From a55209ad24c6c6043db65a215403b62246c98529 Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Mon, 31 Oct 2022 09:54:32 -0400 Subject: [PATCH 01/13] Add alg check --- jwks.go | 44 ++++++++++++++++++++++++++++++-------------- keyfunc.go | 2 +- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/jwks.go b/jwks.go index 9a429bf..44627d6 100644 --- a/jwks.go +++ b/jwks.go @@ -8,9 +8,15 @@ import ( "net/http" "sync" "time" + + "github.com/golang-jwt/jwt/v4" ) var ( + // ErrJWKAlgMismatch indicates that the given JWK was found, but its "alg" parameter's value did not match that of + // the JWT. + ErrJWKAlgMismatch = errors.New(`the given JWK was found, but its "alg" parameter's value did not match the expected algorithm`) + // ErrJWKUseWhitelist indicates that the given JWK was found, but its "use" parameter's value was not whitelisted. ErrJWKUseWhitelist = errors.New(`the given JWK was found, but its "use" parameter's value was not whitelisted`) @@ -39,21 +45,23 @@ type JWKUse string // jsonWebKey represents a JSON Web Key inside a JWKS. type jsonWebKey struct { - Curve string `json:"crv"` - Exponent string `json:"e"` - K string `json:"k"` - ID string `json:"kid"` - Modulus string `json:"n"` - Type string `json:"kty"` - Use string `json:"use"` - X string `json:"x"` - Y string `json:"y"` + Algorithm string `json:"alg"` + Curve string `json:"crv"` + Exponent string `json:"e"` + K string `json:"k"` + ID string `json:"kid"` + Modulus string `json:"n"` + Type string `json:"kty"` + Use string `json:"use"` + X string `json:"x"` + Y string `json:"y"` } // parsedJWK represents a JSON Web Key parsed with fields as the correct Go types. type parsedJWK struct { - use JWKUse - public interface{} + algorithm string + public interface{} + use JWKUse } // JWKS represents a JSON Web Key Set (JWK Set). @@ -124,8 +132,9 @@ func NewJSON(jwksBytes json.RawMessage) (jwks *JWKS, err error) { } jwks.keys[key.ID] = parsedJWK{ - use: JWKUse(key.Use), - public: keyInter, + algorithm: key.Algorithm, + use: JWKUse(key.Use), + public: keyInter, } } @@ -181,7 +190,7 @@ func (j *JWKS) ReadOnlyKeys() map[string]interface{} { } // getKey gets the jsonWebKey from the given KID from the JWKS. It may refresh the JWKS if configured to. -func (j *JWKS) getKey(kid string) (jsonKey interface{}, err error) { +func (j *JWKS) getKey(kid string, token *jwt.Token) (jsonKey interface{}, err error) { j.mux.RLock() pubKey, ok := j.keys[kid] j.mux.RUnlock() @@ -221,5 +230,12 @@ func (j *JWKS) getKey(kid string) (jsonKey interface{}, err error) { } } + tokenAlg, ok := token.Header["alg"].(string) + if ok { + if pubKey.algorithm != "" && pubKey.algorithm != tokenAlg { + return nil, fmt.Errorf(`%w: JWK "alg" parameter value %q does not match token "alg" parameter value %q`, ErrJWKAlgMismatch, pubKey.algorithm, tokenAlg) + } + } + return pubKey.public, nil } diff --git a/keyfunc.go b/keyfunc.go index f74f2fa..6bb342b 100644 --- a/keyfunc.go +++ b/keyfunc.go @@ -25,7 +25,7 @@ func (j *JWKS) Keyfunc(token *jwt.Token) (interface{}, error) { return nil, fmt.Errorf("%w: could not convert kid in JWT header to string", ErrKID) } - return j.getKey(kid) + return j.getKey(kid, token) } // base64urlTrailingPadding removes trailing padding before decoding a string from base64url. Some non-RFC compliant From fd60eda8da74fdfc0bd47302d50b26e2f4abb71a Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Mon, 31 Oct 2022 17:09:55 -0400 Subject: [PATCH 02/13] Make corrections to alg check --- jwks.go | 11 +++-------- keyfunc.go | 7 ++++++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/jwks.go b/jwks.go index 44627d6..a13bb38 100644 --- a/jwks.go +++ b/jwks.go @@ -8,8 +8,6 @@ import ( "net/http" "sync" "time" - - "github.com/golang-jwt/jwt/v4" ) var ( @@ -190,7 +188,7 @@ func (j *JWKS) ReadOnlyKeys() map[string]interface{} { } // getKey gets the jsonWebKey from the given KID from the JWKS. It may refresh the JWKS if configured to. -func (j *JWKS) getKey(kid string, token *jwt.Token) (jsonKey interface{}, err error) { +func (j *JWKS) getKey(alg, kid string) (jsonKey interface{}, err error) { j.mux.RLock() pubKey, ok := j.keys[kid] j.mux.RUnlock() @@ -230,11 +228,8 @@ func (j *JWKS) getKey(kid string, token *jwt.Token) (jsonKey interface{}, err er } } - tokenAlg, ok := token.Header["alg"].(string) - if ok { - if pubKey.algorithm != "" && pubKey.algorithm != tokenAlg { - return nil, fmt.Errorf(`%w: JWK "alg" parameter value %q does not match token "alg" parameter value %q`, ErrJWKAlgMismatch, pubKey.algorithm, tokenAlg) - } + if pubKey.algorithm != "" && pubKey.algorithm != alg { + return nil, fmt.Errorf(`%w: JWK "alg" parameter value %q does not match token "alg" parameter value %q`, ErrJWKAlgMismatch, pubKey.algorithm, alg) } return pubKey.public, nil diff --git a/keyfunc.go b/keyfunc.go index 6bb342b..39bf0fe 100644 --- a/keyfunc.go +++ b/keyfunc.go @@ -25,7 +25,12 @@ func (j *JWKS) Keyfunc(token *jwt.Token) (interface{}, error) { return nil, fmt.Errorf("%w: could not convert kid in JWT header to string", ErrKID) } - return j.getKey(kid, token) + alg, ok := token.Header["alg"].(string) + if !ok { + return nil, fmt.Errorf(`%w: the JWT header did not contain the "alg" parameter, which is required by RFC 7515 section 4.1.1`, ErrJWKAlgMismatch) + } + + return j.getKey(kid, alg) } // base64urlTrailingPadding removes trailing padding before decoding a string from base64url. Some non-RFC compliant From 84fbad5cd1dea94eecae8119b8f0853d0abddeb1 Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Mon, 31 Oct 2022 17:14:50 -0400 Subject: [PATCH 03/13] Fix method argument order --- keyfunc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keyfunc.go b/keyfunc.go index 39bf0fe..683e012 100644 --- a/keyfunc.go +++ b/keyfunc.go @@ -30,7 +30,7 @@ func (j *JWKS) Keyfunc(token *jwt.Token) (interface{}, error) { return nil, fmt.Errorf(`%w: the JWT header did not contain the "alg" parameter, which is required by RFC 7515 section 4.1.1`, ErrJWKAlgMismatch) } - return j.getKey(kid, alg) + return j.getKey(alg, kid) } // base64urlTrailingPadding removes trailing padding before decoding a string from base64url. Some non-RFC compliant From fbc5f8606678ad979ddc8c4cf61097b019231d87 Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Mon, 31 Oct 2022 17:24:21 -0400 Subject: [PATCH 04/13] Add test and note about test coverage --- alg_test.go | 25 +++++++++++++++++++++++++ keyfunc.go | 2 ++ 2 files changed, 27 insertions(+) create mode 100644 alg_test.go diff --git a/alg_test.go b/alg_test.go new file mode 100644 index 0000000..9fafae1 --- /dev/null +++ b/alg_test.go @@ -0,0 +1,25 @@ +package keyfunc_test + +import ( + "encoding/json" + "errors" + "testing" + + "github.com/golang-jwt/jwt/v4" + + "github.com/MicahParks/keyfunc" +) + +func TestAlgMismatch(t *testing.T) { + const jwtB64 = "eyJhbGciOiJSUzUxMiIsInR5cCI6IkpXVCIsImtpZCI6IkM2NXEwRUtReWhwZDFtNGZyN1NLTzJIZV9uQXhnQ3RBZHdzNjRkMkJMdDgifQ.eyJleHAiOjE2MTU0MDcwMjYsImlhdCI6MTYxNTQwNjk2NiwianRpIjoiMzg1NjE4ODItOTA5MS00ODY3LTkzYmYtMmE3YmU4NTc3YmZiIiwiaXNzIjoiaHR0cDovL2xvY2FsaG9zdDo4MDgwL2F1dGgvcmVhbG1zL21hc3RlciIsImF1ZCI6ImFjY291bnQiLCJzdWIiOiJhZDEyOGRmMS0xMTQwLTRlNGMtYjA5Ny1hY2RjZTcwNWJkOWIiLCJ0eXAiOiJCZWFyZXIiLCJhenAiOiJ0b2tlbmRlbG1lIiwiYWNyIjoiMSIsInJlYWxtX2FjY2VzcyI6eyJyb2xlcyI6WyJvZmZsaW5lX2FjY2VzcyIsInVtYV9hdXRob3JpemF0aW9uIl19LCJyZXNvdXJjZV9hY2Nlc3MiOnsiYWNjb3VudCI6eyJyb2xlcyI6WyJtYW5hZ2UtYWNjb3VudCIsIm1hbmFnZS1hY2NvdW50LWxpbmtzIiwidmlldy1wcm9maWxlIl19fSwic2NvcGUiOiJlbWFpbCBwcm9maWxlIiwiY2xpZW50SG9zdCI6IjE3Mi4yMC4wLjEiLCJjbGllbnRJZCI6InRva2VuZGVsbWUiLCJlbWFpbF92ZXJpZmllZCI6ZmFsc2UsInByZWZlcnJlZF91c2VybmFtZSI6InNlcnZpY2UtYWNjb3VudC10b2tlbmRlbG1lIiwiY2xpZW50QWRkcmVzcyI6IjE3Mi4yMC4wLjEifQ.Cmgz3aC_b_kpOmGM-_nRisgQul0d9Jg7BpMLe5F_fdryRhwhW5fQBZtz6FipQ0Tc4jggI6L3Dx1jS2kn823aWCR0x-OAFCawIXnwgAKuM1m2NL7Y6LKC07nytdB_qU4GknAl3jEG-tZIJBHQwYP-K6QKmAT9CdF1ZPbc9u8RgRCPN8UziYcOpvStiG829BO7cTzCt7tp5dJhem8_CnRWBKzelP1fs_z4fAQtW2sgyhX9SUYb5WON-4zrn4i01FlYUwZV-AC83zP6BuHIiy3XpAuTiTp2BjZ-1nzCLWBRpIm_lOObFeo-3AQqWPxzLVAmTFQMKReUF9T8ehL2Osr1XQ" + + jwks, err := keyfunc.NewJSON(json.RawMessage(jwksJSON)) + if err != nil { + t.Fatalf("Failed to create JWKS from JSON: %v", err) + } + + _, err = jwt.Parse(jwtB64, jwks.Keyfunc) + if !errors.Is(err, keyfunc.ErrJWKAlgMismatch) { + t.Fatalf("Expected ErrJWKAlgMismatch, got %v", err) + } +} diff --git a/keyfunc.go b/keyfunc.go index 683e012..967c5a9 100644 --- a/keyfunc.go +++ b/keyfunc.go @@ -27,6 +27,8 @@ func (j *JWKS) Keyfunc(token *jwt.Token) (interface{}, error) { alg, ok := token.Header["alg"].(string) if !ok { + // For test coverage purposes, this should be impossible to reach because the JWT package rejects a token + // without an alg parameter in the header before calling jwt.Keyfunc. return nil, fmt.Errorf(`%w: the JWT header did not contain the "alg" parameter, which is required by RFC 7515 section 4.1.1`, ErrJWKAlgMismatch) } From 47df10b806cbdeef0cb6bf87f46b0e4c4cec474c Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Tue, 1 Nov 2022 09:44:59 -0400 Subject: [PATCH 05/13] Add note about alg check in README.md --- README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/README.md b/README.md index 2fb1a21..b5de6da 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,11 @@ base64url the same as RFC 7515 Section 2: However, this package will remove trailing padding on base64url encoded keys to account for improper implementations of JWKS. +This package will check the `alg` in each JWK. If present, it will confirm the same `alg` is in a given JWT's header +before returning the key for signature verification. If the `alg`s do not match, `keyfunc.ErrJWKAlgMismatch` will +prevent the key being used for signature verification. If the `alg` is not present in the JWK, this check will not +occur. + ## References This project was built and tested using various RFCs and services. The services are listed below: * [Keycloak](https://www.keycloak.org/) From a22a0a6b9bcea062f600ba64fd94fc9f19adda29 Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Tue, 1 Nov 2022 09:46:47 -0400 Subject: [PATCH 06/13] Correct autoformatting in code fences in the README.md --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b5de6da..4a95ebe 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ jwksURL := os.Getenv("JWKS_URL") // Confirm the environment variable is not empty. if jwksURL == "" { -log.Fatalln("JWKS_URL environment variable must be populated.") + log.Fatalln("JWKS_URL environment variable must be populated.") } ``` @@ -81,7 +81,7 @@ Via HTTP: // Create the JWKS from the resource at the given URL. jwks, err := keyfunc.Get(jwksURL, keyfunc.Options{}) // See recommended options in the examples directory. if err != nil { -log.Fatalf("Failed to get the JWKS from the given URL.\nError: %s", err) + log.Fatalf("Failed to get the JWKS from the given URL.\nError: %s", err) } ``` Via JSON: @@ -92,7 +92,7 @@ var jwksJSON = json.RawMessage(`{"keys":[{"kid":"zXew0UJ1h6Q4CCcd_9wxMzvcp5cEBif // Create the JWKS from the resource at the given URL. jwks, err := keyfunc.NewJSON(jwksJSON) if err != nil { -log.Fatalf("Failed to create JWKS from JSON.\nError: %s", err) + log.Fatalf("Failed to create JWKS from JSON.\nError: %s", err) } ``` Via a given key: @@ -103,7 +103,7 @@ uniqueKeyID := "myKeyID" // Create the JWKS from the HMAC key. jwks := keyfunc.NewGiven(map[string]keyfunc.GivenKey{ -uniqueKeyID: keyfunc.NewGivenHMAC(key), + uniqueKeyID: keyfunc.NewGivenHMAC(key), }) ``` @@ -117,7 +117,7 @@ features mentioned at the bottom of this `README.md`. // Parse the JWT. token, err := jwt.Parse(jwtB64, jwks.Keyfunc) if err != nil { -return nil, fmt.Errorf("failed to parse token: %w", err) + return nil, fmt.Errorf("failed to parse token: %w", err) } ``` From d3a7b10fc4caec1e8050969c8851a53d5e03e402 Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Thu, 3 Nov 2022 08:13:41 -0600 Subject: [PATCH 07/13] add failing test for JWKS key with unknown curve --- ecdsa_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 ecdsa_test.go diff --git a/ecdsa_test.go b/ecdsa_test.go new file mode 100644 index 0000000..20c4346 --- /dev/null +++ b/ecdsa_test.go @@ -0,0 +1,30 @@ +package keyfunc + +import ( + "encoding/json" + "testing" + + "github.com/golang-jwt/jwt/v4" +) + +func TestBadCurve(t *testing.T) { + const ( + badJWKS = `{"keys":[{"kty":"EC","crv":"BAD","x":"MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4","y":"4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM","use":"sig","kid":"1"}]}` + someJWT = `eyJhbGciOiJFUzI1NiIsImtpZCI6IjEiLCJ0eXAiOiJKV1QifQ.e30.Q1EeyWUv6XEA0gMLwTFoNhx7Hq1MbVwjI2k9FZPSa-myKW1wYn1X6rHtRyuV-2MEzvimCskFD-afL7UzvdWBQg` + ) + + jwks, err := NewJSON(json.RawMessage(badJWKS)) + if err != nil { + t.Fatalf("Failed to create JWKS from JSON: %v", err) + } + + defer func() { + if r := recover(); r != nil { + t.Fatalf("panic") + } + }() + + if _, err = jwt.Parse(someJWT, jwks.Keyfunc); err == nil { + t.Fatal("No error for bad curve") + } +} From 991e1ab01fb2fad78e6e984fecf7e8147fe5c6ad Mon Sep 17 00:00:00 2001 From: Ben Toews Date: Thu, 3 Nov 2022 08:22:16 -0600 Subject: [PATCH 08/13] handle case of unknown ECDSA curve --- ecdsa.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ecdsa.go b/ecdsa.go index 1105623..bc0d989 100644 --- a/ecdsa.go +++ b/ecdsa.go @@ -48,6 +48,8 @@ func (j *jsonWebKey) ECDSA() (publicKey *ecdsa.PublicKey, err error) { publicKey.Curve = elliptic.P384() case p521: publicKey.Curve = elliptic.P521() + default: + return nil, fmt.Errorf("unknown curve: %s", j.Curve) } // Turn the X coordinate into *big.Int. From 655ef0c3a8de9ddf763a2a4e843ea8dd32a52680 Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Thu, 3 Nov 2022 13:01:16 -0400 Subject: [PATCH 09/13] Add custom error for invalid ECDSA curve and allow test to panic --- ecdsa.go | 8 +++++++- ecdsa_test.go | 18 +++++++++--------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/ecdsa.go b/ecdsa.go index bc0d989..ca0566d 100644 --- a/ecdsa.go +++ b/ecdsa.go @@ -3,6 +3,7 @@ package keyfunc import ( "crypto/ecdsa" "crypto/elliptic" + "errors" "fmt" "math/big" ) @@ -21,6 +22,11 @@ const ( p521 = "P-521" ) +var ( + // ErrECDSACurve indicates an error with the ECDSA curve. + ErrECDSACurve = errors.New("invalid ECDSA curve") +) + // ECDSA parses a jsonWebKey and turns it into an ECDSA public key. func (j *jsonWebKey) ECDSA() (publicKey *ecdsa.PublicKey, err error) { if j.X == "" || j.Y == "" || j.Curve == "" { @@ -49,7 +55,7 @@ func (j *jsonWebKey) ECDSA() (publicKey *ecdsa.PublicKey, err error) { case p521: publicKey.Curve = elliptic.P521() default: - return nil, fmt.Errorf("unknown curve: %s", j.Curve) + return nil, fmt.Errorf("%w: unknown curve: %s", ErrECDSACurve, j.Curve) } // Turn the X coordinate into *big.Int. diff --git a/ecdsa_test.go b/ecdsa_test.go index 20c4346..c71cacb 100644 --- a/ecdsa_test.go +++ b/ecdsa_test.go @@ -1,10 +1,13 @@ -package keyfunc +package keyfunc_test import ( "encoding/json" + "errors" "testing" "github.com/golang-jwt/jwt/v4" + + "github.com/MicahParks/keyfunc" ) func TestBadCurve(t *testing.T) { @@ -13,18 +16,15 @@ func TestBadCurve(t *testing.T) { someJWT = `eyJhbGciOiJFUzI1NiIsImtpZCI6IjEiLCJ0eXAiOiJKV1QifQ.e30.Q1EeyWUv6XEA0gMLwTFoNhx7Hq1MbVwjI2k9FZPSa-myKW1wYn1X6rHtRyuV-2MEzvimCskFD-afL7UzvdWBQg` ) - jwks, err := NewJSON(json.RawMessage(badJWKS)) + jwks, err := keyfunc.NewJSON(json.RawMessage(badJWKS)) if err != nil { t.Fatalf("Failed to create JWKS from JSON: %v", err) } - defer func() { - if r := recover(); r != nil { - t.Fatalf("panic") - } - }() + // The number of parsed keys should be 0. - if _, err = jwt.Parse(someJWT, jwks.Keyfunc); err == nil { - t.Fatal("No error for bad curve") + _, err = jwt.Parse(someJWT, jwks.Keyfunc) + if !errors.Is(err, keyfunc.ErrKIDNotFound) { + t.Fatalf("Expected ErrKIDNotFound, got %v", err) } } From 06dfbcafec3704e4012e60f0eec0c6f81398d6d0 Mon Sep 17 00:00:00 2001 From: Sean Date: Fri, 25 Nov 2022 12:48:31 -0500 Subject: [PATCH 10/13] Allow creation of GivenKeys with a required algorithm To comply with RFC 8725 Section 3.1, we allow specifying an alg for GivenKeys. It behaves equivalently to the 'alg' parameter parsed from JWKS JSON. --- given.go | 19 ++++++++++++++++--- given_test.go | 52 +++++++++++++++++++++++++++++++++++++++++++++++++++ jwks.go | 10 ++++++++++ jwks_test.go | 32 +++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+), 3 deletions(-) diff --git a/given.go b/given.go index 6498431..facf093 100644 --- a/given.go +++ b/given.go @@ -8,7 +8,8 @@ import ( // GivenKey represents a cryptographic key that resides in a JWKS. In conjuncture with Options. type GivenKey struct { - inter interface{} + inter interface{} + algorithm string } // NewGiven creates a JWKS from a map of given keys. @@ -16,7 +17,7 @@ func NewGiven(givenKeys map[string]GivenKey) (jwks *JWKS) { keys := make(map[string]parsedJWK) for kid, given := range givenKeys { - keys[kid] = parsedJWK{public: given.inter} + keys[kid] = parsedJWK{public: given.inter, algorithm: given.algorithm} } return &JWKS{ @@ -25,7 +26,7 @@ func NewGiven(givenKeys map[string]GivenKey) (jwks *JWKS) { } // NewGivenCustom creates a new GivenKey given an untyped variable. The key argument is expected to be a supported -// by the jwt package used. +// by the jwt package used. To specify a required algorithm use NewGivenCustomAlg. // // See the https://pkg.go.dev/github.com/golang-jwt/jwt/v4#RegisterSigningMethod function for registering an unsupported // signing method. @@ -35,6 +36,18 @@ func NewGivenCustom(key interface{}) (givenKey GivenKey) { } } +// NewGivenCustomAlg creates a new GivenKey given an untyped variable and an algorithm. The key argument is expected to +// be a type supported by the jwt package used. The alg argument will be validated against the alg header of tokens. +// +// See the https://pkg.go.dev/github.com/golang-jwt/jwt/v4#RegisterSigningMethod function for registering an unsupported +// signing method. +func NewGivenCustomAlg(key interface{}, alg string) (givenKey GivenKey) { + return GivenKey{ + inter: key, + algorithm: alg, + } +} + // NewGivenECDSA creates a new GivenKey given an ECDSA public key. func NewGivenECDSA(key *ecdsa.PublicKey) (givenKey GivenKey) { return GivenKey{ diff --git a/given_test.go b/given_test.go index f25d533..7f8273b 100644 --- a/given_test.go +++ b/given_test.go @@ -7,6 +7,7 @@ import ( "crypto/rand" "crypto/rsa" "crypto/sha256" + "errors" "fmt" "testing" @@ -45,6 +46,57 @@ func TestNewGivenCustom(t *testing.T) { signParseValidate(t, token, key, jwks) } +// TestNewGivenCustomAlg tests that a custom jwt.SigningMethod can be used to create a JWKS and a proper jwt.Keyfunc. +func TestNewGivenCustomAlg(t *testing.T) { + jwt.RegisterSigningMethod(method.CustomAlg, func() jwt.SigningMethod { + return method.EmptyCustom{} + }) + + const key = "test-key" + givenKeys := make(map[string]keyfunc.GivenKey) + givenKeys[testKID] = keyfunc.NewGivenCustomAlg(key, method.CustomAlg) + + jwks := keyfunc.NewGiven(givenKeys) + + token := jwt.New(method.EmptyCustom{}) + token.Header[algAttribute] = method.CustomAlg + token.Header[kidAttribute] = testKID + + signParseValidate(t, token, key, jwks) +} + +// TestNewGivenCustomAlg_NegativeCase tests that a custom jwt.SigningMethod can be used to create +// a JWKS and a proper jwt.Keyfunc and that a token with a non-matching algorithm will be rejected. +func TestNewGivenCustomAlg_NegativeCase(t *testing.T) { + jwt.RegisterSigningMethod(method.CustomAlg, func() jwt.SigningMethod { + return method.EmptyCustom{} + }) + + const key = jwt.UnsafeAllowNoneSignatureType // So golang-jwt isn't the one blocking this test + givenKeys := make(map[string]keyfunc.GivenKey) + givenKeys[testKID] = keyfunc.NewGivenCustomAlg(key, method.CustomAlg) + + jwks := keyfunc.NewGiven(givenKeys) + + token := jwt.New(method.EmptyCustom{}) + token.Header[algAttribute] = jwt.SigningMethodNone.Alg() + token.Header[kidAttribute] = testKID + + jwtB64, err := token.SignedString(key) + if err != nil { + t.Fatalf(logFmt, "Failed to sign the JWT.", err) + } + + parsed, err := jwt.NewParser().Parse(jwtB64, jwks.Keyfunc) + if !errors.Is(err, keyfunc.ErrJWKAlgMismatch) { + t.Fatalf("Failed to return ErrJWKAlgMismatch") + } + + if parsed.Valid { + t.Fatalf("The JWT was valid.") + } +} + // TestNewGivenKeyECDSA tests that a generated ECDSA key can be added to the JWKS and create a proper jwt.Keyfunc. func TestNewGivenKeyECDSA(t *testing.T) { givenKeys := make(map[string]keyfunc.GivenKey) diff --git a/jwks.go b/jwks.go index a13bb38..b5a0d37 100644 --- a/jwks.go +++ b/jwks.go @@ -160,6 +160,16 @@ func (j *JWKS) KIDs() (kids []string) { return kids } +// KeyAlg returns the algorithm (`alg`) for the key identified by Key ID (`kid`). +func (j *JWKS) KeyAlg(kid string) string { + j.mux.RLock() + defer j.mux.RUnlock() + if pubKey, ok := j.keys[kid]; ok { + return pubKey.algorithm + } + return "" +} + // Len returns the number of keys in the JWKS. func (j *JWKS) Len() int { j.mux.RLock() diff --git a/jwks_test.go b/jwks_test.go index a0b6b77..073ac2a 100644 --- a/jwks_test.go +++ b/jwks_test.go @@ -296,6 +296,38 @@ func TestJWKS_Len(t *testing.T) { } } +// TestJWKS_KeyAlg confirms the JWKS.Len returns the algorithm for keys by kid. +func TestJWKS_KeyAlg(t *testing.T) { + jwks, err := keyfunc.NewJSON([]byte(jwksJSON)) + if err != nil { + t.Fatalf(logFmt, "Failed to create a JWKS from JSON.", err) + } + + expectedAlgs := map[string]string{ + "zXew0UJ1h6Q4CCcd_9wxMzvcp5cEBifH0KWrCz2Kyxc": "PS256", + "ebJxnm9B3QDBljB5XJWEu72qx6BawDaMAhwz4aKPkQ0": "ES512", + "TVAAet63O3xy_KK6_bxVIu7Ra3_z1wlB543Fbwi5VaU": "ES384", + "arlUxX4hh56rNO-XdIPhDT7bqBMqcBwNQuP_TnZJNGs": "RS512", + "tW6ae7TomE6_2jooM-sf9N_6lWg7HNtaQXrDsElBzM4": "PS512", + "Lx1FmayP2YBtxaqS1SKJRJGiXRKnw2ov5WmYIMG-BLE": "PS384", + "gnmAfvmlsi3kKH3VlM1AJ85P2hekQ8ON_XvJqs3xPD8": "RS384", + "CGt0ZWS4Lc5faiKSdi0tU0fjCAdvGROQRGU9iR7tV0A": "ES256", + "C65q0EKQyhpd1m4fr7SKO2He_nAxgCtAdws64d2BLt8": "RS256", + "Q56A": "", + "hmac": "", + "WW91IGdldCBhIGdvbGQgc3RhciDwn4yfCg": "", + } + + for kid, expectedAlg := range expectedAlgs { + t.Run(kid, func(t *testing.T) { + actualAlg := jwks.KeyAlg(kid) + if actualAlg != expectedAlg { + t.Errorf("Unexpected alg for key %v.\n Expected: %v\n Actual: %v\n", kid, expectedAlg, actualAlg) + } + }) + } +} + // TestRateLimit performs a test to confirm the rate limiter works as expected. func TestRateLimit(t *testing.T) { tempDir, err := os.MkdirTemp("", "*") From 9a9ca876b247d863dc8a9f61fbfb61a6971e57e4 Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Sat, 26 Nov 2022 14:10:36 -0500 Subject: [PATCH 11/13] Deprecate old given key creation function and write new ones with option argument --- examples/custom/main.go | 6 ++- examples/custom/method/method.go | 8 +-- examples/given/main.go | 4 +- examples/hmac/main.go | 4 +- given.go | 86 +++++++++++++++++++++++++++++--- given_test.go | 42 ++++++++++------ jwks.go | 10 ---- jwks_test.go | 32 ------------ 8 files changed, 121 insertions(+), 71 deletions(-) diff --git a/examples/custom/main.go b/examples/custom/main.go index 19173e2..1f09865 100644 --- a/examples/custom/main.go +++ b/examples/custom/main.go @@ -15,7 +15,7 @@ func main() { const exampleKID = "exampleKeyID" // Register the custom signing method. - jwt.RegisterSigningMethod(method.CustomAlg, func() jwt.SigningMethod { + jwt.RegisterSigningMethod(method.CustomAlgHeader, func() jwt.SigningMethod { return method.EmptyCustom{} }) @@ -29,7 +29,9 @@ func main() { // Create the JWKS from the given signing method's key. jwks := keyfunc.NewGiven(map[string]keyfunc.GivenKey{ - exampleKID: keyfunc.NewGivenCustom(key), + exampleKID: keyfunc.NewGivenCustomWithOptions(key, keyfunc.GivenKeyOptions{ + Algorithm: method.CustomAlgHeader, + }), }) // Parse the token. diff --git a/examples/custom/method/method.go b/examples/custom/method/method.go index 4d4ddd9..7e03ab5 100644 --- a/examples/custom/method/method.go +++ b/examples/custom/method/method.go @@ -1,7 +1,7 @@ package method -// CustomAlg is the `alg` JSON attribute's value for the example custom jwt.SigningMethod. -const CustomAlg = "customalg" +// CustomAlgHeader is the `alg` JSON attribute's value for the example custom jwt.SigningMethod. +const CustomAlgHeader = "customalg" // EmptyCustom implements the jwt.SigningMethod interface. It will not sign or verify anything. type EmptyCustom struct{} @@ -13,10 +13,10 @@ func (e EmptyCustom) Verify(_, _ string, _ interface{}) error { // Sign helps implement the jwt.SigningMethod interface. It does not sign anything. func (e EmptyCustom) Sign(_ string, _ interface{}) (string, error) { - return CustomAlg, nil + return CustomAlgHeader, nil } // Alg helps implement the jwt.SigningMethod. It returns the `alg` JSON attribute for JWTs signed with this method. func (e EmptyCustom) Alg() string { - return CustomAlg + return CustomAlgHeader } diff --git a/examples/given/main.go b/examples/given/main.go index 33db993..2787b0c 100644 --- a/examples/given/main.go +++ b/examples/given/main.go @@ -23,7 +23,9 @@ func main() { hmacSecret := []byte("example secret") const givenKID = "givenKID" givenKeys := map[string]keyfunc.GivenKey{ - givenKID: keyfunc.NewGivenHMAC(hmacSecret), + givenKID: keyfunc.NewGivenHMACCustomWithOptions(hmacSecret, keyfunc.GivenKeyOptions{ + Algorithm: jwt.SigningMethodHS256.Alg(), + }), } // Create the keyfunc options. Use an error handler that logs. Refresh the JWKS when a JWT signed by an unknown KID diff --git a/examples/hmac/main.go b/examples/hmac/main.go index e3387b4..79eba13 100644 --- a/examples/hmac/main.go +++ b/examples/hmac/main.go @@ -23,7 +23,9 @@ func main() { // Create the JWKS from the HMAC key. jwks := keyfunc.NewGiven(map[string]keyfunc.GivenKey{ - exampleKID: keyfunc.NewGivenHMAC(key), + exampleKID: keyfunc.NewGivenHMACCustomWithOptions(key, keyfunc.GivenKeyOptions{ + Algorithm: jwt.SigningMethodHS512.Alg(), + }), }) // Parse the token. diff --git a/given.go b/given.go index facf093..3279cad 100644 --- a/given.go +++ b/given.go @@ -8,8 +8,20 @@ import ( // GivenKey represents a cryptographic key that resides in a JWKS. In conjuncture with Options. type GivenKey struct { - inter interface{} algorithm string + inter interface{} +} + +type GivenKeyOptions struct { + // Algorithm is the given key's signing algorithm. Its value will be compared to unverified tokens' "alg" header. + // + // See RFC 8725 Section 3.1 for details. + // https://www.rfc-editor.org/rfc/rfc8725#section-3.1 + // + // For a list of possible values, please see: + // https://www.rfc-editor.org/rfc/rfc7518#section-3.1 + // https://www.iana.org/assignments/jose/jose.xhtml#web-signature-encryption-algorithms + Algorithm string } // NewGiven creates a JWKS from a map of given keys. @@ -17,7 +29,10 @@ func NewGiven(givenKeys map[string]GivenKey) (jwks *JWKS) { keys := make(map[string]parsedJWK) for kid, given := range givenKeys { - keys[kid] = parsedJWK{public: given.inter, algorithm: given.algorithm} + keys[kid] = parsedJWK{ + algorithm: given.algorithm, + public: given.inter, + } } return &JWKS{ @@ -26,52 +41,109 @@ func NewGiven(givenKeys map[string]GivenKey) (jwks *JWKS) { } // NewGivenCustom creates a new GivenKey given an untyped variable. The key argument is expected to be a supported -// by the jwt package used. To specify a required algorithm use NewGivenCustomAlg. +// by the jwt package used. // // See the https://pkg.go.dev/github.com/golang-jwt/jwt/v4#RegisterSigningMethod function for registering an unsupported // signing method. +// +// Deprecated: This function does not allow the user to specify the JWT's signing algorithm. Use +// NewGivenCustomWithOptions instead. func NewGivenCustom(key interface{}) (givenKey GivenKey) { return GivenKey{ inter: key, } } -// NewGivenCustomAlg creates a new GivenKey given an untyped variable and an algorithm. The key argument is expected to -// be a type supported by the jwt package used. The alg argument will be validated against the alg header of tokens. +// NewGivenCustomWithOptions creates a new GivenKey given an untyped variable. The key argument is expected to be a type +// supported by the jwt package used. +// +// Consider the options carefully as each field may have a security implication. // // See the https://pkg.go.dev/github.com/golang-jwt/jwt/v4#RegisterSigningMethod function for registering an unsupported // signing method. -func NewGivenCustomAlg(key interface{}, alg string) (givenKey GivenKey) { +func NewGivenCustomWithOptions(key interface{}, options GivenKeyOptions) (givenKey GivenKey) { return GivenKey{ + algorithm: options.Algorithm, inter: key, - algorithm: alg, } } // NewGivenECDSA creates a new GivenKey given an ECDSA public key. +// +// Deprecated: This function does not allow the user to specify the JWT's signing algorithm. Use +// NewGivenECDSACustomWithOptions instead. func NewGivenECDSA(key *ecdsa.PublicKey) (givenKey GivenKey) { return GivenKey{ inter: key, } } +// NewGivenECDSACustomWithOptions creates a new GivenKey given an ECDSA public key. +// +// Consider the options carefully as each field may have a security implication. +func NewGivenECDSACustomWithOptions(key *ecdsa.PublicKey, options GivenKeyOptions) (givenKey GivenKey) { + return GivenKey{ + algorithm: options.Algorithm, + inter: key, + } +} + // NewGivenEdDSA creates a new GivenKey given an EdDSA public key. +// +// Deprecated: This function does not allow the user to specify the JWT's signing algorithm. Use +// NewGivenEdDSACustomWithOptions instead. func NewGivenEdDSA(key ed25519.PublicKey) (givenKey GivenKey) { return GivenKey{ inter: key, } } +// NewGivenEdDSACustomWithOptions creates a new GivenKey given an EdDSA public key. +// +// Consider the options carefully as each field may have a security implication. +func NewGivenEdDSACustomWithOptions(key ed25519.PublicKey, options GivenKeyOptions) (givenKey GivenKey) { + return GivenKey{ + algorithm: options.Algorithm, + inter: key, + } +} + // NewGivenHMAC creates a new GivenKey given an HMAC key in a byte slice. +// +// Deprecated: This function does not allow the user to specify the JWT's signing algorithm. Use +// NewGivenHMACCustomWithOptions instead. func NewGivenHMAC(key []byte) (givenKey GivenKey) { return GivenKey{ inter: key, } } +// NewGivenHMACCustomWithOptions creates a new GivenKey given an HMAC key in a byte slice. +// +// Consider the options carefully as each field may have a security implication. +func NewGivenHMACCustomWithOptions(key []byte, options GivenKeyOptions) (givenKey GivenKey) { + return GivenKey{ + algorithm: options.Algorithm, + inter: key, + } +} + // NewGivenRSA creates a new GivenKey given an RSA public key. +// +// Deprecated: This function does not allow the user to specify the JWT's signing algorithm. Use +// NewGivenRSACustomWithOptions instead. func NewGivenRSA(key *rsa.PublicKey) (givenKey GivenKey) { return GivenKey{ inter: key, } } + +// NewGivenRSACustomWithOptions creates a new GivenKey given an RSA public key. +// +// Consider the options carefully as each field may have a security implication. +func NewGivenRSACustomWithOptions(key *rsa.PublicKey, options GivenKeyOptions) (givenKey GivenKey) { + return GivenKey{ + algorithm: options.Algorithm, + inter: key, + } +} diff --git a/given_test.go b/given_test.go index 7f8273b..16d7eba 100644 --- a/given_test.go +++ b/given_test.go @@ -30,7 +30,7 @@ const ( // TestNewGivenCustom tests that a custom jwt.SigningMethod can be used to create a JWKS and a proper jwt.Keyfunc. func TestNewGivenCustom(t *testing.T) { - jwt.RegisterSigningMethod(method.CustomAlg, func() jwt.SigningMethod { + jwt.RegisterSigningMethod(method.CustomAlgHeader, func() jwt.SigningMethod { return method.EmptyCustom{} }) @@ -40,7 +40,7 @@ func TestNewGivenCustom(t *testing.T) { jwks := keyfunc.NewGiven(givenKeys) token := jwt.New(method.EmptyCustom{}) - token.Header[algAttribute] = method.CustomAlg + token.Header[algAttribute] = method.CustomAlgHeader token.Header[kidAttribute] = testKID signParseValidate(t, token, key, jwks) @@ -48,18 +48,20 @@ func TestNewGivenCustom(t *testing.T) { // TestNewGivenCustomAlg tests that a custom jwt.SigningMethod can be used to create a JWKS and a proper jwt.Keyfunc. func TestNewGivenCustomAlg(t *testing.T) { - jwt.RegisterSigningMethod(method.CustomAlg, func() jwt.SigningMethod { + jwt.RegisterSigningMethod(method.CustomAlgHeader, func() jwt.SigningMethod { return method.EmptyCustom{} }) const key = "test-key" givenKeys := make(map[string]keyfunc.GivenKey) - givenKeys[testKID] = keyfunc.NewGivenCustomAlg(key, method.CustomAlg) + givenKeys[testKID] = keyfunc.NewGivenCustomWithOptions(key, keyfunc.GivenKeyOptions{ + Algorithm: method.CustomAlgHeader, + }) jwks := keyfunc.NewGiven(givenKeys) token := jwt.New(method.EmptyCustom{}) - token.Header[algAttribute] = method.CustomAlg + token.Header[algAttribute] = method.CustomAlgHeader token.Header[kidAttribute] = testKID signParseValidate(t, token, key, jwks) @@ -68,13 +70,15 @@ func TestNewGivenCustomAlg(t *testing.T) { // TestNewGivenCustomAlg_NegativeCase tests that a custom jwt.SigningMethod can be used to create // a JWKS and a proper jwt.Keyfunc and that a token with a non-matching algorithm will be rejected. func TestNewGivenCustomAlg_NegativeCase(t *testing.T) { - jwt.RegisterSigningMethod(method.CustomAlg, func() jwt.SigningMethod { + jwt.RegisterSigningMethod(method.CustomAlgHeader, func() jwt.SigningMethod { return method.EmptyCustom{} }) - const key = jwt.UnsafeAllowNoneSignatureType // So golang-jwt isn't the one blocking this test + const key = jwt.UnsafeAllowNoneSignatureType // Allow the "none" JWT "alg" header value for golang-jwt. givenKeys := make(map[string]keyfunc.GivenKey) - givenKeys[testKID] = keyfunc.NewGivenCustomAlg(key, method.CustomAlg) + givenKeys[testKID] = keyfunc.NewGivenCustomWithOptions(key, keyfunc.GivenKeyOptions{ + Algorithm: method.CustomAlgHeader, + }) jwks := keyfunc.NewGiven(givenKeys) @@ -89,7 +93,7 @@ func TestNewGivenCustomAlg_NegativeCase(t *testing.T) { parsed, err := jwt.NewParser().Parse(jwtB64, jwks.Keyfunc) if !errors.Is(err, keyfunc.ErrJWKAlgMismatch) { - t.Fatalf("Failed to return ErrJWKAlgMismatch") + t.Fatalf("Failed to return ErrJWKAlgMismatch: %v.", err) } if parsed.Valid { @@ -164,7 +168,9 @@ func TestNewGivenKeyRSA(t *testing.T) { // addCustom adds a new key wto the given keys map. The new key is using a test jwt.SigningMethod. func addCustom(givenKeys map[string]keyfunc.GivenKey, kid string) (key string) { key = "" - givenKeys[kid] = keyfunc.NewGivenCustom(key) + givenKeys[kid] = keyfunc.NewGivenCustomWithOptions(key, keyfunc.GivenKeyOptions{ + Algorithm: method.CustomAlgHeader, + }) return key } @@ -175,7 +181,9 @@ func addECDSA(givenKeys map[string]keyfunc.GivenKey, kid string) (key *ecdsa.Pri return nil, fmt.Errorf("failed to create ECDSA key: %w", err) } - givenKeys[kid] = keyfunc.NewGivenECDSA(&key.PublicKey) + givenKeys[kid] = keyfunc.NewGivenECDSACustomWithOptions(&key.PublicKey, keyfunc.GivenKeyOptions{ + Algorithm: jwt.SigningMethodES256.Alg(), + }) return key, nil } @@ -187,7 +195,9 @@ func addEdDSA(givenKeys map[string]keyfunc.GivenKey, kid string) (key ed25519.Pr return nil, fmt.Errorf("failed to create ECDSA key: %w", err) } - givenKeys[kid] = keyfunc.NewGivenEdDSA(pub) + givenKeys[kid] = keyfunc.NewGivenEdDSACustomWithOptions(pub, keyfunc.GivenKeyOptions{ + Algorithm: jwt.SigningMethodEdDSA.Alg(), + }) return key, nil } @@ -200,7 +210,9 @@ func addHMAC(givenKeys map[string]keyfunc.GivenKey, kid string) (secret []byte, return nil, fmt.Errorf("failed to create HMAC secret: %w", err) } - givenKeys[kid] = keyfunc.NewGivenHMAC(secret) + givenKeys[kid] = keyfunc.NewGivenHMACCustomWithOptions(secret, keyfunc.GivenKeyOptions{ + Algorithm: jwt.SigningMethodHS256.Alg(), + }) return secret, nil } @@ -212,7 +224,9 @@ func addRSA(givenKeys map[string]keyfunc.GivenKey, kid string) (key *rsa.Private return nil, fmt.Errorf("failed to create RSA key: %w", err) } - givenKeys[kid] = keyfunc.NewGivenRSA(&key.PublicKey) + givenKeys[kid] = keyfunc.NewGivenRSACustomWithOptions(&key.PublicKey, keyfunc.GivenKeyOptions{ + Algorithm: jwt.SigningMethodRS256.Alg(), + }) return key, nil } diff --git a/jwks.go b/jwks.go index b5a0d37..a13bb38 100644 --- a/jwks.go +++ b/jwks.go @@ -160,16 +160,6 @@ func (j *JWKS) KIDs() (kids []string) { return kids } -// KeyAlg returns the algorithm (`alg`) for the key identified by Key ID (`kid`). -func (j *JWKS) KeyAlg(kid string) string { - j.mux.RLock() - defer j.mux.RUnlock() - if pubKey, ok := j.keys[kid]; ok { - return pubKey.algorithm - } - return "" -} - // Len returns the number of keys in the JWKS. func (j *JWKS) Len() int { j.mux.RLock() diff --git a/jwks_test.go b/jwks_test.go index 073ac2a..a0b6b77 100644 --- a/jwks_test.go +++ b/jwks_test.go @@ -296,38 +296,6 @@ func TestJWKS_Len(t *testing.T) { } } -// TestJWKS_KeyAlg confirms the JWKS.Len returns the algorithm for keys by kid. -func TestJWKS_KeyAlg(t *testing.T) { - jwks, err := keyfunc.NewJSON([]byte(jwksJSON)) - if err != nil { - t.Fatalf(logFmt, "Failed to create a JWKS from JSON.", err) - } - - expectedAlgs := map[string]string{ - "zXew0UJ1h6Q4CCcd_9wxMzvcp5cEBifH0KWrCz2Kyxc": "PS256", - "ebJxnm9B3QDBljB5XJWEu72qx6BawDaMAhwz4aKPkQ0": "ES512", - "TVAAet63O3xy_KK6_bxVIu7Ra3_z1wlB543Fbwi5VaU": "ES384", - "arlUxX4hh56rNO-XdIPhDT7bqBMqcBwNQuP_TnZJNGs": "RS512", - "tW6ae7TomE6_2jooM-sf9N_6lWg7HNtaQXrDsElBzM4": "PS512", - "Lx1FmayP2YBtxaqS1SKJRJGiXRKnw2ov5WmYIMG-BLE": "PS384", - "gnmAfvmlsi3kKH3VlM1AJ85P2hekQ8ON_XvJqs3xPD8": "RS384", - "CGt0ZWS4Lc5faiKSdi0tU0fjCAdvGROQRGU9iR7tV0A": "ES256", - "C65q0EKQyhpd1m4fr7SKO2He_nAxgCtAdws64d2BLt8": "RS256", - "Q56A": "", - "hmac": "", - "WW91IGdldCBhIGdvbGQgc3RhciDwn4yfCg": "", - } - - for kid, expectedAlg := range expectedAlgs { - t.Run(kid, func(t *testing.T) { - actualAlg := jwks.KeyAlg(kid) - if actualAlg != expectedAlg { - t.Errorf("Unexpected alg for key %v.\n Expected: %v\n Actual: %v\n", kid, expectedAlg, actualAlg) - } - }) - } -} - // TestRateLimit performs a test to confirm the rate limiter works as expected. func TestRateLimit(t *testing.T) { tempDir, err := os.MkdirTemp("", "*") From df26d2a797bc691b3492894a51ff322dbc87b731 Mon Sep 17 00:00:00 2001 From: Sean Date: Tue, 29 Nov 2022 15:18:08 -0500 Subject: [PATCH 12/13] Permit construction of a GivenKeys map from JSON This simplifies the use case of supplementing keys loaded from a JWKS url with ones parsed from static JSON --- given.go | 17 +++++++++++++++++ given_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/given.go b/given.go index 3279cad..ce6d52a 100644 --- a/given.go +++ b/given.go @@ -4,6 +4,7 @@ import ( "crypto/ecdsa" "crypto/ed25519" "crypto/rsa" + "encoding/json" ) // GivenKey represents a cryptographic key that resides in a JWKS. In conjuncture with Options. @@ -12,6 +13,7 @@ type GivenKey struct { inter interface{} } +// GivenKeyOptions represents the configuration options for a GivenKey. type GivenKeyOptions struct { // Algorithm is the given key's signing algorithm. Its value will be compared to unverified tokens' "alg" header. // @@ -147,3 +149,18 @@ func NewGivenRSACustomWithOptions(key *rsa.PublicKey, options GivenKeyOptions) ( inter: key, } } + +// NewGivenKeysFromJSON parses a raw JSON message into a map of key IDs (`kid`) to GivenKeys. +// The returned map is suitable for suitable for passing to `NewGiven()` or as `Options.GivenKeys` to `Get()` +func NewGivenKeysFromJSON(jwksBytes json.RawMessage) (map[string]GivenKey, error) { + // Parse by making a temporary JWKS instance. No need to lock its map since it doesn't escape this function. + j, err := NewJSON(jwksBytes) + if err != nil { + return nil, err + } + keys := make(map[string]GivenKey, len(j.keys)) + for kid, cryptoKey := range j.keys { + keys[kid] = NewGivenCustomWithOptions(cryptoKey.public, GivenKeyOptions{Algorithm: cryptoKey.algorithm}) + } + return keys, nil +} diff --git a/given_test.go b/given_test.go index 16d7eba..91e4e88 100644 --- a/given_test.go +++ b/given_test.go @@ -165,6 +165,44 @@ func TestNewGivenKeyRSA(t *testing.T) { signParseValidate(t, token, key, jwks) } +// TestNewGivenKeysFromJSON tests that parsing GivenKeys from JSON can be used to create a JWKS and a proper jwt.Keyfunc. +func TestNewGivenKeysFromJSON(t *testing.T) { + // Construct a JWKS JSON containing a JWK for which we know the private key and thus can sign a token. + key := []byte("test-hmac-secret") + const testJSON = `{ + "keys": [ + { + "kid": "testkid", + "kty": "oct", + "alg": "HS256", + "use": "sig", + "k": "dGVzdC1obWFjLXNlY3JldA" + } + ] + }` + + givenKeys, err := keyfunc.NewGivenKeysFromJSON([]byte(testJSON)) + if err != nil { + t.Fatalf(logFmt, "Failed to parse given keys from JSON.", err) + } + + jwks := keyfunc.NewGiven(givenKeys) + + token := jwt.New(jwt.SigningMethodHS256) + token.Header[kidAttribute] = testKID + + signParseValidate(t, token, key, jwks) +} + +// TestNewGivenKeysFromJSON_BadParse makes sure bad JSON returns an error. +func TestNewGivenKeysFromJSON_BadParse(t *testing.T) { + const testJSON = "{not the best syntax" + _, err := keyfunc.NewGivenKeysFromJSON([]byte(testJSON)) + if err == nil { + t.Fatalf("Expected a JSON parse error") + } +} + // addCustom adds a new key wto the given keys map. The new key is using a test jwt.SigningMethod. func addCustom(givenKeys map[string]keyfunc.GivenKey, kid string) (key string) { key = "" From ac8e1c0a22d25dfbf4485c1f2f36dc4c5eabb2ab Mon Sep 17 00:00:00 2001 From: Micah Parks Date: Wed, 30 Nov 2022 08:42:44 -0500 Subject: [PATCH 13/13] Edit typo and struct literal in NewGivenKeysFromJSON --- given.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/given.go b/given.go index ce6d52a..68c8abd 100644 --- a/given.go +++ b/given.go @@ -150,8 +150,8 @@ func NewGivenRSACustomWithOptions(key *rsa.PublicKey, options GivenKeyOptions) ( } } -// NewGivenKeysFromJSON parses a raw JSON message into a map of key IDs (`kid`) to GivenKeys. -// The returned map is suitable for suitable for passing to `NewGiven()` or as `Options.GivenKeys` to `Get()` +// NewGivenKeysFromJSON parses a raw JSON message into a map of key IDs (`kid`) to GivenKeys. The returned map is +// suitable for passing to `NewGiven()` or as `Options.GivenKeys` to `Get()` func NewGivenKeysFromJSON(jwksBytes json.RawMessage) (map[string]GivenKey, error) { // Parse by making a temporary JWKS instance. No need to lock its map since it doesn't escape this function. j, err := NewJSON(jwksBytes) @@ -160,7 +160,10 @@ func NewGivenKeysFromJSON(jwksBytes json.RawMessage) (map[string]GivenKey, error } keys := make(map[string]GivenKey, len(j.keys)) for kid, cryptoKey := range j.keys { - keys[kid] = NewGivenCustomWithOptions(cryptoKey.public, GivenKeyOptions{Algorithm: cryptoKey.algorithm}) + keys[kid] = GivenKey{ + algorithm: cryptoKey.algorithm, + inter: cryptoKey.public, + } } return keys, nil }