envoyproxy / envoy

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

tls: Expand options to handle client certificate verification failure #34617

Open arulthileeban opened 4 weeks ago

arulthileeban commented 4 weeks ago

Title: Expand options to handle client certificate verification failure

Description:

In envoy's current flow, when a client certificate is required and the client provides no/invalid certificate, envoy sends an SSL error message back error:1404C416:SSL routines:ST_OK:sslv3 alert certificate unknown not completing the TLS handshake. Against untrusted downstream, particularly in the face of DDoS/scanning attacks, it'd be desirable to have an additional set of configurations to handle client cert verification failures differently.

google.protobuf.BoolValue require_client_certificate = 2; 

HandleClientCertVerificationFailure handle_client_cert_verification_failure = 8;

message HandleClientCertVerificationFailure {
    enum HandleFailureType {
        HANDLER_FAILURE_TYPE_UNSPECIFIED = 0;
        RETURN_SSL_FAILURE = 1; // Default
        CLOSE_CONNECTION = 2;   // Close TCP connection without returning SSL failure
        KEEP_CONNECTION_OPEN = 3; // Keep the socket open and don't respond (Tarpitting)
    }

    // Local rate limiting set only for connections that have failed cert verification under KEEP_CONNECTION_OPEN. 
    // This would be useful to avoid self-inflicting DoS by keeping too many sockets open
    LocalRateLimit keep_tcp_connection_open_limit = 1; 

}

I'd be interested in working on this. Would this addition be useful upstream?

alyssawilk commented 3 weeks ago

IMO having the option to tarpit would be generally useful, but having a config specific to client cert verification less so. I think if you make it a general extensible feature and make sure you can configure for various handshake / transport socket failures it'd be a great addition. cc @ggreenway @RyanTheOptimist for further thoughts.

ggreenway commented 3 weeks ago

I think this functionality could probably be made into it's own extension, as a network filter. If you set the tls transport socket to allow unauthorized client certs, and you look at how http route matching works for https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-msg-config-route-v3-routematch-tlscontextmatchoptions, you could make a network filter that does the same thing, and just leaves the connection hanging, and include any limits in that filter. So the existing option (RETURN_SSL_FAILURE) would be handled exactly how it is now, and the other two could be handled by this new network filter. WDYT?

arulthileeban commented 3 weeks ago

@alyssawilk I'd definitely like to make it as generic as possible, but I looked into the way termination of the connection upon failure is handled in a few of these cases:

Making them all work (for eg.) with this new filter might be tricky without a good amount of changes to envoy core code. Please let me know if there's a way we could make it work in a generic fashion

@ggreenway Thanks for the pointer. That makes sense to me and seems straightforward. Is your thought to reuse the validation results stored in the SSL context for this similar to the way HTTP route matching does? Or have it more generic to possibly handle other TLS failures as well?

ggreenway commented 3 weeks ago

Is your thought to reuse the validation results stored in the SSL context for this similar to the way HTTP route matching does? Or have it more generic to possibly handle other TLS failures as well?

Yes, I was thinking to reuse the existing result, and keep it simple.

arulthileeban commented 3 weeks ago

Is your thought to reuse the validation results stored in the SSL context for this similar to the way HTTP route matching does? Or have it more generic to possibly handle other TLS failures as well?

Yes, I was thinking to reuse the existing result, and keep it simple.

Sounds good. I'll get moving on it if there's no further objections from anyone else