From 8df1deaa8a98178c69759d69d39b3ae71bda3054 Mon Sep 17 00:00:00 2001 From: Samantha Date: Thu, 25 May 2023 16:00:33 -0400 Subject: [PATCH 1/4] tlsutil: Fix check TLS configuration --- tlsutil/config.go | 25 +++++++++++++------ tlsutil/config_test.go | 8 +++--- .../checks-configuration-reference.mdx | 4 +-- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/tlsutil/config.go b/tlsutil/config.go index 5cdaf7633eca..55c89e9bd2ac 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -857,10 +857,23 @@ func (c *Configurator) IncomingHTTPSConfig() *tls.Config { return config } -// OutgoingTLSConfigForCheck generates a *tls.Config for outgoing TLS connections -// for checks. This function is separated because there is an extra flag to -// consider for checks. EnableAgentTLSForChecks and InsecureSkipVerify has to -// be checked for checks. +// OutgoingTLSConfigForCheck creates a client *tls.Config for executing checks. +// It is RECOMMENDED that the serverName be left unspecified. The crypto/tls +// client will deduce the ServerName (for SNI) from the check address unless +// it's an IP (RFC 6066, Section 3). However, there are two instances where +// supplying a serverName is useful: +// +// 1. When the check address is an IP, a serverName can be supplied for SNI. +// Note: setting serverName will also override the hostname used to verify +// the certificate presented by the server being checked. +// +// 2. When the a 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 serverName will also override the +// ServerName used for SNI. +// +// Setting skipVerify will disable verification of the server's certificate +// chain and hostname, which is generally not suitable for production use. func (c *Configurator) OutgoingTLSConfigForCheck(skipVerify bool, serverName string) *tls.Config { c.log("OutgoingTLSConfigForCheck") @@ -875,13 +888,9 @@ func (c *Configurator) OutgoingTLSConfigForCheck(skipVerify bool, serverName str } } - if serverName == "" { - serverName = c.serverNameOrNodeName() - } config := c.internalRPCTLSConfig(false) config.InsecureSkipVerify = skipVerify config.ServerName = serverName - return config } diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index 18ba11e38b90..573337222865 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -1374,7 +1374,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { }, }, { - name: "agent tls, default server name", + name: "agent tls, default consul server name, no override", conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ @@ -1387,11 +1387,11 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { }, expected: &tls.Config{ MinVersion: tls.VersionTLS12, - ServerName: "servername", + ServerName: "", }, }, { - name: "agent tls, skip verify, node name for server name", + name: "agent tls, skip verify, consul node name for server name, no override", conf: func() (*Configurator, error) { return NewConfigurator(Config{ InternalRPC: ProtocolConfig{ @@ -1405,7 +1405,7 @@ func TestConfigurator_OutgoingTLSConfigForCheck(t *testing.T) { expected: &tls.Config{ InsecureSkipVerify: true, MinVersion: tls.VersionTLS12, - ServerName: "nodename", + ServerName: "", }, }, { diff --git a/website/content/docs/services/configuration/checks-configuration-reference.mdx b/website/content/docs/services/configuration/checks-configuration-reference.mdx index fee071de51b0..f88130547e5d 100644 --- a/website/content/docs/services/configuration/checks-configuration-reference.mdx +++ b/website/content/docs/services/configuration/checks-configuration-reference.mdx @@ -35,8 +35,8 @@ Specify health check options in the `check` block. To register two or more heath | `h2ping` | String value that specifies the HTTP2 endpoint, including port number, to send HTTP2 requests to. |
  • H2ping
    • | | `h2ping_use_tls` | Boolean value that enables TLS for H2ping checks when set to `true`. |
  • H2ping
    • | | `http` | String value that specifies an HTTP endpoint to send requests to. |
  • HTTP
    • | -| `tls_server_name` | String value that specifies the name of the TLS server that issues certificates. Defaults to the SNI determined by the address specified in the `http` field. Set the `tls_skip_verify` to `false` to disable this field. |
  • HTTP
    • | -| `tls_skip_verify` | Boolean value that disbles TLS for HTTP checks when set to `true`. Default is `false`. |
  • HTTP
    • | +| `tls_server_name` | String value that specifies the server name used to verify the hostname on the returned certificates unless `tls_skip_verify` is given. Also included in the client's handshake to support SNI. It is recommended that this field be left unspecified. The TLS client will deduce the server name for SNI from the check address unless it's an IP ([RFC 6066, Section 3](https://tools.ietf.org/html/rfc6066#section-3)). However, there are two instances where supplying a `tls_server_name` is useful:
  • When the check address is an IP, a `tls_server_name` can be supplied for SNI. Note: setting `tls_server_name` will also override the hostname used to verify the certificate presented by the server being checked.
  • When the a 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.
    • |
  • HTTP
  • H2Ping
  • gRPC
    • | +| `tls_skip_verify` | Boolean value that disables verification of the chain and hostname of the certificate presented by the server being checked. This is generally not suitable for production use. Default is `false`. |
  • HTTP
  • H2Ping
  • gRPC
    • | | `method` | String value that specifies the request method to send during HTTP checks. Default is `GET`. |
  • HTTP
    • | | `header` | Object that specifies header fields to send in HTTP check requests. Each header specified in `header` object contains a list of string values. |
  • HTTP
    • | | `body` | String value that contains JSON attributes to send in HTTP check requests. You must escap the quotation marks around the keys and values for each attribute. |
  • HTTP
    • | From 2e9882a6d75a44b94b091105e6328f9dbb8d5b81 Mon Sep 17 00:00:00 2001 From: Samantha Date: Thu, 25 May 2023 17:53:01 -0400 Subject: [PATCH 2/4] Rewording docs. --- .../services/configuration/checks-configuration-reference.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/services/configuration/checks-configuration-reference.mdx b/website/content/docs/services/configuration/checks-configuration-reference.mdx index f88130547e5d..cf7324d8a657 100644 --- a/website/content/docs/services/configuration/checks-configuration-reference.mdx +++ b/website/content/docs/services/configuration/checks-configuration-reference.mdx @@ -35,7 +35,7 @@ Specify health check options in the `check` block. To register two or more heath | `h2ping` | String value that specifies the HTTP2 endpoint, including port number, to send HTTP2 requests to. |
  • H2ping
    • | | `h2ping_use_tls` | Boolean value that enables TLS for H2ping checks when set to `true`. |
  • H2ping
    • | | `http` | String value that specifies an HTTP endpoint to send requests to. |
  • HTTP
    • | -| `tls_server_name` | String value that specifies the server name used to verify the hostname on the returned certificates unless `tls_skip_verify` is given. Also included in the client's handshake to support SNI. It is recommended that this field be left unspecified. The TLS client will deduce the server name for SNI from the check address unless it's an IP ([RFC 6066, Section 3](https://tools.ietf.org/html/rfc6066#section-3)). However, there are two instances where supplying a `tls_server_name` is useful:
  • When the check address is an IP, a `tls_server_name` can be supplied for SNI. Note: setting `tls_server_name` will also override the hostname used to verify the certificate presented by the server being checked.
  • When the a 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.
    • |
  • HTTP
  • H2Ping
  • gRPC
    • | +| `tls_server_name` | String value that specifies the server name used to verify the hostname on the returned certificates unless `tls_skip_verify` is given. Also included in the client's handshake to support SNI. It is recommended that this field be left unspecified. The TLS client will deduce the server name for SNI from the check address unless it's an IP ([RFC 6066, Section 3](https://tools.ietf.org/html/rfc6066#section-3)). There are two common circumstances where supplying a `tls_server_name` can be beneficial:
  • 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.
  • When the a 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.
    • |
  • HTTP
  • H2Ping
  • gRPC
    • | | `tls_skip_verify` | Boolean value that disables verification of the chain and hostname of the certificate presented by the server being checked. This is generally not suitable for production use. Default is `false`. |
  • HTTP
  • H2Ping
  • gRPC
    • | | `method` | String value that specifies the request method to send during HTTP checks. Default is `GET`. |
  • HTTP
    • | | `header` | Object that specifies header fields to send in HTTP check requests. Each header specified in `header` object contains a list of string values. |
  • HTTP
    • | From b459ae3da8003d55268e42a12dd3a9fb32cb36de Mon Sep 17 00:00:00 2001 From: Samantha Date: Mon, 26 Jun 2023 17:32:37 -0400 Subject: [PATCH 3/4] Update website/content/docs/services/configuration/checks-configuration-reference.mdx Co-authored-by: trujillo-adam <47586768+trujillo-adam@users.noreply.github.com> --- .../services/configuration/checks-configuration-reference.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/services/configuration/checks-configuration-reference.mdx b/website/content/docs/services/configuration/checks-configuration-reference.mdx index cf7324d8a657..4ac32163acec 100644 --- a/website/content/docs/services/configuration/checks-configuration-reference.mdx +++ b/website/content/docs/services/configuration/checks-configuration-reference.mdx @@ -36,7 +36,7 @@ Specify health check options in the `check` block. To register two or more heath | `h2ping_use_tls` | Boolean value that enables TLS for H2ping checks when set to `true`. |
  • H2ping
    • | | `http` | String value that specifies an HTTP endpoint to send requests to. |
  • HTTP
    • | | `tls_server_name` | String value that specifies the server name used to verify the hostname on the returned certificates unless `tls_skip_verify` is given. Also included in the client's handshake to support SNI. It is recommended that this field be left unspecified. The TLS client will deduce the server name for SNI from the check address unless it's an IP ([RFC 6066, Section 3](https://tools.ietf.org/html/rfc6066#section-3)). There are two common circumstances where supplying a `tls_server_name` can be beneficial:
  • 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.
  • When the a 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.
    • |
  • HTTP
  • H2Ping
  • gRPC
    • | -| `tls_skip_verify` | Boolean value that disables verification of the chain and hostname of the certificate presented by the server being checked. This is generally not suitable for production use. Default is `false`. |
  • HTTP
  • H2Ping
  • gRPC
    • | +| `tls_skip_verify` | Boolean value that determines if the check verifies the chain and hostname of the certificate that the server presents. Set to `true` to disable verification. We recommend setting to `false` for production use. Default is `false`. |
  • HTTP
  • H2Ping
  • gRPC
    • | | `method` | String value that specifies the request method to send during HTTP checks. Default is `GET`. |
  • HTTP
    • | | `header` | Object that specifies header fields to send in HTTP check requests. Each header specified in `header` object contains a list of string values. |
  • HTTP
    • | | `body` | String value that contains JSON attributes to send in HTTP check requests. You must escap the quotation marks around the keys and values for each attribute. |
  • HTTP
    • | From c75c60215829219a0c7aac7d99c4c567de01c249 Mon Sep 17 00:00:00 2001 From: Samantha Date: Wed, 28 Jun 2023 11:40:22 -0400 Subject: [PATCH 4/4] Fix typos and add changelog entry. --- .changelog/17481.txt | 3 +++ tlsutil/config.go | 2 +- .../services/configuration/checks-configuration-reference.mdx | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) create mode 100644 .changelog/17481.txt diff --git a/.changelog/17481.txt b/.changelog/17481.txt new file mode 100644 index 000000000000..89ad16998e83 --- /dev/null +++ b/.changelog/17481.txt @@ -0,0 +1,3 @@ +```release-note:bug +tlsutil: Default setting of ServerName field in outgoing TLS configuration for checks now handled by crypto/tls. +``` diff --git a/tlsutil/config.go b/tlsutil/config.go index 55c89e9bd2ac..a52d6b6ad829 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -867,7 +867,7 @@ func (c *Configurator) IncomingHTTPSConfig() *tls.Config { // Note: setting serverName will also override the hostname used to verify // the certificate presented by the server being checked. // -// 2. When the a hostname in the check address won't be present in the SAN +// 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 serverName will also override the // ServerName used for SNI. diff --git a/website/content/docs/services/configuration/checks-configuration-reference.mdx b/website/content/docs/services/configuration/checks-configuration-reference.mdx index 4ac32163acec..c0d3e24cfde6 100644 --- a/website/content/docs/services/configuration/checks-configuration-reference.mdx +++ b/website/content/docs/services/configuration/checks-configuration-reference.mdx @@ -35,7 +35,7 @@ Specify health check options in the `check` block. To register two or more heath | `h2ping` | String value that specifies the HTTP2 endpoint, including port number, to send HTTP2 requests to. |
  • H2ping
    • | | `h2ping_use_tls` | Boolean value that enables TLS for H2ping checks when set to `true`. |
  • H2ping
    • | | `http` | String value that specifies an HTTP endpoint to send requests to. |
  • HTTP
    • | -| `tls_server_name` | String value that specifies the server name used to verify the hostname on the returned certificates unless `tls_skip_verify` is given. Also included in the client's handshake to support SNI. It is recommended that this field be left unspecified. The TLS client will deduce the server name for SNI from the check address unless it's an IP ([RFC 6066, Section 3](https://tools.ietf.org/html/rfc6066#section-3)). There are two common circumstances where supplying a `tls_server_name` can be beneficial:
  • 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.
  • When the a 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.
    • |
  • HTTP
  • H2Ping
  • gRPC
    • | +| `tls_server_name` | String value that specifies the server name used to verify the hostname on the returned certificates unless `tls_skip_verify` is given. Also included in the client's handshake to support SNI. It is recommended that this field be left unspecified. The TLS client will deduce the server name for SNI from the check address unless it's an IP ([RFC 6066, Section 3](https://tools.ietf.org/html/rfc6066#section-3)). There are two common circumstances where supplying a `tls_server_name` can be beneficial:
  • 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.
  • 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.
    • |
  • HTTP
  • H2Ping
  • gRPC
    • | | `tls_skip_verify` | Boolean value that determines if the check verifies the chain and hostname of the certificate that the server presents. Set to `true` to disable verification. We recommend setting to `false` for production use. Default is `false`. |
  • HTTP
  • H2Ping
  • gRPC
    • | | `method` | String value that specifies the request method to send during HTTP checks. Default is `GET`. |
  • HTTP
    • | | `header` | Object that specifies header fields to send in HTTP check requests. Each header specified in `header` object contains a list of string values. |
  • HTTP
    • |