Skip to content

K
k3s
Unverified Commit 18f6d75f authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub
Browse files
Options

Merge pull request #62993 from WanLinghao/sa_token_fix

Automatic merge from submit-queue (batch tested with PRs 63033, 62993). 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>. fix a bug in serviceaccount validate. **What this PR does / why we need it**: As the patch shows, the original idea here is to make sure that the bounded object is still exists in cluster. But the compare is wrong. It could cause recreate object validate through bug. For example, a user requests a token which bounded with Pod A. The token should become invalid after Pod A's deletion. But if someone create a Pod with same name with Pod A, the token would be valid which should be not. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes # **Special notes for your reviewer**: **Release note**: ```release-note NONE ```
parents 6917ed47 198b9e48
Hide whitespace changes
Inline Side-by-side
Showing with 86 additions and 4 deletions
pkg/serviceaccount/claims.go
View file @ 18f6d75f
...@@ -155,8 +155,8 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ ...@@ -155,8 +155,8 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{
glog.V(4).Infof("Bound secret is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, secref.Name, namespace, saref.Name) glog.V(4).Infof("Bound secret is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, secref.Name, namespace, saref.Name)
return "", "", "", errors.New("Token has been invalidated") return "", "", "", errors.New("Token has been invalidated")
} }
if string(secref.UID) != secref.UID { if secref.UID != string(secret.UID) {
glog.V(4).Infof("Secret UID no longer matches %s/%s: %q != %q", namespace, secref.Name, string(serviceAccount.UID), secref.UID) glog.V(4).Infof("Secret UID no longer matches %s/%s: %q != %q", namespace, secref.Name, string(secret.UID), secref.UID)
return "", "", "", fmt.Errorf("Secret UID (%s) does not match claim (%s)", secret.UID, secref.UID) return "", "", "", fmt.Errorf("Secret UID (%s) does not match claim (%s)", secret.UID, secref.UID)
} }
} }
...@@ -172,8 +172,8 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{ ...@@ -172,8 +172,8 @@ func (v *validator) Validate(_ string, public *jwt.Claims, privateObj interface{
glog.V(4).Infof("Bound pod is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, podref.Name, namespace, saref.Name) glog.V(4).Infof("Bound pod is deleted and awaiting removal: %s/%s for service account %s/%s", namespace, podref.Name, namespace, saref.Name)
return "", "", "", errors.New("Token has been invalidated") return "", "", "", errors.New("Token has been invalidated")
} }
if string(podref.UID) != podref.UID { if podref.UID != string(pod.UID) {
glog.V(4).Infof("Pod UID no longer matches %s/%s: %q != %q", namespace, podref.Name, string(serviceAccount.UID), podref.UID) glog.V(4).Infof("Pod UID no longer matches %s/%s: %q != %q", namespace, podref.Name, string(pod.UID), podref.UID)
return "", "", "", fmt.Errorf("Pod UID (%s) does not match claim (%s)", pod.UID, podref.UID) return "", "", "", fmt.Errorf("Pod UID (%s) does not match claim (%s)", pod.UID, podref.UID)
} }
} }
......
test/integration/auth/svcaccttoken_test.go
View file @ 18f6d75f
...@@ -342,6 +342,88 @@ func TestServiceAccountTokenCreate(t *testing.T) { ...@@ -342,6 +342,88 @@ func TestServiceAccountTokenCreate(t *testing.T) {
doTokenReview(t, cs, treq, false) doTokenReview(t, cs, treq, false)
}) })
t.Run("a token should be invalid after recreating same name pod", func(t *testing.T) {
treq := &authenticationv1.TokenRequest{
Spec: authenticationv1.TokenRequestSpec{
Audiences: []string{"api"},
ExpirationSeconds: &one,
BoundObjectRef: &authenticationv1.BoundObjectReference{
Kind: "Pod",
APIVersion: "v1",
Name: pod.Name,
},
},
}
sa, del := createDeleteSvcAcct(t, cs, sa)
defer del()
originalPod, originalDelPod := createDeletePod(t, cs, pod)
defer originalDelPod()
treq.Spec.BoundObjectRef.UID = originalPod.UID
if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err != nil {
t.Fatalf("err: %v", err)
}
checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
checkPayload(t, treq.Status.Token, `["api"]`, "aud")
checkPayload(t, treq.Status.Token, `"test-pod"`, "kubernetes.io", "pod", "name")
checkPayload(t, treq.Status.Token, "null", "kubernetes.io", "secret")
checkPayload(t, treq.Status.Token, `"myns"`, "kubernetes.io", "namespace")
checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name")
doTokenReview(t, cs, treq, false)
originalDelPod()
doTokenReview(t, cs, treq, true)
_, recreateDelPod := createDeletePod(t, cs, pod)
defer recreateDelPod()
doTokenReview(t, cs, treq, true)
})
t.Run("a token should be invalid after recreating same name secret", func(t *testing.T) {
treq := &authenticationv1.TokenRequest{
Spec: authenticationv1.TokenRequestSpec{
Audiences: []string{"api"},
ExpirationSeconds: &one,
BoundObjectRef: &authenticationv1.BoundObjectReference{
Kind: "Secret",
APIVersion: "v1",
Name: secret.Name,
UID: secret.UID,
},
},
}
sa, del := createDeleteSvcAcct(t, cs, sa)
defer del()
originalSecret, originalDelSecret := createDeleteSecret(t, cs, secret)
defer originalDelSecret()
treq.Spec.BoundObjectRef.UID = originalSecret.UID
if treq, err = cs.CoreV1().ServiceAccounts(sa.Namespace).CreateToken(sa.Name, treq); err != nil {
t.Fatalf("err: %v", err)
}
checkPayload(t, treq.Status.Token, `"system:serviceaccount:myns:test-svcacct"`, "sub")
checkPayload(t, treq.Status.Token, `["api"]`, "aud")
checkPayload(t, treq.Status.Token, `null`, "kubernetes.io", "pod")
checkPayload(t, treq.Status.Token, `"test-secret"`, "kubernetes.io", "secret", "name")
checkPayload(t, treq.Status.Token, `"myns"`, "kubernetes.io", "namespace")
checkPayload(t, treq.Status.Token, `"test-svcacct"`, "kubernetes.io", "serviceaccount", "name")
doTokenReview(t, cs, treq, false)
originalDelSecret()
doTokenReview(t, cs, treq, true)
_, recreateDelSecret := createDeleteSecret(t, cs, secret)
defer recreateDelSecret()
doTokenReview(t, cs, treq, true)
})
} }
func doTokenReview(t *testing.T, cs externalclientset.Interface, treq *authenticationv1.TokenRequest, expectErr bool) { func doTokenReview(t *testing.T, cs externalclientset.Interface, treq *authenticationv1.TokenRequest, expectErr bool) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment