From 0c07a364088b8ad32013202f5e82702328bf3779 Mon Sep 17 00:00:00 2001 From: Derek Menteer <105233703+hashi-derek@users.noreply.github.com> Date: Fri, 11 Nov 2022 14:29:22 -0600 Subject: [PATCH] Prevent serving TLS via ports.grpc (#15339) Prevent serving TLS via ports.grpc We remove the ability to run the ports.grpc in TLS mode to avoid confusion and to simplify configuration. This breaking change ensures that any user currently using ports.grpc in an encrypted mode will receive an error message indicating that ports.grpc_tls must be explicitly used. The suggested action for these users is to simply swap their ports.grpc to ports.grpc_tls in the configuration file. If both ports are defined, or if the user has not configured TLS for grpc, then the error message will not be printed. --- .changelog/{14294.txt => 15339.txt} | 2 +- agent/agent.go | 11 +-- agent/config/builder.go | 10 +++ agent/config/builder_test.go | 84 +++++++++++++++++++ .../docs/agent/config/config-files.mdx | 8 +- .../docs/upgrading/upgrade-specific.mdx | 19 ++--- 6 files changed, 109 insertions(+), 25 deletions(-) rename .changelog/{14294.txt => 15339.txt} (64%) diff --git a/.changelog/14294.txt b/.changelog/15339.txt similarity index 64% rename from .changelog/14294.txt rename to .changelog/15339.txt index 7fcb497b1..e5392e818 100644 --- a/.changelog/14294.txt +++ b/.changelog/15339.txt @@ -2,5 +2,5 @@ config: Add new `ports.grpc_tls` configuration option. Introduce a new port to better separate TLS config from the existing `ports.grpc` config. The new `ports.grpc_tls` only supports TLS encrypted communication. -The existing `ports.grpc` currently supports both plain-text and tls communication, but tls support will be removed in a future release. +The existing `ports.grpc` now only supports plain-text communication. ``` diff --git a/agent/agent.go b/agent/agent.go index 672c55252..d4ea4e07d 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -886,16 +886,9 @@ func (a *Agent) listenAndServeGRPC() error { return nil } - // The original grpc port may spawn in either plain-text or TLS mode (for backwards compatibility). - // TODO: Simplify this block to only spawn plain-text after 1.14 when deprecated TLS support is removed. + // Only allow grpc to spawn with a plain-text listener. if a.config.GRPCPort > 0 { - // Only allow the grpc port to spawn TLS connections if the other grpc_tls port is NOT defined. - protocol := middleware.ProtocolPlaintext - if a.config.GRPCTLSPort <= 0 && a.tlsConfigurator.GRPCServerUseTLS() { - a.logger.Warn("deprecated gRPC TLS configuration detected. Consider using `ports.grpc_tls` instead") - protocol = middleware.ProtocolTLS - } - if err := start("grpc", a.config.GRPCAddrs, protocol); err != nil { + if err := start("grpc", a.config.GRPCAddrs, middleware.ProtocolPlaintext); err != nil { closeListeners(listeners) return err } diff --git a/agent/config/builder.go b/agent/config/builder.go index 4819d64dd..6645b3a33 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1089,6 +1089,16 @@ func (b *builder) build() (rt RuntimeConfig, err error) { return RuntimeConfig{}, err } + // `ports.grpc` previously supported TLS, but this was changed for Consul 1.14. + // This check is done to warn users that a config change is mandatory. + if rt.TLS.GRPC.CertFile != "" || (rt.TLS.AutoTLS && rt.TLS.GRPC.UseAutoCert) { + // If only `ports.grpc` is enabled, and the gRPC TLS port is not explicitly defined by the user, + // check the grpc TLS settings for incompatibilities. + if rt.GRPCPort > 0 && c.Ports.GRPCTLS == nil { + return RuntimeConfig{}, fmt.Errorf("the `ports.grpc` listener no longer supports TLS. Use `ports.grpc_tls` instead. This message is appearing because GRPC is configured to use TLS, but `ports.grpc_tls` is not defined") + } + } + rt.UseStreamingBackend = boolValWithDefault(c.UseStreamingBackend, true) if c.RaftBoltDBConfig != nil { diff --git a/agent/config/builder_test.go b/agent/config/builder_test.go index cdd6ce7b4..9ee6a7af2 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -365,6 +365,90 @@ func TestBuilder_tlsVersion(t *testing.T) { require.Contains(t, b.err.Error(), invalidTLSVersion) } +func TestBuilder_WarnGRPCTLS(t *testing.T) { + tests := []struct { + name string + hcl string + expectErr bool + }{ + { + name: "success", + hcl: ``, + expectErr: false, + }, + { + name: "grpc_tls is disabled but explicitly defined", + hcl: ` + ports { grpc_tls = -1 } + tls { grpc { cert_file = "defined" }} + `, + // This behavior is a little strange, but it allows users + // to setup TLS and disable the port if they wish. + expectErr: false, + }, + { + name: "grpc is disabled", + hcl: ` + ports { grpc = -1 } + tls { grpc { cert_file = "defined" }} + `, + expectErr: false, + }, + { + name: "grpc_tls is undefined with default manual cert", + hcl: ` + tls { defaults { cert_file = "defined" }} + `, + expectErr: true, + }, + { + name: "grpc_tls is undefined with manual cert", + hcl: ` + tls { grpc { cert_file = "defined" }} + `, + expectErr: true, + }, + { + name: "grpc_tls is undefined with auto encrypt", + hcl: ` + auto_encrypt { tls = true } + tls { grpc { use_auto_cert = true }} + `, + expectErr: true, + }, + { + name: "grpc_tls is undefined with auto config", + hcl: ` + auto_config { enabled = true } + tls { grpc { use_auto_cert = true }} + `, + expectErr: true, + }, + } + for _, tc := range tests { + // using dev mode skips the need for a data dir + // and enables both grpc ports by default. + devMode := true + builderOpts := LoadOpts{ + DevMode: &devMode, + Overrides: []Source{ + FileSource{ + Name: "overrides", + Format: "hcl", + Data: tc.hcl, + }, + }, + } + _, err := Load(builderOpts) + if tc.expectErr { + require.Error(t, err) + require.Contains(t, err.Error(), "listener no longer supports TLS") + } else { + require.NoError(t, err) + } + } +} + func TestBuilder_tlsCipherSuites(t *testing.T) { b := builder{} diff --git a/website/content/docs/agent/config/config-files.mdx b/website/content/docs/agent/config/config-files.mdx index ad5c1c6c8..7ff61f399 100644 --- a/website/content/docs/agent/config/config-files.mdx +++ b/website/content/docs/agent/config/config-files.mdx @@ -607,14 +607,12 @@ Valid time units are 'ns', 'us' (or 'µs'), 'ms', 's', 'm', 'h'." - `grpc` ((#grpc_port)) - The gRPC API, -1 to disable. Default -1 (disabled). **We recommend using `8502` for `grpc`** as your conventional gRPC port number, as it allows some tools to work automatically. This parameter is set to `8502` by default when the agent runs - in `-dev` mode. The `grpc` port currently supports either plaintext or TLS traffic for - backwards-compatibility, but TLS support is deprecated and will be removed in a future - release. Refer to `grpc_tls` for more information on configuring a TLS-enabled port. + in `-dev` mode. The `grpc` port only supports plaintext traffic starting in Consul 1.14. + Refer to `grpc_tls` for more information on configuring a TLS-enabled port. - `grpc_tls` ((#grpc_tls_port)) - The gRPC API with TLS connections, -1 to disable. gRPC_TLS is enabled by default on port 8503 for Consul servers. **We recommend using `8503` for `grpc_tls`** as your conventional gRPC port number, as it allows some tools to work automatically. `grpc_tls` is always guaranteed to be encrypted. Both `grpc` and `grpc_tls` - can be configured at the same time, but they may not utilize the same port number. If both `grpc` and - `grpc_tls` are defined, then `grpc` will always be plaintext. This field was added in Consul 1.14. + can be configured at the same time, but they may not utilize the same port number. This field was added in Consul 1.14. - `serf_lan` ((#serf_lan_port)) - The Serf LAN port. Default 8301. TCP and UDP. Equivalent to the [`-serf-lan-port` command line flag](/docs/agent/config/cli-flags#_serf_lan_port). - `serf_wan` ((#serf_wan_port)) - The Serf WAN port. Default 8302. diff --git a/website/content/docs/upgrading/upgrade-specific.mdx b/website/content/docs/upgrading/upgrade-specific.mdx index 65b40e358..cb59d74c9 100644 --- a/website/content/docs/upgrading/upgrade-specific.mdx +++ b/website/content/docs/upgrading/upgrade-specific.mdx @@ -26,22 +26,21 @@ A breaking change was made in Consul 1.14 that: #### Changes to gRPC TLS configuration -**Make configuration changes** if using sidecar proxies or gateways that include any of the following configuration file values: -1. [`ports.https`](/docs/agent/config/config-files#https_port) - Encrypts gRPC in Consul 1.12 and prior -1. [`auto_encrypt`](/docs/agent/config/config-files#auto_encrypt) - Encrypts gRPC in Consul 1.13 and prior -1. [`auto_config`](/docs/agent/config/config-files#auto_config) - Encrypts gRPC in Consul 1.13 and prior +**Make configuration changes** if using [`ports.grpc`](/docs/agent/config/config-files#grpc_port) in conjunction with any of the following settings that enables encryption: +1. [`tls.grpc`](/docs/agent/config/config-files#tls_grpc) +1. [`tls.defaults`](/docs/agent/config/config-files#tls_defaults) +1. [`auto_encrypt`](/docs/agent/config/config-files#auto_encrypt) +1. [`auto_config`](/docs/agent/config/config-files#auto_config) Prior to Consul 1.14, it was possible to encrypt communication between Consul and Envoy over `ports.grpc` using these settings. Consul 1.14 introduces [`ports.grpc_tls`](/docs/agent/config/config-files#grpc_tls_port), a new configuration for encrypting communication over gRPC. The existing [`ports.grpc`](/docs/agent/config/config- -files#grpc_port) configuration **will stop supporting encryption in a future release**. As of version 1.14, -[`ports.grpc_tls`](/docs/agent/config/config-files#grpc_tls_port) is the recommended configuration to encrypt gRPC traffic. -The default value for gRPC TLS port is 8503 for Consul servers. To disable the gRPC TLS port, use value -1. +files#grpc_port) configuration **no longer supports encryption**. As of version 1.14, +[`ports.grpc_tls`](/docs/agent/config/config-files#grpc_tls_port) is the only port that serves encrypted gRPC traffic. +The default value for the gRPC TLS port is 8503 for Consul servers. To disable the gRPC TLS port, use value -1. -For most environments, the Envoy communication to Consul is loop-back only and does not benefit from encryption. - -If you already use gRPC encryption, change the existing `ports.grpc` to `ports.grpc_tls` in your configuration to ensure compatibility with future releases. +If you already use gRPC encryption, change the existing `ports.grpc` to `ports.grpc_tls` in your configuration to ensure compatibility. #### Changes to peering