From 16a0aca4db086346e959cab019f8e6cf1a1bb1c8 Mon Sep 17 00:00:00 2001 From: Patrick East Date: Mon, 25 Nov 2019 16:24:21 -0800 Subject: [PATCH] topdown/tokens: Fix verifying with multi-key JWKS We were previously ignoring any keys beyond the first index in a set of keys contained in a JWKS provided to the JWT builtins. We now will attempt to use any of the keys found. In addition we no longer raise an error if the key type doesn't match the header. We will return the correct result as if the verification failed (because it did..) Fixes: #1901 Signed-off-by: Patrick East --- topdown/tokens.go | 72 +++++++++++++++++++++----------- topdown/tokens_test.go | 94 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 138 insertions(+), 28 deletions(-) diff --git a/topdown/tokens.go b/topdown/tokens.go index d5a5bf7f09..35ef766ade 100644 --- a/topdown/tokens.go +++ b/topdown/tokens.go @@ -169,9 +169,9 @@ func builtinJWTVerifyES256(a ast.Value, b ast.Value) (ast.Value, error) { }) } -// getKeyFromCertOrJWK returns the public key found in a X.509 certificate or JWK key. +// getKeyFromCertOrJWK returns the public key found in a X.509 certificate or JWK key(s). // A valid PEM block is never valid JSON (and vice versa), hence can try parsing both. -func getKeyFromCertOrJWK(certificate string) (key interface{}, err error) { +func getKeyFromCertOrJWK(certificate string) ([]interface{}, error) { if block, rest := pem.Decode([]byte(certificate)); block != nil { if block.Type != "CERTIFICATE" { return nil, fmt.Errorf("failed to find a PEM certificate block") @@ -186,15 +186,24 @@ func getKeyFromCertOrJWK(certificate string) (key interface{}, err error) { return nil, errors.Wrap(err, "failed to parse a PEM certificate") } - return cert.PublicKey, nil + return []interface{}{cert.PublicKey}, nil } - keys, err := jwk.ParseString(certificate) + jwks, err := jwk.ParseString(certificate) if err != nil { return nil, errors.Wrap(err, "failed to parse a JWK key (set)") } - return keys.Keys[0].Materialize() + var keys []interface{} + for _, k := range jwks.Keys { + key, err := k.Materialize() + if err != nil { + return nil, err + } + keys = append(keys, key) + } + + return keys, nil } // Implements JWT signature verification. @@ -209,7 +218,7 @@ func builtinJWTVerify(a ast.Value, b ast.Value, verify func(publicKey interface{ return nil, err } - key, err := getKeyFromCertOrJWK(string(s)) + keys, err := getKeyFromCertOrJWK(string(s)) if err != nil { return nil, err } @@ -220,14 +229,18 @@ func builtinJWTVerify(a ast.Value, b ast.Value, verify func(publicKey interface{ } // Validate the JWT signature - err = verify(key, - getInputSHA([]byte(token.header+"."+token.payload)), - []byte(signature)) + for _, key := range keys { + err = verify(key, + getInputSHA([]byte(token.header+"."+token.payload)), + []byte(signature)) - if err != nil { - return ast.Boolean(false), nil + if err == nil { + return ast.Boolean(true), nil + } } - return ast.Boolean(true), nil + + // None of the keys worked, return false + return ast.Boolean(false), nil } // Implements HS256 (secret) JWT signature verification @@ -265,8 +278,8 @@ func builtinJWTVerifyHS256(a ast.Value, b ast.Value) (ast.Value, error) { // tokenConstraints holds decoded JWT verification constraints. type tokenConstraints struct { - // The single asymmetric key we will verify with. - key interface{} + // The set of asymmetric keys we can verify with. + keys []interface{} // The single symmetric key we will verify with. secret string @@ -317,7 +330,7 @@ func tokenConstraintCert(value ast.Value, constraints *tokenConstraints) (err er return fmt.Errorf("cert constraint: must be a string") } - constraints.key, err = getKeyFromCertOrJWK(string(s)) + constraints.keys, err = getKeyFromCertOrJWK(string(s)) return } @@ -386,7 +399,7 @@ func parseTokenConstraints(a ast.Value) (constraints tokenConstraints, err error // validate validates the constraints argument. func (constraints *tokenConstraints) validate() (err error) { keys := 0 - if constraints.key != nil { + if constraints.keys != nil { keys++ } if constraints.secret != "" { @@ -404,7 +417,7 @@ func (constraints *tokenConstraints) validate() (err error) { } // verify verifies a JWT using the constraints and the algorithm from the header -func (constraints *tokenConstraints) verify(kid, alg, header, payload, signature string) (err error) { +func (constraints *tokenConstraints) verify(kid, alg, header, payload, signature string) error { // Construct the payload plaintext := []byte(header) plaintext = append(plaintext, []byte(".")...) @@ -414,19 +427,28 @@ func (constraints *tokenConstraints) verify(kid, alg, header, payload, signature var a tokenAlgorithm a, ok = tokenAlgorithms[alg] if !ok { - err = fmt.Errorf("unknown JWS algorithm: %s", alg) - return - } - // If we're configured with a single key then only trust that - if constraints.key != nil { - return a.verify(constraints.key, a.hash, plaintext, []byte(signature)) + return fmt.Errorf("unknown JWS algorithm: %s", alg) + } + // If we're configured with asymmetric key(s) then only trust that + if constraints.keys != nil { + verified := false + for _, key := range constraints.keys { + err := a.verify(key, a.hash, plaintext, []byte(signature)) + if err == nil { + verified = true + break + } + } + if !verified { + return errSignatureNotVerified + } + return nil } if constraints.secret != "" { return a.verify([]byte(constraints.secret), a.hash, plaintext, []byte(signature)) } // (*tokenConstraints)validate() should prevent this happening - err = errors.New("unexpectedly found no keys to trust") - return + return errors.New("unexpectedly found no keys to trust") } // validAudience checks the audience of the JWT. diff --git a/topdown/tokens_test.go b/topdown/tokens_test.go index 7a83d99237..fe97ba67bd 100644 --- a/topdown/tokens_test.go +++ b/topdown/tokens_test.go @@ -4,6 +4,7 @@ import ( "context" "crypto/ecdsa" "crypto/elliptic" + "crypto/rsa" "encoding/base64" "encoding/json" "errors" @@ -31,8 +32,8 @@ func TestParseTokenConstraints(t *testing.T) { if constraints.alg != "" { t.Errorf("alg: %v", constraints.alg) } - if constraints.key != nil { - t.Errorf("key: %v", constraints.key) + if constraints.keys != nil { + t.Errorf("key: %v", constraints.keys) } }) t.Run("Alg", func(t *testing.T) { @@ -66,7 +67,7 @@ OHoCIHmNX37JOqTcTzGn2u9+c8NlnvZ0uDvsd1BmKPaUmjmm if err != nil { t.Fatalf("parseTokenConstraints: %v", err) } - pubKey := constraints.key.(*ecdsa.PublicKey) + pubKey := constraints.keys[0].(*ecdsa.PublicKey) if pubKey.Curve != elliptic.P256() { t.Errorf("curve: %v", pubKey.Curve) } @@ -77,6 +78,46 @@ OHoCIHmNX37JOqTcTzGn2u9+c8NlnvZ0uDvsd1BmKPaUmjmm t.Errorf("y: %x", pubKey.Y) } }) + t.Run("Cert Multi Key", func(t *testing.T) { + var constraints tokenConstraints + var err error + c := ast.NewObject() + c.Insert(ast.StringTerm("cert"), ast.StringTerm(`{ + "keys": [ + { + "kty": "EC", + "use": "sig", + "crv": "P-256", + "kid": "k1", + "x": "9Qq5S5VqMQoH-FOI4atcH6V3bua03C-5ZMZMG1rszwA", + "y": "LLbFxWkGBEBrTm1GMYZJy1OXCH1KLweJMCgIEPIsibU", + "alg": "ES256" + }, + { + "kty": "RSA", + "e": "AQAB", + "use": "enc", + "kid": "k2", + "alg": "RS256", + "n": "sGu-fYVE2nq2dPxJlqAMI0Z8G3FD0XcWDnD8mkfO1ddKRGuUQZmfj4gWeZGyIk3cnuoy7KJCEqa3daXc08QHuFZyfn0rH33t8_AFsvb0q0i7R2FK-Gdqs_E0-sGpYMsRJdZWfCioLkYjIHEuVnRbi3DEsWqe484rEGbKF60jNRgGC4b-8pz-E538ZkssWxcqHrYIj5bjGEU36onjS3M_yrTuNvzv_8wRioK4fbcwmGne9bDxu8LcoSReWpPn0CnUkWnfqroRcMJnC87ZuJagDW1ZWCmU3psdsVanmFFh0DP6z0fsA4h8G2n9-qp-LEKFaWwo3IWlOsIzU3MHdcEiGw" + } + ] +} +`)) + constraints, err = parseTokenConstraints(c) + if err != nil { + t.Fatalf("parseTokenConstraints: %v", err) + } + elPubKey := constraints.keys[0].(*ecdsa.PublicKey) + if elPubKey.Curve != elliptic.P256() { + t.Errorf("curve: %v", elPubKey.Curve) + } + + rsaPubKey := constraints.keys[1].(*rsa.PublicKey) + if rsaPubKey.Size() != 256 { + t.Errorf("expected size 256 found %d", rsaPubKey.Size()) + } + }) t.Run("Unrecognized", func(t *testing.T) { var err error c := ast.NewObject() @@ -831,6 +872,27 @@ const ( certPemExtraData = `-----BEGIN CERTIFICATE-----\nMIIFiDCCA3ACCQCGV6XsfG/oRTANBgkqhkiG9w0BAQUFADCBhTELMAkGA1UEBhMC\nVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFTATBgNVBAcMDFJlZHdvb2QgQ2l0eTEO\nMAwGA1UECgwFU3R5cmExDDAKBgNVBAsMA0RldjESMBAGA1UEAwwJbG9jYWxob3N0\nMRgwFgYJKoZIhvcNAQkBFglhc2hAc3R5cmEwHhcNMTgwMzA2MDAxNTU5WhcNMTkw\nMzA2MDAxNTU5WjCBhTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWEx\nFTATBgNVBAcMDFJlZHdvb2QgQ2l0eTEOMAwGA1UECgwFU3R5cmExDDAKBgNVBAsM\nA0RldjESMBAGA1UEAwwJbG9jYWxob3N0MRgwFgYJKoZIhvcNAQkBFglhc2hAc3R5\ncmEwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDucnAwTRA0zqDQ671L\nKWOVwhjhycFyzyhZUd7vhsnslOBiYM6TYIDXhETfAk2RQoRE/9xF16woMD8FOglc\nlSuhi+GNfFRif6LfArm84ZFj1ZS1MX2logikhXhRJQ7AOHe5+ED0re3KH5lWyqfz\nR6bQuPYwTQSBJy6Tq7T9RiOM29yadCX64OaCEbzEFmHtNlbb5px4zCVvgskg/fpV\nGGCMpAYjGDatbxE5eAloVs1EJuI5RSqWr1JRm6EejxM04BFdfGn1HgWrsKXtlvBa\n00/AC0zXL5n6LK7+L3WbRguVTZcE4Yu70gDwhmM+VsKeT9LKClX003BNj0NJDRB9\ndw9MaWxsXDNHNOWEfbnASXeP7ZRv3D81ftij6P8SL14ZnxyrRty8TAN4ij3wd41l\nastRQCtrJFi+HzO606XOp6HDzBoWT0DGl8Sn2hZ6RLPyBnD04vvvcSGeCVjHGOQ8\nc3OTroK58u5MR/q4T00sTkeeVAxuKoEWKsjIBYYrJTe/a2mEq9yiDGbPNYDnWnQZ\njSUZm+Us23Y2sm/agZ5zKXcEuoecGL6sYCixr/xeB9BPxEiTthH+0M8OY99qpIhz\nSmj41wdgQfzZi/6B8pIr77V/KywYKxJEmzw8Uy48aC/rZ8WsT8QdKwclo1aiNJhx\n79OvGbZFoeHD/w7igpx+ttpF/wIDAQABMA0GCSqGSIb3DQEBBQUAA4ICAQC3wWUs\nfXz+aSfFVz+O3mLFkr65NIgazbGAySgMgMNVuadheIkPL4k21atyflfpx4pg9FGv\n40vWCLMajpvynfz4oqah0BACnpqzQ8Dx6HYkmlXK8fLB+WtPrZBeUEsGPKuJYt4M\nd5TeY3VpNgWOPXmnE4lvxHZqh/8OwmOpjBfC9E3e2eqgwiwOkXnMaZEPgKP6JiWk\nEFaQ9jgMQqJZnNcv6NmiqqsZeI0/NNjBpkmEWQl+wLegVusHiQ0FMBMQ0taEo21r\nzUwHoNJR3h3wgGQiKxKOH1FUKHBV7hEqObLraD/hfG5xYucJfvvAAP1iH0ycPs+9\nhSccrn5/HY1c9AZnW8Kh7atp/wFP+sHjtECWK/lUmXfhASS293hprCpJk2n9pkmR\nziXKJhjwkxlC8NcHuiVfaxdfDa4+1Qta2gK7GEypbvLoEmIt/dsYUsxUg84lwJJ9\nnyC/pfZ5a8wFSf186JeVH4kHd3bnkzlQz460HndOMSJ/Xi1wSfuZlOVupFf8TVKl\np4j28MTLH2Wqx50NssKThdaX6hoCiMqreYa+EVaN1f/cIGQxZSCzdzMCKqdB8lKB\n3Eax+5zsIa/UyPwGxZcyXBRHAlz5ZnkjuRxInyiMkBWWz3IZXjTe6Fq8BNd2UWNc\nw35+2nO5n1LKXgR2+nzhZUOk8TPsi9WUywRluQ==\n-----END CERTIFICATE-----\nEXTRA` certPemBadCertificate = `-----BEGIN CERTIFICATE-----\ndeadiDCCA3ACCQCGV6XsfG/oRTANBgkqhkiG9w0BAQUFADCBhTELMAkGA1UEBhMC\nVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFTATBgNVBAcMDFJlZHdvb2QgQ2l0eTEO\nMAwGA1UECgwFU3R5cmExDDAKBgNVBAsMA0RldjESMBAGA1UEAwwJbG9jYWxob3N0\nMRgwFgYJKoZIhvcNAQkBFglhc2hAc3R5cmEwHhcNMTgwMzA2MDAxNTU5WhcNMTkw\nMzA2MDAxNTU5WjCBhTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWEx\nFTATBgNVBAcMDFJlZHdvb2QgQ2l0eTEOMAwGA1UECgwFU3R5cmExDDAKBgNVBAsM\nA0RldjESMBAGA1UEAwwJbG9jYWxob3N0MRgwFgYJKoZIhvcNAQkBFglhc2hAc3R5\ncmEwggIiMA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDucnAwTRA0zqDQ671L\nKWOVwhjhycFyzyhZUd7vhsnslOBiYM6TYIDXhETfAk2RQoRE/9xF16woMD8FOglc\nlSuhi+GNfFRif6LfArm84ZFj1ZS1MX2logikhXhRJQ7AOHe5+ED0re3KH5lWyqfz\nR6bQuPYwTQSBJy6Tq7T9RiOM29yadCX64OaCEbzEFmHtNlbb5px4zCVvgskg/fpV\nGGCMpAYjGDatbxE5eAloVs1EJuI5RSqWr1JRm6EejxM04BFdfGn1HgWrsKXtlvBa\n00/AC0zXL5n6LK7+L3WbRguVTZcE4Yu70gDwhmM+VsKeT9LKClX003BNj0NJDRB9\ndw9MaWxsXDNHNOWEfbnASXeP7ZRv3D81ftij6P8SL14ZnxyrRty8TAN4ij3wd41l\nastRQCtrJFi+HzO606XOp6HDzBoWT0DGl8Sn2hZ6RLPyBnD04vvvcSGeCVjHGOQ8\nc3OTroK58u5MR/q4T00sTkeeVAxuKoEWKsjIBYYrJTe/a2mEq9yiDGbPNYDnWnQZ\njSUZm+Us23Y2sm/agZ5zKXcEuoecGL6sYCixr/xeB9BPxEiTthH+0M8OY99qpIhz\nSmj41wdgQfzZi/6B8pIr77V/KywYKxJEmzw8Uy48aC/rZ8WsT8QdKwclo1aiNJhx\n79OvGbZFoeHD/w7igpx+ttpF/wIDAQABMA0GCSqGSIb3DQEBBQUAA4ICAQC3wWUs\nfXz+aSfFVz+O3mLFkr65NIgazbGAySgMgMNVuadheIkPL4k21atyflfpx4pg9FGv\n40vWCLMajpvynfz4oqah0BACnpqzQ8Dx6HYkmlXK8fLB+WtPrZBeUEsGPKuJYt4M\nd5TeY3VpNgWOPXmnE4lvxHZqh/8OwmOpjBfC9E3e2eqgwiwOkXnMaZEPgKP6JiWk\nEFaQ9jgMQqJZnNcv6NmiqqsZeI0/NNjBpkmEWQl+wLegVusHiQ0FMBMQ0taEo21r\nzUwHoNJR3h3wgGQiKxKOH1FUKHBV7hEqObLraD/hfG5xYucJfvvAAP1iH0ycPs+9\nhSccrn5/HY1c9AZnW8Kh7atp/wFP+sHjtECWK/lUmXfhASS293hprCpJk2n9pkmR\nziXKJhjwkxlC8NcHuiVfaxdfDa4+1Qta2gK7GEypbvLoEmIt/dsYUsxUg84lwJJ9\nnyC/pfZ5a8wFSf186JeVH4kHd3bnkzlQz460HndOMSJ/Xi1wSfuZlOVupFf8TVKl\np4j28MTLH2Wqx50NssKThdaX6hoCiMqreYa+EVaN1f/cIGQxZSCzdzMCKqdB8lKB\n3Eax+5zsIa/UyPwGxZcyXBRHAlz5ZnkjuRxInyiMkBWWz3IZXjTe6Fq8BNd2UWNc\nw35+2nO5n1LKXgR2+nzhZUOk8TPsi9WUywRluQ==\n-----END CERTIFICATE-----` keyJWKBadKey = `{"kty":"bogus key type","e":"AQAB","kid":"4db88b6b-cda9-4242-b79e-51346edc313c","n":"7nJwME0QNM6g0Ou9SyljlcIY4cnBcs8oWVHe74bJ7JTgYmDOk2CA14RE3wJNkUKERP_cRdesKDA_BToJXJUroYvhjXxUYn-i3wK5vOGRY9WUtTF9paIIpIV4USUOwDh3ufhA9K3tyh-ZVsqn80em0Lj2ME0EgScuk6u0_UYjjNvcmnQl-uDmghG8xBZh7TZW2-aceMwlb4LJIP36VRhgjKQGIxg2rW8ROXgJaFbNRCbiOUUqlq9SUZuhHo8TNOARXXxp9R4Fq7Cl7ZbwWtNPwAtM1y-Z-iyu_i91m0YLlU2XBOGLu9IA8IZjPlbCnk_SygpV9NNwTY9DSQ0QfXcPTGlsbFwzRzTlhH25wEl3j-2Ub9w_NX7Yo-j_Ei9eGZ8cq0bcvEwDeIo98HeNZWrLUUArayRYvh8zutOlzqehw8waFk9AxpfEp9oWekSz8gZw9OL773EhnglYxxjkPHNzk66CufLuTEf6uE9NLE5HnlQMbiqBFirIyAWGKyU3v2tphKvcogxmzzWA51p0GY0lGZvlLNt2NrJv2oGecyl3BLqHnBi-rGAosa_8XgfQT8RIk7YR_tDPDmPfaqSIc0po-NcHYEH82Yv-gfKSK--1fyssGCsSRJs8PFMuPGgv62fFrE_EHSsHJaNWojSYce_Trxm2RaHhw_8O4oKcfrbaRf8"}` + multiKeyJWkS = `{ + "keys": [ + { + "kty": "EC", + "use": "sig", + "crv": "P-256", + "kid": "k1", + "x": "9Qq5S5VqMQoH-FOI4atcH6V3bua03C-5ZMZMG1rszwA", + "y": "LLbFxWkGBEBrTm1GMYZJy1OXCH1KLweJMCgIEPIsibU", + "alg": "ES256" + }, + { + "kty": "RSA", + "e": "AQAB", + "use": "enc", + "kid": "k2", + "alg": "RS256", + "n": "sGu-fYVE2nq2dPxJlqAMI0Z8G3FD0XcWDnD8mkfO1ddKRGuUQZmfj4gWeZGyIk3cnuoy7KJCEqa3daXc08QHuFZyfn0rH33t8_AFsvb0q0i7R2FK-Gdqs_E0-sGpYMsRJdZWfCioLkYjIHEuVnRbi3DEsWqe484rEGbKF60jNRgGC4b-8pz-E538ZkssWxcqHrYIj5bjGEU36onjS3M_yrTuNvzv_8wRioK4fbcwmGne9bDxu8LcoSReWpPn0CnUkWnfqroRcMJnC87ZuJagDW1ZWCmU3psdsVanmFFh0DP6z0fsA4h8G2n9-qp-LEKFaWwo3IWlOsIzU3MHdcEiGw" + } + ] +}` ) func TestTopDownJWTVerifyRSA(t *testing.T) { @@ -1296,6 +1358,32 @@ func TestTopDownJWTDecodeVerify(t *testing.T) { `{}`, "", }, + { + "multiple-keys-one-valid", + "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.ZcLZbBKpPFFz8YGD2jEbXzwHT7DWtqRVk1PTV-cAWUV8jr6f2a--Fw9SFR3vSbrtFif06AQ3aWY7PMM2AuxDjiUVGjItmHRz0sJBEijcE2QVkDN7MNK3Kk1fsM_hbEXzNCzChZpEkTZnLy9ijkJJFD0j6lBat4lO5Zc_LC2lXUftV_hU2aW9mQ7pLSgJjItzRymivnN0g-WUDq5IPK_M8b3yPy_N9iByj8B2FO0sC3TuOrXWbrYrX4ve4bAaSqOFOXiL5Z5BJfmmtT--xKdWDGJxnei8lbv7in7t223fVsUpsH-zmybp529Fya37BsaIlcgLrl38ghvoqy2sHu2wAA", + fmt.Sprintf("{\"cert\": `%s`, \"time\": 1574723450396363500}", multiKeyJWkS), + true, + `{ + "alg": "RS256", + "typ": "JWT" + }`, + `{ + "admin": true, + "iat": 1516239022, + "name": "John Doe", + "sub": "1234567890" + }`, + "", + }, + { + "multiple-keys-no-valid", + "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiYWRtaW4iOnRydWUsImlhdCI6MTUxNjIzOTAyMn0.G051ZlKno4XdDz4pdPthPKH1cKlFqkREvx_dHhl6kwM", + fmt.Sprintf("{\"cert\": `%s`, \"time\": 1574723450396363500}", multiKeyJWkS), + false, + `{}`, + `{}`, + "", + }, } type test struct {