Skip to content

Cherry-pick #9258 and #9332 into v1.83.x - #9335

Merged
easwars merged 2 commits into
grpc:v1.83.xfrom
easwars:backport-v1.83.1
Aug 19, 2026
Merged

Cherry-pick #9258 and #9332 into v1.83.x#9335
easwars merged 2 commits into
grpc:v1.83.xfrom
easwars:backport-v1.83.1

Conversation

@easwars

@easwars easwars commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Original PRs: #9258 and #9332

RELEASE NOTES:

  • xds/rbac: Fix a bug where nested Principal or Permission rules with :scheme or grpc- prefixed header matchers were not rejected, which could cause DENY rules to fail open.
  • xds/rbac: Fix a bug where the host header matcher was not being replaced with :authority in nested Principal or Permission rules.
  • xds/rbac: Fix a bug where a header matcher whose name was not lowercase, such as X-Role, matched no header, which could cause DENY rules to fail open.
  • xds/rbac: Fix a bug where a :scheme or grpc- prefixed header matcher was accepted when its name was not lowercase.
  • xds/rbac: Fix a bug where a Host header matcher was not replaced with :authority.

@easwars easwars added the Type: Security A bug or other problem affecting security label Aug 19, 2026
@easwars easwars added this to the 1.83 Release milestone Aug 19, 2026
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.24%. Comparing base (8cfeca0) to head (0b093a6).

Files with missing lines Patch % Lines
internal/xds/httpfilter/rbac/rbac.go 87.50% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           v1.83.x    #9335      +/-   ##
===========================================
- Coverage    83.36%   83.24%   -0.12%     
===========================================
  Files          421      421              
  Lines        34086    34119      +33     
===========================================
- Hits         28415    28402      -13     
- Misses        4247     4277      +30     
- Partials      1424     1440      +16     
Files with missing lines Coverage Δ
internal/xds/httpfilter/rbac/rbac.go 74.07% <87.50%> (+11.40%) ⬆️

... and 32 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nvxbug and others added 2 commits August 19, 2026 06:26
…c#9258)

parseConfig only walks the top-level Permissions and Principals of each
RBAC policy when it applies the A41 header-name rules, so a header
matcher nested inside an and_rules, or_rules, or not_rule is never
checked. A control plane can put a `:scheme` or `grpc-` prefixed matcher
inside a nested rule to slip past the validation A41 says must reject
it, and a nested `host` matcher never gets rewritten to `:authority`, so
it silently fails to match the header grpc-go actually carries (a deny
policy on a nested host matcher fails open).

Walk the full permission and principal trees so both the :scheme/grpc-
rejection and the host to :authority rewrite reach matchers at any
depth. Doing it in parseConfig keeps the check in the one place that
already owns A41 validation, and folds the two former top-level passes
into a single recursive walk shared by permissions and principals.

RELEASE NOTES:
- xds/rbac: Fix a bug where nested `Principal` or `Permission` rules
with `:scheme` or `grpc-` prefixed header matchers were not rejected,
which could cause DENY rules to fail open.
- xds/rbac: Fix a bug where the `host` header matcher was not being
replaced with `:authority` in nested `Principal` or `Permission` rules.

(cherry picked from commit 1f4c6f3)
The RBAC filter passes the name of a header matcher to the matching
engine unchanged. The metadata that the engine matches against always
has lowercase keys. A name that contains an uppercase character
therefore matches no header, and the rule that holds it never fires. The
policy parses, reports no error and looks active. A DENY policy written
this way fails open.

The A41 validation reads the same unnormalized name, so the rejection of
:scheme and grpc- prefixed matchers misses the name Grpc-Status. A Host
matcher also keeps its name, although A41 makes host and :authority
equivalent.

Lowercase the name in normalizeHeaderMatcher. That function already owns
the A41 rules and already rewrites the name in place, so the matching
engine, the :scheme and grpc- rejection, and the host alias all read one
normalized name. Envoy holds each header matcher name in a
LowerCaseString, grpc-java lowercases the name before it looks the
header up, and authz/rbac_translator.go lowercases the name on the
non-xDS path.

The new test in test/xds shows the effect on an end user. A DENY policy
on the header name User-Agent returns OK for every RPC before the change
and PermissionDenied after it. The unit tests cover the name that the
parse gives to the engine, the case of the :scheme and grpc- rejection,
and the host alias, at the top level and inside a nested rule.

RELEASE NOTES:

- xds/rbac: Fix a bug where a header matcher whose name was not
lowercase, such as `X-Role`, matched no header, which could cause DENY
rules to fail open.
- xds/rbac: Fix a bug where a `:scheme` or `grpc-` prefixed header
matcher was accepted when its name was not lowercase.
- xds/rbac: Fix a bug where a `Host` header matcher was not replaced
with `:authority`.

Co-authored-by: Markus Magnuson <331091+alimony@users.noreply.github.com>
(cherry picked from commit db94828)
@easwars
easwars merged commit ebba6f3 into grpc:v1.83.x Aug 19, 2026
14 checks passed
@easwars
easwars deleted the backport-v1.83.1 branch August 19, 2026 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: Security A bug or other problem affecting security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants