Repository navigation
controlplane: log the served_by and audience refusals at /sts/token - #795
Open
asivanadi0 wants to merge 1 commit into
Open
asivanadi0 wants to merge 1 commit into
asivanadi0 wants to merge 1 commit into
Conversation
The served_by check and the resolveEgressAudience check in HandleSTSToken return 403 before the caller's Biscuit is verified, and neither wrote a Border Crossing line, so an audit join on the deny lines could not see a node probing destinations it does not serve. Both refusals now write a deny line in the same shape as the existing /sts/token one: direction, endpoint, decision, actor_node (proven by the PoP challenge), destination, the requested audience and the reason. client_peer_id is empty because nothing in the Biscuit has been verified at that point. Refusals before the challenge verifies stay as they are. TestSTSTokenBorderCrossingDeniesBeforeBiscuit covers both refusals and checks for exactly one deny line each. Refs google#742 Signed-off-by: Anpoo Sivanadi <sivanadi08@gmail.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces auditing for /sts/token denials that occur before the caller's Biscuit is verified, specifically for served_by and audience refusals. It adds the logSTSDenyBeforeBiscuit helper to log these 'Border Crossing' deny events with an empty client_peer_id, implements a comprehensive integration test (TestSTSTokenBorderCrossingDeniesBeforeBiscuit) to verify this behavior, and updates the security architecture documentation. No review comments were provided, and the changes comply with the repository's style guide, so I have no feedback to provide.
This branch has not been deployed
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.
Refs #742
Follow-up to the audit gap raised in the review of #750. Two refusals at
POST /sts/tokenreturn 403 before the caller's Biscuit is verified and wrote noBorder Crossingline: theserved_bycheck and theresolveEgressAudiencecheck inHandleSTSToken. A node probing destinations it doesn't serve left no trace in an audit join on the deny lines.Both now write a deny line in the same shape as the existing
/sts/tokenone:direction=outbound,endpoint=/sts/token,decision=deny,actor_node,destination,reason, and the requestedaudience(the resolved one doesn't exist yet when the audience check is what failed). Both checks run after the PoP challenge has verified, soactor_nodeis proven.client_peer_idis empty, since nothing in the Biscuit has been verified at that point. Refusals before the challenge (no node credential, router role, rate limit, bad body, failed challenge) are unchanged. The Mint endpoint row insecurity-architecture.mdnow says so.Test:
TestSTSTokenBorderCrossingDeniesBeforeBiscuit, next toTestSTSTokenBorderCrossingNamesTheOriginPeerand using the same go-log pipe. One case calls anEgressDestinationwhoseserved_bythe egress node doesn't match, the other sends an audience that doesn't match the destination'soidc_federationaudience. Each checks for a 403 and exactly one deny line with the calling node, the destination, the requested audience, the refusal reason and an emptyclient_peer_id. The caller's Biscuit is a Task Biscuit bound to another peer, so a line logged after Biscuit verification would show up as a non-emptyclient_peer_id. The test fails onmainwith no line for theserved_bycase.go test -race ./internal/controlplane/andgolangci-lint run ./internal/controlplane/...pass.