-
Notifications
You must be signed in to change notification settings - Fork 249
A82: xDS System Root Certificates #436
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
Conversation
This allows using system root certs in gRPC. For details, see grpc/proposal#436. Risk Level: Low Testing: N/A Docs Changes: Included in PR Signed-off-by: Mark D. Roth <roth@google.com>
This allows using system root certs in gRPC. For details, see grpc/proposal#436. Risk Level: Low Testing: N/A Docs Changes: Included in PR Signed-off-by: Mark D. Roth <roth@google.com> Mirrored from https://github.com/envoyproxy/envoy @ 6364882088d5fce4b39d5ad3d0c0fac51c761b09
As per gRFC A82 (grpc/proposal#436). Closes #37185 COPYBARA_INTEGRATE_REVIEW=#37185 from markdroth:xds_system_root_certs 9ee1e82 PiperOrigin-RevId: 651896612
As per gRFC A82 (grpc/proposal#436). Closes grpc#37185 COPYBARA_INTEGRATE_REVIEW=grpc#37185 from markdroth:xds_system_root_certs 9ee1e82 PiperOrigin-RevId: 651896612
|
|
||
| ### Temporary environment variable protection | ||
|
|
||
| Use of the `use_system_root_certs` field in CDS and LDS will be guarded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I noticed that we were going to have different validation for client-side even though this is in a shared message with server-side. Up above it says "Note that LDS validation will be unchanged" which appears to disagree with this line. Only supporting it on client-side might be a bit harder to support, since this is in CommonTlsContext. I agree we don't need it on server-side, but can we add support for it anyway when there is shared code?
Also: s/use_system_root_certs/system_root_certs/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We did talk about this question specifically before finalizing this gRFC. I had originally intended to support this option on both the client side and server side, just for consistency, but when I went to implement this in C-core, it turned out to be non-trivial, because we don't already have server-side code for using system root certs, so we decided to exclude it.
You're right about the typo. I'll send a separate PR to fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sent #451 to fix this.
As per gRFC A82 (grpc/proposal#436). Closes grpc#37185 COPYBARA_INTEGRATE_REVIEW=grpc#37185 from markdroth:xds_system_root_certs 9ee1e82 PiperOrigin-RevId: 651896612
As per gRFC A82 (grpc/proposal#436). Closes grpc#37185 COPYBARA_INTEGRATE_REVIEW=grpc#37185 from markdroth:xds_system_root_certs 9ee1e82 PiperOrigin-RevId: 651896612
No description provided.