server: k8s_psat: validate pod UID & fix error message grammar#7117
Open
c4rlo wants to merge 3 commits into
Open
server: k8s_psat: validate pod UID & fix error message grammar#7117c4rlo wants to merge 3 commits into
c4rlo wants to merge 3 commits into
Conversation
TokenReview authenticates the projected service account token and returns the pod UID bound to that token. The server then fetches the live pod by namespace and name to derive node identity, selectors, and the agent SPIFFE ID. Reject the attestation when the fetched pod UID does not match the TokenReview pod UID so a stale or raced pod lookup cannot bind authenticated token data to a different live pod. Update the unit tests to model pod UIDs explicitly and cover the mismatch case. Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net>
c4rlo
requested review from
MarcosDY,
amartinezfayo,
rturner3 and
sorindumitru
as code owners
There was a problem hiding this comment.
Pull request overview
This PR tightens the Kubernetes PSAT node attestation flow by ensuring the live pod fetched from the Kubernetes API is the same pod identity returned by the TokenReview, preventing a TOCTOU-style mix-up between authenticated token data and a different live pod.
Changes:
- Add a pod UID equality check between TokenReview status and the fetched Pod object, rejecting attestation on mismatch.
- Update unit test fixtures to include explicit pod UIDs and add a mismatch test case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/server/plugin/nodeattestor/k8spsat/psat.go | Adds enforcement that the fetched pod UID matches the TokenReview pod UID before deriving agent identity/selectors. |
| pkg/server/plugin/nodeattestor/k8spsat/psat_test.go | Updates pod creation helper to set pod UID and adds a test covering UID mismatch behavior. |
Comment on lines
+227
to
+229
| if string(pod.UID) != podUID { | ||
| return status.Errorf(codes.Internal, "pod UID mismatch for cluster %q", attestationData.Cluster) | ||
| } |
Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net>
Signed-off-by: Carlo Teubner <cteubner1@bloomberg.net>
c4rlo
changed the title
| return status.Errorf(codes.Internal, "failed to get pod from k8s API server for cluster %q: %v", attestationData.Cluster, err) | ||
| } | ||
| if string(pod.UID) != podUID { | ||
| return status.Errorf(codes.PermissionDenied, "pod UID mismatch for cluster %q", attestationData.Cluster) |
Member
There was a problem hiding this comment.
I'm wondering if we could include a bit more context in this error to help operators diagnose a rejection. Including the pod name and the token's UID, for example pod UID mismatch for pod %q in cluster %q: token bound to pod UID %q, would make it clearer which pod was involved, while keeping the error limited to values the caller already presented in its token.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TokenReview authenticates the projected service account token and returns the pod UID bound to that token. The server then fetches the live pod by namespace and name to derive node identity, selectors, and the agent SPIFFE ID.
Reject the attestation when the fetched pod UID does not match the TokenReview pod UID so a stale or raced pod lookup cannot bind authenticated token data to a different live pod. Update the unit tests to model pod UIDs explicitly and cover the mismatch case.
This closes a TOCTOU issue, though it's probably somewhat theoretical.
While I was in there, I also fixed some error message grammar (
fail to→failed to).