From 54d66dd7c15d65c59a6356b4bffbf16c6aaf3904 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 21 Jul 2026 21:58:02 +0200 Subject: [PATCH 1/2] fix(iac): provision EC P-256 signing keys to match ES256 OIDC code PR #882 switched the OIDC issuer (JWKS + client-assertion signing) to require ECDSA P-256 (ES256) keys and reject any non-ECDSA key at resolve time. The Terraform modules that provision the actual signing key still specified RSA, so a fresh deploy would provision a key the Go code immediately rejects, taking down the OIDC issuer. Update all three provisioning modules to match: - AWS: aws_kms_key customer_master_key_spec -> ECC_NIST_P256 - GCP: google_kms_crypto_key version_template.algorithm -> EC_SIGN_P256_SHA256 - Azure: azurerm_key_vault_key key_type -> EC with curve = P-256 Also update the design spec (specs/azure-wif-redesign.md) to describe the ES256/EC design instead of the superseded RSA/RS256 plan. Changing the key spec forces key replacement on the next terraform apply (new kid), which is expected. --- specs/azure-wif-redesign.md | 21 ++++++++++--------- .../modules/compute/aws/lambda/signing-key.tf | 4 ++-- .../compute/gcp/cloud-run/signing-key.tf | 2 +- .../modules/secrets/azure/signing-key.tf | 6 +++--- 4 files changed, 17 insertions(+), 16 deletions(-) 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", From 03c3c9cede2fa56359a5e71381b09191f81924ce Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 21 Jul 2026 21:58:35 +0200 Subject: [PATCH 2/2] fix(oidc): reject non-P-256 ECDSA keys instead of publishing a corrupt JWK PublicJWK's doc comment claimed only P-256 keys are accepted, but the code accepted any *ecdsa.PublicKey and unconditionally hardcoded Crv: "P-256" / Alg: "ES256" in the output. A misconfigured wrong-curve key (e.g. P-384) would be published through /.well-known/jwks.json as a mislabeled P-256 JWK with actually-longer coordinates, and the real failure would only surface later at the KMS signing call, far from the misconfiguration's cause. Check ecPub.Curve against elliptic.P256() and return an explicit error immediately in PublicJWK, and in the AWS KMS / GCP KMS signers' resolveOnce so a wrong-curve key fails fast with a clear message at key resolution instead of producing a corrupt public JWKS. (Azure Key Vault's resolveOnce already forces P-256 when parsing the raw point, so it needed no change.) Adds regression tests generating a P-384 key and asserting each path returns an error; verified failing pre-fix and passing post-fix. --- internal/oidc/aws_signer.go | 5 +++++ internal/oidc/aws_signer_test.go | 18 ++++++++++++++++++ internal/oidc/backend_jws_test.go | 18 ++++++++++++++++++ internal/oidc/gcp_signer.go | 5 +++++ internal/oidc/jwks.go | 9 +++++++++ internal/oidc/signer_test.go | 19 +++++++++++++++++++ 6 files changed, 74 insertions(+) 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" {