diff --git a/internal/oidc/aws_signer.go b/internal/oidc/aws_signer.go index 3e1b9c435..ebc98dabe 100644 --- a/internal/oidc/aws_signer.go +++ b/internal/oidc/aws_signer.go @@ -4,6 +4,7 @@ import ( "context" "crypto" "crypto/ecdsa" + "crypto/elliptic" "crypto/x509" "fmt" "sync" @@ -99,6 +100,10 @@ func (s *AWSKMSSigner) resolveOnce(ctx context.Context) { s.err = fmt.Errorf("oidc: kms key is not ECDSA (got %T); key must be ECC_NIST_P256", pub) return } + if ecPub.Curve != elliptic.P256() { + s.err = fmt.Errorf("oidc: kms key curve is %s; key must be ECC_NIST_P256 (ES256)", ecPub.Curve.Params().Name) + return + } kid, err := ComputeKeyID(ecPub) if err != nil { s.err = err diff --git a/internal/oidc/aws_signer_test.go b/internal/oidc/aws_signer_test.go index 788f213c8..4399b0651 100644 --- a/internal/oidc/aws_signer_test.go +++ b/internal/oidc/aws_signer_test.go @@ -75,3 +75,21 @@ func TestAWSKMSSignerRoundTrip(t *testing.T) { // convert it to the RFC 7518 raw R||S form before Mint encodes it. assertRawES256JWS(t, jws, ecPub) } + +// TestAWSKMSSignerRejectsWrongCurve guards against a misconfigured KMS +// key (e.g. ECC_NIST_P384 instead of the required ECC_NIST_P256) being +// accepted silently. resolveOnce must fail fast with a clear error +// instead of caching a wrong-curve public key that only breaks later +// at JWKS-publish or signing time. +func TestAWSKMSSignerRejectsWrongCurve(t *testing.T) { + ctx := context.Background() + key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + if err != nil { + t.Fatalf("gen p384 key: %v", err) + } + signer := NewAWSKMSSignerFromClient(&fakeKMSClient{key: key}, "alias/wrong-curve") + + if _, err := signer.PublicKey(ctx); err == nil { + t.Fatal("PublicKey accepted a P-384 KMS key; want an error rejecting the wrong curve") + } +} diff --git a/internal/oidc/backend_jws_test.go b/internal/oidc/backend_jws_test.go index ec94f30c6..afa29063b 100644 --- a/internal/oidc/backend_jws_test.go +++ b/internal/oidc/backend_jws_test.go @@ -72,6 +72,24 @@ func TestGCPKMSSignerEmitsRawES256(t *testing.T) { assertRawES256JWS(t, jws, &key.PublicKey) } +// TestGCPKMSSignerRejectsWrongCurve guards against a misconfigured KMS +// key (e.g. EC_SIGN_P384_SHA384 instead of the required +// EC_SIGN_P256_SHA256) being accepted silently. resolveOnce must fail +// fast instead of caching a wrong-curve public key. +func TestGCPKMSSignerRejectsWrongCurve(t *testing.T) { + ctx := context.Background() + key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + if err != nil { + t.Fatalf("gen p384 key: %v", err) + } + signer := NewGCPKMSSignerFromClient(&fakeGCPKMSClient{key: key}, + "projects/p/locations/global/keyRings/r/cryptoKeys/k/cryptoKeyVersions/1") + + if _, err := signer.PublicKey(ctx); err == nil { + t.Fatal("PublicKey accepted a P-384 KMS key; want an error rejecting the wrong curve") + } +} + // --- Azure: raw R||S path --- type fakeAzureKVClient struct { diff --git a/internal/oidc/gcp_signer.go b/internal/oidc/gcp_signer.go index 554d09835..a132f8a87 100644 --- a/internal/oidc/gcp_signer.go +++ b/internal/oidc/gcp_signer.go @@ -4,6 +4,7 @@ import ( "context" "crypto" "crypto/ecdsa" + "crypto/elliptic" "crypto/x509" "encoding/pem" "fmt" @@ -125,6 +126,10 @@ func (s *GCPKMSSigner) resolveOnce(ctx context.Context) { s.err = fmt.Errorf("oidc: gcp kms key is not ECDSA (got %T); key must be EC_SIGN_P256_SHA256", pub) return } + if ecPub.Curve != elliptic.P256() { + s.err = fmt.Errorf("oidc: gcp kms key curve is %s; key must be EC_SIGN_P256_SHA256 (ES256)", ecPub.Curve.Params().Name) + return + } kid, err := ComputeKeyID(ecPub) if err != nil { s.err = err diff --git a/internal/oidc/jwks.go b/internal/oidc/jwks.go index 91a0d9f51..20398bf94 100644 --- a/internal/oidc/jwks.go +++ b/internal/oidc/jwks.go @@ -4,6 +4,7 @@ import ( "context" "crypto" "crypto/ecdsa" + "crypto/elliptic" "encoding/base64" "fmt" ) @@ -38,6 +39,14 @@ func PublicJWK(pub crypto.PublicKey, kid string) (JWK, error) { if !ok || ecPub == nil { return JWK{}, fmt.Errorf("oidc: PublicJWK requires *ecdsa.PublicKey, got %T", pub) } + // Fail loud on a wrong-curve key instead of publishing a corrupt JWK: + // the Crv/Alg fields below are hardcoded to "P-256"/ES256, so a + // misconfigured P-384 (or other) key would otherwise be mislabeled + // as P-256 with coordinates of the wrong length, and only surface as + // a KMS signing failure far from the actual misconfiguration. + if ecPub.Curve != elliptic.P256() { + return JWK{}, fmt.Errorf("oidc: PublicJWK requires a P-256 key (ES256), got curve %s", ecPub.Curve.Params().Name) + } 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 diff --git a/internal/oidc/signer_test.go b/internal/oidc/signer_test.go index 2759c7be6..2bc51d0e9 100644 --- a/internal/oidc/signer_test.go +++ b/internal/oidc/signer_test.go @@ -3,6 +3,8 @@ package oidc import ( "context" "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" "crypto/sha256" "encoding/base64" "encoding/json" @@ -157,6 +159,23 @@ func TestBuildJWKS(t *testing.T) { // RSA fields are structurally absent from the EC JWK type. } +// TestPublicJWKRejectsWrongCurve guards against a misconfigured +// non-P-256 KMS/Key Vault key (e.g. P-384) silently producing a +// corrupt JWK: pre-fix, PublicJWK hardcoded Crv/Alg to "P-256"/ES256 +// regardless of ecPub.Curve, so a P-384 key would be published as a +// mislabeled P-256 JWK with 48-byte (not 32-byte) coordinates, and the +// misconfiguration would only surface later as an opaque KMS signing +// failure. Post-fix, PublicJWK must reject it immediately. +func TestPublicJWKRejectsWrongCurve(t *testing.T) { + key, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + if err != nil { + t.Fatalf("gen p384 key: %v", err) + } + if _, err := PublicJWK(&key.PublicKey, "some-kid"); err == nil { + t.Fatal("PublicJWK accepted a P-384 key; want an error rejecting the wrong curve") + } +} + func TestBuildDiscovery(t *testing.T) { d := BuildDiscovery("https://cudly.example.com") if d.Issuer != "https://cudly.example.com" { diff --git a/specs/azure-wif-redesign.md b/specs/azure-wif-redesign.md index 27019dbe5..b2732781d 100644 --- a/specs/azure-wif-redesign.md +++ b/specs/azure-wif-redesign.md @@ -37,7 +37,7 @@ True Azure federation, where CUDly stores **no** Azure-specific secret: 1. CUDly's own AWS Lambda exposes OIDC discovery + JWKS endpoints (`/.well-known/openid-configuration`, `/.well-known/jwks.json`) on its Function URL. The JWKS publishes the public half of an AWS KMS asymmetric - signing key (RSA_2048, SIGN_VERIFY). The private half never leaves KMS. + signing key (ECC_NIST_P256, SIGN_VERIFY). The private half never leaves KMS. 2. The Azure AD App Registration gets a **federated identity credential** (`az ad app federated-credential create`) pointing at CUDly's OIDC issuer with `subject=cudly-controller` and `audience=api://AzureADTokenExchange`. @@ -57,7 +57,7 @@ No PEMs, no passwords, no symmetric secrets. │ │ │ ┌────────────────────┐ kms:Sign ┌────────────────────────┐ │ │ │ CUDly Lambda │◀─────────────▶│ KMS asymmetric │ │ - │ │ (api handler) │ GetPublicKey │ (RSA_2048 SIGN_VERIFY) │ │ + │ │ (api handler) │ GetPublicKey │ (ECC_NIST_P256 SIGN) │ │ │ └────────────────────┘ └────────────────────────┘ │ │ │ │ │ │ serves on Function URL: │ @@ -92,19 +92,20 @@ One commit per layer, each independently reviewable. asymmetric key, alias, IAM policy on the Lambda role for `kms:Sign` / `kms:GetPublicKey`, new Lambda env var `CUDLY_SIGNING_KEY_ID`. **DONE**. 3. **`internal/oidc/signer.go`** — cloud-agnostic `Signer` interface: - - `Sign(ctx, digest []byte) ([]byte, error)` — raw RSA-PKCS1v15 over - SHA-256 digest. Called by the JWT minter. - - `PublicKey(ctx) (*rsa.PublicKey, error)` — used to build the JWK and - to compute a stable `kid`. - - `Algorithm() string` — currently always `RS256`. + - `Sign(ctx, digest []byte) ([]byte, error)` — raw ECDSA over a + SHA-256 digest, converted to the RFC 7518 section 3.4 raw R||S + form. Called by the JWT minter. + - `PublicKey(ctx) (*ecdsa.PublicKey, error)` — used to build the JWK + and to compute a stable `kid`. + - `Algorithm() string` — currently always `ES256`. Backed by three implementations: - `AWSKMSSigner` — `aws-sdk-go-v2/service/kms`, `kms:Sign` with - `RSASSA_PKCS1_V1_5_SHA_256`, `kms:GetPublicKey` once at startup. - - `AzureKeyVaultSigner` — `azkeys.Client.Sign` with `RS256`, + `ECDSA_SHA_256`, `kms:GetPublicKey` once at startup. + - `AzureKeyVaultSigner` — `azkeys.Client.Sign` with `ES256`, `GetKey` to read the public half. - `GCPKMSSigner` — `cloud.google.com/go/kms/apiv1`, `AsymmetricSign` with digest SHA-256, `GetPublicKey` once at startup. - Plus a `LocalSigner` (in-process RSA key) used only in tests. + Plus a `LocalSigner` (in-process P-256 ECDSA key) used only in tests. 4. **`internal/oidc/jwt.go`** — cloud-agnostic JWT minter that takes a `Signer` and a `jwt.MapClaims`, builds the header (with the Signer's `kid`), composes `base64url(header).base64url(claims)`, hashes it, diff --git a/terraform/modules/compute/aws/lambda/signing-key.tf b/terraform/modules/compute/aws/lambda/signing-key.tf index ffba769c9..8e5d7dec7 100644 --- a/terraform/modules/compute/aws/lambda/signing-key.tf +++ b/terraform/modules/compute/aws/lambda/signing-key.tf @@ -6,8 +6,8 @@ # without CUDly holding any long-lived secret. resource "aws_kms_key" "signing" { - description = "CUDly ${var.stack_name} OIDC issuer signing key (RSA_2048, RS256)" - customer_master_key_spec = "RSA_2048" + description = "CUDly ${var.stack_name} OIDC issuer signing key (ECC_NIST_P256, ES256)" + customer_master_key_spec = "ECC_NIST_P256" key_usage = "SIGN_VERIFY" deletion_window_in_days = 7 enable_key_rotation = false # KMS asymmetric keys do not support automatic rotation diff --git a/terraform/modules/compute/gcp/cloud-run/signing-key.tf b/terraform/modules/compute/gcp/cloud-run/signing-key.tf index 33965cc2d..1f31809c1 100644 --- a/terraform/modules/compute/gcp/cloud-run/signing-key.tf +++ b/terraform/modules/compute/gcp/cloud-run/signing-key.tf @@ -28,7 +28,7 @@ resource "google_kms_crypto_key" "signing" { destroy_scheduled_duration = "86400s" # 1 day — tests redeploy often version_template { - algorithm = "RSA_SIGN_PKCS1_2048_SHA256" + algorithm = "EC_SIGN_P256_SHA256" protection_level = "SOFTWARE" } diff --git a/terraform/modules/secrets/azure/signing-key.tf b/terraform/modules/secrets/azure/signing-key.tf index d903c5e64..a23fe77e5 100644 --- a/terraform/modules/secrets/azure/signing-key.tf +++ b/terraform/modules/secrets/azure/signing-key.tf @@ -12,13 +12,13 @@ resource "azurerm_role_assignment" "current_user_crypto_officer" { resource "azurerm_key_vault_key" "signing" { name = "cudly-oidc-signing" key_vault_id = azurerm_key_vault.main.id - key_type = "RSA" - key_size = 2048 + key_type = "EC" + curve = "P-256" # Only the operations the Signer actually needs. Sign covers the # kms-equivalent signing op; get is consulted at Signer startup to # derive the JWK. verify is a no-op for CUDly but is conventional - # for RSA signing keys. + # for EC signing keys. key_opts = [ "sign", "verify",