From 10b406941ed75584cdef8d28c6f95ec19b55e6b6 Mon Sep 17 00:00:00 2001 From: sergioperezcheco Date: Wed, 1 Jul 2026 23:52:22 +0800 Subject: [PATCH] fix: gracefully skip JWKS keys with unsupported elliptic curves When a JWKS endpoint returns keys with curves not supported by go-jose (e.g. secp256k1 used by Telegram), the entire key set unmarshal fails, breaking OIDC verification for all providers that use mixed key types. Add a fallback in RemoteKeySet.updateKeys() that parses each key individually when the full set unmarshal fails, skipping any keys that can't be decoded. This allows providers like Telegram (whose JWKS mixes RS256 and ES256K keys) to work as long as the token's signing key uses a supported curve. Includes tests for both the mixed-key and all-unsupported-curve cases. Closes ory/kratos#4572 --- oidc/jwks.go | 40 ++++++++++++++++++++- oidc/jwks_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/oidc/jwks.go b/oidc/jwks.go index 6a846ec..da7dc83 100644 --- a/oidc/jwks.go +++ b/oidc/jwks.go @@ -6,6 +6,7 @@ import ( "crypto/ecdsa" "crypto/ed25519" "crypto/rsa" + "encoding/json" "errors" "fmt" "io" @@ -263,7 +264,44 @@ func (r *RemoteKeySet) updateKeys() ([]jose.JSONWebKey, error) { var keySet jose.JSONWebKeySet err = unmarshalResp(resp, body, &keySet) if err != nil { - return nil, fmt.Errorf("oidc: failed to decode keys: %v %s", err, body) + // If unmarshaling the full key set fails (e.g. due to unsupported + // elliptic curves like secp256k1), fall back to parsing each key + // individually and skipping any that can't be decoded. This allows + // providers like Telegram (whose JWKS mixes RS256 and ES256K keys) + // to work as long as the token's signing key uses a supported curve. + keys, fallbackErr := unmarshalKeysIndividually(body) + if fallbackErr != nil { + return nil, fmt.Errorf("oidc: failed to decode keys: %v %s", err, body) + } + return keys, nil } return keySet.Keys, nil } + +// unmarshalKeysIndividually parses a JWKS JSON body one key at a time, +// silently skipping any individual key that fails to decode (e.g. keys +// using unsupported elliptic curves). This is a fallback for providers +// whose JWKS contains keys with curves not supported by go-jose. +func unmarshalKeysIndividually(body []byte) ([]jose.JSONWebKey, error) { + var raw struct { + Keys []json.RawMessage `json:"keys"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, err + } + + var keys []jose.JSONWebKey + for _, rawKey := range raw.Keys { + var key jose.JSONWebKey + if err := json.Unmarshal(rawKey, &key); err != nil { + // Skip keys with unsupported curves or other parse errors. + continue + } + keys = append(keys, key) + } + + if len(keys) == 0 { + return nil, fmt.Errorf("oidc: no supported keys found in JWK set") + } + return keys, nil +} diff --git a/oidc/jwks_test.go b/oidc/jwks_test.go index 2cbf38b..1974402 100644 --- a/oidc/jwks_test.go +++ b/oidc/jwks_test.go @@ -289,6 +289,96 @@ func TestRotation(t *testing.T) { } } +// TestUnsupportedCurveFallback verifies that when a JWKS contains keys with +// unsupported elliptic curves (e.g. secp256k1), the fetcher gracefully skips +// those keys and still returns the supported ones. This mirrors the real-world +// scenario with Telegram's OIDC provider which mixes RS256 and ES256K keys. +func TestUnsupportedCurveFallback(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Create a valid RSA key that should still work. + validKey := newRSAKey(t) + validKey.keyID = "oidc-1" + + payload := []byte("a secret") + jws, err := jose.ParseSigned(validKey.sign(t, payload), allAlgs) + if err != nil { + t.Fatal(err) + } + + // Build a JWKS JSON that includes both a valid RSA key and an + // unsupported secp256k1 key (like Telegram's JWKS). + rsaJWK := validKey.jwk() + rsaJSON, err := json.Marshal(rsaJWK) + if err != nil { + t.Fatal(err) + } + + unsupportedKeyJSON := `{ + "kty": "EC", + "crv": "secp256k1", + "kid": "oidc-es256k-1", + "x": "abc123", + "y": "def456", + "use": "sig" + }` + + jwksJSON := fmt.Sprintf(`{"keys":[%s,%s]}`, rsaJSON, unsupportedKeyJSON) + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(jwksJSON)) + })) + defer s.Close() + + rks := newRemoteKeySet(ctx, s.URL, nil) + + // The RSA key should still verify even though the JWKS contains an + // unsupported secp256k1 key. + gotPayload, err := rks.verify(ctx, jws) + if err != nil { + t.Fatalf("failed to verify valid signature with unsupported curve present: %v", err) + } + if !bytes.Equal(gotPayload, payload) { + t.Errorf("expected payload %s got %s", payload, gotPayload) + } +} + +// TestUnsupportedCurveFallbackAllUnsupported verifies that when ALL keys in a +// JWKS use unsupported curves, the fetcher returns an error rather than +// silently returning an empty key set. +func TestUnsupportedCurveFallbackAllUnsupported(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + jwksJSON := `{"keys":[ + {"kty":"EC","crv":"secp256k1","kid":"k1","x":"abc","y":"def","use":"sig"}, + {"kty":"EC","crv":"secp256k1","kid":"k2","x":"ghi","y":"jkl","use":"sig"} + ]}` + + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(jwksJSON)) + })) + defer s.Close() + + rks := newRemoteKeySet(ctx, s.URL, nil) + + // Attempting to verify should fail because no keys could be parsed. + payload := []byte("test") + key := newRSAKey(t) + jws, err := jose.ParseSigned(key.sign(t, payload), allAlgs) + if err != nil { + t.Fatal(err) + } + + _, err = rks.verify(ctx, jws) + if err == nil { + t.Fatal("expected error when all keys use unsupported curves, got nil") + } +} + func BenchmarkVerify(b *testing.B) { ctx, cancel := context.WithCancel(context.Background()) defer cancel()