A29: xDS-Based Security for gRPC Clients and Servers #243

sanjaypujare · 2021-06-11T16:49:36Z

CC @ejona86 @dfawley @yashykt @easwars pls review.

A29-xds-tls-security.md

ejona86 · 2021-06-18T18:55:24Z

A29-xds-tls-security.md

+
+##### Server Authorization aka Subject-Alt-Name Checks
+
+If [match_subject_alt_names](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/common.proto#L373)


What if it isn't present? Is no verification of the server performed? If so, it seems we have to guarantee we aren't missing other important configuration that might be used instead, like verify_certificate_hash, verify_certificate_spki.

Don't know how I missed this comment earlier.

What if it isn't present? Is no verification of the server performed?

The server cert trust verification is performed of course. But there is no verification (or rather matching) of the ID presented in the certificate because we don't know what to match it to.

... verify_certificate_hash, verify_certificate_spki....

verify_certificate_hash, verify_certificate_spki are purely cryptographic verification of the peer certificate based on out of band values supplied by the control plane and I don't think is a substitute for either hostname verification in Web PKI or the subject-alt-name check here.

However I should probably add here that we should NACK an update that has these fields populated. Thanks for pointing that out.

ejona86 · 2021-06-18T19:12:13Z

A29-xds-tls-security.md

+[`FilterChain`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/config/listener/v3/listener_components.proto#L193)s.
+The `FilterChain` contains the `DownstreamTlsContext` that is applied to an incoming connection
+after selecting the best-matching `FilterChain` as described in
+[FilterChainMatch](A36-xds-for-servers.md#filterchainmatch). If the server is using `XdsServerCredentials`,


The server doesn't know if it is using XdsServerCredentials.

True. I understand what you are saying and our implementation does incorporate that. But let me know how this should be reworded here.

ejona86 · 2021-06-18T19:14:31Z

A29-xds-tls-security.md

+service mesh requires the security features described in this proposal.
+
+The Certificate Provider Plugin Framework does not depend on an agent (SDS server). The
+SDS server/agent based solution compromised security (by sharing the private key with the


There is still an agent here, in order to dump files on disk, so this seems questionable. The security property here is the same independent of whether it dumps files on disk or a UDS socket. I can understand if we say we want to allow for systems that don't share the private key, although I think we believe the best security is for the workload to not have the private key at all and keep it solely within an agent.

Yes, your comment applies to the file_watcher plugin but the "Certificate Provider Plugin Framework" itself does not assume the existence of an agent. For example, a plugin that mints CSRs and gets them signed by a CA. But I will modify the text to address your comment.

A29-xds-tls-security.md

markdroth

This looks like a great start!

Please let me know if you have any questions. Thanks!

A29-xds-tls-security.md

markdroth · 2021-06-24T16:06:46Z

A29-xds-tls-security.md

+The caller of the framework registers a watcher with the plugin to receive
+certificate updates as described below.
+
+### Implementing Security in the xDS Flow


I don't think we really need to rehash the overall flow here; that should already be covered earlier in the document. Instead, I think this section should be converted into an "Implementation Details" section, which includes things like the internals of the CertificateProvider API (e.g., flow from CertificateProvider to distributor to TLS creds) or how data flows through LB policies in various languages.

Okay, good point and I will convert this into an "Implementation Details" section. Regarding your suggestion

...or how data flows through LB policies in various languages.

I thought it would be better to leave out language specific implementation details from this gRFC similar to how other gRFCs have done. What do you think?

It's kind of a gray area. I think that for a gRFC in the A category (as opposed to those in the L category), the overall design and functional specification needs to be described in a language-agnostic way. However, it's also fine to include some language-specific details to help implementors understand how things need to work, as long as those details are not the core of the document.

In this case, I do think it would be useful to include a high-level description of how data flows through the LB policies. The comments you have earlier in the doc about how C-core uses channel args and Java uses an SslContextProvider are appropriate to mention as part of that.

A29-xds-tls-security.md

yashykt · 2021-06-26T00:34:24Z

A29-xds-tls-security.md

+the CertificateProvider plugin framework enables gRPC to acquire the certificates necessary for the TLS handshake.
+
+The field [`tls_certificate_certificate_provider_instance`][TCCPI] is used for the local certificate and the private
+key. And either [`validation_context_certificate_provider_instance`][VCCPI] or


I think we should just support one way of providing validation_context_certificate_provider_instance and that should be the one inside of combined_validation_context. Having multiple way unnecessarily expands the API surface. That being said, I see that @markdroth added this field to the proto in https://github.com/envoyproxy/envoy/pull/12237/files so I might be missing some context here.

(Core only supports validation_context_certificate_provider_instance inside of combined_validation_context at the moment.)

As discussed, I agree with you. I'll change the text to only support validation_context_certificate_provider_instance inside of combined_validation_context.

I was talking with @ejona86 about this just last week. I think we misunderstood how the xDS validation config worked when we added those fields. The validation_context_certificate_provider and validation_context_certificate_provider_instance fields don't actually make any sense, because cert providers don't return a complete CertificateValidationContext proto; they just provide the CA certs, which are basically just the trusted_ca field within that proto.

What we probably should have done was simply add the new cert provider fields in CommonTlsContext but not inside of the validation_context_type oneof. The idea would be that the CertificateValidationContext is obtained from one of validation_context, validation_context_sds_secret_config, or combined_validation_context, fully populated except for the trusted_ca field. Then, as a second step, the cert provider is used to populate the trusted_ca field.

I've put together envoyproxy/envoy#17201 for this. We'll need to update all 3 languages and TD.

CC @jaychenatr

I think the open issue of potential changes to the CertificateProvider fields in xDS is the biggest open issue in this document. We really need to get this nailed down before we finalize this gRFC.

yashykt · 2021-06-26T00:47:25Z

A29-xds-tls-security.md

+[`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L259)
+or [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)
+inside [`combined_validation_context`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L251)
+is used for the root certificate-chain for validating peer certificates. For mTLS both the local certificate


+1
Core's behavior: If root certificates (validation context) are present, then client certificates are requested. If additionally, require_client_certificate is set, client certificates are mandated. https://github.com/grpc/grpc/blob/69a130c6ecc1353689b903cf5785a9dc296b39cc/src/core/lib/security/credentials/xds/xds_credentials.cc#L211

If the gRFC adopts this behavior, a question arises on whether we NACK a config where require_client_certificate is true, but no validation context is provided.

yashykt · 2021-06-26T00:50:35Z

A29-xds-tls-security.md

+and the client needs only the root certificate
+(the [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)).
+
+gRPC silently ignores the other fields in `CommonTlsContext` pertaining to other ways of fetching


+1
Core's behavior:
On the server side, NACK if no tls_certificate_certificate_provider_instance is found.
On the client side, NACK if no validation_context_certificate_provider_instance is found.

yashykt · 2021-06-26T01:04:13Z

A29-xds-tls-security.md

+[`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L259)
+or [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)
+inside [`combined_validation_context`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L251)
+is used for the root certificate-chain for validating peer certificates. For mTLS both the local certificate


We should probably also mention the behavior if the instance_name is not recognized, i.e., not present in the bootstrap file.
Core's behavior - Don't check at parsing time, so no NACKing. It's checked later when it needs to be used, at which point an error is propagated. I believe that results in a TransientFailurePicker on the client side and closing the connection on the server side.

A29-xds-tls-security.md

Add C++ section for XdsServerCredentials

A29-xds-tls-security.md

easwars · 2021-06-29T22:01:12Z

A29-xds-tls-security.md

+   server := xds.NewGRPCServer(grpc.Creds(creds))
+```
+
+### xDS Protocol


Sorry for being late to the party. But the sub-sections underneath and hard to read since they are structured as paragraphs. What do you think about using bullet points instead?

Is that because you are trying to view it as a raw text file? How about using the markdown mode of the browser? Also what's the exact issue of "hard to read" due to paragraphs?

I was viewing the markdown, the issue is not that. I feel that each paragraph is just a stream of words, with references to a lot of protos and fields within them. If we split them into points, and show relevant proto snippets with relevant fields, I feel it would make it easier to read.

+1, had the same feeling. It's not a blocker, but bullet points would've been easier to follow.

easwars · 2021-06-29T22:03:29Z

A29-xds-tls-security.md

+[A27:CDS][] describes the flow. The field [`transport_socket`][CL-TS] is used to extract the
+[`UpstreamTlsContext`][UTC] as described [here][CL-TS-comment].
+Note that we don't (currently) support [`transport_socket_matches`][CL-TS-matches].
+The `UpstreamTlsContext` thus obtained is passed down to all the child policies and connections.


The actual UpstreamTlsContext message is not passed down to the child policies. Instead whatever information is of use to us is extracted and that is what is passed down. Now sure if that distinction is useful to highlight here.

A29-xds-tls-security.md

easwars · 2021-06-29T22:09:33Z

A29-xds-tls-security.md

+[`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L259)
+or [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)
+inside [`combined_validation_context`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L251)
+is used for the root certificate-chain for validating peer certificates. For mTLS both the local certificate


gRPC-Go NACKs the resource if require_client_certificate is set and a certificate provider instance name for the root certificate is not provided. I think we discussed this long back and decided to NACK in this case.

easwars · 2021-06-29T22:27:27Z

A29-xds-tls-security.md

+[`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L259)
+or [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)
+inside [`combined_validation_context`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L251)
+is used for the root certificate-chain for validating peer certificates. For mTLS both the local certificate


We should probably also mention the behavior if the instance_name is not recognized, i.e., not present in the bootstrap file.

In gRPC-Go, we do something similar to Core. In fact, we cannot verify at parsing time as to whether the given certificate provider instance name is present in the bootstrap config or not. So, we simply accept it. But when the update it given to the CDS balancer, it is able to verify if the instance name is present in the bootstrap config or not. If the instance name is not found in the bootstrap config, the CDS balancer propagates such errors down to its child policy, which eventually leads to the channel being put in transient failure.

On the server side, since we are not required to put the server in not-serving mode for bad configuration (we are only required to do that when the configuration is deleted), we simply fail the connection at handshake time if the provider instance is not found in the bootstrap file.

easwars · 2021-06-29T22:29:34Z

A29-xds-tls-security.md

+and the client needs only the root certificate
+(the [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)).
+
+gRPC silently ignores the other fields in `CommonTlsContext` pertaining to other ways of fetching


Same behavior in Go as in Core.

A29-xds-tls-security.md

markdroth · 2021-06-30T17:00:42Z

A29-xds-tls-security.md

+the CertificateProvider plugin framework enables gRPC to acquire the certificates necessary for the TLS handshake.
+
+The field [`tls_certificate_certificate_provider_instance`][TCCPI] is used for the local certificate and the private
+key. And either [`validation_context_certificate_provider_instance`][VCCPI] or


I was talking with @ejona86 about this just last week. I think we misunderstood how the xDS validation config worked when we added those fields. The validation_context_certificate_provider and validation_context_certificate_provider_instance fields don't actually make any sense, because cert providers don't return a complete CertificateValidationContext proto; they just provide the CA certs, which are basically just the trusted_ca field within that proto.

What we probably should have done was simply add the new cert provider fields in CommonTlsContext but not inside of the validation_context_type oneof. The idea would be that the CertificateValidationContext is obtained from one of validation_context, validation_context_sds_secret_config, or combined_validation_context, fully populated except for the trusted_ca field. Then, as a second step, the cert provider is used to populate the trusted_ca field.

I've put together envoyproxy/envoy#17201 for this. We'll need to update all 3 languages and TD.

CC @jaychenatr

markdroth · 2021-06-30T17:15:00Z

A29-xds-tls-security.md

+[`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L259)
+or [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)
+inside [`combined_validation_context`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L251)
+is used for the root certificate-chain for validating peer certificates. For mTLS both the local certificate


Why can't we validate that the certificate provider instance is valid at config parsing time? I think that would be a trivial change in C-core, and it seems like the right thing to do. We basically do the same thing for supported HTTP filters (the only difference being that those are not configured in the bootstrap file, they're set at compile time).

sanjaypujare · 2021-07-09T20:48:16Z

All the reviewers: thanks for your comments. I have addressed all your comments. Pls take a look.

sergiitk

LGTM.

nit: I'd recommend to put it though a spell checker. My Grammarly shows a bunch of complaints on missing commas, etc.

A29-xds-tls-security.md

sergiitk · 2021-07-13T21:15:22Z

A29-xds-tls-security.md

+provider whereas a client running in cloud may use a cloud-vendor provided implementation to obtain
+the certificate.
+
+gRPC provides a `CertificateProvider` plugin API that can support multiple implementations. The plugin API


editorial nit: tautology

Suggested change

gRPC provides a `CertificateProvider` plugin API that can support multiple implementations. The plugin API

gRPC offers a `CertificateProvider` plugin API that can support multiple implementations. The plugin API

A29-xds-tls-security.md

Co-authored-by: Sergii Tkachenko <hi@sergii.org>

markdroth

This is moving in the right direction!

Please let me know if you have any questions. Thanks!

markdroth · 2021-07-22T20:30:22Z

A29-xds-tls-security.md

+always validates all security configurations in CDS or LDS regardless of whether an XdsChannelCredentials
+or XdsServerCredentials is in force. If validation fails, the update is NACKed. When an update is
+accepted the correponding security configuration in CDS or LDS is used if the corresponding
+Xds*Credentials is in force.


Instead of "if the corresponding Xds*Credentials is in force", I suggest saying "if the application used Xds*Credentials".

markdroth · 2021-07-22T20:32:41Z

A29-xds-tls-security.md

-field which is optional. The client certificate is sent to the server if the server requests
-or requires it in the TLS handshake. If the server requires the client certificate but is not
-present then the TLS handshake fails.
+in the TLS mode. As part of validating all CDS updates, if this field is not set in a CDS,


s/in a CDS/in a CDS resource/

markdroth · 2021-07-22T20:36:09Z

A29-xds-tls-security.md


-If any of the following fields are present, gRPC will NACK the CDS update:
+As part of validating all CDS updates, if any of the following fields are present, gRPC


@yashykt Doesn't look like C-core is checking these fields.

What's the rationale for NACKing if these are set, as opposed to just ignoring them, as we do with every other xDS field we don't yet support?

To be clear, I'm not necessarily objecting to NACKing these fields -- there are cases where we do that, usually because not doing so could cause a security problem or a forward-compatibility issue in the future. But our default approach should be to ignore fields unless that kind of situation applies. So if there is a reason to NACK if these fields are set, let's document why that behavior is justified. If there isn't any such reason, then we should ignore these fields instead of NACKing.

It's not clear to me that ignoring these fields causes a security concern, so I'll wait for a resolution before making the change.

Eric and I talked a bit about this: we can ignore a field (instead of NACKint it as unsupported) if that doesn't make the implementation any more insecure. Will re-evaluate the list with that goal.

markdroth · 2021-07-22T20:38:19Z

A29-xds-tls-security.md


-If any of the following fields are present, gRPC will NACK the LDS update:
+If any of the following fields are present, gRPC will NACK an LDS update as


@yashykt Looks like C-core isn't checking these fields either.

My comments above about needing a justification for NACKing fields applies here as well.

markdroth · 2021-07-22T20:41:55Z

A29-xds-tls-security.md

+[`combined_validation_context`][CVC] is validated in any CDS/LDS update as follows:
+[default_validation_context][] is accepted only on the client side i.e. inside `UpstreamTlsContext`.
+And inside that field only [match_subject_alt_names][] is processed as described [above][server-authz].
+If any of the other fields (listed below) are present, the update is NACKed.


@yashykt C-core isn't checking these fields.

My comments above about needing a justification for NACKing fields applies here as well.

A29-xds-tls-security.md

markdroth · 2021-07-22T21:01:49Z

A29-xds-tls-security.md

+The caller of the framework registers a watcher with the plugin to receive
+certificate updates as described below.
+
+### Implementing Security in the xDS Flow


It's kind of a gray area. I think that for a gRFC in the A category (as opposed to those in the L category), the overall design and functional specification needs to be described in a language-agnostic way. However, it's also fine to include some language-specific details to help implementors understand how things need to work, as long as those details are not the core of the document.

In this case, I do think it would be useful to include a high-level description of how data flows through the LB policies. The comments you have earlier in the doc about how C-core uses channel args and Java uses an SslContextProvider are appropriate to mention as part of that.

markdroth · 2021-07-22T21:04:24Z

A29-xds-tls-security.md

+the CertificateProvider plugin framework enables gRPC to acquire the certificates necessary for the TLS handshake.
+
+The field [`tls_certificate_certificate_provider_instance`][TCCPI] is used for the local certificate and the private
+key. And either [`validation_context_certificate_provider_instance`][VCCPI] or


I think the open issue of potential changes to the CertificateProvider fields in xDS is the biggest open issue in this document. We really need to get this nailed down before we finalize this gRFC.

markdroth · 2021-07-22T21:07:16Z

A29-xds-tls-security.md

+[`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L259)
+or [`validation_context_certificate_provider_instance`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L200)
+inside [`combined_validation_context`](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/tls.proto#L251)
+is used for the root certificate-chain for validating peer certificates. For mTLS both the local certificate


Sanjay, please clarify in the text that we will NACK if the specified certificate provider instance name is not one of the instances configured in the bootstrap file.

ejona86 · 2021-07-22T19:43:49Z

A29-xds-tls-security.md

+
+A secure client at a minimum requires [`validation_context_certificate_provider_instance`][VCCPI1]
+inside [`combined_validation_context`][CVC] to be able to validate the server certificate
+in the TLS mode. If this field is not set, gRPC will NACK the CDS update. The client identity


Right now the language could imply that validation_context_certificate_provider_instance is mandatory all the time, so if combined_validation_context is not present that would still mean to NACK. The tweaked language "if this field is not set in a CDS" seems to make that even stronger.

ejona86 · 2021-07-22T23:05:30Z

A29-xds-tls-security.md

+* if there is no leaf server certificate, or there are no SAN entries in the certificate
+the check fails.
+
+* each SAN entry of type DNS, URI and IP address is considered for the below match


Envoy also supports RFC 822 (email) names. Are we purposefully not supporting them?
https://github.com/envoyproxy/envoy/blob/0fae6970ddaf93f024908ba304bbd2b34e997a51/source/extensions/transport_sockets/tls/utility.cc#L174

Envoy being a general purpose proxy and supporting other protocols (http1 etc) I thought they see more use cases for email names etc. I am not aware of use-cases or usage of email names in gRPC (specifically in the context of proxyless service mesh with security) so I thought it's okay to not support it

As discussed separately, I wasn't saying we should support it but think we should either support it or call out that we are purposefully not supporting (or requiring support for) this. It seems very easy to support, so I'd just assume to support it, but I accept it isn't much of a mainstream feature and if we didn't support it we may never hear from a user noticing it isn't available.

I think it's almost identical to URI matching so I don't see any reason to exclude support since the implementation cost is almost zero. Will update the text.

Added support for email type

A29-xds-tls-security.md

ejona86 · 2021-07-26T19:55:20Z

A29-xds-tls-security.md

+* `trust_chain_verification`
+* `custom_validator_config`
+
+Only [`validation_context_certificate_provider_instance`][VCCPI1] is accepted. When that value is present


...on server-side? default_validation_context is accepted on client-side above.

Clarified that the field references are for combined_validation_context

A29-xds-tls-security.md

yashykt · 2021-07-24T01:33:11Z

A29-xds-tls-security.md


-If any of the following fields are present, gRPC will NACK the CDS update:
+As part of validating all CDS updates, if any of the following fields are present, gRPC


It's not clear to me that ignoring these fields causes a security concern, so I'll wait for a resolution before making the change.

A29-xds-tls-security.md

ejona86

Sending what I have

A29-xds-tls-security.md

…romise security

A29-xds-tls-security.md

markdroth

This is getting closer, but I think we still need to make some of the wording clearer, and I think we need to ditch all of the references to the deprecated fields.

Please let me know if you have any questions. Thanks!

A29-xds-tls-security.md

sanjaypujare · 2021-08-24T06:24:43Z

This is getting closer, but I think we still need to make some of the wording clearer, and I think we need to ditch all of the references to the deprecated fields.

Please let me know if you have any questions. Thanks!

I have addressed all the comments. PTAL

markdroth

Just a few minor changes remaining here.

Please let me know if you have any questions. Thanks!

A29-xds-tls-security.md

sanjaypujare · 2021-08-24T22:56:17Z

Just a few minor changes remaining here.

Please let me know if you have any questions. Thanks!

Addressed the latest comments

markdroth

Looks good! Thanks for doing this!

markdroth · 2021-08-26T18:32:36Z

I've just announced this on the Envoy slack instance, so I'll let this sit open for another couple of days, in case we get some feedback from there. But otherwise, I think this is ready to go!

sanjaypujare · 2021-08-26T18:40:42Z

I've just announced this on the Envoy slack instance, so I'll let this sit open for another couple of days, in case we get some feedback from there. But otherwise, I think this is ready to go!

Thanks Mark! I'll plan on merging this on Monday (unless there is feedback).

sanjaypujare · 2021-08-30T16:07:53Z

I've just announced this on the Envoy slack instance, so I'll let this sit open for another couple of days, in case we get some feedback from there. But otherwise, I think this is ready to go!

Thanks Mark! I'll plan on merging this on Monday (unless there is feedback).

I'll be merging this today as planned assuming there are no further comments.

A29: xDS-Based Security for gRPC Clients and Servers

f0e341c

sanjaypujare requested a review from markdroth June 11, 2021 16:49

yayapichfficial approved these changes Jun 11, 2021

View reviewed changes

sanjaypujare mentioned this pull request Jun 11, 2021

A41: xDS RBAC Support #237

Merged

ejona86 added a commit to ejona86/proposal that referenced this pull request Jun 11, 2021

A29 is now in grpc#243

08bed38

ejona86 reviewed Jun 18, 2021

View reviewed changes

yashykt mentioned this pull request Jun 24, 2021

De-experimentalize XdsCredentials and XdsServerCredentials API grpc/grpc#26544

Merged

address review comments

414e01c

markdroth requested changes Jun 24, 2021

View reviewed changes

Add C++ section for XdsServerCredentials

1103b7f

yashykt reviewed Jun 26, 2021

View reviewed changes

A29-xds-tls-security.md Show resolved Hide resolved

sanjaypujare added 2 commits June 28, 2021 10:14

typo - added the missing L in line number anchors

19b175a

Merge pull request #1 from yashykt/xds-security-new

fa67b42

Add C++ section for XdsServerCredentials

easwars reviewed Jun 29, 2021

View reviewed changes

A29-xds-tls-security.md Outdated Show resolved Hide resolved

A29-xds-tls-security.md Show resolved Hide resolved

address review comments and add Go snippets

95f01ba

easwars reviewed Jun 29, 2021

View reviewed changes

markdroth reviewed Jun 30, 2021

View reviewed changes

address more review comments

ecd85ca

This was referenced Jul 3, 2021

NACK xDS updates where transport_name is not recognized grpc/grpc#26612

Merged

NACK xds updates requiring client certificates when no validation certificate provider instance is mentioned grpc/grpc#26613

Merged

address some more review comments

cf2fff4

easwars mentioned this pull request Jul 8, 2021

NACK xDS updates when certificate provider instance names are unrecognized grpc/grpc-go#4591

Closed

sanjaypujare added 3 commits July 9, 2021 01:08

address next set of comments

c80082f

minor typos and fixes

929e4fa

add further content about validation of combined_validation_context

c1f94eb

sergiitk approved these changes Jul 13, 2021

View reviewed changes

Update A29-xds-tls-security.md

0f01937

Co-authored-by: Sergii Tkachenko <hi@sergii.org>

add IPv6 canonical string representation spec

f0aea10

markdroth requested changes Jul 22, 2021

View reviewed changes

ejona86 reviewed Jul 22, 2021

View reviewed changes

ejona86 mentioned this pull request Jul 26, 2021

Emit security configuration by default GoogleCloudPlatform/traffic-director-grpc-bootstrap#19

Merged

ejona86 reviewed Jul 26, 2021

View reviewed changes

sanjaypujare added 2 commits August 3, 2021 20:26

address latest review comments

a948366

address latest review comments - part2

f3b7a96

yashykt reviewed Aug 4, 2021

View reviewed changes

ejona86 reviewed Aug 4, 2021

View reviewed changes

A29-xds-tls-security.md Show resolved Hide resolved

sanjaypujare added 5 commits August 5, 2021 09:26

move a few fields from NACK to ignored category since they don't comp…

3ce1617

…romise security

add text saying certificate_name is ignored by file_watcher

c227dd7

update tls.proto and common.proto versions and line numbers

749f4fa

update content to reference the new fields in tls and common.proto

875cca7

update the last updated date and minor typos

878817c

ejona86 approved these changes Aug 12, 2021

View reviewed changes

A29-xds-tls-security.md Outdated Show resolved Hide resolved

markdroth requested changes Aug 13, 2021

View reviewed changes

sanjaypujare added 2 commits August 23, 2021 18:06

address latest review comments

c22f8fe

address latest review comments-2

356b843

markdroth reviewed Aug 24, 2021

View reviewed changes

A29-xds-tls-security.md Outdated Show resolved Hide resolved

A29-xds-tls-security.md Outdated Show resolved Hide resolved

A29-xds-tls-security.md Outdated Show resolved Hide resolved

address the latest review comments-3

ce24e58

clarify the match_subject_alt_names semantics on the server side

e9f99d5

markdroth approved these changes Aug 26, 2021

View reviewed changes

make the grfc final

3681210

sanjaypujare merged commit deaf1bc into grpc:master Aug 30, 2021

sanjaypujare deleted the xds-security-new branch August 30, 2021 18:42


		##### Server Authorization aka Subject-Alt-Name Checks

		If [match_subject_alt_names](https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/transport_sockets/tls/v3/common.proto#L373)

	gRPC provides a `CertificateProvider` plugin API that can support multiple implementations. The plugin API
	gRPC offers a `CertificateProvider` plugin API that can support multiple implementations. The plugin API


		If any of the following fields are present, gRPC will NACK the CDS update:
		As part of validating all CDS updates, if any of the following fields are present, gRPC

A29: xDS-Based Security for gRPC Clients and Servers #243

A29: xDS-Based Security for gRPC Clients and Servers #243

Conversation

sanjaypujare commented Jun 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markdroth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanjaypujare commented Jul 9, 2021

sergiitk left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markdroth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sanjaypujare Jul 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ejona86 left a comment

Choose a reason for hiding this comment

markdroth left a comment

Choose a reason for hiding this comment

sanjaypujare commented Aug 24, 2021

markdroth left a comment

Choose a reason for hiding this comment

sanjaypujare commented Aug 24, 2021

markdroth left a comment

Choose a reason for hiding this comment

markdroth commented Aug 26, 2021

sanjaypujare commented Aug 26, 2021

sanjaypujare commented Aug 30, 2021

sergiitk left a comment •

edited

Loading

sanjaypujare Jul 27, 2021 •

edited

Loading