Repository navigation
sec(oidc): validate Azure Key Vault Crv field to enforce P-256 on EC key fetch #46
Description
Summary
In internal/oidc/azure_signer.go resolveOnce, the signer fetches the EC public key by reading resp.Key.X and resp.Key.Y and constructs an *ecdsa.PublicKey with the curve hardcoded to elliptic.P256(), without validating the resp.Key.Crv field.
If someone misconfigures a P-384 or P-521 key in Key Vault:
PublicKey()will return a key with P-256 curve but P-384/P-521 coordinate sizes, which is an invalid point on P-256.- The
Signcall will return an error (Key Vault rejects ES256 signing with a non-P-256 key), so no silently wrong signatures are produced.
The gap is that the error surfaces at sign-time rather than at key-fetch time, and the public key returned by PublicKey() is structurally invalid.
Fix: check resp.Key.Crv != nil && *resp.Key.Crv == azkeys.CurveNameP256 in resolveOnce and return an error if the curve is absent or not P-256.
This is a correctness issue but low severity in practice because Azure Key Vault enforces the algorithm on Sign. Discovered during adversarial review of PR LeanerCloud/cloud-commitments-cli#882.
Activity
cristim commentedon Jul 28, 2026
Reviewed commit:
be11bdcb5. Note:origin/mainmoved to3e9660d06during the review; re-verify against currentmainbefore changing code, since a finding may have been fixed or moved.The 2026-07-28 full-repo review re-derived this and found one field beyond the
Crvcheck this issue already tracks, plus a sharper argument for why it should not stay atseverity/low.Where
internal/oidc/azure_signer.go:103-135(resolveOnce)internal/oidc/jwks.go:47(PublicJWK), which does reject a non-P-256 curveinternal/oidc/gcp_signer.go:52-55, which pins an explicitcryptoKeyVersionWhat the review adds
Ktyis unchecked as well asCrv. Neitherresp.Key.Ktynorresp.Key.Crvis ever read. The code checks only thatXandYare non-nil and at most 32 bytes, right-aligns them into a 65-byte uncompressed point, and parses withecdsa.ParseUncompressedPublicKey(elliptic.P256(), ...). The curve is assumed, not verified, and so is the key type.Signthen unconditionally requestsazkeys.SignatureAlgorithmES256.The validation already exists in this package.
PublicJWK(jwks.go:47) explicitly rejects a non-P-256 curve. The check is present on the JWKS-publishing side and simply absent from this backend's key-fetch side, so the two disagree about what has been validated.A
P-256Kkey is the case that is not merely a loud failure. This issue notes that Key Vault rejects ES256 signing for a P-384/P-521 key, so nothing wrong is signed. That reasoning holds for the size-mismatched curves. It does not hold as cleanly for secp256k1 (P-256K), whose coordinates are also 32 bytes: a coordinate pair that happens to satisfy the P-256 curve equation parses successfully, and CUDly then publishes a JWK labelled"crv":"P-256"describing a key from a different curve.Failure scenario
An operator points
CUDLY_SIGNING_KEY_NAMEat aP-256KorP-384key. The failure surfaces far from its cause: either the point fails to parse with an opaque "parse EC public key" error at first use, or the JWKS advertises a mislabelled key while Key Vault rejects the ES256 sign request. Federated credential resolution for every cloud account then breaks with a diagnostic pointing at the JWKS rather than at the misconfigured key, which is a slow outage to diagnose across all three clouds at once.Fix direction
Assert
*resp.Key.KtyisECorEC-HSMand*resp.Key.CrvisP-256before constructing the point, and return an error naming the actual values so the operator sees the misconfigured key rather than a parse failure.IAmJSD commentedon Oct 20, 2026
Claiming: working on this