Merge pull request kubernetes#28542 from liggitt/sa-invalid-token

Automatic merge from submit-queue Check for valid serviceaccount JWT token before inspecting claims Moved claims check after the error check that ensures we have a valid JWT token
lavalamp · Jul 8, 2016 · 9a414d2 · 9a414d2
2 parents d6d846f + cce6772
commit 9a414d2
Show file tree

Hide file tree

Showing 21 changed files with 1,176 additions and 298 deletions.
diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json
diff --git a/pkg/serviceaccount/jwt.go b/pkg/serviceaccount/jwt.go
@@ -92,17 +92,19 @@ type jwtTokenGenerator struct {
 func (j *jwtTokenGenerator) GenerateToken(serviceAccount api.ServiceAccount, secret api.Secret) (string, error) {
 	token := jwt.New(jwt.SigningMethodRS256)
 
+	claims, _ := token.Claims.(jwt.MapClaims)
+
 	// Identify the issuer
-	token.Claims[IssuerClaim] = Issuer
+	claims[IssuerClaim] = Issuer
 
 	// Username
-	token.Claims[SubjectClaim] = MakeUsername(serviceAccount.Namespace, serviceAccount.Name)
+	claims[SubjectClaim] = MakeUsername(serviceAccount.Namespace, serviceAccount.Name)
 
 	// Persist enough structured info for the authenticator to be able to look up the service account and secret
-	token.Claims[NamespaceClaim] = serviceAccount.Namespace
-	token.Claims[ServiceAccountNameClaim] = serviceAccount.Name
-	token.Claims[ServiceAccountUIDClaim] = serviceAccount.UID
-	token.Claims[SecretNameClaim] = secret.Name
+	claims[NamespaceClaim] = serviceAccount.Namespace
+	claims[ServiceAccountNameClaim] = serviceAccount.Name
+	claims[ServiceAccountUIDClaim] = serviceAccount.UID
+	claims[SecretNameClaim] = secret.Name
 
 	// Sign and get the complete encoded token as a string
 	return token.SignedString(j.key)
@@ -156,30 +158,32 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(token string) (user.Info, bool
 
 		// If we get here, we have a token with a recognized signature
 
+		claims, _ := parsedToken.Claims.(jwt.MapClaims)
+
 		// Make sure we issued the token
-		iss, _ := parsedToken.Claims[IssuerClaim].(string)
+		iss, _ := claims[IssuerClaim].(string)
 		if iss != Issuer {
 			return nil, false, nil
 		}
 
 		// Make sure the claims we need exist
-		sub, _ := parsedToken.Claims[SubjectClaim].(string)
+		sub, _ := claims[SubjectClaim].(string)
 		if len(sub) == 0 {
 			return nil, false, errors.New("sub claim is missing")
 		}
-		namespace, _ := parsedToken.Claims[NamespaceClaim].(string)
+		namespace, _ := claims[NamespaceClaim].(string)
 		if len(namespace) == 0 {
 			return nil, false, errors.New("namespace claim is missing")
 		}
-		secretName, _ := parsedToken.Claims[SecretNameClaim].(string)
+		secretName, _ := claims[SecretNameClaim].(string)
 		if len(namespace) == 0 {
 			return nil, false, errors.New("secretName claim is missing")
 		}
-		serviceAccountName, _ := parsedToken.Claims[ServiceAccountNameClaim].(string)
+		serviceAccountName, _ := claims[ServiceAccountNameClaim].(string)
 		if len(serviceAccountName) == 0 {
 			return nil, false, errors.New("serviceAccountName claim is missing")
 		}
-		serviceAccountUID, _ := parsedToken.Claims[ServiceAccountUIDClaim].(string)
+		serviceAccountUID, _ := claims[ServiceAccountUIDClaim].(string)
 		if len(serviceAccountUID) == 0 {
 			return nil, false, errors.New("serviceAccountUID claim is missing")
 		}

diff --git a/pkg/serviceaccount/jwt_test.go b/pkg/serviceaccount/jwt_test.go
@@ -225,6 +225,12 @@ func TestTokenGenerateAndValidate(t *testing.T) {
 		getter := serviceaccountcontroller.NewGetterFromClient(tc.Client)
 		authenticator := serviceaccount.JWTTokenAuthenticator(tc.Keys, tc.Client != nil, getter)
 
+		// An invalid, non-JWT token should always fail
+		if _, ok, err := authenticator.AuthenticateToken("invalid token"); err != nil || ok {
+			t.Errorf("%s: Expected err=nil, ok=false for non-JWT token", k)
+			continue
+		}
+
 		user, ok, err := authenticator.AuthenticateToken(token)
 		if (err != nil) != tc.ExpectedErr {
 			t.Errorf("%s: Expected error=%v, got %v", k, tc.ExpectedErr, err)

diff --git a/vendor/github.com/dgrijalva/jwt-go/.travis.yml b/vendor/github.com/dgrijalva/jwt-go/.travis.yml
diff --git a/vendor/github.com/dgrijalva/jwt-go/MIGRATION_GUIDE.md b/vendor/github.com/dgrijalva/jwt-go/MIGRATION_GUIDE.md
diff --git a/vendor/github.com/dgrijalva/jwt-go/README.md b/vendor/github.com/dgrijalva/jwt-go/README.md
diff --git a/vendor/github.com/dgrijalva/jwt-go/VERSION_HISTORY.md b/vendor/github.com/dgrijalva/jwt-go/VERSION_HISTORY.md