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

WIP: Implement client certificate validation (mTLS) #1226

Closed
wants to merge 12 commits into from

Conversation

uablrek
Copy link
Contributor

@uablrek uablrek commented Jul 5, 2019

This PR implements mTLS and updates #1090 .
Please see design documentation in PR #1233 .

Work items

  • contour/apis: Parse the clientValidation configuration item
  • CRD: Extend the validation to include clientValidation
  • internal/dag: Add ClientValidation in SecureVirtualHost
  • internal/dag: unit-tests
  • internal/envoy: Handle clientValidation in the DownstreamTLSContext() function
  • internal/envoy: Set RequireClientCertificate to "true" if clientValidation is != nil
  • internal/envoy: unit-tests
  • internal/e2e: Add tests
  • internal/contour: Handle SPKI validation items
  • internal/contour: Handle Hash validation items
  • internal/contour: Handle CA validation (Secret)
  • internal/contour: unit-tests
  • User documentation (ingressroute.md)

Example

mTLS must be configured with an IngressRoute;

apiVersion: contour.heptio.com/v1beta1
kind: IngressRoute
metadata:
  name: kahttp
  namespace: default
spec:
  virtualhost:
    fqdn: kahttp.com
    tls:
      secretName: contour-secret
      clientValidation:
        secretName: clientsecret
        spkis:
          - 2IEpPESU/mmC30tPsnOfbGKdwKdQfN/wZw1QWpjGlmk=
  routes:
    - match: /
      services:
        - name: kahttp
          port: 80

Client certificate validation can be tested with curl;

# curl -v --cacert /cert/contour.crt \
>   --cert /cert/contour.crt --key /cert/contour.key https://kahttp.com
* Rebuilt URL to: https://kahttp.com/
*   Trying 12.0.251.249...
* TCP_NODELAY set
* Connected to kahttp.com (12.0.251.249) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /cert/contour.crt
  CApath: /etc/ssl/certs
* (304) (OUT), TLS handshake, Client hello (1):
* (304) (IN), TLS handshake, Server hello (2):
* (304) (IN), TLS Unknown, Certificate Status (22):
* (304) (IN), TLS handshake, Unknown (8):
* (304) (IN), TLS handshake, Request CERT (13):
* (304) (IN), TLS handshake, Certificate (11):
* (304) (IN), TLS handshake, CERT verify (15):
* (304) (IN), TLS handshake, Finished (20):
* (304) (OUT), TLS change cipher, Client hello (1):
* (304) (OUT), TLS Unknown, Certificate Status (22):
* (304) (OUT), TLS handshake, Certificate (11):
* (304) (OUT), TLS Unknown, Certificate Status (22):
* (304) (OUT), TLS handshake, CERT verify (15):
* (304) (OUT), TLS Unknown, Certificate Status (22):
* (304) (OUT), TLS handshake, Finished (20):
* SSL connection using unknown / TLS_CHACHA20_POLY1305_SHA256
* ALPN, server accepted to use h2
* Server certificate:
*  subject: C=SE; ST=Sweden; L=Kista; O=Xcluster; OU=Contour; CN=kahttp.com; emailAddress=nemo@/dev/null
*  start date: May  9 07:51:44 2019 GMT
*  expire date: May  6 07:51:44 2029 GMT
*  common name: kahttp.com (matched)
*  issuer: C=SE; ST=Sweden; L=Kista; O=Xcluster; OU=Contour; CN=kahttp.com; emailAddress=nemo@/dev/null
*  SSL certificate verify ok.
* Using HTTP2, server supports multi-use
* Connection state changed (HTTP/2 confirmed)
* Copying HTTP/2 data in stream buffer to connection buffer after upgrade: len=0
* (304) (OUT), TLS Unknown, Unknown (23):
* (304) (OUT), TLS Unknown, Unknown (23):
* (304) (OUT), TLS Unknown, Unknown (23):
* Using Stream ID: 1 (easy handle 0x55ef86b27b50)
* (304) (OUT), TLS Unknown, Unknown (23):
> GET / HTTP/2
> Host: kahttp.com
> User-Agent: curl/7.58.0
> Accept: */*
> 
* (304) (IN), TLS Unknown, Certificate Status (22):
* (304) (IN), TLS handshake, Newsession Ticket (4):
* (304) (IN), TLS handshake, Newsession Ticket (4):
* (304) (IN), TLS Unknown, Unknown (23):
* Connection state changed (MAX_CONCURRENT_STREAMS updated)!
* (304) (OUT), TLS Unknown, Unknown (23):
* (304) (IN), TLS Unknown, Unknown (23):
< HTTP/2 200 
< x-kahttp-server-host: Kahttp/v0.4@kahttp-deployment-6874689df6-5c6hf
< date: Fri, 05 Jul 2019 21:36:21 GMT
< content-length: 578
< content-type: text/plain; charset=utf-8
< x-envoy-upstream-service-time: 1
< server: envoy
< 
Method: GET
URL: /
Proto: HTTP/1.1
ContentLength: 0
TransferEncoding: []
Host: kahttp.com
RemoteAddr: 11.0.2.2:42700
RequestURI: /
X-Kahttp-Server-Host: Kahttp/v0.4@kahttp-deployment-6874689df6-5c6hf
X-Request-Start: [t=1562362581.768]
X-Forwarded-Proto: [https]
X-Envoy-External-Address: [11.0.2.1]
X-Forwarded-Client-Cert: [Hash=c49c6930b9fbfb72a0d9d07504133d26c87588aa2d32b2919475f647a62f6fec]
X-Envoy-Expected-Rq-Timeout-Ms: [15000]
X-Request-Id: [4fa1b422-c086-447d-8aaa-1e7c48a7cafc]
Content-Length: [0]
User-Agent: [curl/7.58.0]
Accept: [*/*]
X-Forwarded-For: [11.0.2.1]
* Connection #0 to host kahttp.com left intact

Copy link
Contributor

@davecheney davecheney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this.

Can you please split this PR into two. We will review the design document first then move on to the implementation when the design has been nailed down.

Thank you.

@davecheney davecheney added this to the 0.14.0 milestone Jul 8, 2019
uablrek pushed a commit to Nordix/contour that referenced this pull request Jul 8, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Lars Ekman added 12 commits July 8, 2019 14:12
Updates: projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Updates projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>

Add a "clientValidation" in the IngressRoute for validation of
client certificated (mTLS). This commit does not include CRD validation.
Updates projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>

The clientValidation is used for client certificate validation (mTLS)
Update projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>

If ClientValidation is configured for a virtual host Envoy is
configured to validate the client certificate (mTLS)
Updates projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>

If ClientValidation is configured validation data is sent
to Envoy for client validation (mTLS)
No unit-tests for mTLS added in this commit

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Updates projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Updates projectcontour#1090

Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>

Also remove misplaced ForwardCertificateDetails items.
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>

Moved to a separate PR
@uablrek
Copy link
Contributor Author

uablrek commented Jul 8, 2019

The new and improved design proposal is now in #1233 and can be viewed here;
https://github.com/Nordix/contour/blob/mtls-design/design/tls-client-verification.md

uablrek pushed a commit to Nordix/contour that referenced this pull request Jul 9, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
uablrek pushed a commit to Nordix/contour that referenced this pull request Jul 11, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
@davecheney davecheney modified the milestones: 0.14.0, 0.15.0 Jul 12, 2019
uablrek pushed a commit to Nordix/contour that referenced this pull request Jul 20, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
uablrek pushed a commit to Nordix/contour that referenced this pull request Jul 23, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
@davecheney davecheney modified the milestones: 0.15.0, 1.0.0-beta.1 Aug 23, 2019
uablrek pushed a commit to Nordix/contour that referenced this pull request Sep 23, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
@davecheney davecheney modified the milestones: 1.0.0-beta.1, 1.0.0-rc.1 Sep 24, 2019
@davecheney
Copy link
Contributor

Thank you for working on this. I'm sorry its taken so long to reply. Sadly I don't see it will be possible to land this before contour 1.0 as we're deep in code freeze now. I'll assign this to the backlog to review once Contour 1.0 is out the door.

@davecheney davecheney modified the milestones: 1.0.0-rc.1, Backlog Sep 29, 2019
tsaarni pushed a commit to Nordix/contour that referenced this pull request Nov 13, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
tsaarni pushed a commit to Nordix/contour that referenced this pull request Nov 28, 2019
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
jpeach pushed a commit that referenced this pull request Jan 26, 2020
Signed-off-by: Lars Ekman <lars.g.ekman@est.tech>
@github-actions
Copy link

Marking this PR stale since there has been no activity for 14 days. It will be closed if there is no activity for another 90 days.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 30, 2020
@uablrek
Copy link
Contributor Author

uablrek commented Mar 9, 2020

Replaced by #2250. Closing...

@uablrek uablrek closed this Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants