envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.26k stars 4.68k forks source link

Extend TrustChainVerification to support SSL_VERIFY_NONE mode #30091

Open bladedancer opened 8 months ago

bladedancer commented 8 months ago

Title: Add a trust chain mode that accepts a client certificate if one is presented but does not request it.

Description: The scenario is that I'm terminating TLS in Envoy but for legacy reasons doing authorization in the backend. One of the authorization formats is mTLS, so I need to extract the client certificate and forward it on to the backend. The problem is the endpoint is shared and not all requests use the same authorizatio, this is a business rule and (currently) isn't something that can be known when setting the Envoy configuration.

Our initial approach was to use ACCEPT_UNTRUSTED. This allowed us to terminate TLS at Envoy and still extract the client certificate which we passed over XFF headers to the backend. This works fine except ACCEPT_UNTRUSTED sets SSL_VERIFY_PEER: https://github.com/envoyproxy/envoy/blob/main/source/extensions/transport_sockets/tls/cert_validator/default_validator.cc#L65

"SSL_VERIFY_PEER...On a server it requests a client certificate and makes errors fatal. However, anonymous clients are still allowed." https://commondatastorage.googleapis.com/chromium-boringssl-docs/ssl.h.html#SSL_VERIFY_PEER

It's the requesting a client cert that's the issue. We need to accept one if one is presented but not request one - this is causing unexpected behaviour (browsers are prompting for cert selection). What would be ideal is to accept a certificate if one is presented but not request it - which based on the description seems to be what SSL_VERIFY_NONE does (though I've never used this so I'm basing this purely on the one line description and my own confirmation bias).

kyessenov commented 8 months ago

That sounds reasonable. I'm not sure why SSL_VERIFY_PEER is set when the config does not explicitly configure to request the client certificate. Muxing TLS and mTLS on the same listener is certainly useful if supported.

CC @ggreenway

ggreenway commented 8 months ago

@kyessenov I believe SSL_VERIFY_PEER is only set when we have a validation context, eg we've configured envoy to do some validation.

@bladedancer I think your proposal makes sense; maybe add another enum value ACCEPT_UNTRUSTED_AND_DO_NOT_REQUEST_PEER_CERT (or something less wordy if you come up with a good idea).

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 7 months ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

bladedancer commented 7 months ago

Per the bot - can this be kept open and marked as help wanted?

jewertow commented 5 months ago

@bladedancer if you didn't work on it yet, I could implement this feature.

jewertow commented 5 months ago

@bladedancer I was trying to reproduce your issue, but Envoy didn't request client certificates when trust_chain_verification: ACCEPT_UNTRUSTED. Envoy requested certificates only when I set require_client_certificate: true.

bladedancer commented 2 months ago

Sorry wasn't tracking the thread. This was a very specific scenario - if the client was a browser and the browser had client certificates installed then it would prompt for the client certificate.

Envoy was quite happy whether or not one was specified - it was optional after all - but there's something in the ssl handshake prompting the browser to ask for a client cert if it has one - based on my investigation that was SSL_VERIFY_PEER.

Updated @jewertow demo to illustrate the problem: test case

jewertow commented 3 weeks ago

@bladedancer thanks for reproducer. I will try to fix it.

jewertow commented 5 days ago

@bladedancer I was trying to fix your issue, but when I set SSL_VERIFY_NONE, then client certificate is not requested by the server and browser does not prompt to choose certificate, but at the same time, certificate is not verified by the server if it's provided and does not appear in the XFCC header, so I think we can't fix it.

arulthileeban commented 5 days ago

I'm not sure if the boringSSL docs are clear about "SSL_VERIFY_PEER" errors being fatal. AFAIK it should be used in conjunction with "SSL_VERIFY_FAIL_IF_NO_PEER_CERT" to fail if cert isn't presented (which is how "required_client_certificate" is set in the code). If you setup a config like this for eg.,

 common_tls_context:
            tls_certificates:
              - certificate_chain:
                  filename: "test_configs/certificates/envoy-server.pem"
                private_key:
                  filename: "test_configs/certificates/envoy-server.key"
            validation_context:
              trusted_ca:
                { filename: test_configs/certificates/envoy-root.pem }
              trust_chain_verification: ACCEPT_UNTRUSTED

which only sets "SSL_VERIFY_PEER", it'll request you for a client certificate and TLS will succeed even if you don't present the cert. Requesting client cert is required from a mTLS standpoint (SSL_VERIFY_NONE cannot be used for mTLS) in the standard TLS flow.

I suspect the reason you see that prompt in that browser is because browsers tend to be smart about these things. In the handshake, the server will request a client certificate with certain specs (signed by this CA, etc.). So browsers prompt you if you have that cert issued by that CA installed. If you don't have any client certs matching the specs installed, I believe it will go through the flow without prompting.

On a separate note, you can use policies like this to auto select the certs without a user prompt: https://cloud.google.com/access-context-manager/docs/enterprise-certificates#configure_an_autoselectcertificateforurls_policy

bladedancer commented 1 day ago

I haven't tested with different validation contexts - in my case the mtls validation is business logic that will happen in my ext_authz.....mainly because the authorization I need is not layer 4.

I'll give it a whirl just to see how much is browser being smart.

On the boringssl handshake....it's been a while since I'd consider myself proficient in CPP so I could be wildly off but I was hoping we'd end up here: https://github.com/google/boringssl/blob/master/ssl/handshake.cc#L377 But I'd need to a lot of digging to see if I'm even in the right ballpark here.