From ceb102f352fed3ab229af752725223fc5d904488 Mon Sep 17 00:00:00 2001 From: hc-github-team-consul-core Date: Fri, 11 Nov 2022 15:29:50 -0500 Subject: [PATCH] Backport of Prevent serving TLS via ports.grpc into release/1.14.x (#15342) This pull request was automerged via backport-assistant --- .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 7fcb497b1e..e5392e818a 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 2489ee3e67..a095f1d5a3 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -887,16 +887,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 a7f9bcb5e2..3c7c8ecc6b 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1090,6 +1090,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 abb7be0e1d..c3afbd1fd0 100644 --- a/agent/config/builder_test.go +++ b/agent/config/builder_test.go @@ -366,6 +366,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 ad5c1c6c82..7ff61f3993 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 65b40e3580..cb59d74c92 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