xds: fail closed when ext_authz response processing throws - #13051
Merged
Conversation
CheckResponseHandler.handleResponse() only catches the checked HeaderMutationDisallowedException. An unchecked exception raised while processing a CheckResponse -- for example HeaderValue.create() rejecting a header value that is not valid ASCII -- escaped AuthzCallbackObserver.onNext(). gRPC then cancelled the stream and invoked onError(), where failure_mode_allow sent the request to the backend, turning an explicit PERMISSION_DENIED into an ALLOW. failure_mode_allow covers the authorization service being unreachable or returning an error. It does not cover a failure to process a response that the service successfully returned, so onNext() now handles its own failures and fails the call with INTERNAL instead of letting them reach onError(). Also fail the RPC, rather than silently dropping the mutation, when the authz server attempts to mutate a gRPC-owned header.
kannanjgithub
approved these changes
Sep 11, 2026
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.
When the ext_authz filter processes a
CheckResponse,CheckResponseHandler.handleResponse()only catches the checkedHeaderMutationDisallowedException. An unchecked exception raised anywhere else in that path escapedAuthzCallbackObserver.onNext().gRPC reacts to an exception from an application callback by cancelling the stream, which invokes
onError(). There,failure_mode_allowsent the request on to the backend. The net effect is that an explicitPERMISSION_DENIEDfrom the authorization server could be turned into an ALLOW.The most accessible trigger is a header value that gRPC metadata cannot represent.
HeaderValue.create()rejects anything outside horizontal tab, space and printable ASCII, while the value arrives in a protostringfield carrying arbitrary UTF-8. An authorization server that echoes back a non-ASCII character in a header value is enough; a malicious server is not required.failure_mode_allowis defined in terms of the authorization service being unreachable or returning an error. It does not cover a failure to process a response that the service successfully returned. Routing local processing errors through that policy is the underlying defect, soonNext()now handles its own failures and fails the call withINTERNALrather than letting them reachonError().Fixing this at the observer rather than at each individual throw site means the invariant holds for future call paths too. The
orElseThrow()on the DENY branch is a second instance that was already reachable in principle and is now covered.This change also makes a mutation targeting a gRPC-owned header key fail the RPC instead of being silently dropped.
Testing:
deny_withMalformedHeader_failOpen_doesNotReachBackendis the regression test. Without the fix it fails with the backend having received the request despite an explicit denial.deny_withMissingStatus_failsCallWithInternalcovers theorElseThrow()path; it previously asserted the exception escaped.handleResponse_ok_grpcOwnedHeader_deniesCallcovers the gRPC-owned header behavior, split out of the former combined binary-header test.