envoy icon indicating copy to clipboard operation
envoy copied to clipboard

SNI TLS alert "unrecognized_name"

Open rmillet-rs opened this issue 3 years ago • 6 comments

Hello,

When SNI does not match any known server name (in filter_chain_match), is it possible to send an "unrecognized_name" TLS alert instead of resetting the connection?

Current behavior with envoy:

curl -v https://127.0.0.1
*   Trying 127.0.0.1:443...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: /etc/ssl/certs
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* OpenSSL SSL_connect: Connection reset by peer in connection to 127.0.0.1:443 
* Closing connection 0
curl: (35) OpenSSL SSL_connect: Connection reset by peer in connection to 127.0.0.1:443

The expected behavior (like with HAProxy)

curl -v https://127.0.0.1
*   Trying 127.0.0.1:443...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/ssl/certs/ca-certificates.crt
  CApath: /etc/ssl/certs
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS alert, unrecognized name (624):
* error:14094458:SSL routines:ssl3_read_bytes:tlsv1 unrecognized name
* Closing connection 0

https://www.rfc-editor.org/rfc/rfc6066#section-3

rmillet-rs avatar Jan 13 '23 15:01 rmillet-rs

Sounds like a good improvement, marking it as a feature request.

yanavlasov avatar Jan 20 '23 20:01 yanavlasov

I was thinking that TLS inspector could handle it in SSL_CTX_set_tlsext_servername_callback, but it's called after matching filter_chain_match.server_names. So if SNI does not match any filter, SSL_CTX_set_tlsext_servername_callback will not be called and "unrecognized name" can't be returned, because the connection is closed earlier.

If SSL_CTX_set_tlsext_servername_callback could be reached, then we could set the alert to SSL_AD_UNRECOGNIZED_NAME if does not match with the filter SNI.

@yanavlasov any ideas?

jewertow avatar Jan 25 '24 12:01 jewertow

Currently, the way SNI/ALPN termination is handled in Envoy, is not in accordance with RFC. Both require a particular SSL fatal message SNI - "unrecognized name" and ALPN - "no_application_protocol" on no match. We use the filter_chain_matchers to catch the mismatch and, do a termination abruptly since we don't operate with TLS context in the filter_chain_matcher logic. I'd argue that we should do the termination returning a SSL error in the TLS inspector as part of the SNI/ALPN callbacks before moving to the filter_chain_matcher,. A possible way would be to continue using filter_chain_matchers for SNI/ALPN at filter chain level with current logic; Additionally we add two config values in TLS inspector to handle it for the whole listener, which admittedly, could be confusing and likely not intended functionality of TLS "inspector".

- name: tls_inspector
  typed_config:
    "@type": type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector
    allowed_alpn: ["h2","http/1.1"]
    allowed_sni: ["example.com", "example1.com", "examplealias.com"]
filter_chains:
  - filter_chain_match:
      application_protocols: ["h2"]
      server_names: ["example.com"]
 ...
  - filter_chain_match:
      application_protocols: ["http/1.1"]
      server_names: ["example1.com"]

IMO while it'd be nice to modify the current way it is done, I'm not sure of wider implications of changing this match mechanism and given this is likely widely used, I'm not sure the pros of this change outweigh the cons. Although, happy to work on this if there's a clear way forward. @ggreenway @yanavlasov thoughts?

arulthileeban avatar Jul 14 '24 14:07 arulthileeban

I think it would be better to integrate the error handling with the existing filter chain matching. This could be done by creating a transport socket (or a configuration of the existing tls transport socket) that always returns a configurable TLS alert and rejects the handshake.

So in a case where a config has server_names matches for various names, the default/fallback filter chain could be configured to always respond with unrecognized_name.

This approach would avoid duplicating all the server name matching between the filter chain matching and the tls inspector configs.

ggreenway avatar Jul 17 '24 00:07 ggreenway

I think it would be better to integrate the error handling with the existing filter chain matching. This could be done by creating a transport socket (or a configuration of the existing tls transport socket) that always returns a configurable TLS alert and rejects the handshake.

So in a case where a config has server_names matches for various names, the default/fallback filter chain could be configured to always respond with unrecognized_name.

This approach would avoid duplicating all the server name matching between the filter chain matching and the tls inspector configs.

This makes sense. Thanks. I'll take a look

arulthileeban avatar Jul 23 '24 22:07 arulthileeban

@ggreenway Just got the time to work on this. I was able to get a new transport socket to work as intended. I have run into a bit of a roadblock around the final structure of this fallback filter chain. If we define no filters in this chain, we encounter a failure earlier in the flow. I used a dummy echo filter to overcome this check for testing purposes, but how would you like this to be managed in the final solution:

  • Create a private no-op filter for this flow
  • Use one of the existing filters with a clear designation
  • Modify existing code to avoid needing a filter for this

arulthileeban avatar Oct 17 '24 23:10 arulthileeban

I think you can just document that a filter needs to be specified, and put in an example that uses the echo filter or something similar.

ggreenway avatar Oct 22 '24 17:10 ggreenway

Shouldn't this change be part of envoy's flow without user having to specify anything? i.e. If envoy receives a SNI with server name that doesn't match what the server supports, envoy should respond with unrecognized_name instead of closing the connection, which it does currently.

listener_filters:
- name: "envoy.filters.listener.tls_inspector"
  typed_config:
    "@type": type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector
filter_chains:
- filter_chain_match:
    server_names: ["www.example.com"]

For eg., if you send a request to this server with SNI abc.example.com, currently it'd close the connection and we'd want it to respond with a TLS error unrecognized_name

arulthileeban avatar Oct 22 '24 23:10 arulthileeban

What if the filter chain matching has both server_name and alpn and source IP address matching rules? If we don't match a filter chain, it's not clear what error to send. That's why I'm suggesting it be something configured by the user (who can evaluate their entire set of matching rules and configure appropriate errors in the appropriate cases).

ggreenway avatar Oct 22 '24 23:10 ggreenway

Maybe I'm missing something, but my plan was to initialize a fallback filter chain(an internal dummy one with the configurable TLS failure socket) as part of the filter chain manager and return it here on failure for Server name with SNI alert, here for ALPN alert. The IP and other non-TLS matching can remain as-is with returning a nullptr which will terminate here. This also shouldn't override an user specified default filter.

arulthileeban avatar Oct 23 '24 00:10 arulthileeban

I think we miscommunicated. There should not be any automatic behavior; what I am proposing is a way to configure a filter chain so that all it does is send back a configurable TLS alert. A user could configure this filter chain with any set of filter-chain-match criteria they would like, including possibly as the default/fallback filter-chain.

Adding a non-nullptr return to the points you noted would break filter-chain matching behavior.

ggreenway avatar Oct 23 '24 01:10 ggreenway

Got it, thanks. That clears it up for me. Will follow up soon.

arulthileeban avatar Oct 23 '24 01:10 arulthileeban