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

Connect: Validate Server Certificates when using Envoy #6364

Closed
banks opened this issue Aug 20, 2019 · 1 comment · Fixed by #10621
Closed

Connect: Validate Server Certificates when using Envoy #6364

banks opened this issue Aug 20, 2019 · 1 comment · Fixed by #10621
Labels
theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies type/enhancement Proposed improvement or new feature

Comments

@banks
Copy link
Member

banks commented Aug 20, 2019

Scope:

  • No design work needed
  • O(days) with familiarity

Envoy upstream clusters currently don't validate the server certificate. Adding this to the cluster isn't too hard as part of the cluster's TLS context. For Connect, we always know the URI we expect in the server cert and can just encode that using verify_subject_alt_name list.

The one wrinkle here is that to support External Federation we need a way to register some opaque SAN value when registering a service from an external mesh so we can configure that value instead of the Connect-formatted one. This is similar to the recent work on ExternalSNI (#6324) but subtly different - SNI interacts with gateway routing and discovery chain while SAN doesn't.

We could add this as a field to the service instances themselves - there could technically be a case where you'd want some instances to be external and some not meaning different SAN expected per instance. In that case we can just collect all the distinct SANs returned and allow any of them.

It would be simpler and sufficient for all known use cases now to only allow overriding allowed SANs explicitly in service-defaults in the same way we do with ExternalURI. The problem is that ideally we should return the correct SAN in all service-discovery requests to ensure those contain complete info needed to establish a secure connection without having to also cross-reference the discovery chain stuff. I don't like the idea of only adding this as service meta when it's a core part of our model for key features.

So I think we should add a SAN field somewhere in the service definition/response to encode this. By default that would be empty for regular services, and would contain the normal Connect spiffe ID for connect-enabled (native of proxy) services. External federation would then populate this field explicitly on registration with the external SAN to validate and proxy integrations (e.g. the SDK can just rely on validating against whatever is in the result(s) without needing to duplicate the current Connect Spiffe ID formatting logic.

We should update the SDK/built-in proxy to use that field as well as Envoy (#6362).

[edit] external federation will happen eventiually and the above will likely be relevant then, but it's not here today so we don't really need to solve for it while fixing the basic validation case. We can work that out later as part of that work.

@banks banks added type/enhancement Proposed improvement or new feature theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies labels Aug 20, 2019
@banks banks modified the milestones: 1.6.x, 1.7.0 Aug 20, 2019
@hanshasselberg hanshasselberg modified the milestones: 1.7.0, 1.7.x Jan 30, 2020
@dnephin dnephin removed this from the 1.7.x milestone Jun 30, 2020
@banks
Copy link
Member Author

banks commented Apr 30, 2021

Envoy now has match_subject_alt_name which supports using a regex to validate the server.

It probably did when we wrote this, just noting we'd need that to match the right parts of the URI SAN just like we do on the public listener.

Also, we still use separate clusters for each destiantion service even when they are going through the same set of mesh gateways which means we should be able to add the SAN validation to each one easily.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies type/enhancement Proposed improvement or new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants