Skip to content

xds/rbac: enforce strict presence-based short-circuit in authenticatedMatcher#9111

Merged
arjan-bal merged 2 commits into
grpc:masterfrom
al4an444:fix/xds-rbac-auth-bypass
May 6, 2026
Merged

xds/rbac: enforce strict presence-based short-circuit in authenticatedMatcher#9111
arjan-bal merged 2 commits into
grpc:masterfrom
al4an444:fix/xds-rbac-auth-bypass

Conversation

@al4an444

@al4an444 al4an444 commented May 5, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a bug in the xDS RBAC authenticatedMatcher where it incorrectly falls through URI/DNS SANs to the Subject DN. According to gRFC A41 and Envoy's specification, only the first non-empty identity source must be consulted. This bug allows an authorization bypass in certain scenarios. A regression test is included.

RELEASE NOTES:

  • xds/rbac: Fix Authenticated matcher to use the first non-empty identity source (URI SAN, DNS SAN, and Subject DN) in the order specified in gRFC A41.

@linux-foundation-easycla

linux-foundation-easycla Bot commented May 5, 2026

Copy link
Copy Markdown

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.09%. Comparing base (811290b) to head (be15b0e).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9111      +/-   ##
==========================================
+ Coverage   83.06%   83.09%   +0.02%     
==========================================
  Files         413      413              
  Lines       33487    33491       +4     
==========================================
+ Hits        27817    27829      +12     
+ Misses       4243     4238       -5     
+ Partials     1427     1424       -3     
Files with missing lines Coverage Δ
internal/xds/rbac/matchers.go 77.57% <100.00%> (+1.85%) ⬆️

... and 25 files with indirect coverage changes

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

@al4an444 al4an444 force-pushed the fix/xds-rbac-auth-bypass branch from 8339cc5 to 7717b98 Compare May 5, 2026 21:11

@easwars easwars left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, modulo minor nits

Comment thread internal/xds/rbac/rbac_engine_test.go Outdated
Comment thread internal/xds/rbac/rbac_engine_test.go Outdated
Comment thread internal/xds/rbac/rbac_engine_test.go Outdated
@easwars easwars added the Type: Security A bug or other problem affecting security label May 5, 2026
@easwars easwars added this to the 1.82 Release milestone May 5, 2026
@easwars

easwars commented May 5, 2026

Copy link
Copy Markdown
Contributor

@arjan-bal for second set of eyes

@easwars

easwars commented May 5, 2026

Copy link
Copy Markdown
Contributor

@arjan-bal : I also added a release note. Please review that too. Thanks.

@al4an444 al4an444 force-pushed the fix/xds-rbac-auth-bypass branch from 7717b98 to 2fd46b8 Compare May 5, 2026 22:32
@al4an444 al4an444 force-pushed the fix/xds-rbac-auth-bypass branch from 2fd46b8 to f3832d8 Compare May 5, 2026 22:36
@al4an444

al4an444 commented May 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I addressed the nits by removing the zero-value fields from the table cases, cleaning up the nearby test comments, and running gofmt. The targeted RBAC tests pass locally.

@arjan-bal arjan-bal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good, some minor comments related to the tests.

Comment thread internal/xds/rbac/rbac_engine_test.go
Comment thread internal/xds/rbac/rbac_engine_test.go Outdated
@arjan-bal arjan-bal modified the milestones: 1.82 Release, 1.81 Release May 6, 2026
@arjan-bal arjan-bal added the Area: xDS Includes everything xDS related, including LB policies used with xDS. label May 6, 2026
@arjan-bal arjan-bal assigned al4an444 and unassigned arjan-bal May 6, 2026
@al4an444

al4an444 commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

All good, I added a case covering empty PeerCertificates, which expects OK, and updated the URL parsing in the identity precedence test to fail with t.Fatalf on parse errors. The RBAC package tests pass locally.

@arjan-bal arjan-bal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@arjan-bal arjan-bal assigned arjan-bal and unassigned al4an444 May 6, 2026
@arjan-bal arjan-bal merged commit 94b9449 into grpc:master May 6, 2026
21 of 22 checks passed
easwars pushed a commit to easwars/grpc-go that referenced this pull request May 12, 2026
…dMatcher (grpc#9111)

This PR fixes a bug in the xDS RBAC authenticatedMatcher where it
incorrectly falls through URI/DNS SANs to the Subject DN. According to
gRFC A41 and Envoy's specification, only the first non-empty identity
source must be consulted. This bug allows an authorization bypass in
certain scenarios. A regression test is included.

RELEASE NOTES:
- xds/rbac: Fix `Authenticated` matcher to use the first non-empty
identity source (URI SAN, DNS SAN, and Subject DN) in the order
specified in gRFC A41.
easwars added a commit that referenced this pull request May 12, 2026
RELEASE NOTES:
- xds/rbac: Fix Authenticated matcher to use the first non-empty
identity source (URI SAN, DNS SAN, and Subject DN) in the order
specified in gRFC A41.

Co-authored-by: Alan Ortega Alamo <alanortega7312@gmail.com>
eleboucher pushed a commit to eleboucher/talos-mcp that referenced this pull request May 18, 2026
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [google.golang.org/grpc](https://github.com/grpc/grpc-go) | `v1.81.0` → `v1.81.1` | ![age](https://developer.mend.io/api/mc/badges/age/go/google.golang.org%2fgrpc/v1.81.1?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/go/google.golang.org%2fgrpc/v1.81.0/v1.81.1?slim=true) |

---

### Release Notes

<details>
<summary>grpc/grpc-go (google.golang.org/grpc)</summary>

### [`v1.81.1`](https://github.com/grpc/grpc-go/releases/tag/v1.81.1): Release 1.81.1

[Compare Source](grpc/grpc-go@v1.81.0...v1.81.1)

### Security

- xds/rbac: Fix a potential authorization bypass caused by incorrectly falling through URI/DNS SANs to Subject Distinguished Name (DN) when matching the authenticated principal name. With this fix, only the first non-empty identity source will be used, as per [gRFC A41](https://github.com/grpc/proposal/blob/master/A41-xds-rbac.md). ([#&#8203;9111](grpc/grpc-go#9111))
  - Special Thanks: [@&#8203;al4an444](https://github.com/al4an444)

### Bug Fixes

- otel: Segregate client and server RPC information used for metrics and traces, to avoid one overwriting the other. ([#&#8203;9081](grpc/grpc-go#9081))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJ0eXBlL3BhdGNoIl19-->

Reviewed-on: https://git.erwanleboucher.dev/eleboucher/talos-mcp/pulls/2
santiago-praetorian pushed a commit to praetorian-inc/hadrian that referenced this pull request May 26, 2026
Verified end-to-end alongside #109 in worktree lab-3865 (LAB-3865): build/vet/lint clean, go test -race ./... 19/19 packages PASS, live tests 7/7 PASS (vulnerable-api × 4 auth modes, grpc, dvga, crapi, 284 findings). Includes upstream security fix for xds/rbac authorization bypass (grpc/grpc-go#9111).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: xDS Includes everything xDS related, including LB policies used with xDS. Type: Security A bug or other problem affecting security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants