fix: use loaded public key in oidc service, fixes #860#874
Conversation
dosubot
Bot
added
the
size:S
📝 WalkthroughWalkthroughThe ChangesRSA Public Key Storage and Usage
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/service/oidc_service.go (2)
226-239:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject non-RSA or mismatched public keys before constructing the service.
x509.ParsePKIXPublicKeyaccepts multiple key types (*rsa.PublicKey, *dsa.PublicKey, *ecdsa.PublicKey, ed25519.PublicKey, *ecdh.PublicKey). The unchecked type assertionpublicKey.(*rsa.PublicKey)at line 274 will panic at runtime if the parsed key is not RSA. Additionally, validate that the loaded public key matches the private key—if they differ, the service will advertise a key it cannot use for signing.Suggested fix
} else { block, _ := pem.Decode(fpublicKey) if block == nil { return nil, errors.New("failed to decode public key") @@ default: return nil, fmt.Errorf("unsupported public key type: %s", block.Type) } } + + rsaPublicKey, ok := publicKey.(*rsa.PublicKey) + if !ok { + return nil, fmt.Errorf("unsupported public key type %T: RSA required", publicKey) + } + if rsaPublicKey.N.Cmp(privateKey.N) != 0 || rsaPublicKey.E != privateKey.E { + return nil, errors.New("public key does not match private key") + } @@ - publicKey: publicKey.(*rsa.PublicKey), + publicKey: rsaPublicKey,Also applies to: 272-275
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/service/oidc_service.go` around lines 226 - 239, The parsed public key from x509.ParsePKIXPublicKey may not be RSA and the code later does an unchecked assertion publicKey.(*rsa.PublicKey), so update the parsing branch to perform a safe type check (type switch or type assertion with ok) and return an explicit error if the key is not an *rsa.PublicKey; additionally, verify the loaded public key actually matches the service's private key by comparing the RSA public components (e.g., compare rsaPub.N and rsaPub.E with privateKey.PublicKey.N and .E) and return an error if they differ so the service cannot be constructed with mismatched keys (refer to variables publicKey, privateKey and the parsing branches that call x509.ParsePKCS1PublicKey / x509.ParsePKIXPublicKey).
816-831:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBuild the JWK from
service.publicKeyfor consistency with the loaded key.The
KeyIDis derived from the explicitly loadedservice.publicKey, but the JWK is built fromservice.privateKey.Public(). Since the public key is loaded independently from its own file, these can diverge if keys are rotated or mismatched files are provided. The JWK should be built from the explicitly loadedservice.publicKeyto ensure theKeyIDmatches the actual served key.Suggested fix
jwk := jose.JSONWebKey{ - Key: service.privateKey, + Key: service.publicKey, Algorithm: string(jose.RS256), Use: "sig", KeyID: base64.URLEncoding.EncodeToString(hasher.Sum(nil)), } - return jwk.Public().MarshalJSON() + return jwk.MarshalJSON()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/service/oidc_service.go` around lines 816 - 831, The JWK is being created from service.privateKey which can diverge from the separately loaded service.publicKey used to compute KeyID; update the JSONWebKey construction in the function that marshals the key (the block using x509.MarshalPKCS1PublicKey, hasher.Write and creating jose.JSONWebKey) to use service.publicKey (the explicit loaded public key) as the Key field so the JWK contents and the KeyID derived from der remain consistent; keep Algorithm as RS256 and Use as "sig" and continue returning jwk.Public().MarshalJSON().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/service/oidc_service.go`:
- Around line 226-239: The parsed public key from x509.ParsePKIXPublicKey may
not be RSA and the code later does an unchecked assertion
publicKey.(*rsa.PublicKey), so update the parsing branch to perform a safe type
check (type switch or type assertion with ok) and return an explicit error if
the key is not an *rsa.PublicKey; additionally, verify the loaded public key
actually matches the service's private key by comparing the RSA public
components (e.g., compare rsaPub.N and rsaPub.E with privateKey.PublicKey.N and
.E) and return an error if they differ so the service cannot be constructed with
mismatched keys (refer to variables publicKey, privateKey and the parsing
branches that call x509.ParsePKCS1PublicKey / x509.ParsePKIXPublicKey).
- Around line 816-831: The JWK is being created from service.privateKey which
can diverge from the separately loaded service.publicKey used to compute KeyID;
update the JSONWebKey construction in the function that marshals the key (the
block using x509.MarshalPKCS1PublicKey, hasher.Write and creating
jose.JSONWebKey) to use service.publicKey (the explicit loaded public key) as
the Key field so the JWK contents and the KeyID derived from der remain
consistent; keep Algorithm as RS256 and Use as "sig" and continue returning
jwk.Public().MarshalJSON().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ac789c9c-942c-4051-a015-a9aa1da80767
📒 Files selected for processing (1)
internal/service/oidc_service.go
1 participant
Summary by CodeRabbit