xds/rbac: enforce strict presence-based short-circuit in authenticatedMatcher#9111
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 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
🚀 New features to boost your workflow:
|
8339cc5 to
7717b98
Compare
|
@arjan-bal for second set of eyes |
|
@arjan-bal : I also added a release note. Please review that too. Thanks. |
7717b98 to
2fd46b8
Compare
2fd46b8 to
f3832d8
Compare
|
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
left a comment
There was a problem hiding this comment.
Mostly looks good, some minor comments related to the tests.
|
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. |
…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.
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>
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` |  |  | --- ### 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). ([#​9111](grpc/grpc-go#9111)) - Special Thanks: [@​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. ([#​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
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).
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:
Authenticatedmatcher to use the first non-empty identity source (URI SAN, DNS SAN, and Subject DN) in the order specified in gRFC A41.