diff --git a/pkg/serviceaccount/BUILD b/pkg/serviceaccount/BUILD index d28601f8f66..bfb6f959084 100644 --- a/pkg/serviceaccount/BUILD +++ b/pkg/serviceaccount/BUILD @@ -57,7 +57,9 @@ go_test( "//pkg/controller/serviceaccount:go_default_library", "//pkg/routes:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//staging/src/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library", "//staging/src/k8s.io/client-go/kubernetes:go_default_library", "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library", diff --git a/pkg/serviceaccount/claims.go b/pkg/serviceaccount/claims.go index 3d48b6f2dce..543efc20e20 100644 --- a/pkg/serviceaccount/claims.go +++ b/pkg/serviceaccount/claims.go @@ -98,8 +98,9 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ klog.Errorf("jwt validator expected private claim of type *privateClaims but got: %T", privateObj) return nil, errors.New("Token could not be validated.") } + nowTime := now() err := public.Validate(jwt.Expected{ - Time: now(), + Time: nowTime, }) switch { case err == nil: @@ -110,6 +111,8 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ return nil, errors.New("Token could not be validated.") } + // consider things deleted prior to now()-leeway to be invalid + invalidIfDeletedBefore := nowTime.Add(-jwt.DefaultLeeway) namespace := private.Kubernetes.Namespace saref := private.Kubernetes.Svcacct podref := private.Kubernetes.Pod @@ -120,7 +123,7 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ klog.V(4).Infof("Could not retrieve service account %s/%s: %v", namespace, saref.Name, err) return nil, err } - if serviceAccount.DeletionTimestamp != nil { + if serviceAccount.DeletionTimestamp != nil && serviceAccount.DeletionTimestamp.Time.Before(invalidIfDeletedBefore) { klog.V(4).Infof("Service account has been deleted %s/%s", namespace, saref.Name) return nil, fmt.Errorf("ServiceAccount %s/%s has been deleted", namespace, saref.Name) } @@ -136,7 +139,7 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ klog.V(4).Infof("Could not retrieve bound secret %s/%s for service account %s/%s: %v", namespace, secref.Name, namespace, saref.Name, err) return nil, errors.New("Token has been invalidated") } - if secret.DeletionTimestamp != nil { + if secret.DeletionTimestamp != nil && secret.DeletionTimestamp.Time.Before(invalidIfDeletedBefore) { klog.V(4).Infof("Bound secret is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, secref.Name, namespace, saref.Name) return nil, errors.New("Token has been invalidated") } @@ -154,7 +157,7 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ klog.V(4).Infof("Could not retrieve bound pod %s/%s for service account %s/%s: %v", namespace, podref.Name, namespace, saref.Name, err) return nil, errors.New("Token has been invalidated") } - if pod.DeletionTimestamp != nil { + if pod.DeletionTimestamp != nil && pod.DeletionTimestamp.Time.Before(invalidIfDeletedBefore) { klog.V(4).Infof("Bound pod is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, podref.Name, namespace, saref.Name) return nil, errors.New("Token has been invalidated") } diff --git a/pkg/serviceaccount/claims_test.go b/pkg/serviceaccount/claims_test.go index 91ed25ed0d3..8c5ac79a915 100644 --- a/pkg/serviceaccount/claims_test.go +++ b/pkg/serviceaccount/claims_test.go @@ -24,7 +24,10 @@ import ( "gopkg.in/square/go-jose.v2/jwt" + v1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/kubernetes/pkg/apis/core" ) @@ -182,3 +185,166 @@ func TestClaims(t *testing.T) { }) } } + +type deletionTestCase struct { + name string + time *metav1.Time + expectErr bool +} + +type claimTestCase struct { + name string + getter ServiceAccountTokenGetter + private *privateClaims + expectErr bool +} + +func TestValidatePrivateClaims(t *testing.T) { + var ( + nowUnix = int64(1514764800) + + serviceAccount = &v1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "saname", Namespace: "ns", UID: "sauid"}} + secret = &v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "secretname", Namespace: "ns", UID: "secretuid"}} + pod = &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "podname", Namespace: "ns", UID: "poduid"}} + ) + + deletionTestCases := []deletionTestCase{ + { + name: "valid", + time: nil, + }, + { + name: "deleted now", + time: &metav1.Time{Time: time.Unix(nowUnix, 0)}, + }, + { + name: "deleted near past", + time: &metav1.Time{Time: time.Unix(nowUnix-1, 0)}, + }, + { + name: "deleted near future", + time: &metav1.Time{Time: time.Unix(nowUnix+1, 0)}, + }, + { + name: "deleted now-leeway", + time: &metav1.Time{Time: time.Unix(nowUnix-60, 0)}, + }, + { + name: "deleted now-leeway-1", + time: &metav1.Time{Time: time.Unix(nowUnix-61, 0)}, + expectErr: true, + }, + } + + testcases := []claimTestCase{ + { + name: "missing serviceaccount", + getter: fakeGetter{nil, nil, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Namespace: "ns"}}, + expectErr: true, + }, + { + name: "missing secret", + getter: fakeGetter{serviceAccount, nil, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Secret: &ref{Name: "secretname", UID: "secretuid"}, Namespace: "ns"}}, + expectErr: true, + }, + { + name: "missing pod", + getter: fakeGetter{serviceAccount, nil, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Pod: &ref{Name: "podname", UID: "poduid"}, Namespace: "ns"}}, + expectErr: true, + }, + { + name: "different uid serviceaccount", + getter: fakeGetter{serviceAccount, nil, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauidold"}, Namespace: "ns"}}, + expectErr: true, + }, + { + name: "different uid secret", + getter: fakeGetter{serviceAccount, secret, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Secret: &ref{Name: "secretname", UID: "secretuidold"}, Namespace: "ns"}}, + expectErr: true, + }, + { + name: "different uid pod", + getter: fakeGetter{serviceAccount, nil, pod}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Pod: &ref{Name: "podname", UID: "poduidold"}, Namespace: "ns"}}, + expectErr: true, + }, + } + + for _, deletionTestCase := range deletionTestCases { + var ( + deletedServiceAccount = serviceAccount.DeepCopy() + deletedPod = pod.DeepCopy() + deletedSecret = secret.DeepCopy() + ) + deletedServiceAccount.DeletionTimestamp = deletionTestCase.time + deletedPod.DeletionTimestamp = deletionTestCase.time + deletedSecret.DeletionTimestamp = deletionTestCase.time + + testcases = append(testcases, + claimTestCase{ + name: deletionTestCase.name + " serviceaccount", + getter: fakeGetter{deletedServiceAccount, nil, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Namespace: "ns"}}, + expectErr: deletionTestCase.expectErr, + }, + claimTestCase{ + name: deletionTestCase.name + " secret", + getter: fakeGetter{serviceAccount, deletedSecret, nil}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Secret: &ref{Name: "secretname", UID: "secretuid"}, Namespace: "ns"}}, + expectErr: deletionTestCase.expectErr, + }, + claimTestCase{ + name: deletionTestCase.name + " pod", + getter: fakeGetter{serviceAccount, nil, deletedPod}, + private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Pod: &ref{Name: "podname", UID: "poduid"}, Namespace: "ns"}}, + expectErr: deletionTestCase.expectErr, + }, + ) + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + v := &validator{tc.getter} + _, err := v.Validate("", &jwt.Claims{Expiry: jwt.NumericDate(nowUnix)}, tc.private) + if err != nil && !tc.expectErr { + t.Fatal(err) + } + if err == nil && tc.expectErr { + t.Fatal("expected error, got none") + } + if err != nil { + return + } + }) + } +} + +type fakeGetter struct { + serviceAccount *v1.ServiceAccount + secret *v1.Secret + pod *v1.Pod +} + +func (f fakeGetter) GetServiceAccount(namespace, name string) (*v1.ServiceAccount, error) { + if f.serviceAccount == nil { + return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "serviceaccounts"}, name) + } + return f.serviceAccount, nil +} +func (f fakeGetter) GetPod(namespace, name string) (*v1.Pod, error) { + if f.pod == nil { + return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, name) + } + return f.pod, nil +} +func (f fakeGetter) GetSecret(namespace, name string) (*v1.Secret, error) { + if f.secret == nil { + return nil, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "secrets"}, name) + } + return f.secret, nil +}