Bug 1477349 - [RFE] SSL/TLS client certificate validation for OpenShift router
[RFE] SSL/TLS client certificate validation for OpenShift router
Status: NEW
Product: OpenShift Container Platform
Classification: Red Hat
Component: RFE (Show other bugs)
3.5.0
Unspecified Unspecified
high Severity high
: ---
: ---
Assigned To: Eric Paris
zhaozhanqi
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-08-01 16:44 EDT by Ryan Howe
Modified: 2018-05-16 09:22 EDT (History)
13 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Ryan Howe 2017-08-01 16:44:15 EDT
Proposed title of this feature request
SSL/TLS client certificate validation for OpenShift router

Description:

Request a feature to enable mTLS (mutual TLS) authentication with the OpenShift Router. With options to set ACL used to match the certificate common name, rules to delete the three headers, and headers added for transparency. 


Example diff of features added:

--- examples/haproxy-config.template	2017-06-29 12:54:18.131120000 -0400
+++ custom/haproxy-config.template	2017-07-25 16:45:31.829700000 -0400
@@ -186,12 +186,45 @@
 
 frontend fe_sni
   # terminate ssl on edge
-  bind 127.0.0.1:{{env "ROUTER_SERVICE_SNI_PORT" "10444"}} ssl no-sslv3 {{ if (len .DefaultCertificate) gt 0 }}crt {{.DefaultCertificate}}{{ else }}crt /var/lib/haproxy/conf/default_pub_keys.pem{{ end }} crt-list /var/lib/haproxy/conf/cert_config.map accept-proxy
+  bind 127.0.0.1:{{env "ROUTER_SERVICE_SNI_PORT" "10444"}} ssl no-sslv3 {{ if (len .DefaultCertificate) gt 0 }}crt {{.DefaultCertificate}}{{ else }}crt /var/lib/haproxy/conf/default_pub_keys.pem{{ end }} crt-list /var/lib/haproxy/conf/cert_config.map accept-proxy ca-file /var/lib/haproxy/conf/custom/client-chain.pem verify optional
   mode http
 
   # Remove port from Host header
   http-request replace-header Host (.*):.* \1
 
+  # Check the CN of the client certificate to see if it is in the list
+  # An example of checking the CN in OpenShift is in the OpenShift docs
+  # https://docs.openshift.com/container-platform/3.4/install_config/router/customized_haproxy_router.html#using-annotations
+  acl match ssl_c_s_dn(CN) -m sub CustomName\ TAM\ Web\ Client
+
+  # Default X-Headers-Sanitized to false, set to true if test fails
+  http-request set-header X-Headers-Sanitized false
+
+  # Debugging information. Add headers to indicate front-end and status.
+  http-request set-header X-HAProxy-Frontend fe_sni
+  http-request set-header X-Headers-Sanitized true unless { ssl_c_verify 0 } match
+
+  # Debugging information. Store a copy of iv- headers in X-Sanitized
+  http-request set-header X-Sanitized-iv-user %[req.hdr(iv-user)] unless { ssl_c_verify 0 } match
+  http-request set-header X-Sanitized-iv-creds %[req.hdr(iv-creds)] unless { ssl_c_verify 0 } match
+  http-request set-header X-Sanitized-iv-groups %[req.hdr(iv-groups)] unless { ssl_c_verify 0 } match
+
+  # Sanitize iv- headers unless verification succeeded and CN matches
+  http-request del-header iv-user unless { ssl_c_verify 0 } match
+  http-request del-header iv-groups unless { ssl_c_verify 0 } match
+  http-request del-header iv-creds unless { ssl_c_verify 0 } match
+
+  # Pass the certificate information to the client (informational)
+  # https://raymii.org/s/tutorials/haproxy_client_side_ssl_certificates.html
+  http-request set-header X-SSL %[ssl_fc]
+  http-request set-header X-SSL-Client-Verify %[ssl_c_verify]
+  http-request set-header X-SSL-Client-SHA1 %{+Q}[ssl_c_sha1]
+  http-request set-header X-SSL-Client-DN %{+Q}[ssl_c_s_dn]
+  http-request set-header X-SSL-Client-CN %{+Q}[ssl_c_s_dn(cn)]
+  http-request set-header X-SSL-Issuer %{+Q}[ssl_c_i_dn]
+  http-request set-header X-SSL-Client-Not-Before %{+Q}[ssl_c_notbefore]
+  http-request set-header X-SSL-Client-Not-After %{+Q}[ssl_c_notafter]
+
   # check re-encrypt backends first - from most specific to general path.
   acl reencrypt base,map_beg(/var/lib/haproxy/conf/os_reencrypt.map) -m found
 
@@ -237,12 +270,43 @@
 
 frontend fe_no_sni
   # terminate ssl on edge
-  bind 127.0.0.1:{{env "ROUTER_SERVICE_NO_SNI_PORT" "10443"}} ssl no-sslv3 {{ if (len .DefaultCertificate) gt 0 }}crt {{.DefaultCertificate}}{{ else }}crt /var/lib/haproxy/conf/default_pub_keys.pem{{ end }} accept-proxy
+  bind 127.0.0.1:{{env "ROUTER_SERVICE_NO_SNI_PORT" "10443"}} ssl no-sslv3 {{ if (len .DefaultCertificate) gt 0 }}crt {{.DefaultCertificate}}{{ else }}crt /var/lib/haproxy/conf/default_pub_keys.pem{{ end }} accept-proxy ca-file /var/lib/haproxy/conf/custom/client-chain.pem verify optional
   mode http
 
   # Remove port from Host header
   http-request replace-header Host (.*):.* \1
 
+  # Check the CN of the client certificate to see if it is in the list
+  acl match ssl_c_s_dn(CN) -m sub CustomName \ TAM\ Web\ Client
+
+  # Default X-Headers-Sanitized to false, set to true if test fails
+  http-request set-header X-Headers-Sanitized false
+
+  # Debugging information. Add headers to indicate front-end and status.
+  http-request set-header X-HAProxy-Frontend fe_no_sni
+  http-request set-header X-Headers-Sanitized true unless { ssl_c_verify 0 } match
+
+  # Debugging information. Store a copy of iv- headers in X-Sanitized
+  http-request set-header X-Sanitized-iv-user %[req.hdr(iv-user)] unless { ssl_c_verify 0 } match
+  http-request set-header X-Sanitized-iv-creds %[req.hdr(iv-creds)] unless { ssl_c_verify 0 } match
+  http-request set-header X-Sanitized-iv-groups %[req.hdr(iv-groups)] unless { ssl_c_verify 0 } match
+
+  # Sanitize iv- headers unless verification succeeded and CN matches
+  http-request del-header iv-user unless { ssl_c_verify 0 } match
+  http-request del-header iv-groups unless { ssl_c_verify 0 } match
+  http-request del-header iv-creds unless { ssl_c_verify 0 } match
+
+  # Pass the certificate information to the client (informational)
+  # https://raymii.org/s/tutorials/haproxy_client_side_ssl_certificates.html
+  http-request set-header X-SSL %[ssl_fc]
+  http-request set-header X-SSL-Client-Verify %[ssl_c_verify]
+  http-request set-header X-SSL-Client-SHA1 %{+Q}[ssl_c_sha1]
+  http-request set-header X-SSL-Client-DN %{+Q}[ssl_c_s_dn]
+  http-request set-header X-SSL-Client-CN %{+Q}[ssl_c_s_dn(cn)]
+  http-request set-header X-SSL-Issuer %{+Q}[ssl_c_i_dn]
+  http-request set-header X-SSL-Client-Not-Before %{+Q}[ssl_c_notbefore]
+  http-request set-header X-SSL-Client-Not-After %{+Q}[ssl_c_notafter]
+
   # check re-encrypt backends first - path or host based.
   acl reencrypt base,map_beg(/var/lib/haproxy/conf/os_reencrypt.map) -m found
Comment 7 Ben Bennett 2018-04-05 15:23:00 EDT
The big question that we need to work out is what control there needs to be on a per-route basis.

i.e. there are a lot of options for this, and if you need per-route control of most of them this becomes harder.

What can be defined router-wide?

- The ca that are trusted for the client certs?
- The http headers that are set for the requests?

Obviously, things like the CNs to trust or whether to allow or drop connections that were not verified will need to vary per route, but that can be done with an annotation.



Is it reasonable to mandate that all routes handled by a particular router would be validated with the same CA cert (if validation was requested by a per-route annotation)?

Then, if validation was requested, we would set some X-SSL headers as this does: https://raymii.org/s/tutorials/haproxy_client_side_ssl_certificates.html

Then on each route you would have an annotation that would specify whether client verification was required, or optional.  If required, it would drop connections that didn't validate.  If optional, or required with a valid connection, the http headers would be set.

There would also be an optional annotation on the routes that would let you provide a CN to match against.



However, we would not support adding arbitrary headers, or having alternate backends depending on marches.


Is that sufficient?

Note You need to log in before you can comment on or make changes to this bug.