Repository navigation
Commit 5900a08
authored
sec(oidc): switch JWT signing from PKCS1v15 to ES256 (#882)
* sec(oidc): switch JWT signing from PKCS1v15 to ES256 (closes #422)
Replace RSA-PKCS1v15 (RS256) with ECDSA P-256 (ES256) across the entire
oidc package:
- LocalSigner: ecdsa.GenerateKey(P256) + ecdsa.SignASN1 instead of
rsa.GenerateKey + rsa.SignPKCS1v15
- Signer interface: PublicKey() returns crypto.PublicKey (was *rsa.PublicKey)
- Algorithm constant: "ES256" (was "RS256")
- ComputeKeyID: accepts crypto.PublicKey, hashes uncompressed EC point
- JWK: EC fields (kty=EC, crv=P-256, x, y); RSA fields (n, e) removed
- AWSKMSSigner: SigningAlgorithmSpecEcdsaSha256 + *ecdsa.PublicKey assertion
- AzureKeyVaultSigner: SignatureAlgorithmES256 + EC key (X/Y) extraction
- GCPKMSSigner: *ecdsa.PublicKey assertion (digest call is key-type-agnostic)
- All tests: positive assertions on ES256 algorithm and EC JWK fields
* fix(oidc): emit RFC7518 raw R||S ES256 JWS signatures across all signer backends (refs #422)
Mint base64url-encoded the signer's raw output directly into the JWS
signature segment, but the Local/AWS/GCP signers return DER/ASN.1 ECDSA
signatures (ecdsa.SignASN1, AWS ECDSA_SHA_256, GCP EC sign all yield
ASN.1 DER). RFC 7518 section 3.4 requires an ES256 JWS signature to be
the raw fixed-length R || S concatenation (32 bytes each = 64 bytes for
P-256), NOT DER. As a result AWS/GCP/Local minted tokens that real OIDC
consumers (the stated Azure AD target) reject. Azure Key Vault already
returns raw R||S, so only Azure was correct, leaving the four backends
mutually inconsistent. The Azure Sign comment also wrongly claimed
Key Vault returns DER.
Contract: Signer.Sign now returns the RFC 7518 raw R||S form. DER
backends (Local/AWS/GCP) convert via a single shared
derToRawECDSASignature helper; Azure passes through unchanged (no
double-conversion). Mint encodes the result directly and defensively
rejects any signature that is not 64 bytes.
- signer.go: add derToRawECDSASignature; LocalSigner.Sign converts;
Mint enforces the 64-byte contract; document the contract on the
Signer interface.
- aws_signer.go, gcp_signer.go: convert KMS DER output to raw R||S.
- azure_signer.go: fix the wrong "DER-encoded" comment; keep passthrough.
- tests: add assertRawES256JWS asserting the JWS sig is exactly 64 bytes,
splitting R||S for ecdsa.Verify, and parsing with a real golang-jwt
ES256 parser. Cover Local, AWS (DER fake), GCP (DER fake) and Azure
(raw fake). The DER-path tests fail on the pre-fix code (71/69-byte
signatures) and pass after; the Azure raw path stays correct.
- go.mod: promote golang-jwt/jwt/v5 to a direct dependency (test use).
* fix(oidc): rebase on main, fix test/lint regressions after ES256 migration
- Replace TestAzureSigner_ExponentRange (RSA exponent tests) with
TestAzureSigner_ECKeyCompleteness: the azure_factory_test.go added on
main (#1044) tested RSA exponent validation removed by this PR; new
test covers EC key nil-field rejection to keep coverage parity.
- Apply fieldalignment ordering (govet) to aws_signer.go, azure_signer.go,
gcp_signer.go, and the updated azure_factory_test.go struct literals.
- Suppress staticcheck SA1019 on elliptic.Marshal in ComputeKeyID with an
inline nolint; elliptic.Marshal is the only stdlib path from *ecdsa.PublicKey
to the uncompressed point without converting through crypto/ecdh.
- Fix staticcheck QF1008 (remove unnecessary .PublicKey embed selector) in
aws_signer_test.go and backend_jws_test.go.
- Fix shadow declarations in signer_test.go (json.Unmarshal err vars).
- Replace deprecated ecdsa.Sign with ecdsa.SignASN1 + derToRawECDSASignature
in the fakeAzureKVClient test stub (backend_jws_test.go).
- Fix gofmt import ordering in backend_jws_test.go (cloud.google.com before
github.com).
* fix(oidc): replace deprecated elliptic.Marshal with ECDH().Bytes() in ComputeKeyID
Use (*ecdsa.PublicKey).ECDH() and (*ecdh.PublicKey).Bytes() to obtain the
uncompressed EC point, replacing the SA1019-deprecated elliptic.Marshal.
The wire format is identical (0x04 || X || Y), so kid stability is preserved.
Removes the staticcheck nolint directive.
* fix(oidc): update stubOIDCSigner in credentials tests to crypto.PublicKey
The oidc.Signer interface PublicKey method was changed from *rsa.PublicKey
to crypto.PublicKey as part of the ES256 migration. Update the test stub
in internal/credentials to match, fixing the go vet type-assertion failure.
* fix(oidc): avoid deprecated ecdsa.PublicKey.X/Y in ES256 tests
Rebasing onto main (Go 1.26.5) surfaces staticcheck SA1019: the raw
ecdsa.PublicKey.X/Y fields are deprecated since Go 1.26. Replace the two
uses introduced by the ES256 test code:
- aws_signer_test.go: compare public keys via (*ecdsa.PublicKey).Equal
instead of X.Cmp/Y.Cmp.
- azure_factory_test.go: derive the fixed-width JWK coordinates from the
uncompressed SEC 1 point via crypto/ecdh (ECDH().Bytes()), matching
ComputeKeyID and PublicJWK.
No behaviour change; the Azure fake now emits left-padded coordinates,
which big.Int.SetBytes reconstructs identically.
* fix(oidc): replace deprecated ecdsa.PublicKey X/Y fields with ParseUncompressedPublicKey
Staticcheck SA1019 flagged direct struct-literal initialization of
ecdsa.PublicKey.X and ecdsa.PublicKey.Y (deprecated since Go 1.23).
azure_signer.go: build the 65-byte SEC 1 uncompressed point from the
JWK X/Y bytes (right-aligned to 32 bytes each, per JWK spec), then
call ecdsa.ParseUncompressedPublicKey(elliptic.P256(), ...) which
validates point membership and produces a PublicKey without touching
the deprecated fields. Drop the now-unused math/big import. Add a
guard for coordinate lengths > 32 bytes.
backend_jws_test.go: replace f.key.X.Bytes()/f.key.Y.Bytes() with
key.PublicKey.ECDH().Bytes() (uncompressed point), then slice X from
[1:33] and Y from [33:65]. Add "fmt" import for the new error path.1 parent f8ccda6 commit 5900a08
1 file changed
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
49 | 49 | | |
50 | 50 | | |
51 | 51 | | |
52 | | - | |
| 52 | + | |
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
| |||
0 commit comments