mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-23 11:50:44 +00:00
serviceaccount/claims: include validation failure error in the log
Without this fix, the errors are logged as: unexpected validation error: *errors.errorString Signed-off-by: Monis Khan <mok@vmware.com>
This commit is contained in:
Monis Khan
parent
0fec47582c
commit
92c8596002
@ -111,20 +111,31 @@ var _ = Validator(&validator{})
|
||||
func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims, privateObj interface{}) (*apiserverserviceaccount.ServiceAccountInfo, error) {
|
||||
private, ok := privateObj.(*privateClaims)
|
||||
if !ok {
|
||||
klog.Errorf("jwt validator expected private claim of type *privateClaims but got: %T", privateObj)
|
||||
return nil, errors.New("Token could not be validated.")
|
||||
klog.Errorf("service account jwt validator expected private claim of type *privateClaims but got: %T", privateObj)
|
||||
return nil, errors.New("service account token claims could not be validated due to unexpected private claim")
|
||||
}
|
||||
nowTime := now()
|
||||
err := public.Validate(jwt.Expected{
|
||||
Time: nowTime,
|
||||
})
|
||||
switch {
|
||||
case err == nil:
|
||||
case err == jwt.ErrExpired:
|
||||
return nil, errors.New("Token has expired.")
|
||||
switch err {
|
||||
case nil:
|
||||
// successful validation
|
||||
|
||||
case jwt.ErrExpired:
|
||||
return nil, errors.New("service account token has expired")
|
||||
|
||||
case jwt.ErrNotValidYet:
|
||||
return nil, errors.New("service account token is not valid yet")
|
||||
|
||||
// our current use of jwt.Expected above should make these cases impossible to hit
|
||||
case jwt.ErrInvalidAudience, jwt.ErrInvalidID, jwt.ErrInvalidIssuer, jwt.ErrInvalidSubject:
|
||||
klog.Errorf("service account token claim validation got unexpected validation failure: %v", err)
|
||||
return nil, fmt.Errorf("service account token claims could not be validated: %w", err) // safe to pass these errors back to the user
|
||||
|
||||
default:
|
||||
klog.Errorf("unexpected validation error: %T", err)
|
||||
return nil, errors.New("Token could not be validated.")
|
||||
klog.Errorf("service account token claim validation got unexpected error type: %T", err) // avoid leaking unexpected information into the logs
|
||||
return nil, errors.New("service account token claims could not be validated due to unexpected validation error") // return an opaque error
|
||||
}
|
||||
|
||||
// consider things deleted prior to now()-leeway to be invalid
|
||||
@ -141,11 +152,11 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
|
||||
}
|
||||
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)
|
||||
return nil, fmt.Errorf("service account %s/%s has been deleted", namespace, saref.Name)
|
||||
}
|
||||
if string(serviceAccount.UID) != saref.UID {
|
||||
klog.V(4).Infof("Service account UID no longer matches %s/%s: %q != %q", namespace, saref.Name, string(serviceAccount.UID), saref.UID)
|
||||
return nil, fmt.Errorf("ServiceAccount UID (%s) does not match claim (%s)", serviceAccount.UID, saref.UID)
|
||||
return nil, fmt.Errorf("service account UID (%s) does not match claim (%s)", serviceAccount.UID, saref.UID)
|
||||
}
|
||||
|
||||
if secref != nil {
|
||||
@ -153,15 +164,15 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
|
||||
secret, err := v.getter.GetSecret(namespace, secref.Name)
|
||||
if err != nil {
|
||||
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")
|
||||
return nil, errors.New("service account token has been invalidated")
|
||||
}
|
||||
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")
|
||||
return nil, errors.New("service account token has been invalidated")
|
||||
}
|
||||
if secref.UID != string(secret.UID) {
|
||||
klog.V(4).Infof("Secret UID no longer matches %s/%s: %q != %q", namespace, secref.Name, string(secret.UID), secref.UID)
|
||||
return nil, fmt.Errorf("Secret UID (%s) does not match claim (%s)", secret.UID, secref.UID)
|
||||
return nil, fmt.Errorf("secret UID (%s) does not match service account secret ref claim (%s)", secret.UID, secref.UID)
|
||||
}
|
||||
}
|
||||
|
||||
@ -171,15 +182,15 @@ func (v *validator) Validate(ctx context.Context, _ string, public *jwt.Claims,
|
||||
pod, err := v.getter.GetPod(namespace, podref.Name)
|
||||
if err != nil {
|
||||
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")
|
||||
return nil, errors.New("service account token has been invalidated")
|
||||
}
|
||||
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")
|
||||
return nil, errors.New("service account token has been invalidated")
|
||||
}
|
||||
if podref.UID != string(pod.UID) {
|
||||
klog.V(4).Infof("Pod UID no longer matches %s/%s: %q != %q", namespace, podref.Name, string(pod.UID), podref.UID)
|
||||
return nil, fmt.Errorf("Pod UID (%s) does not match claim (%s)", pod.UID, podref.UID)
|
||||
return nil, fmt.Errorf("pod UID (%s) does not match service account pod ref claim (%s)", pod.UID, podref.UID)
|
||||
}
|
||||
podName = podref.Name
|
||||
podUID = podref.UID
|
||||
|
||||
@ -223,7 +223,9 @@ type claimTestCase struct {
|
||||
name string
|
||||
getter ServiceAccountTokenGetter
|
||||
private *privateClaims
|
||||
expectErr bool
|
||||
expiry jwt.NumericDate
|
||||
notBefore jwt.NumericDate
|
||||
expectErr string
|
||||
}
|
||||
|
||||
func TestValidatePrivateClaims(t *testing.T) {
|
||||
@ -264,41 +266,61 @@ func TestValidatePrivateClaims(t *testing.T) {
|
||||
}
|
||||
|
||||
testcases := []claimTestCase{
|
||||
{
|
||||
name: "good",
|
||||
getter: fakeGetter{serviceAccount, nil, nil},
|
||||
private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Namespace: "ns"}},
|
||||
expectErr: "",
|
||||
},
|
||||
{
|
||||
name: "expired",
|
||||
getter: fakeGetter{serviceAccount, nil, nil},
|
||||
private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Namespace: "ns"}},
|
||||
expiry: jwt.NewNumericDate(now().Add(-1_000 * time.Hour)),
|
||||
expectErr: "service account token has expired",
|
||||
},
|
||||
{
|
||||
name: "not yet valid",
|
||||
getter: fakeGetter{serviceAccount, nil, nil},
|
||||
private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Namespace: "ns"}},
|
||||
notBefore: jwt.NewNumericDate(now().Add(1_000 * time.Hour)),
|
||||
expectErr: "service account token is not valid yet",
|
||||
},
|
||||
{
|
||||
name: "missing serviceaccount",
|
||||
getter: fakeGetter{nil, nil, nil},
|
||||
private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauid"}, Namespace: "ns"}},
|
||||
expectErr: true,
|
||||
expectErr: `serviceaccounts "saname" not found`,
|
||||
},
|
||||
{
|
||||
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,
|
||||
expectErr: "service account token has been invalidated",
|
||||
},
|
||||
{
|
||||
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,
|
||||
expectErr: "service account token has been invalidated",
|
||||
},
|
||||
{
|
||||
name: "different uid serviceaccount",
|
||||
getter: fakeGetter{serviceAccount, nil, nil},
|
||||
private: &privateClaims{Kubernetes: kubernetes{Svcacct: ref{Name: "saname", UID: "sauidold"}, Namespace: "ns"}},
|
||||
expectErr: true,
|
||||
expectErr: "service account UID (sauid) does not match claim (sauidold)",
|
||||
},
|
||||
{
|
||||
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,
|
||||
expectErr: "secret UID (secretuid) does not match service account secret ref claim (secretuidold)",
|
||||
},
|
||||
{
|
||||
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,
|
||||
expectErr: "pod UID (poduid) does not match service account pod ref claim (poduidold)",
|
||||
},
|
||||
}
|
||||
|
||||
@ -312,24 +334,30 @@ func TestValidatePrivateClaims(t *testing.T) {
|
||||
deletedPod.DeletionTimestamp = deletionTestCase.time
|
||||
deletedSecret.DeletionTimestamp = deletionTestCase.time
|
||||
|
||||
var saDeletedErr, deletedErr string
|
||||
if deletionTestCase.expectErr {
|
||||
saDeletedErr = "service account ns/saname has been deleted"
|
||||
deletedErr = "service account token has been invalidated"
|
||||
}
|
||||
|
||||
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,
|
||||
expectErr: saDeletedErr,
|
||||
},
|
||||
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,
|
||||
expectErr: deletedErr,
|
||||
},
|
||||
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,
|
||||
expectErr: deletedErr,
|
||||
},
|
||||
)
|
||||
}
|
||||
@ -337,20 +365,30 @@ func TestValidatePrivateClaims(t *testing.T) {
|
||||
for _, tc := range testcases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
v := &validator{tc.getter}
|
||||
_, err := v.Validate(context.Background(), "", &jwt.Claims{Expiry: jwt.NumericDate(nowUnix)}, tc.private)
|
||||
if err != nil && !tc.expectErr {
|
||||
t.Fatal(err)
|
||||
expiry := jwt.NumericDate(nowUnix)
|
||||
if tc.expiry != 0 {
|
||||
expiry = tc.expiry
|
||||
}
|
||||
if err == nil && tc.expectErr {
|
||||
t.Fatal("expected error, got none")
|
||||
}
|
||||
if err != nil {
|
||||
return
|
||||
_, err := v.Validate(context.Background(), "", &jwt.Claims{Expiry: expiry, NotBefore: tc.notBefore}, tc.private)
|
||||
if len(tc.expectErr) > 0 {
|
||||
if errStr := errString(err); tc.expectErr != errStr {
|
||||
t.Fatalf("expected error %q but got %q", tc.expectErr, errStr)
|
||||
}
|
||||
} else if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func errString(err error) string {
|
||||
if err == nil {
|
||||
return ""
|
||||
}
|
||||
|
||||
return err.Error()
|
||||
}
|
||||
|
||||
type fakeGetter struct {
|
||||
serviceAccount *v1.ServiceAccount
|
||||
secret *v1.Secret
|
||||
|
||||
Loading…
Reference in New Issue
Block a user