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

Support TLS verification of backend services #813

Closed
robbiemcmichael opened this issue Nov 19, 2018 · 4 comments · Fixed by #1045
Closed

Support TLS verification of backend services #813

robbiemcmichael opened this issue Nov 19, 2018 · 4 comments · Fixed by #1045
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@robbiemcmichael
Copy link
Contributor

Contour should support TLS verification of backend services with a CA certificate. This feature is supported in Envoy, we just need to add the trustedCA field to the TLS validation_context: https://www.envoyproxy.io/docs/envoy/latest/intro/arch_overview/ssl#enabling-certificate-verification

@robbiemcmichael
Copy link
Contributor Author

Some personal opinion on implementation of this:

  • Ideally the CA certificate should be configurable via a configmap, similar to how the TLS certificate and private key can be configured via a secret
  • The ingress route that defines the backend service should define the configmap which is used for TLS verification (configmap would be in the same namespace as this ingress route)
  • If TLS verification is enabled for a service in the ingress route then the contour.heptio.com/upstream-protocol.h2 annotation should not be required on the upstream service - This is probably the most controversial as most ingress controllers for Kubernetes use service annotations for this, but I've always disliked needing to add it to the service as it should be the client (i.e. Envoy in this case) that specifies whether they are expecting to use HTTP or HTTPS

I'm working on the implementation of this feature for my own fork of Contour as we require TLS verification for an internal project, but I'd like to get some feedback so that hopefully it can be merged back into the main project eventually.

I'd like to extend the ingress route resource to support specifying the configmap for the CA certificate. I'm not sure how open you are to extending the resource, but from a technical standpoint it's not too bad as the tlsVerification field is optional. What I have right now is:

apiVersion: contour.heptio.com/v1beta1
kind: IngressRoute
metadata:
  name: example
spec:
  routes:
  - match: /
    services:
    - name: service
      port: 443
      tlsVerification:
        configMapName: example-ca

The main thing I dislike about the above is that if the same CA is being used for every service then there would be a lot of unnecessary repetition. The different levels at which the CA could potentially be specified are:

  1. At the ingress route level, all routes/services use the same CA - I don't like this as it seems possible that someone would want to use different CAs for some of the routes
  2. At the route level, every service under a particular route uses the same CA - Seems like an improvement on the first option
  3. At the service level, every service can have a different CA (as shown above) - Gets repetitive if specifying multiple services under a single route
  4. Some combination of the above options, which are listed in order of increasing precedence

@stevesloka
Copy link
Member

Hey, @robbiemcmichael thanks for the detailed work here! I agree with you on the points you mentioned regarding the effects of putting the CA on each route.

It seems like this should maybe be an annotation on the service (since there's no good place to specify this in v1.Service today), but to your point, the IngressRoute still wouldn't know if it's proxying to a TLS endpoint or not (short of convention i.e. port 443).

I wonder if we could have multiple levels of the tlsVerification struct. We had this in the initial design of IngressRoute, where you could say specify the lbAlgorithm in the root and it would use that as the default for all routes unless it was specified otherwise.

Doing this I think would be neat because if you could use the same CA for all the services in the route, you could default this once, but still have the opportunity to further define it if needed.

@rbankston rbankston added ZD1535 and removed ZD1535 labels Apr 2, 2019
@davecheney davecheney added this to the 0.11.0 milestone Apr 5, 2019
@timh timh added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Apr 5, 2019
@davecheney davecheney modified the milestones: 0.11.0, 0.12.0 Apr 8, 2019
@davecheney
Copy link
Contributor

Due to the need to release Contour 0.11 to address the security issue in Envoy 1.9.0 and there being no defined implementation path for this issue yet I have moved this issue to the 0.12 milestone.

@rbankston please be aware the milestone for this issue has changed.

@timh timh added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 10, 2019
davecheney added a commit to davecheney/contour that referenced this issue Apr 23, 2019
Updates projectcontour#813

This PR adds a draft design for validation of backend services accessed
over TLS.

Signed-off-by: Dave Cheney <dave@cheney.net>
@davecheney
Copy link
Contributor

@here please review and give feedback on #1040.

davecheney added a commit to davecheney/contour that referenced this issue Apr 23, 2019
Updates projectcontour#813

This PR adds a draft design for validation of backend services accessed
over TLS.

Signed-off-by: Dave Cheney <dave@cheney.net>
davecheney added a commit to davecheney/contour that referenced this issue Apr 23, 2019
Updates projectcontour#813

This PR adds a draft design for validation of backend services accessed
over TLS.

Signed-off-by: Dave Cheney <dave@cheney.net>
davecheney added a commit to davecheney/contour that referenced this issue Apr 23, 2019
Updates projectcontour#813

This PR adds a draft design for validation of backend services accessed
over TLS.

Signed-off-by: Dave Cheney <dave@cheney.net>
davecheney added a commit to davecheney/contour that referenced this issue Apr 23, 2019
Updates projectcontour#813

This PR adds a draft design for validation of backend services accessed
over TLS.

Signed-off-by: Dave Cheney <dave@cheney.net>
@stevesloka stevesloka self-assigned this Apr 23, 2019
@shabx shabx removed the ZD1539 label Apr 23, 2019
davecheney added a commit to davecheney/contour that referenced this issue May 6, 2019
Updates projectcontour#813

This PR adds a draft design for validation of backend services accessed
over TLS.

Signed-off-by: Dave Cheney <dave@cheney.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants