From 2862fb333a0d0ecfb45ac1bb2789976c24510175 Mon Sep 17 00:00:00 2001 From: Mike Danese Date: Thu, 15 Feb 2018 09:45:43 -0800 Subject: [PATCH] svcacct: make token authenticator fully generic so it can be used for both new and legacy svcacct tokens. Also move the legacy validator into legacy.go. --- pkg/kubeapiserver/authenticator/config.go | 2 +- pkg/serviceaccount/jwt.go | 103 ++++-------------- pkg/serviceaccount/jwt_test.go | 2 +- pkg/serviceaccount/legacy.go | 91 ++++++++++++++++ .../serviceaccount/service_account_test.go | 2 +- 5 files changed, 114 insertions(+), 86 deletions(-) diff --git a/pkg/kubeapiserver/authenticator/config.go b/pkg/kubeapiserver/authenticator/config.go index c182365bfa..5e47f22194 100644 --- a/pkg/kubeapiserver/authenticator/config.go +++ b/pkg/kubeapiserver/authenticator/config.go @@ -278,7 +278,7 @@ func newServiceAccountAuthenticator(keyfiles []string, lookup bool, serviceAccou allPublicKeys = append(allPublicKeys, publicKeys...) } - tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, allPublicKeys, lookup, serviceAccountGetter) + tokenAuthenticator := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, allPublicKeys, serviceaccount.NewLegacyValidator(lookup, serviceAccountGetter)) return tokenAuthenticator, nil } diff --git a/pkg/serviceaccount/jwt.go b/pkg/serviceaccount/jwt.go index 22cc76f61e..4371d98717 100644 --- a/pkg/serviceaccount/jwt.go +++ b/pkg/serviceaccount/jwt.go @@ -17,7 +17,6 @@ limitations under the License. package serviceaccount import ( - "bytes" "crypto/ecdsa" "crypto/elliptic" "crypto/rsa" @@ -30,10 +29,8 @@ import ( "k8s.io/api/core/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/authentication/authenticator" - apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount" "k8s.io/apiserver/pkg/authentication/user" - "github.com/golang/glog" jose "gopkg.in/square/go-jose.v2" "gopkg.in/square/go-jose.v2/jwt" ) @@ -113,14 +110,11 @@ func (j *jwtTokenGenerator) GenerateToken(claims *jwt.Claims, privateClaims inte // 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(iss string, keys []interface{}, lookup bool, getter ServiceAccountTokenGetter) authenticator.Token { +func JWTTokenAuthenticator(iss string, keys []interface{}, validator Validator) authenticator.Token { return &jwtTokenAuthenticator{ - iss: iss, - keys: keys, - validator: &legacyValidator{ - lookup: lookup, - getter: getter, - }, + iss: iss, + keys: keys, + validator: validator, } } @@ -130,8 +124,19 @@ type jwtTokenAuthenticator struct { validator Validator } +// Validator is called by the JWT token authentictaor to apply domain specific +// validation to a token and extract user information. type Validator interface { - Validate(tokenData string, public *jwt.Claims, private *legacyPrivateClaims) error + // Validate validates a token and returns user information or an error. + // Validator can assume that the issuer and signature of a token are already + // verified when this function is called. + Validate(tokenData string, public *jwt.Claims, private interface{}) (namespace, name, uid string, err error) + // NewPrivateClaims returns a struct that the authenticator should + // deserialize the JWT payload into. The authenticator may then pass this + // struct back to the Validator as the 'private' argument to a Validate() + // call. This struct should contain fields for any private claims that the + // Validator requires to validate the JWT. + NewPrivateClaims() interface{} } var errMismatchedSigningMethod = errors.New("invalid signing method") @@ -147,7 +152,7 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(tokenData string) (user.Info, } public := &jwt.Claims{} - private := &legacyPrivateClaims{} + private := j.validator.NewPrivateClaims() var ( found bool @@ -168,12 +173,12 @@ func (j *jwtTokenAuthenticator) AuthenticateToken(tokenData string) (user.Info, // If we get here, we have a token with a recognized signature and // issuer string. - if err := j.validator.Validate(tokenData, public, private); err != nil { + ns, name, uid, err := j.validator.Validate(tokenData, public, private) + if err != nil { return nil, false, err } - return UserInfo(private.Namespace, private.ServiceAccountName, private.ServiceAccountUID), true, nil - + return UserInfo(ns, name, uid), true, nil } // hasCorrectIssuer returns true if tokenData is a valid JWT in compact @@ -204,71 +209,3 @@ func (j *jwtTokenAuthenticator) hasCorrectIssuer(tokenData string) bool { return true } - -type legacyValidator struct { - lookup bool - getter ServiceAccountTokenGetter -} - -func (v *legacyValidator) Validate(tokenData string, public *jwt.Claims, private *legacyPrivateClaims) error { - - // Make sure the claims we need exist - if len(public.Subject) == 0 { - return errors.New("sub claim is missing") - } - namespace := private.Namespace - if len(namespace) == 0 { - return errors.New("namespace claim is missing") - } - secretName := private.SecretName - if len(secretName) == 0 { - return errors.New("secretName claim is missing") - } - serviceAccountName := private.ServiceAccountName - if len(serviceAccountName) == 0 { - return errors.New("serviceAccountName claim is missing") - } - serviceAccountUID := private.ServiceAccountUID - if len(serviceAccountUID) == 0 { - return errors.New("serviceAccountUID claim is missing") - } - - subjectNamespace, subjectName, err := apiserverserviceaccount.SplitUsername(public.Subject) - if err != nil || subjectNamespace != namespace || subjectName != serviceAccountName { - return errors.New("sub claim is invalid") - } - - if v.lookup { - // Make sure token hasn't been invalidated by deletion of the secret - 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") - } - if secret.DeletionTimestamp != nil { - glog.V(4).Infof("Token is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, secretName, namespace, serviceAccountName) - return errors.New("Token has been invalidated") - } - if bytes.Compare(secret.Data[v1.ServiceAccountTokenKey], []byte(tokenData)) != 0 { - glog.V(4).Infof("Token contents no longer matches %s/%s for service account %s/%s", namespace, secretName, namespace, serviceAccountName) - return errors.New("Token does not match server's copy") - } - - // Make sure service account still exists (name and UID) - 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 - } - if serviceAccount.DeletionTimestamp != nil { - glog.V(4).Infof("Service account has been deleted %s/%s", namespace, serviceAccountName) - return fmt.Errorf("ServiceAccount %s/%s has been deleted", namespace, serviceAccountName) - } - if string(serviceAccount.UID) != serviceAccountUID { - glog.V(4).Infof("Service account UID no longer matches %s/%s: %q != %q", namespace, serviceAccountName, string(serviceAccount.UID), serviceAccountUID) - return fmt.Errorf("ServiceAccount UID (%s) does not match claim (%s)", serviceAccount.UID, serviceAccountUID) - } - } - - return nil -} diff --git a/pkg/serviceaccount/jwt_test.go b/pkg/serviceaccount/jwt_test.go index 668be00c41..a8cffd9f39 100644 --- a/pkg/serviceaccount/jwt_test.go +++ b/pkg/serviceaccount/jwt_test.go @@ -267,7 +267,7 @@ func TestTokenGenerateAndValidate(t *testing.T) { for k, tc := range testCases { getter := serviceaccountcontroller.NewGetterFromClient(tc.Client) - authenticator := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, tc.Keys, tc.Client != nil, getter) + authenticator := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, tc.Keys, serviceaccount.NewLegacyValidator(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/pkg/serviceaccount/legacy.go b/pkg/serviceaccount/legacy.go index f5fa96eb43..5055db7cb7 100644 --- a/pkg/serviceaccount/legacy.go +++ b/pkg/serviceaccount/legacy.go @@ -17,9 +17,14 @@ limitations under the License. package serviceaccount import ( + "bytes" + "errors" + "fmt" + "k8s.io/api/core/v1" apiserverserviceaccount "k8s.io/apiserver/pkg/authentication/serviceaccount" + "github.com/golang/glog" "gopkg.in/square/go-jose.v2/jwt" ) @@ -42,3 +47,89 @@ type legacyPrivateClaims struct { SecretName string `json:"kubernetes.io/serviceaccount/secret.name"` Namespace string `json:"kubernetes.io/serviceaccount/namespace"` } + +func NewLegacyValidator(lookup bool, getter ServiceAccountTokenGetter) Validator { + return &legacyValidator{ + lookup: lookup, + getter: getter, + } +} + +type legacyValidator struct { + lookup bool + getter ServiceAccountTokenGetter +} + +var _ = Validator(&legacyValidator{}) + +func (v *legacyValidator) Validate(tokenData string, public *jwt.Claims, privateObj interface{}) (string, string, string, error) { + private, ok := privateObj.(*legacyPrivateClaims) + if !ok { + glog.Errorf("jwt validator expected private claim of type *legacyPrivateClaims but got: %T", privateObj) + return "", "", "", errors.New("Token could not be validated.") + } + + // Make sure the claims we need exist + if len(public.Subject) == 0 { + return "", "", "", errors.New("sub claim is missing") + } + namespace := private.Namespace + if len(namespace) == 0 { + return "", "", "", errors.New("namespace claim is missing") + } + secretName := private.SecretName + if len(secretName) == 0 { + return "", "", "", errors.New("secretName claim is missing") + } + serviceAccountName := private.ServiceAccountName + if len(serviceAccountName) == 0 { + return "", "", "", errors.New("serviceAccountName claim is missing") + } + serviceAccountUID := private.ServiceAccountUID + if len(serviceAccountUID) == 0 { + return "", "", "", errors.New("serviceAccountUID claim is missing") + } + + subjectNamespace, subjectName, err := apiserverserviceaccount.SplitUsername(public.Subject) + if err != nil || subjectNamespace != namespace || subjectName != serviceAccountName { + return "", "", "", errors.New("sub claim is invalid") + } + + if v.lookup { + // Make sure token hasn't been invalidated by deletion of the secret + 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") + } + if secret.DeletionTimestamp != nil { + glog.V(4).Infof("Token is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, secretName, namespace, serviceAccountName) + return "", "", "", errors.New("Token has been invalidated") + } + if bytes.Compare(secret.Data[v1.ServiceAccountTokenKey], []byte(tokenData)) != 0 { + glog.V(4).Infof("Token contents no longer matches %s/%s for service account %s/%s", namespace, secretName, namespace, serviceAccountName) + return "", "", "", errors.New("Token does not match server's copy") + } + + // Make sure service account still exists (name and UID) + 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 + } + if serviceAccount.DeletionTimestamp != nil { + glog.V(4).Infof("Service account has been deleted %s/%s", namespace, serviceAccountName) + return "", "", "", fmt.Errorf("ServiceAccount %s/%s has been deleted", namespace, serviceAccountName) + } + if string(serviceAccount.UID) != serviceAccountUID { + glog.V(4).Infof("Service account UID no longer matches %s/%s: %q != %q", namespace, serviceAccountName, string(serviceAccount.UID), serviceAccountUID) + return "", "", "", fmt.Errorf("ServiceAccount UID (%s) does not match claim (%s)", serviceAccount.UID, serviceAccountUID) + } + } + + return private.Namespace, private.ServiceAccountName, private.ServiceAccountUID, nil +} + +func (v *legacyValidator) NewPrivateClaims() interface{} { + return &legacyPrivateClaims{} +} diff --git a/test/integration/serviceaccount/service_account_test.go b/test/integration/serviceaccount/service_account_test.go index 7bd3bacb23..90f12d35b3 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(serviceaccount.LegacyIssuer, []interface{}{&serviceAccountKey.PublicKey}, true, serviceAccountTokenGetter) + serviceAccountTokenAuth := serviceaccount.JWTTokenAuthenticator(serviceaccount.LegacyIssuer, []interface{}{&serviceAccountKey.PublicKey}, serviceaccount.NewLegacyValidator(true, serviceAccountTokenGetter)) authenticator := union.New( bearertoken.New(rootTokenAuth), bearertoken.New(serviceAccountTokenAuth),