From 057b7af798793c6c3daf9319ab496b07665d5ad5 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Wed, 24 Jan 2018 20:21:07 -0800 Subject: [PATCH] serviceaccount: check token is issued by correct iss before verifying Right now if a JWT for an unknown issuer, for any subject hits the serviceaccount token authenticator, we return a errors as if the token was meant for us but we couldn't find a key to verify it. We should instead return nil, false, nil. This change helps us support multiple service account token authenticators with different issuers. --- .../app/controllermanager.go | 2 +- pkg/kubeapiserver/authenticator/config.go | 2 +- pkg/serviceaccount/jwt.go | 67 +++++++++++++++---- pkg/serviceaccount/jwt_test.go | 20 +++++- .../serviceaccount/service_account_test.go | 4 +- 5 files changed, 75 insertions(+), 20 deletions(-) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index 407732862a..34ef8f3567 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -568,7 +568,7 @@ func (c serviceAccountTokenControllerStarter) startServiceAccountTokenController ctx.InformerFactory.Core().V1().Secrets(), c.rootClientBuilder.ClientOrDie("tokens-controller"), serviceaccountcontroller.TokensControllerOptions{ - TokenGenerator: serviceaccount.JWTTokenGenerator(privateKey), + TokenGenerator: serviceaccount.JWTTokenGenerator(serviceaccount.LegacyIssuer, privateKey), RootCA: rootCA, }, ) diff --git a/pkg/kubeapiserver/authenticator/config.go b/pkg/kubeapiserver/authenticator/config.go index d47ec98d53..aa45099184 100644 --- a/pkg/kubeapiserver/authenticator/config.go +++ b/pkg/kubeapiserver/authenticator/config.go @@ -289,7 +289,7 @@ func newServiceAccountAuthenticator(keyfiles []string, lookup bool, serviceAccou allPublicKeys = append(allPublicKeys, publicKeys...) } - tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(allPublicKeys, lookup, serviceAccountGetter) + tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, allPublicKeys, lookup, serviceAccountGetter) return tokenAuthenticator, nil } diff --git a/pkg/serviceaccount/jwt.go b/pkg/serviceaccount/jwt.go index 9b6dd83852..4453ee0dda 100644 --- a/pkg/serviceaccount/jwt.go +++ b/pkg/serviceaccount/jwt.go @@ -21,8 +21,11 @@ import ( "crypto/ecdsa" "crypto/elliptic" "crypto/rsa" + "encoding/base64" + "encoding/json" "errors" "fmt" + "strings" "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -35,7 +38,7 @@ import ( "gopkg.in/square/go-jose.v2/jwt" ) -const Issuer = "kubernetes/serviceaccount" +const LegacyIssuer = "kubernetes/serviceaccount" type privateClaims struct { ServiceAccountName string `json:"kubernetes.io/serviceaccount/service-account.name"` @@ -59,11 +62,15 @@ type TokenGenerator interface { // JWTTokenGenerator returns a TokenGenerator that generates signed JWT tokens, using the given privateKey. // privateKey is a PEM-encoded byte array of a private RSA key. // JWTTokenAuthenticator() -func JWTTokenGenerator(privateKey interface{}) TokenGenerator { - return &jwtTokenGenerator{privateKey} +func JWTTokenGenerator(iss string, privateKey interface{}) TokenGenerator { + return &jwtTokenGenerator{ + iss: iss, + privateKey: privateKey, + } } type jwtTokenGenerator struct { + iss string privateKey interface{} } @@ -100,7 +107,7 @@ func (j *jwtTokenGenerator) GenerateToken(serviceAccount v1.ServiceAccount, secr return jwt.Signed(signer). Claims(&jwt.Claims{ - Issuer: Issuer, + Issuer: j.iss, Subject: apiserverserviceaccount.MakeUsername(serviceAccount.Namespace, serviceAccount.Name), }). Claims(&privateClaims{ @@ -114,11 +121,17 @@ func (j *jwtTokenGenerator) GenerateToken(serviceAccount v1.ServiceAccount, secr // JWTTokenAuthenticator authenticates tokens as JWT tokens produced by JWTTokenGenerator // Token signatures are verified using each of the given public keys until one works (allowing key rotation) // If lookup is true, the service account and secret referenced as claims inside the token are retrieved and verified with the provided ServiceAccountTokenGetter -func JWTTokenAuthenticator(keys []interface{}, lookup bool, getter ServiceAccountTokenGetter) authenticator.Token { - return &jwtTokenAuthenticator{keys, lookup, getter} +func JWTTokenAuthenticator(iss string, keys []interface{}, lookup bool, getter ServiceAccountTokenGetter) authenticator.Token { + return &jwtTokenAuthenticator{ + iss: iss, + keys: keys, + lookup: lookup, + getter: getter, + } } type jwtTokenAuthenticator struct { + iss string keys []interface{} lookup bool getter ServiceAccountTokenGetter @@ -127,6 +140,10 @@ type jwtTokenAuthenticator struct { var errMismatchedSigningMethod = errors.New("invalid signing method") func (j *jwtTokenAuthenticator) AuthenticateToken(tokenData string) (user.Info, bool, error) { + if !j.hasCorrectIssuer(tokenData) { + return nil, false, nil + } + tok, err := jwt.ParseSigned(tokenData) if err != nil { return nil, false, nil @@ -152,13 +169,8 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(tokenData string) (user.Info, return nil, false, utilerrors.NewAggregate(errlist) } - // If we get here, we have a token with a recognized signature - - // Make sure we issued the token - if public.Issuer != Issuer { - return nil, false, nil - } - + // If we get here, we have a token with a recognized signature and + // issuer string. if err := j.Validate(tokenData, public, private); err != nil { return nil, false, err } @@ -167,6 +179,35 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(tokenData string) (user.Info, } +// hasCorrectIssuer returns true if tokenData is a valid JWT in compact +// serialization format and the "iss" claim matches the iss field of this token +// authenticator, and otherwise returns false. +// +// Note: go-jose currently does not allow access to unverified JWS payloads. +// See https://github.com/square/go-jose/issues/169 +func (j *jwtTokenAuthenticator) hasCorrectIssuer(tokenData string) bool { + parts := strings.Split(tokenData, ".") + if len(parts) != 3 { + return false + } + payload, err := base64.RawURLEncoding.DecodeString(parts[1]) + if err != nil { + return false + } + claims := struct { + // WARNING: this JWT is not verified. Do not trust these claims. + Issuer string `json:"iss"` + }{} + if err := json.Unmarshal(payload, &claims); err != nil { + return false + } + if claims.Issuer != j.iss { + return false + } + return true + +} + func (j *jwtTokenAuthenticator) Validate(tokenData string, public *jwt.Claims, private *privateClaims) error { // Make sure the claims we need exist diff --git a/pkg/serviceaccount/jwt_test.go b/pkg/serviceaccount/jwt_test.go index e21af9584d..d8b326a50c 100644 --- a/pkg/serviceaccount/jwt_test.go +++ b/pkg/serviceaccount/jwt_test.go @@ -128,7 +128,7 @@ func TestTokenGenerateAndValidate(t *testing.T) { } // Generate the RSA token - rsaGenerator := serviceaccount.JWTTokenGenerator(getPrivateKey(rsaPrivateKey)) + rsaGenerator := serviceaccount.JWTTokenGenerator(serviceaccount.LegacyIssuer, getPrivateKey(rsaPrivateKey)) rsaToken, err := rsaGenerator.GenerateToken(*serviceAccount, *rsaSecret) if err != nil { t.Fatalf("error generating token: %v", err) @@ -141,7 +141,7 @@ func TestTokenGenerateAndValidate(t *testing.T) { } // Generate the ECDSA token - ecdsaGenerator := serviceaccount.JWTTokenGenerator(getPrivateKey(ecdsaPrivateKey)) + ecdsaGenerator := serviceaccount.JWTTokenGenerator(serviceaccount.LegacyIssuer, getPrivateKey(ecdsaPrivateKey)) ecdsaToken, err := ecdsaGenerator.GenerateToken(*serviceAccount, *ecdsaSecret) if err != nil { t.Fatalf("error generating token: %v", err) @@ -153,6 +153,13 @@ func TestTokenGenerateAndValidate(t *testing.T) { "token": []byte(ecdsaToken), } + // Generate signer with same keys as RSA signer but different issuer + badIssuerGenerator := serviceaccount.JWTTokenGenerator("foo", getPrivateKey(rsaPrivateKey)) + badIssuerToken, err := badIssuerGenerator.GenerateToken(*serviceAccount, *rsaSecret) + if err != nil { + t.Fatalf("error generating token: %v", err) + } + testCases := map[string]struct { Client clientset.Interface Keys []interface{} @@ -195,6 +202,13 @@ func TestTokenGenerateAndValidate(t *testing.T) { ExpectedUserUID: expectedUserUID, ExpectedGroups: []string{"system:serviceaccounts", "system:serviceaccounts:test"}, }, + "valid key, invalid issuer (rsa)": { + Token: badIssuerToken, + Client: nil, + Keys: []interface{}{getPublicKey(rsaPublicKey)}, + ExpectedErr: false, + ExpectedOK: false, + }, "valid key (ecdsa)": { Token: ecdsaToken, Client: nil, @@ -253,7 +267,7 @@ func TestTokenGenerateAndValidate(t *testing.T) { for k, tc := range testCases { getter := serviceaccountcontroller.NewGetterFromClient(tc.Client) - authenticator := serviceaccount.JWTTokenAuthenticator(tc.Keys, tc.Client != nil, getter) + authenticator := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, tc.Keys, tc.Client != nil, getter) // An invalid, non-JWT token should always fail if _, ok, err := authenticator.AuthenticateToken("invalid token"); err != nil || ok { diff --git a/test/integration/serviceaccount/service_account_test.go b/test/integration/serviceaccount/service_account_test.go index b1efed5879..68655d2c1e 100644 --- a/test/integration/serviceaccount/service_account_test.go +++ b/test/integration/serviceaccount/service_account_test.go @@ -375,7 +375,7 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie }) serviceAccountKey, _ := rsa.GenerateKey(rand.Reader, 2048) serviceAccountTokenGetter := serviceaccountcontroller.NewGetterFromClient(rootClientset) - serviceAccountTokenAuth := serviceaccount.JWTTokenAuthenticator([]interface{}{&serviceAccountKey.PublicKey}, true, serviceAccountTokenGetter) + serviceAccountTokenAuth := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, []interface{}{&serviceAccountKey.PublicKey}, true, serviceAccountTokenGetter) authenticator := union.New( bearertoken.New(rootTokenAuth), bearertoken.New(serviceAccountTokenAuth), @@ -442,7 +442,7 @@ func startServiceAccountTestServer(t *testing.T) (*clientset.Clientset, restclie informers.Core().V1().ServiceAccounts(), informers.Core().V1().Secrets(), rootClientset, - serviceaccountcontroller.TokensControllerOptions{TokenGenerator: serviceaccount.JWTTokenGenerator(serviceAccountKey)}, + serviceaccountcontroller.TokensControllerOptions{TokenGenerator: serviceaccount.JWTTokenGenerator(serviceaccount.LegacyIssuer, serviceAccountKey)}, ) if err != nil { return rootClientset, clientConfig, stop, err