Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

authz: support empty principals and fix rbac authenticated matcher #4883

Merged
merged 4 commits into from
Oct 21, 2021
Merged

authz: support empty principals and fix rbac authenticated matcher #4883

merged 4 commits into from
Oct 21, 2021

Conversation

ashithasantosh
Copy link
Contributor

@ashithasantosh ashithasantosh commented Oct 17, 2021

Previously, if we had empty principals in SDK policy, the translator would create RBAC policy with ANY rule for "Principal". With the changes in this PR, an empty principal in SDK policy will check for authenticated connection.
Similar behavior to unset principal_name in RBAC proto
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/rbac/v3/rbac.proto#config-rbac-v3-principal-authenticated

This PR also fixes rbac authenticated matcher, to check for TLS authenticated connection and correctly handle empty strings for TLS and plaintext case.

RELEASE NOTES:

  • authz: support empty principals and fix rbac authenticated matcher

authz/rbac_translator.go Outdated Show resolved Hide resolved
@ashithasantosh ashithasantosh added this to the 1.42 Release milestone Oct 19, 2021
@ashithasantosh ashithasantosh added the Type: Feature New features or improvements in behavior label Oct 19, 2021
@ejona86
Copy link
Member

ejona86 commented Oct 21, 2021

Could this see review soon, as it is holding up enabling RBAC in Go?

authz/sdk_end2end_test.go Outdated Show resolved Hide resolved
authz/sdk_end2end_test.go Outdated Show resolved Hide resolved
internal/xds/rbac/rbac_engine.go Outdated Show resolved Hide resolved
authz/sdk_end2end_test.go Outdated Show resolved Hide resolved
authz/sdk_end2end_test.go Outdated Show resolved Hide resolved
@dfawley dfawley assigned ashithasantosh and unassigned dfawley Oct 21, 2021
@ashithasantosh
Copy link
Contributor Author

Thank you for the review, @dfawley!:) I resolved all the comments and even the tests passed. So merging the PR..

@ashithasantosh ashithasantosh merged commit 4f21cde into grpc:master Oct 21, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants