oidc: ignore JWKs with unknown signing algorithms rather than failing

Fixes #490

Author: ericchiang

oidc/jose.go

@@ -30,3 +30,11 @@ var allAlgs = []jose.SignatureAlgorithm{
 	jose.PS512,
 	jose.EdDSA,
 }
+
+var supportedJOSEAlgs = map[string]bool{}
+
+func init() {
+	for _, alg := range allAlgs {
+		supportedJOSEAlgs[string(alg)] = true
+	}
+}

oidc/jwks.go

@@ -6,6 +6,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/rsa"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -235,6 +236,55 @@ func (r *RemoteKeySet) keysFromRemote(ctx context.Context) ([]jose.JSONWebKey, e
 	}
 }
 
+// jwkJSON implements custom logic for unmarshaling JSON Web Key Sets.
+type jwkJSON struct {
+	Keys []jose.JSONWebKey
+}
+
+func (j *jwkJSON) UnmarshalJSON(data []byte) error {
+	var raw struct {
+		Keys []json.RawMessage `json:"keys"`
+	}
+	if err := json.Unmarshal(data, &raw); err != nil {
+		return err
+	}
+	// For each key, attempt to determine if the algorithm is supported before
+	// unmarshaling the key. This allows us to ignore keys with unsupported
+	// algorithms, rather than failing to load the entire set.
+	for _, key := range raw.Keys {
+		var metadata struct {
+			Alg string `json:"alg"`
+		}
+		if err := json.Unmarshal(key, &metadata); err != nil {
+			return err
+		}
+		if metadata.Alg != "" {
+			// Algorithms are technically optional, but if the key advertises an
+			// algorithm we don't support, ignore it rather than failing to load
+			// the entire key set.
+			//
+			// https://datatracker.ietf.org/doc/html/rfc7517#section-4.4
+			//
+			// This skip is currently implemented due to secp256k1, which some
+			// providers use for signing with "ES256K". While we could also
+			// ignore the curve, this seems like a more general check in case
+			// providers start throwing in post-quantum algorithims or something
+			// like that.
+			//
+			// https://github.com/coreos/go-oidc/issues/490
+			if !supportedJOSEAlgs[metadata.Alg] {
+				continue
+			}
+		}
+		var jwk jose.JSONWebKey
+		if err := json.Unmarshal(key, &jwk); err != nil {
+			return err
+		}
+		j.Keys = append(j.Keys, jwk)
+	}
+	return nil
+}
+
 func (r *RemoteKeySet) updateKeys() ([]jose.JSONWebKey, error) {
 	req, err := http.NewRequest("GET", r.jwksURL, nil)
 	if err != nil {
@@ -257,7 +307,7 @@ func (r *RemoteKeySet) updateKeys() ([]jose.JSONWebKey, error) {
 		return nil, fmt.Errorf("oidc: get keys failed: %s %s", resp.Status, body)
 	}
 
-	var keySet jose.JSONWebKeySet
+	var keySet jwkJSON
 	err = unmarshalResp(resp, body, &keySet)
 	if err != nil {
 		return nil, fmt.Errorf("oidc: failed to decode keys: %v %s", err, body)

oidc/jwks_test.go

@@ -362,3 +362,67 @@ func BenchmarkVerify(b *testing.B) {
 		}
 	}
 }
+
+func TestJWKParseWithIgnoredKeys(t *testing.T) {
+	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	pubKey := jose.JSONWebKey{
+		Key:       priv.Public(),
+		Use:       "sig",
+		Algorithm: string(jose.ES256),
+		KeyID:     "key1",
+	}
+	pubRaw, err := json.Marshal(pubKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	privKey := jose.JSONWebKey{
+		Key:       priv,
+		Use:       "sig",
+		Algorithm: string(jose.ES256),
+		KeyID:     "key1",
+	}
+
+	hf := func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		io.WriteString(w, `{
+			"keys": [
+				`+string(pubRaw)+`,
+				{
+					"alg": "ES256K",
+					"kty": "EC",
+					"x": "vsk5i5YJu8H_VPL7DWTgVGXBPrqgkyNmYvfgOrVut38",
+					"y": "Mrg56tBhVeorHPXK1LbTX2jP7rEqOHIatM96HFzVMIU",
+					"crv": "secp256k1",
+					"kid": "oidc-es256k-1",
+					"use": "sig"
+				}
+			]
+		}`)
+	}
+
+	ks := NewRemoteKeySet(t.Context(), httptest.NewServer(http.HandlerFunc(hf)).URL)
+
+	signer, err := jose.NewSigner(jose.SigningKey{Algorithm: jose.ES256, Key: &privKey}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	jws, err := signer.Sign([]byte("payload"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	serialized, err := jws.CompactSerialize()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	got, err := ks.VerifySignature(t.Context(), serialized)
+	if err != nil {
+		t.Fatalf("failed to verify signature: %v", err)
+	}
+	if string(got) != "payload" {
+		t.Errorf("expected payload %q got %q", "payload", string(got))
+	}
+}