From a475617ec663525e507b0c79f9c1a6230893412d Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 13 Aug 2026 16:47:25 +0100 Subject: [PATCH] R65: validate ES client-assertion key curve matches alg --- client_assertion.go | 24 +++++++++++++++++++++- review_r65_es_curve_test.go | 40 +++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 review_r65_es_curve_test.go diff --git a/client_assertion.go b/client_assertion.go index cbfd253..0502678 100644 --- a/client_assertion.go +++ b/client_assertion.go @@ -3,6 +3,7 @@ package traefikoidc import ( "crypto" "crypto/ecdsa" + "crypto/elliptic" "crypto/rand" "crypto/rsa" "crypto/sha256" @@ -98,9 +99,30 @@ func validateAlgKeyMatch(alg string, key crypto.PrivateKey) error { return fmt.Errorf("alg %q requires an RSA key, got %T", alg, key) } case 'E': // ES* - if _, ok := key.(*ecdsa.PrivateKey); !ok { + ecKey, ok := key.(*ecdsa.PrivateKey) + if !ok { return fmt.Errorf("alg %q requires an EC key, got %T", alg, key) } + // ES* maps 1:1 to a NIST curve (ES256→P-256, ES384→P-384, + // ES512→P-521). A key on the wrong curve would otherwise pass this + // check yet always produce a signature the IdP rejects. + if want := ecdsaAlgorithmCurve(alg); want != nil && ecKey.Curve != want { + return fmt.Errorf("alg %q requires a key on curve %s, got %s", alg, want.Params().Name, ecKey.Curve.Params().Name) + } + } + return nil +} + +// ecdsaAlgorithmCurve returns the NIST curve required by an ES* algorithm, or +// nil for any non-ES algorithm. +func ecdsaAlgorithmCurve(alg string) elliptic.Curve { + switch alg { + case "ES256": + return elliptic.P256() + case "ES384": + return elliptic.P384() + case "ES512": + return elliptic.P521() } return nil } diff --git a/review_r65_es_curve_test.go b/review_r65_es_curve_test.go new file mode 100644 index 0000000..e63075d --- /dev/null +++ b/review_r65_es_curve_test.go @@ -0,0 +1,40 @@ +package traefikoidc + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "encoding/pem" + "testing" +) + +// TestClientAssertionESAlgCurveMatch ensures the client-assertion signer +// validates that an ES* key is on the NIST curve the algorithm requires +// (ES256→P-256, ES384→P-384, ES512→P-521). Previously any EC key passed +// construction regardless of curve, so a wrong-curve key built successfully +// yet always produced a signature the IdP rejected (deterministic +// client-authentication failure). +func TestClientAssertionESAlgCurveMatch(t *testing.T) { + ecKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + der, err := x509.MarshalECPrivateKey(ecKey) + if err != nil { + t.Fatal(err) + } + pemBytes := pem.EncodeToMemory(&pem.Block{Type: "EC PRIVATE KEY", Bytes: der}) + + t.Run("ES512 with P-256 key rejected", func(t *testing.T) { + if _, err := NewClientAssertionSigner(pemBytes, "ES512", "k"); err == nil { + t.Fatal("expected error for ES512 with a P-256 key") + } + }) + + t.Run("ES256 with P-256 key accepted", func(t *testing.T) { + if _, err := NewClientAssertionSigner(pemBytes, "ES256", "k"); err != nil { + t.Fatalf("ES256 with P-256 key should build: %v", err) + } + }) +}