diff --git a/go.mod b/go.mod index 0b858817a..ae0da12cd 100644 --- a/go.mod +++ b/go.mod @@ -49,7 +49,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/go-logr/logr v1.4.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect - github.com/golang-jwt/jwt/v5 v5.3.1 // indirect + github.com/golang-jwt/jwt/v5 v5.3.1 github.com/google/s2a-go v0.1.9 // indirect github.com/googleapis/enterprise-certificate-proxy v0.3.14 // indirect github.com/googleapis/gax-go/v2 v2.21.0 // indirect diff --git a/internal/api/handler_oidc_test.go b/internal/api/handler_oidc_test.go index 50fe8800d..afa20c59e 100644 --- a/internal/api/handler_oidc_test.go +++ b/internal/api/handler_oidc_test.go @@ -88,8 +88,10 @@ func TestHandleOIDCJWKS(t *testing.T) { t.Fatalf("keys=%d want 1", len(jwks.Keys)) } k := jwks.Keys[0] - if k.Kty != "RSA" || k.Alg != "RS256" || k.Kid == "" || k.N == "" { - t.Errorf("jwk malformed: %+v", k) + // ES256: EC key with P-256 curve. Positively assert the algorithm + // change -- regression to RSA/RS256 must not pass. + if k.Kty != "EC" || k.Alg != "ES256" || k.Crv != "P-256" || k.Kid == "" || k.X == "" || k.Y == "" { + t.Errorf("jwk malformed (want EC/ES256/P-256): %+v", k) } } diff --git a/internal/credentials/resolver_extra_test.go b/internal/credentials/resolver_extra_test.go index d31959406..2d861662b 100644 --- a/internal/credentials/resolver_extra_test.go +++ b/internal/credentials/resolver_extra_test.go @@ -2,6 +2,7 @@ package credentials import ( "context" + "crypto" "crypto/rand" "crypto/rsa" "crypto/x509" @@ -24,7 +25,7 @@ type stubOIDCSigner struct{} func (stubOIDCSigner) Sign(context.Context, []byte) ([]byte, error) { return nil, assert.AnError } -func (stubOIDCSigner) PublicKey(context.Context) (*rsa.PublicKey, error) { +func (stubOIDCSigner) PublicKey(context.Context) (crypto.PublicKey, error) { return nil, assert.AnError } func (stubOIDCSigner) KeyID(context.Context) (string, error) { diff --git a/internal/oidc/aws_signer.go b/internal/oidc/aws_signer.go index df90793ec..3e1b9c435 100644 --- a/internal/oidc/aws_signer.go +++ b/internal/oidc/aws_signer.go @@ -2,7 +2,8 @@ package oidc import ( "context" - "crypto/rsa" + "crypto" + "crypto/ecdsa" "crypto/x509" "fmt" "sync" @@ -24,15 +25,16 @@ type AWSKMSClient interface { type AWSKMSSigner struct { client AWSKMSClient err error - pubKey *rsa.PublicKey + pubKey *ecdsa.PublicKey keyID string kid string once sync.Once } // NewAWSKMSSigner constructs a signer bound to the given KMS key. The -// keyID may be the key ARN, alias ARN, or alias name — anything -// accepted by kms:Sign and kms:GetPublicKey. +// keyID may be the key ARN, alias ARN, or alias name -- anything +// accepted by kms:Sign and kms:GetPublicKey. The KMS key must be an +// ECC_NIST_P256 key configured for signing (ES256). func NewAWSKMSSigner(ctx context.Context, keyID string) (*AWSKMSSigner, error) { if keyID == "" { return nil, fmt.Errorf("oidc: empty AWS KMS keyID") @@ -51,24 +53,25 @@ func NewAWSKMSSignerFromClient(client AWSKMSClient, keyID string) *AWSKMSSigner } // Sign calls kms:Sign with the raw SHA-256 digest and the signing -// algorithm RSASSA_PKCS1_V1_5_SHA_256, which matches what RS256 JWS -// signatures expect. +// algorithm ECDSA_SHA_256. AWS KMS returns a DER/ASN.1-encoded ECDSA +// signature, which is converted to the RFC 7518 section 3.4 raw R || S +// form required by ES256 JWS signatures (the Signer.Sign contract). func (s *AWSKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) { out, err := s.client.Sign(ctx, &kms.SignInput{ KeyId: &s.keyID, Message: digest, MessageType: types.MessageTypeDigest, - SigningAlgorithm: types.SigningAlgorithmSpecRsassaPkcs1V15Sha256, + SigningAlgorithm: types.SigningAlgorithmSpecEcdsaSha256, }) if err != nil { return nil, fmt.Errorf("oidc: kms:Sign: %w", err) } - return out.Signature, nil + return derToRawECDSASignature(out.Signature) } // PublicKey fetches the public half of the KMS key once and caches it. // Subsequent calls return the cached value. -func (s *AWSKMSSigner) PublicKey(ctx context.Context) (*rsa.PublicKey, error) { +func (s *AWSKMSSigner) PublicKey(ctx context.Context) (crypto.PublicKey, error) { s.resolveOnce(ctx) return s.pubKey, s.err } @@ -91,17 +94,17 @@ func (s *AWSKMSSigner) resolveOnce(ctx context.Context) { s.err = fmt.Errorf("oidc: parse kms public key: %w", err) return } - rsaPub, ok := pub.(*rsa.PublicKey) + ecPub, ok := pub.(*ecdsa.PublicKey) if !ok { - s.err = fmt.Errorf("oidc: kms key is not RSA (got %T)", pub) + s.err = fmt.Errorf("oidc: kms key is not ECDSA (got %T); key must be ECC_NIST_P256", pub) return } - kid, err := ComputeKeyID(rsaPub) + kid, err := ComputeKeyID(ecPub) if err != nil { s.err = err return } - s.pubKey = rsaPub + s.pubKey = ecPub s.kid = kid }) } diff --git a/internal/oidc/aws_signer_test.go b/internal/oidc/aws_signer_test.go index b6721e7f5..788f213c8 100644 --- a/internal/oidc/aws_signer_test.go +++ b/internal/oidc/aws_signer_test.go @@ -2,28 +2,24 @@ package oidc import ( "context" - "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" - "crypto/sha256" "crypto/x509" - "encoding/base64" "testing" "github.com/aws/aws-sdk-go-v2/service/kms" ) -var base64RawURL = base64.RawURLEncoding - -// fakeKMSClient is a minimal AWSKMSClient backed by an in-process RSA -// key. It lets TestAWSKMSSigner exercise the Signer contract without -// touching real AWS. +// fakeKMSClient is a minimal AWSKMSClient backed by an in-process P-256 +// ECDSA key. It lets TestAWSKMSSigner exercise the Signer contract without +// touching real AWS. Like real AWS KMS, it returns a DER/ASN.1 signature. type fakeKMSClient struct { - key *rsa.PrivateKey + key *ecdsa.PrivateKey } func (f *fakeKMSClient) Sign(_ context.Context, in *kms.SignInput, _ ...func(*kms.Options)) (*kms.SignOutput, error) { - sig, err := rsa.SignPKCS1v15(rand.Reader, f.key, crypto.SHA256, in.Message) + sig, err := ecdsa.SignASN1(rand.Reader, f.key, in.Message) if err != nil { return nil, err } @@ -40,19 +36,23 @@ func (f *fakeKMSClient) GetPublicKey(_ context.Context, _ *kms.GetPublicKeyInput func TestAWSKMSSignerRoundTrip(t *testing.T) { ctx := context.Background() - key, err := rsa.GenerateKey(rand.Reader, 2048) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { t.Fatalf("gen key: %v", err) } signer := NewAWSKMSSignerFromClient(&fakeKMSClient{key: key}, "alias/test-key") - // Signer contract: PublicKey returns the RSA pub half. - pub, err := signer.PublicKey(ctx) + // Signer contract: PublicKey returns the ECDSA pub half. + rawPub, err := signer.PublicKey(ctx) if err != nil { t.Fatalf("public key: %v", err) } - if pub.N.Cmp(key.N) != 0 { - t.Fatal("public key modulus mismatch") + ecPub, ok := rawPub.(*ecdsa.PublicKey) + if !ok { + t.Fatalf("public key is not *ecdsa.PublicKey, got %T", rawPub) + } + if !ecPub.Equal(&key.PublicKey) { + t.Fatal("public key point mismatch") } // KeyID stable across calls. @@ -62,7 +62,7 @@ func TestAWSKMSSignerRoundTrip(t *testing.T) { t.Errorf("kid unstable or empty: %s vs %s", k1, k2) } - // Mint a JWT and verify the signature end-to-end. + // Mint a JWT and verify the ECDSA signature end-to-end. jws, err := Mint(ctx, signer, map[string]any{ "iss": "https://cudly.example.com", "sub": "cudly-controller", @@ -71,43 +71,7 @@ func TestAWSKMSSignerRoundTrip(t *testing.T) { if err != nil { t.Fatalf("mint: %v", err) } - // Verify using the underlying pub half. - parts := splitJWS(t, jws) - signingInput := parts[0] + "." + parts[1] - digest := sha256.Sum256([]byte(signingInput)) - if err := rsa.VerifyPKCS1v15(pub, crypto.SHA256, digest[:], decodeB64(t, parts[2])); err != nil { - t.Errorf("signature verify: %v", err) - } -} - -// helpers — unit tests only, kept private. -func splitJWS(t *testing.T, jws string) [3]string { - t.Helper() - var out [3]string - last := 0 - idx := 0 - for i := 0; i < len(jws); i++ { - if jws[i] == '.' { - if idx >= 3 { - t.Fatalf("too many dots in JWS: %q", jws) - } - out[idx] = jws[last:i] - idx++ - last = i + 1 - } - } - if idx != 2 { - t.Fatalf("expected 2 dots in JWS, got %d: %q", idx, jws) - } - out[2] = jws[last:] - return out -} - -func decodeB64(t *testing.T, s string) []byte { - t.Helper() - b, err := base64RawURL.DecodeString(s) - if err != nil { - t.Fatalf("b64 decode: %v", err) - } - return b + // The AWS KMS fake returns DER (ecdsa.SignASN1); the signer must + // convert it to the RFC 7518 raw R||S form before Mint encodes it. + assertRawES256JWS(t, jws, ecPub) } diff --git a/internal/oidc/azure_factory_test.go b/internal/oidc/azure_factory_test.go index 246b8c83d..6a014613c 100644 --- a/internal/oidc/azure_factory_test.go +++ b/internal/oidc/azure_factory_test.go @@ -2,9 +2,9 @@ package oidc import ( "context" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" - "math/big" "testing" "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys" @@ -13,11 +13,15 @@ import ( ) // fakeAzureKeyVaultClient is a minimal AzureKeyVaultClient backed by an -// in-process RSA key. Used to exercise resolveOnce without a real Key Vault. +// in-process EC key. Used to exercise resolveOnce without a real Key Vault. type fakeAzureKeyVaultClient struct { signErr error - key *rsa.PublicKey - eBytes []byte + key *ecdsa.PublicKey + // xBytes and yBytes allow callers to inject nil to simulate incomplete + // responses from the Key Vault API. + xBytes []byte + yBytes []byte + nilKey bool // if true, return a KeyBundle with Key==nil } func (f *fakeAzureKeyVaultClient) Sign(_ context.Context, _, _ string, _ azkeys.SignParameters, _ *azkeys.SignOptions) (azkeys.SignResponse, error) { @@ -25,15 +29,42 @@ func (f *fakeAzureKeyVaultClient) Sign(_ context.Context, _, _ string, _ azkeys. } func (f *fakeAzureKeyVaultClient) GetKey(_ context.Context, _, _ string, _ *azkeys.GetKeyOptions) (azkeys.GetKeyResponse, error) { - eBytes := f.eBytes - if eBytes == nil { - // Normal path: real exponent from the RSA key. - e := big.NewInt(int64(f.key.E)) - eBytes = e.Bytes() + if f.nilKey { + return azkeys.GetKeyResponse{ + KeyBundle: azkeys.KeyBundle{Key: nil}, + }, nil + } + xBytes := f.xBytes + yBytes := f.yBytes + if xBytes == nil || yBytes == nil { + // Derive the fixed-width coordinates from the uncompressed SEC 1 + // point (0x04 || X || Y) via crypto/ecdh, matching ComputeKeyID + // and avoiding the deprecated ecdsa.PublicKey.X/Y fields. + ecdhKey, err := f.key.ECDH() + if err != nil { + return azkeys.GetKeyResponse{}, err + } + uncompressed := ecdhKey.Bytes() + byteLen := (f.key.Curve.Params().BitSize + 7) / 8 + if xBytes == nil { + xBytes = uncompressed[1 : 1+byteLen] + } + if yBytes == nil { + yBytes = uncompressed[1+byteLen:] + } + } + // Use sentinel value to represent "caller explicitly passed nil" vs + // "caller didn't override" -- an empty slice signals nil field. + var jwkX, jwkY []byte + if f.xBytes != nil || xBytes != nil { + jwkX = xBytes + } + if f.yBytes != nil || yBytes != nil { + jwkY = yBytes } keyBundle := azkeys.JSONWebKey{ - N: f.key.N.Bytes(), - E: eBytes, + X: jwkX, + Y: jwkY, } return azkeys.GetKeyResponse{ KeyBundle: azkeys.KeyBundle{Key: &keyBundle}, @@ -41,45 +72,45 @@ func (f *fakeAzureKeyVaultClient) GetKey(_ context.Context, _, _ string, _ *azke } // --------------------------------------------------------------------------- -// M6 — Azure public-exponent overflow guard +// M6 -- Azure EC key completeness guard (replaces RSA exponent-range check) // --------------------------------------------------------------------------- -// TestAzureSigner_ExponentRange verifies that resolveOnce rejects oversized or -// non-positive exponents before constructing the rsa.PublicKey (03-M6). -func TestAzureSigner_ExponentRange(t *testing.T) { - key, err := rsa.GenerateKey(rand.Reader, 2048) +// TestAzureSigner_ECKeyCompleteness verifies that resolveOnce rejects Key +// Vault responses that are missing the EC public-key coordinates, which +// would otherwise produce a zero-valued *ecdsa.PublicKey and sign invalid +// tokens. A valid response with both X and Y must be accepted. +func TestAzureSigner_ECKeyCompleteness(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, err) cases := []struct { + client *fakeAzureKeyVaultClient name string errSubstr string - eBytes []byte wantErr bool }{ { - name: "normal exponent 65537 accepted", - eBytes: big.NewInt(65537).Bytes(), + name: "valid EC key accepted", + client: &fakeAzureKeyVaultClient{ + key: &key.PublicKey, + }, wantErr: false, }, { - name: "exponent 0 rejected", - eBytes: big.NewInt(0).Bytes(), - wantErr: true, - errSubstr: "exponent", - }, - { - name: "exponent too large (> MaxInt32) rejected", - eBytes: new(big.Int).Add(big.NewInt(0x7fffffff), big.NewInt(1)).Bytes(), + name: "key bundle with Key=nil rejected", + client: &fakeAzureKeyVaultClient{ + key: &key.PublicKey, + nilKey: true, + }, wantErr: true, - errSubstr: "exponent", + errSubstr: "missing X or Y", }, } for _, tc := range cases { tc := tc t.Run(tc.name, func(t *testing.T) { - client := &fakeAzureKeyVaultClient{key: &key.PublicKey, eBytes: tc.eBytes} - signer := NewAzureKeyVaultSignerFromClient(client, "test-key", "") + signer := NewAzureKeyVaultSignerFromClient(tc.client, "test-key", "") ctx := context.Background() _, err := signer.PublicKey(ctx) @@ -94,7 +125,7 @@ func TestAzureSigner_ExponentRange(t *testing.T) { } // --------------------------------------------------------------------------- -// M7 — factory precise error for half-configured Azure +// M7 -- factory precise error for half-configured Azure // --------------------------------------------------------------------------- // TestNewSignerFromEnv_AzureHalfConfigured verifies that exactly one of the diff --git a/internal/oidc/azure_signer.go b/internal/oidc/azure_signer.go index 013b5f4f9..4007110f5 100644 --- a/internal/oidc/azure_signer.go +++ b/internal/oidc/azure_signer.go @@ -2,10 +2,10 @@ package oidc import ( "context" - "crypto/rsa" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" "fmt" - "math" - "math/big" "sync" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -19,12 +19,12 @@ type AzureKeyVaultClient interface { GetKey(ctx context.Context, name, version string, options *azkeys.GetKeyOptions) (azkeys.GetKeyResponse, error) } -// AzureKeyVaultSigner signs JWTs via an Azure Key Vault RSA key. The +// AzureKeyVaultSigner signs JWTs via an Azure Key Vault EC key (P-256). The // private half never leaves the vault. type AzureKeyVaultSigner struct { client AzureKeyVaultClient err error - pubKey *rsa.PublicKey + pubKey *ecdsa.PublicKey keyName string keyVersion string kid string @@ -34,7 +34,7 @@ type AzureKeyVaultSigner struct { // NewAzureKeyVaultSigner constructs a signer against a Key Vault using // the standard azidentity default credential chain. vaultURL is the // full vault URL (e.g. https://cudly-vault.vault.azure.net/); keyName -// is the name of the RSA key in that vault. +// is the name of the EC (P-256) key in that vault. func NewAzureKeyVaultSigner(ctx context.Context, vaultURL, keyName string) (*AzureKeyVaultSigner, error) { if vaultURL == "" || keyName == "" { return nil, fmt.Errorf("oidc: azure key vault signer requires vaultURL + keyName") @@ -57,10 +57,13 @@ func NewAzureKeyVaultSignerFromClient(client AzureKeyVaultClient, keyName, keyVe return &AzureKeyVaultSigner{client: client, keyName: keyName, keyVersion: keyVersion} } -// Sign calls Key Vault's Sign operation with RS256, passing the raw -// SHA-256 digest. Key Vault returns the raw RSA signature bytes. +// Sign calls Key Vault's Sign operation with ES256, passing the raw +// SHA-256 digest. Unlike AWS KMS and GCP Cloud KMS (which return DER), +// Azure Key Vault's ES256 already returns the IEEE P1363 / RFC 7518 +// raw R || S signature (64 bytes for P-256), so it satisfies the +// Signer.Sign contract directly and must NOT be DER-converted. func (s *AzureKeyVaultSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) { - alg := azkeys.SignatureAlgorithmRS256 + alg := azkeys.SignatureAlgorithmES256 resp, err := s.client.Sign(ctx, s.keyName, s.keyVersion, azkeys.SignParameters{ Algorithm: &alg, Value: digest, @@ -73,12 +76,12 @@ func (s *AzureKeyVaultSigner) Sign(ctx context.Context, digest []byte) ([]byte, // PublicKey fetches the public half of the Key Vault key once and // caches it. -func (s *AzureKeyVaultSigner) PublicKey(ctx context.Context) (*rsa.PublicKey, error) { +func (s *AzureKeyVaultSigner) PublicKey(ctx context.Context) (crypto.PublicKey, error) { s.resolveOnce(ctx) return s.pubKey, s.err } -// KeyID returns a stable kid derived from the public key modulus. +// KeyID returns a stable kid derived from the public key point. func (s *AzureKeyVaultSigner) KeyID(ctx context.Context) (string, error) { s.resolveOnce(ctx) return s.kid, s.err @@ -91,25 +94,30 @@ func (s *AzureKeyVaultSigner) resolveOnce(ctx context.Context) { s.err = fmt.Errorf("oidc: azure keyvault GetKey: %w", err) return } - if resp.Key == nil || resp.Key.N == nil || resp.Key.E == nil { - s.err = fmt.Errorf("oidc: azure keyvault returned incomplete key") + if resp.Key == nil || resp.Key.X == nil || resp.Key.Y == nil { + s.err = fmt.Errorf("oidc: azure keyvault returned incomplete EC key (missing X or Y)") return } - e := new(big.Int).SetBytes(resp.Key.E) - if !e.IsInt64() || e.Int64() <= 0 || e.Int64() > math.MaxInt32 { - s.err = fmt.Errorf("oidc: azure keyvault returned unexpected RSA exponent %s", e) + // JWK coordinates may omit leading zeros; right-align each into 32 bytes. + if len(resp.Key.X) > 32 || len(resp.Key.Y) > 32 { + s.err = fmt.Errorf("oidc: azure keyvault EC key coordinate exceeds 32 bytes") return } - rsaPub := &rsa.PublicKey{ - N: new(big.Int).SetBytes(resp.Key.N), - E: int(e.Int64()), + var uncompressed [65]byte + uncompressed[0] = 0x04 + copy(uncompressed[1+32-len(resp.Key.X):33], resp.Key.X) + copy(uncompressed[33+32-len(resp.Key.Y):65], resp.Key.Y) + ecPub, err := ecdsa.ParseUncompressedPublicKey(elliptic.P256(), uncompressed[:]) + if err != nil { + s.err = fmt.Errorf("oidc: azure keyvault parse EC public key: %w", err) + return } - kid, err := ComputeKeyID(rsaPub) + kid, err := ComputeKeyID(ecPub) if err != nil { s.err = err return } - s.pubKey = rsaPub + s.pubKey = ecPub s.kid = kid }) } diff --git a/internal/oidc/backend_jws_test.go b/internal/oidc/backend_jws_test.go new file mode 100644 index 000000000..ec94f30c6 --- /dev/null +++ b/internal/oidc/backend_jws_test.go @@ -0,0 +1,131 @@ +package oidc + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "encoding/pem" + "fmt" + "testing" + + kmspb "cloud.google.com/go/kms/apiv1/kmspb" + "github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys" +) + +// These tests assert that every signer backend produces an RFC 7518 +// section 3.4 ES256 JWS signature (raw R||S, 64 bytes) that a real +// JOSE/JWT parser accepts. They cover the two on-wire shapes a backend +// can return: +// +// - DER/ASN.1 (AWS KMS, GCP Cloud KMS, in-process LocalSigner): the +// signer must convert to raw R||S. Exercised here via the GCP fake +// (the AWS DER path is covered by TestAWSKMSSignerRoundTrip). +// - raw R||S / P1363 (Azure Key Vault ES256): the signer must pass it +// through unchanged, with NO double-conversion. +// +// Both must yield a 64-byte JWS signature; on the pre-fix code the DER +// path emitted ~70-72 bytes and these tests fail. + +// --- GCP: DER path --- + +type fakeGCPKMSClient struct { + key *ecdsa.PrivateKey +} + +func (f *fakeGCPKMSClient) AsymmetricSign(_ context.Context, req *kmspb.AsymmetricSignRequest, _ ...interface{}) (*kmspb.AsymmetricSignResponse, error) { + // Real GCP Cloud KMS returns a DER/ASN.1 ECDSA signature. + der, err := ecdsa.SignASN1(rand.Reader, f.key, req.GetDigest().GetSha256()) + if err != nil { + return nil, err + } + return &kmspb.AsymmetricSignResponse{Signature: der}, nil +} + +func (f *fakeGCPKMSClient) GetPublicKey(_ context.Context, _ *kmspb.GetPublicKeyRequest, _ ...interface{}) (*kmspb.PublicKey, error) { + der, err := x509.MarshalPKIXPublicKey(&f.key.PublicKey) + if err != nil { + return nil, err + } + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: der}) + return &kmspb.PublicKey{Pem: string(pemBytes)}, nil +} + +func TestGCPKMSSignerEmitsRawES256(t *testing.T) { + ctx := context.Background() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("gen key: %v", err) + } + signer := NewGCPKMSSignerFromClient(&fakeGCPKMSClient{key: key}, + "projects/p/locations/global/keyRings/r/cryptoKeys/k/cryptoKeyVersions/1") + + jws, err := Mint(ctx, signer, map[string]any{ + "iss": "https://cudly.example.com", + "sub": "cudly-controller", + "aud": "api://AzureADTokenExchange", + }) + if err != nil { + t.Fatalf("mint: %v", err) + } + assertRawES256JWS(t, jws, &key.PublicKey) +} + +// --- Azure: raw R||S path --- + +type fakeAzureKVClient struct { + key *ecdsa.PrivateKey +} + +func (f *fakeAzureKVClient) Sign(_ context.Context, _, _ string, params azkeys.SignParameters, _ *azkeys.SignOptions) (azkeys.SignResponse, error) { + // Real Azure Key Vault ES256 returns the raw R||S (IEEE P1363) form, + // NOT DER. Mirror that here so the test fails if the signer wrongly + // DER-converts it. Sign over the digest the caller passed in Value. + der, err := ecdsa.SignASN1(rand.Reader, f.key, params.Value) + if err != nil { + return azkeys.SignResponse{}, err + } + raw, err := derToRawECDSASignature(der) + if err != nil { + return azkeys.SignResponse{}, err + } + return azkeys.SignResponse{KeyOperationResult: azkeys.KeyOperationResult{Result: raw}}, nil +} + +func (f *fakeAzureKVClient) GetKey(_ context.Context, _, _ string, _ *azkeys.GetKeyOptions) (azkeys.GetKeyResponse, error) { + // Use ECDH() to get the uncompressed point bytes (avoids deprecated X/Y fields). + ecdhPub, err := f.key.PublicKey.ECDH() + if err != nil { + return azkeys.GetKeyResponse{}, fmt.Errorf("fake azure client: ECDH: %w", err) + } + raw := ecdhPub.Bytes() // 0x04 || X (32 bytes) || Y (32 bytes) for P-256 + crv := azkeys.CurveNameP256 + kty := azkeys.KeyTypeEC + jwk := &azkeys.JSONWebKey{ + Kty: &kty, + Crv: &crv, + X: raw[1:33], + Y: raw[33:65], + } + return azkeys.GetKeyResponse{KeyBundle: azkeys.KeyBundle{Key: jwk}}, nil +} + +func TestAzureKeyVaultSignerPassesThroughRawES256(t *testing.T) { + ctx := context.Background() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("gen key: %v", err) + } + signer := NewAzureKeyVaultSignerFromClient(&fakeAzureKVClient{key: key}, "cudly-key", "") + + jws, err := Mint(ctx, signer, map[string]any{ + "iss": "https://cudly.example.com", + "sub": "cudly-controller", + "aud": "api://AzureADTokenExchange", + }) + if err != nil { + t.Fatalf("mint: %v", err) + } + assertRawES256JWS(t, jws, &key.PublicKey) +} diff --git a/internal/oidc/gcp_signer.go b/internal/oidc/gcp_signer.go index c3e23985b..554d09835 100644 --- a/internal/oidc/gcp_signer.go +++ b/internal/oidc/gcp_signer.go @@ -2,7 +2,8 @@ package oidc import ( "context" - "crypto/rsa" + "crypto" + "crypto/ecdsa" "crypto/x509" "encoding/pem" "fmt" @@ -35,12 +36,12 @@ func (w gcpKMSWrapper) GetPublicKey(ctx context.Context, req *kmspb.GetPublicKey return w.real.GetPublicKey(ctx, req) } -// GCPKMSSigner signs JWTs using a GCP Cloud KMS asymmetric key. The +// GCPKMSSigner signs JWTs using a GCP Cloud KMS asymmetric key (EC P-256). The // private half never leaves the KMS. type GCPKMSSigner struct { client GCPKMSClient err error - pubKey *rsa.PublicKey + pubKey *ecdsa.PublicKey keyResource string kid string once sync.Once @@ -50,6 +51,8 @@ type GCPKMSSigner struct { // version resource. Example resource: // // projects/.../locations/global/keyRings/.../cryptoKeys/.../cryptoKeyVersions/1 +// +// The key must be an EC_SIGN_P256_SHA256 key. func NewGCPKMSSigner(ctx context.Context, keyResource string) (*GCPKMSSigner, error) { if keyResource == "" { return nil, fmt.Errorf("oidc: empty GCP KMS key resource") @@ -68,8 +71,10 @@ func NewGCPKMSSignerFromClient(client GCPKMSClient, keyResource string) *GCPKMSS // Sign calls AsymmetricSign with the SHA-256 digest. The caller must // have already hashed the signing input; the digest is forwarded -// as-is with the expected algorithm -// RSA_SIGN_PKCS1_2048_SHA256 (implicit in the key config). +// as-is. The key must be configured as EC_SIGN_P256_SHA256 in GCP KMS. +// GCP Cloud KMS returns a DER/ASN.1-encoded ECDSA signature, which is +// converted to the RFC 7518 section 3.4 raw R || S form required by +// ES256 JWS signatures (the Signer.Sign contract). func (s *GCPKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) { crc := int64(crc32.Checksum(digest, crc32.MakeTable(crc32.Castagnoli))) req := &kmspb.AsymmetricSignRequest{ @@ -83,11 +88,11 @@ func (s *GCPKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) if err != nil { return nil, fmt.Errorf("oidc: gcp kms AsymmetricSign: %w", err) } - return resp.Signature, nil + return derToRawECDSASignature(resp.Signature) } // PublicKey fetches the public half of the KMS key once and caches it. -func (s *GCPKMSSigner) PublicKey(ctx context.Context) (*rsa.PublicKey, error) { +func (s *GCPKMSSigner) PublicKey(ctx context.Context) (crypto.PublicKey, error) { s.resolveOnce(ctx) return s.pubKey, s.err } @@ -115,17 +120,17 @@ func (s *GCPKMSSigner) resolveOnce(ctx context.Context) { s.err = fmt.Errorf("oidc: parse gcp kms public key: %w", err) return } - rsaPub, ok := pub.(*rsa.PublicKey) + ecPub, ok := pub.(*ecdsa.PublicKey) if !ok { - s.err = fmt.Errorf("oidc: gcp kms key is not RSA (got %T)", pub) + s.err = fmt.Errorf("oidc: gcp kms key is not ECDSA (got %T); key must be EC_SIGN_P256_SHA256", pub) return } - kid, err := ComputeKeyID(rsaPub) + kid, err := ComputeKeyID(ecPub) if err != nil { s.err = err return } - s.pubKey = rsaPub + s.pubKey = ecPub s.kid = kid }) } diff --git a/internal/oidc/jwks.go b/internal/oidc/jwks.go index 5e66bc0e3..91a0d9f51 100644 --- a/internal/oidc/jwks.go +++ b/internal/oidc/jwks.go @@ -2,22 +2,25 @@ package oidc import ( "context" - "crypto/rsa" + "crypto" + "crypto/ecdsa" "encoding/base64" "fmt" ) -// JWK is a minimal RFC 7517 JSON Web Key for a public RSA signing key. +// JWK is a minimal RFC 7517 JSON Web Key for a public EC signing key. // Only the fields needed by Azure AD federated credential validation -// are serialized. +// are serialized. RSA fields (N, E) are omitted; EC fields (Crv, X, Y) +// carry the P-256 public point per RFC 7518 §6.2. type JWK struct { - Kty string `json:"kty"` // always "RSA" - Use string `json:"use"` // always "sig" - Alg string `json:"alg"` // always "RS256" - Kid string `json:"kid"` // stable key id - N string `json:"n"` // base64url modulus - E string `json:"e"` // base64url exponent - X5c []string `json:"x5c,omitempty"` + Kty string `json:"kty"` // "EC" for ECDSA keys + Use string `json:"use"` // always "sig" + Alg string `json:"alg"` // always "ES256" + Kid string `json:"kid"` // stable key id + Crv string `json:"crv"` // "P-256" + X string `json:"x"` // base64url x-coordinate + Y string `json:"y"` // base64url y-coordinate + X5c []string `json:"x5c,omitempty"` // certificate chain (unused) } // JWKS is the container returned by /.well-known/jwks.json. @@ -25,22 +28,38 @@ type JWKS struct { Keys []JWK `json:"keys"` } -// PublicJWK derives a JWK from an RSA public key and a kid. -func PublicJWK(pub *rsa.PublicKey, kid string) (JWK, error) { - if pub == nil || pub.N == nil { - return JWK{}, fmt.Errorf("oidc: nil rsa public key") - } +// PublicJWK derives a JWK from an ECDSA public key and a kid. +// Only P-256 keys (matching ES256) are accepted. +func PublicJWK(pub crypto.PublicKey, kid string) (JWK, error) { if kid == "" { return JWK{}, fmt.Errorf("oidc: empty kid") } - eBytes := bigEndianExponent(pub.E) + ecPub, ok := pub.(*ecdsa.PublicKey) + if !ok || ecPub == nil { + return JWK{}, fmt.Errorf("oidc: PublicJWK requires *ecdsa.PublicKey, got %T", pub) + } + byteLen := (ecPub.Curve.Params().BitSize + 7) / 8 + // Derive the fixed-width, already left-padded coordinates from the + // uncompressed SEC 1 point (0x04 || X || Y) via crypto/ecdh, matching + // ComputeKeyID and avoiding the deprecated ecdsa.PublicKey.X/Y fields. + ecdhKey, err := ecPub.ECDH() + if err != nil { + return JWK{}, fmt.Errorf("oidc: convert ecdsa public key to ecdh: %w", err) + } + uncompressed := ecdhKey.Bytes() + if len(uncompressed) != 1+2*byteLen { + return JWK{}, fmt.Errorf("oidc: unexpected uncompressed point length %d, want %d", len(uncompressed), 1+2*byteLen) + } + xPadded := uncompressed[1 : 1+byteLen] + yPadded := uncompressed[1+byteLen:] return JWK{ - Kty: "RSA", + Kty: "EC", Use: "sig", Alg: Algorithm, Kid: kid, - N: base64.RawURLEncoding.EncodeToString(pub.N.Bytes()), - E: base64.RawURLEncoding.EncodeToString(eBytes), + Crv: "P-256", + X: base64.RawURLEncoding.EncodeToString(xPadded), + Y: base64.RawURLEncoding.EncodeToString(yPadded), }, nil } @@ -62,20 +81,3 @@ func BuildJWKS(ctx context.Context, signer Signer) (JWKS, error) { } return JWKS{Keys: []JWK{jwk}}, nil } - -// bigEndianExponent returns the RSA exponent as the minimal big-endian -// byte slice. RFC 7518 §6.3.1 requires the byte representation with -// leading zero bytes stripped; e=65537 → 0x01 0x00 0x01. -func bigEndianExponent(e int) []byte { - buf := make([]byte, 0, 4) - for shift := 24; shift >= 0; shift -= 8 { - b := byte(e >> shift) // #nosec G115 -- intentional byte extraction: RSA exponent fits in 3 bytes; byte(e>>shift) extracts each octet per RFC 7518 §6.3.1 - if b != 0 || len(buf) > 0 { - buf = append(buf, b) - } - } - if len(buf) == 0 { - buf = []byte{0} - } - return buf -} diff --git a/internal/oidc/signer.go b/internal/oidc/signer.go index 8aebdd94d..29e37aaf3 100644 --- a/internal/oidc/signer.go +++ b/internal/oidc/signer.go @@ -4,7 +4,7 @@ // // The package exposes: // -// - Signer: a cloud-agnostic interface for producing raw RSA-PKCS1v15 +// - Signer: a cloud-agnostic interface for producing ECDSA (ES256) // signatures over a SHA-256 digest. Backed by AWS KMS, Azure Key Vault, // or GCP Cloud KMS depending on where CUDly runs. The private key // never leaves the cloud KMS. @@ -15,33 +15,49 @@ // configured with a federated identity credential pointing at // CUDly's OIDC issuer. // -// - LocalSigner: an in-process RSA signer used only by tests. +// - LocalSigner: an in-process ECDSA signer used only by tests. package oidc import ( "context" "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "crypto/sha256" + "encoding/asn1" "encoding/base64" "encoding/json" "fmt" + "math/big" ) -// Signer abstracts raw RSA-PKCS1v15 signing over a SHA-256 digest. +// p256SigComponentLen is the fixed byte length of each of the R and S +// components of a P-256 (ES256) signature. RFC 7518 section 3.4 requires +// the JWS signature to be the concatenation R || S, each left-padded to +// the curve's octet length (ceil(256/8) = 32 bytes), so the full JWS +// signature is exactly 64 bytes. +const p256SigComponentLen = 32 + +// Signer abstracts ECDSA (ES256) signing over a SHA-256 digest. // Implementations delegate the actual private-key operation to a cloud // KMS so CUDly never handles the private key material. type Signer interface { - // Sign returns the RSA-PKCS1v15 signature over digest (the SHA-256 - // digest of the signing input). The caller is responsible for - // hashing the input; this matches what AWS KMS, Azure Key Vault, - // and GCP Cloud KMS all expect. + // Sign returns the raw fixed-length ECDSA signature over digest in the + // RFC 7518 section 3.4 JWS form: the R || S concatenation, each + // component left-padded to 32 bytes (64 bytes total for P-256). The + // caller is responsible for hashing the input. Backends whose KMS + // returns DER/ASN.1 (AWS, GCP, and the in-process LocalSigner) MUST + // convert via derToRawECDSASignature before returning; Azure Key Vault + // already returns this raw form and passes it through unchanged. This + // lets Mint base64url-encode the result directly into the JWS without + // any per-backend special-casing. Sign(ctx context.Context, digest []byte) ([]byte, error) - // PublicKey returns the RSA public key corresponding to the signer. - // Cached after the first call per implementation. - PublicKey(ctx context.Context) (*rsa.PublicKey, error) + // PublicKey returns the public key corresponding to the signer. + // Cached after the first call per implementation. Returns + // *ecdsa.PublicKey for ES256 signers. + PublicKey(ctx context.Context) (crypto.PublicKey, error) // KeyID returns a stable identifier used as the JWT `kid` header // and as the JWK `kid`. Derived from the public key so Azure AD's @@ -50,14 +66,48 @@ type Signer interface { } // Algorithm is the JWS algorithm used throughout the package. All three -// backends (AWS KMS, Azure Key Vault, GCP Cloud KMS) support RS256 over -// an RSA 2048-bit key, which is also what Azure AD accepts for a -// federated identity credential's client_assertion. -const Algorithm = "RS256" +// backends (AWS KMS, Azure Key Vault, GCP Cloud KMS) support ES256 over +// a P-256 key, which is also what Azure AD accepts for a federated +// identity credential's client_assertion. +const Algorithm = "ES256" + +// derToRawECDSASignature converts a DER/ASN.1-encoded ECDSA signature +// (an ASN.1 SEQUENCE of two INTEGERs R and S, as returned by AWS KMS, +// GCP Cloud KMS, and crypto/ecdsa.SignASN1) into the RFC 7518 section +// 3.4 raw form: R || S, each left-padded with leading zeros to +// p256SigComponentLen bytes. The result is exactly 2*p256SigComponentLen +// (64) bytes for P-256. It returns an error if the input is not a valid +// two-INTEGER SEQUENCE or if R/S exceed the component length. +func derToRawECDSASignature(der []byte) ([]byte, error) { + var sig struct { + R, S *big.Int + } + rest, err := asn1.Unmarshal(der, &sig) + if err != nil { + return nil, fmt.Errorf("oidc: parse DER ecdsa signature: %w", err) + } + if len(rest) != 0 { + return nil, fmt.Errorf("oidc: trailing bytes after DER ecdsa signature") + } + if sig.R == nil || sig.S == nil || sig.R.Sign() <= 0 || sig.S.Sign() <= 0 { + return nil, fmt.Errorf("oidc: DER ecdsa signature has non-positive R or S") + } + rb := sig.R.Bytes() + sb := sig.S.Bytes() + if len(rb) > p256SigComponentLen || len(sb) > p256SigComponentLen { + return nil, fmt.Errorf("oidc: ecdsa signature component exceeds %d bytes (R=%d S=%d); not a P-256 signature", p256SigComponentLen, len(rb), len(sb)) + } + raw := make([]byte, 2*p256SigComponentLen) + copy(raw[p256SigComponentLen-len(rb):p256SigComponentLen], rb) + copy(raw[2*p256SigComponentLen-len(sb):], sb) + return raw, nil +} // Mint produces a compact JWS signed by the given Signer. claims is // serialized as the JWT payload; the header is constructed from the -// Signer's key id plus the RS256 algorithm. +// Signer's key id plus the ES256 algorithm. The Signer.Sign contract +// returns the raw R || S signature (RFC 7518 section 3.4), so Mint +// base64url-encodes it directly into the JWS signature segment. func Mint(ctx context.Context, signer Signer, claims map[string]any) (string, error) { kid, err := signer.KeyID(ctx) if err != nil { @@ -87,26 +137,33 @@ func Mint(ctx context.Context, signer Signer, claims map[string]any) (string, er if err != nil { return "", fmt.Errorf("oidc: sign jwt: %w", err) } + // Enforce the Signer.Sign contract: a conforming ES256 backend returns + // the 64-byte raw R || S signature. Reject anything else (e.g. a DER + // blob that slipped through) rather than emitting a JWS that real OIDC + // consumers like Azure AD would reject. + if len(signature) != 2*p256SigComponentLen { + return "", fmt.Errorf("oidc: signer returned %d-byte signature, want %d-byte raw R||S (RFC 7518 ES256)", len(signature), 2*p256SigComponentLen) + } return signingInput + "." + base64.RawURLEncoding.EncodeToString(signature), nil } -// LocalSigner is an in-process RSA signer used only by tests. Callers +// LocalSigner is an in-process ECDSA signer used only by tests. Callers // must NOT use it outside of test code; real deployments must back the // Signer interface with a cloud KMS so the private key never hits the // CUDly process. type LocalSigner struct { - key *rsa.PrivateKey + key *ecdsa.PrivateKey kid string } -// NewLocalSigner generates a new 2048-bit RSA key and wraps it in a +// NewLocalSigner generates a new P-256 ECDSA key and wraps it in a // test-only Signer. The returned kid is a hash-based identifier stable // across calls for the same key. func NewLocalSigner() (*LocalSigner, error) { - key, err := rsa.GenerateKey(rand.Reader, 2048) + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - return nil, fmt.Errorf("oidc: generate local rsa key: %w", err) + return nil, fmt.Errorf("oidc: generate local ecdsa key: %w", err) } kid, err := ComputeKeyID(&key.PublicKey) if err != nil { @@ -115,13 +172,20 @@ func NewLocalSigner() (*LocalSigner, error) { return &LocalSigner{key: key, kid: kid}, nil } -// Sign signs digest with the test RSA key using PKCS1v15 + SHA-256. +// Sign signs digest with the test ECDSA P-256 key. crypto/ecdsa.SignASN1 +// produces a DER/ASN.1 signature (the same format cloud KMS ECDSA +// operations return), which is converted to the RFC 7518 raw R || S form +// to satisfy the Signer.Sign contract. func (s *LocalSigner) Sign(_ context.Context, digest []byte) ([]byte, error) { - return rsa.SignPKCS1v15(rand.Reader, s.key, crypto.SHA256, digest) + der, err := ecdsa.SignASN1(rand.Reader, s.key, digest) + if err != nil { + return nil, fmt.Errorf("oidc: local ecdsa sign: %w", err) + } + return derToRawECDSASignature(der) } // PublicKey returns the test key's public half. -func (s *LocalSigner) PublicKey(_ context.Context) (*rsa.PublicKey, error) { +func (s *LocalSigner) PublicKey(_ context.Context) (crypto.PublicKey, error) { return &s.key.PublicKey, nil } @@ -130,13 +194,24 @@ func (s *LocalSigner) KeyID(_ context.Context) (string, error) { return s.kid, nil } -// ComputeKeyID returns a stable kid for a public key. Uses the SHA-256 -// of the raw RSA modulus big-endian bytes, base64url-encoded without -// padding. Stable across restarts, new on every key rotation. -func ComputeKeyID(pub *rsa.PublicKey) (string, error) { - if pub == nil || pub.N == nil { - return "", fmt.Errorf("oidc: nil rsa public key") +// ComputeKeyID returns a stable kid for a public key. +// For *ecdsa.PublicKey: SHA-256 of the uncompressed point (0x04 || X || Y), +// base64url-encoded without padding. +// Stable across restarts, new on every key rotation. +func ComputeKeyID(pub crypto.PublicKey) (string, error) { + switch k := pub.(type) { + case *ecdsa.PublicKey: + if k == nil { + return "", fmt.Errorf("oidc: nil ecdsa public key") + } + ecdhKey, err := k.ECDH() + if err != nil { + return "", fmt.Errorf("oidc: convert ecdsa public key to ecdh: %w", err) + } + uncompressed := ecdhKey.Bytes() + sum := sha256.Sum256(uncompressed) + return base64.RawURLEncoding.EncodeToString(sum[:]), nil + default: + return "", fmt.Errorf("oidc: unsupported public key type %T", pub) } - sum := sha256.Sum256(pub.N.Bytes()) - return base64.RawURLEncoding.EncodeToString(sum[:]), nil } diff --git a/internal/oidc/signer_test.go b/internal/oidc/signer_test.go index a8f592634..2759c7be6 100644 --- a/internal/oidc/signer_test.go +++ b/internal/oidc/signer_test.go @@ -2,15 +2,59 @@ package oidc import ( "context" - "crypto" - "crypto/rsa" + "crypto/ecdsa" "crypto/sha256" "encoding/base64" "encoding/json" + "math/big" "strings" "testing" + + "github.com/golang-jwt/jwt/v5" ) +// assertRawES256JWS verifies that jws is a compact JWS whose signature +// segment is the RFC 7518 section 3.4 raw R || S form (exactly 64 bytes +// for P-256) and that it verifies against pub. It checks the signature +// three independent ways so the test fails on the pre-fix DER-emitting +// code: (1) the decoded signature is exactly 64 bytes; (2) the R || S +// split verifies via ecdsa.Verify; (3) a real JOSE/JWT ES256 parser +// (golang-jwt) accepts the token, proving real OIDC consumers like +// Azure AD would too. +func assertRawES256JWS(t *testing.T, jws string, pub *ecdsa.PublicKey) { + t.Helper() + + parts := strings.Split(jws, ".") + if len(parts) != 3 { + t.Fatalf("expected 3 JWS parts, got %d", len(parts)) + } + + sig, err := base64.RawURLEncoding.DecodeString(parts[2]) + if err != nil { + t.Fatalf("decode signature: %v", err) + } + // (1) Raw R || S is exactly 64 bytes for P-256. DER signatures are + // ~70-72 bytes and start with 0x30, so this rejects the pre-fix output. + if len(sig) != 64 { + t.Fatalf("JWS signature is %d bytes, want 64-byte raw R||S (RFC 7518 ES256); first byte 0x%02x", len(sig), sig[0]) + } + + // (2) Split R || S and verify with the public key over the signing input. + r := new(big.Int).SetBytes(sig[:32]) + s := new(big.Int).SetBytes(sig[32:]) + digest := sha256.Sum256([]byte(parts[0] + "." + parts[1])) + if !ecdsa.Verify(pub, digest[:], r, s) { + t.Errorf("raw R||S ECDSA signature verify failed") + } + + // (3) A real JOSE/JWT ES256 parser must accept the token. + if _, err := jwt.Parse(jws, func(*jwt.Token) (any, error) { + return pub, nil + }, jwt.WithValidMethods([]string{"ES256"}), jwt.WithoutClaimsValidation()); err != nil { + t.Errorf("golang-jwt ES256 parse/verify failed: %v", err) + } +} + func TestLocalSignerMintAndVerify(t *testing.T) { ctx := context.Background() signer, err := NewLocalSigner() @@ -40,12 +84,12 @@ func TestLocalSignerMintAndVerify(t *testing.T) { t.Fatalf("decode header: %v", err) } var header map[string]any - err = json.Unmarshal(headerBytes, &header) - if err != nil { - t.Fatalf("unmarshal header: %v", err) + if umErr := json.Unmarshal(headerBytes, &header); umErr != nil { + t.Fatalf("unmarshal header: %v", umErr) } - if header["alg"] != "RS256" { - t.Errorf("alg=%v, want RS256", header["alg"]) + // Positively assert ES256 -- this is the invariant the fix-422 change guards. + if header["alg"] != "ES256" { + t.Errorf("alg=%v, want ES256 (PKCS1v15/RS256 is no longer permitted)", header["alg"]) } if header["typ"] != "JWT" { t.Errorf("typ=%v, want JWT", header["typ"]) @@ -60,28 +104,24 @@ func TestLocalSignerMintAndVerify(t *testing.T) { t.Fatalf("decode claims: %v", err) } var decoded map[string]any - err = json.Unmarshal(claimsBytes, &decoded) - if err != nil { - t.Fatalf("unmarshal claims: %v", err) + if umErr := json.Unmarshal(claimsBytes, &decoded); umErr != nil { + t.Fatalf("unmarshal claims: %v", umErr) } if decoded["iss"] != claims["iss"] { t.Errorf("iss mismatch: %v vs %v", decoded["iss"], claims["iss"]) } - // Verify the signature end-to-end with the signer's public key. - sigBytes, err := base64.RawURLEncoding.DecodeString(parts[2]) - if err != nil { - t.Fatalf("decode signature: %v", err) - } - pub, err := signer.PublicKey(ctx) + // Verify the JWS signature end-to-end. The signature MUST be the + // RFC 7518 section 3.4 raw R || S form (64 bytes for P-256), not DER. + rawPub, err := signer.PublicKey(ctx) if err != nil { t.Fatalf("public key: %v", err) } - signingInput := parts[0] + "." + parts[1] - digest := sha256.Sum256([]byte(signingInput)) - if err := rsa.VerifyPKCS1v15(pub, crypto.SHA256, digest[:], sigBytes); err != nil { - t.Errorf("signature verify failed: %v", err) + ecPub, ok := rawPub.(*ecdsa.PublicKey) + if !ok { + t.Fatalf("public key is not *ecdsa.PublicKey, got %T", rawPub) } + assertRawES256JWS(t, jws, ecPub) } func TestBuildJWKS(t *testing.T) { @@ -98,18 +138,23 @@ func TestBuildJWKS(t *testing.T) { t.Fatalf("want 1 key, got %d", len(jwks.Keys)) } k := jwks.Keys[0] - if k.Kty != "RSA" || k.Use != "sig" || k.Alg != "RS256" { + // Positively assert EC/ES256 -- guards against regression back to RSA/RS256. + if k.Kty != "EC" || k.Use != "sig" || k.Alg != "ES256" { t.Errorf("jwk metadata wrong: %+v", k) } - if k.Kid == "" || k.N == "" || k.E == "" { - t.Errorf("jwk missing kid/n/e: %+v", k) + if k.Crv != "P-256" { + t.Errorf("jwk crv wrong: got %q, want P-256", k.Crv) + } + if k.Kid == "" || k.X == "" || k.Y == "" { + t.Errorf("jwk missing kid/x/y: %+v", k) } - if _, err := base64.RawURLEncoding.DecodeString(k.N); err != nil { - t.Errorf("n not base64url: %v", err) + if _, err := base64.RawURLEncoding.DecodeString(k.X); err != nil { + t.Errorf("x not base64url: %v", err) } - if _, err := base64.RawURLEncoding.DecodeString(k.E); err != nil { - t.Errorf("e not base64url: %v", err) + if _, err := base64.RawURLEncoding.DecodeString(k.Y); err != nil { + t.Errorf("y not base64url: %v", err) } + // RSA fields are structurally absent from the EC JWK type. } func TestBuildDiscovery(t *testing.T) { @@ -120,32 +165,9 @@ func TestBuildDiscovery(t *testing.T) { if d.JWKSURI != "https://cudly.example.com/.well-known/jwks.json" { t.Errorf("jwks_uri=%s", d.JWKSURI) } - if len(d.IDTokenSigningAlgValuesSupported) != 1 || d.IDTokenSigningAlgValuesSupported[0] != "RS256" { - t.Errorf("alg values wrong: %v", d.IDTokenSigningAlgValuesSupported) - } -} - -func TestBigEndianExponent(t *testing.T) { - cases := []struct { - in int - want []byte - }{ - {65537, []byte{0x01, 0x00, 0x01}}, - {3, []byte{0x03}}, - {0, []byte{0x00}}, - {256, []byte{0x01, 0x00}}, - } - for _, c := range cases { - got := bigEndianExponent(c.in) - if len(got) != len(c.want) { - t.Errorf("%d: len %d want %d", c.in, len(got), len(c.want)) - continue - } - for i := range got { - if got[i] != c.want[i] { - t.Errorf("%d: byte %d = 0x%02x want 0x%02x", c.in, i, got[i], c.want[i]) - } - } + // Positively assert ES256 in the discovery document. + if len(d.IDTokenSigningAlgValuesSupported) != 1 || d.IDTokenSigningAlgValuesSupported[0] != "ES256" { + t.Errorf("alg values wrong: %v, want [ES256]", d.IDTokenSigningAlgValuesSupported) } } @@ -166,3 +188,19 @@ func TestComputeKeyIDStableAcrossCalls(t *testing.T) { t.Errorf("kid changed between calls: %s vs %s", a, b) } } + +func TestComputeKeyIDChangesOnKeyRotation(t *testing.T) { + s1, err := NewLocalSigner() + if err != nil { + t.Fatalf("new signer 1: %v", err) + } + s2, err := NewLocalSigner() + if err != nil { + t.Fatalf("new signer 2: %v", err) + } + k1, _ := s1.KeyID(context.Background()) + k2, _ := s2.KeyID(context.Background()) + if k1 == k2 { + t.Errorf("different keys produced the same kid: %s", k1) + } +}