Skip to content

Commit

Permalink
topdown/tokens: Fix verifying with multi-key JWKS
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@patrick-east
patrick-east committed Nov 26, 2019
1 parent 3258838 commit 16a0aca
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 28 deletions.
72 changes: 47 additions & 25 deletions topdown/tokens.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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.
Expand All @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 != "" {
Expand All @@ -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(".")...)
Expand All @@ -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.
Expand Down
94 changes: 91 additions & 3 deletions topdown/tokens_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"crypto/ecdsa"
"crypto/elliptic"
"crypto/rsa"
"encoding/base64"
"encoding/json"
"errors"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 16a0aca

Please sign in to comment.