Description

Since the introduction of #5394, the OutgoingTLSConfigForCheck method of tlsutil.Configurator has been setting the ServerName field of TLS configurations used for executing health checks to the Consul node or server name by default.

According to the crypto/tls documentation:

// ServerName is used to verify the hostname on the returned
// certificates unless InsecureSkipVerify is given. It is also included
// in the client's handshake to support virtual hosting unless it is
// an IP address.

Unless the check.tls_server_name indicates otherwise, it is generally better to leave the ServerName field empty. The crypto/tls client will infer the ServerName for SNI from the check address, unless it's an IP (as per RFC 6066, Section 3). There are two common situations where specifying this field can be beneficial:

1. When the check address is an IP, tls_server_name can be specified for SNI. Note: setting tls_server_name will also override the hostname used to verify the certificate presented by the server being checked.

2. When the hostname in the check address won't be present in the SAN (Subject Alternative Name) field of the certificate presented by the server being checked. Note: setting tls_server_name will also override the hostname used for SNI.

This PR also updates the documentation for tls_server_name and tls_skip_verify to indicate that both fields can be used for H2Ping and gRPC checks, not just HTTP and provides a slightly slimmed down version of the above circumstances for tls_server_name.

Testing & Reproduction steps

I encountered this issue while drafting letsencrypt/boulder#6908. After experiencing certificate validation failures due the Consul node name failing to be present in the SAN field of certificate of the component being health checked, we dug into the Consul code and diagnosed the issue above.

Links

Substantiating my claim that crypto/tls client will infer the ServerName based on the address being dialed: link

PR Checklist

• updated test coverage
• external facing docs updated
• appropriate backport labels added
• not a security concern