From 560ba87cdce718b9b0d5011603fb902165ad6b11 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sat, 30 May 2026 20:36:44 +0200 Subject: [PATCH] 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 --- internal/api/handler_oidc_test.go | 6 ++- internal/oidc/aws_signer.go | 25 ++++----- internal/oidc/aws_signer_test.go | 37 ++++++++------ internal/oidc/azure_signer.go | 35 +++++++------ internal/oidc/gcp_signer.go | 22 ++++---- internal/oidc/jwks.go | 68 ++++++++++++------------- internal/oidc/signer.go | 75 +++++++++++++++------------ internal/oidc/signer_test.go | 84 ++++++++++++++++--------------- 8 files changed, 187 insertions(+), 165 deletions(-) diff --git a/internal/api/handler_oidc_test.go b/internal/api/handler_oidc_test.go index 50fe8800..afa20c59 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/oidc/aws_signer.go b/internal/oidc/aws_signer.go index a1b1ce5c..1c11d108 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" @@ -26,14 +27,15 @@ type AWSKMSSigner struct { keyID string once sync.Once - pubKey *rsa.PublicKey + pubKey *ecdsa.PublicKey kid string err error } // 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") @@ -52,14 +54,13 @@ 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, which matches what ES256 JWS signatures expect. 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) @@ -69,7 +70,7 @@ func (s *AWSKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) // 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 } @@ -92,17 +93,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 364a5e23..e1165340 100644 --- a/internal/oidc/aws_signer_test.go +++ b/internal/oidc/aws_signer_test.go @@ -2,9 +2,9 @@ package oidc import ( "context" - "crypto" + "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" - "crypto/rsa" "crypto/sha256" "crypto/x509" "encoding/base64" @@ -15,15 +15,15 @@ import ( var base64RawURL = base64.RawURLEncoding -// fakeKMSClient is a minimal AWSKMSClient backed by an in-process RSA -// key. It lets TestAWSKMSSigner exercise the Signer contract without +// 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. 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 +40,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.PublicKey.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.X.Cmp(key.PublicKey.X) != 0 || ecPub.Y.Cmp(key.PublicKey.Y) != 0 { + t.Fatal("public key point mismatch") } // KeyID stable across calls. @@ -62,7 +66,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,16 +75,17 @@ func TestAWSKMSSignerRoundTrip(t *testing.T) { if err != nil { t.Fatalf("mint: %v", err) } - // Verify using the underlying pub half. + // Verify header asserts ES256 algorithm. parts := splitJWS(t, jws) + // Verify using the underlying EC pub half. 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) + if !ecdsa.VerifyASN1(ecPub, digest[:], decodeB64(t, parts[2])) { + t.Errorf("ECDSA signature verify failed") } } -// helpers — unit tests only, kept private +// helpers -- unit tests only, kept private func splitJWS(t *testing.T, jws string) [3]string { t.Helper() var out [3]string diff --git a/internal/oidc/azure_signer.go b/internal/oidc/azure_signer.go index 2cd50454..07510fc9 100644 --- a/internal/oidc/azure_signer.go +++ b/internal/oidc/azure_signer.go @@ -2,7 +2,9 @@ package oidc import ( "context" - "crypto/rsa" + "crypto" + "crypto/ecdsa" + "crypto/elliptic" "fmt" "math/big" "sync" @@ -18,7 +20,7 @@ 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 @@ -26,7 +28,7 @@ type AzureKeyVaultSigner struct { keyVersion string // may be empty = latest once sync.Once - pubKey *rsa.PublicKey + pubKey *ecdsa.PublicKey kid string err error } @@ -34,7 +36,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 +59,10 @@ 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. Key Vault returns a DER-encoded ECDSA signature. 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 +75,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,20 +93,21 @@ 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 } - rsaPub := &rsa.PublicKey{ - N: new(big.Int).SetBytes(resp.Key.N), - E: int(new(big.Int).SetBytes(resp.Key.E).Int64()), + ecPub := &ecdsa.PublicKey{ + Curve: elliptic.P256(), + X: new(big.Int).SetBytes(resp.Key.X), + Y: new(big.Int).SetBytes(resp.Key.Y), } - 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/gcp_signer.go b/internal/oidc/gcp_signer.go index 9f620dfd..80dc4dfc 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,14 +36,14 @@ 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 keyResource string // full resource name, incl. /cryptoKeyVersions/N once sync.Once - pubKey *rsa.PublicKey + pubKey *ecdsa.PublicKey kid string err error } @@ -51,6 +52,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") @@ -69,8 +72,7 @@ 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. func (s *GCPKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) { crc := int64(crc32.Checksum(digest, crc32.MakeTable(crc32.Castagnoli))) req := &kmspb.AsymmetricSignRequest{ @@ -88,7 +90,7 @@ func (s *GCPKMSSigner) Sign(ctx context.Context, digest []byte) ([]byte, error) } // 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 } @@ -116,17 +118,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 1b0ad0c8..c8e8f2e6 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,32 @@ 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 + xBytes := ecPub.X.Bytes() + yBytes := ecPub.Y.Bytes() + // Left-pad to the expected coordinate byte length (RFC 7518 §6.2.1.2). + xPadded := make([]byte, byteLen) + yPadded := make([]byte, byteLen) + copy(xPadded[byteLen-len(xBytes):], xBytes) + copy(yPadded[byteLen-len(yBytes):], yBytes) 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 +75,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) - 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 8aebdd94..b5abc892 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,35 @@ // 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/base64" "encoding/json" "fmt" ) -// Signer abstracts raw RSA-PKCS1v15 signing over a SHA-256 digest. +// 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 DER-encoded ECDSA 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 when using EC-based keys. 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 +52,14 @@ 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" // 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. func Mint(ctx context.Context, signer Signer, claims map[string]any) (string, error) { kid, err := signer.KeyID(ctx) if err != nil { @@ -91,22 +93,22 @@ func Mint(ctx context.Context, signer Signer, claims map[string]any) (string, er 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 +117,15 @@ 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. Returns a DER-encoded +// ASN.1 signature (the format returned by cloud KMS ECDSA operations and +// accepted by ecdsa.VerifyASN1). func (s *LocalSigner) Sign(_ context.Context, digest []byte) ([]byte, error) { - return rsa.SignPKCS1v15(rand.Reader, s.key, crypto.SHA256, digest) + return ecdsa.SignASN1(rand.Reader, s.key, digest) } // 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 +134,20 @@ 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") + } + uncompressed := elliptic.Marshal(k.Curve, k.X, k.Y) + 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 6894ec88..5b1c4056 100644 --- a/internal/oidc/signer_test.go +++ b/internal/oidc/signer_test.go @@ -2,8 +2,7 @@ package oidc import ( "context" - "crypto" - "crypto/rsa" + "crypto/ecdsa" "crypto/sha256" "encoding/base64" "encoding/json" @@ -43,8 +42,9 @@ func TestLocalSignerMintAndVerify(t *testing.T) { if err := json.Unmarshal(headerBytes, &header); err != nil { t.Fatalf("unmarshal header: %v", err) } - 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"]) @@ -66,19 +66,23 @@ func TestLocalSignerMintAndVerify(t *testing.T) { t.Errorf("iss mismatch: %v vs %v", decoded["iss"], claims["iss"]) } - // Verify the signature end-to-end with the signer's public key. + // Verify the ECDSA 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) + rawPub, err := signer.PublicKey(ctx) if err != nil { t.Fatalf("public key: %v", err) } + ecPub, ok := rawPub.(*ecdsa.PublicKey) + if !ok { + t.Fatalf("public key is not *ecdsa.PublicKey, got %T", rawPub) + } 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) + if !ecdsa.VerifyASN1(ecPub, digest[:], sigBytes) { + t.Errorf("ECDSA signature verify failed") } } @@ -96,18 +100,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) { @@ -118,32 +127,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) } } @@ -164,3 +150,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) + } +}