Merge pull request #58791 from mikedanese/jwt0

Automatic merge from submit-queue (batch tested with PRs 58626, 58791). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

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.

https://github.com/kubernetes/kubernetes/issues/58790

```release-note
NONE
```

cmd/kube-controller-manager/app/controllermanager.go

@@ -571,7 +571,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,
 		},
 	)

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
 }
 

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 {
+func JWTTokenGenerator(iss string, privateKey interface{}) TokenGenerator {
-	return &jwtTokenGenerator{privateKey}
+	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,19 +121,34 @@ 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 {
+func JWTTokenAuthenticator(iss string, keys []interface{}, lookup bool, getter ServiceAccountTokenGetter) authenticator.Token {
-	return &jwtTokenAuthenticator{keys, lookup, getter}
+	return &jwtTokenAuthenticator{
+		iss:  iss,
+		keys: keys,
+		validator: &legacyValidator{
+			lookup: lookup,
+			getter: getter,
+		},
+	}
 }
 
 type jwtTokenAuthenticator struct {
-	keys   []interface{}
+	iss       string
-	lookup bool
+	keys      []interface{}
-	getter ServiceAccountTokenGetter
+	validator Validator
+}
+
+type Validator interface {
+	Validate(tokenData string, public *jwt.Claims, private *privateClaims) error
 }
 
 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,14 +174,9 @@ 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
+	// If we get here, we have a token with a recognized signature and
-
+	// issuer string.
-	// Make sure we issued the token
+	if err := j.validator.Validate(tokenData, public, private); err != nil {
-	if public.Issuer != Issuer {
-		return nil, false, nil
-	}
-
-	if err := j.Validate(tokenData, public, private); err != nil {
 		return nil, false, err
 	}
 
@@ -167,7 +184,41 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(tokenData string) (user.Info,
 
 }
 
-func (j *jwtTokenAuthenticator) Validate(tokenData string, public *jwt.Claims, private *privateClaims) error {
+// 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
+
+}
+
+type legacyValidator struct {
+	lookup bool
+	getter ServiceAccountTokenGetter
+}
+
+func (v *legacyValidator) Validate(tokenData string, public *jwt.Claims, private *privateClaims) error {
 
 	// Make sure the claims we need exist
 	if len(public.Subject) == 0 {
@@ -195,9 +246,9 @@ func (j *jwtTokenAuthenticator) Validate(tokenData string, public *jwt.Claims, p
 		return errors.New("sub claim is invalid")
 	}
 
-	if j.lookup {
+	if v.lookup {
 		// Make sure token hasn't been invalidated by deletion of the secret
-		secret, err := j.getter.GetSecret(namespace, secretName)
+		secret, err := v.getter.GetSecret(namespace, secretName)
 		if err != nil {
 			glog.V(4).Infof("Could not retrieve token %s/%s for service account %s/%s: %v", namespace, secretName, namespace, serviceAccountName, err)
 			return errors.New("Token has been invalidated")
@@ -212,7 +263,7 @@ func (j *jwtTokenAuthenticator) Validate(tokenData string, public *jwt.Claims, p
 		}
 
 		// Make sure service account still exists (name and UID)
-		serviceAccount, err := j.getter.GetServiceAccount(namespace, serviceAccountName)
+		serviceAccount, err := v.getter.GetServiceAccount(namespace, serviceAccountName)
 		if err != nil {
 			glog.V(4).Infof("Could not retrieve service account %s/%s: %v", namespace, serviceAccountName, err)
 			return err

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 {

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