Allow setting TLS for gRPC with deprecated options [1.13.x] (#14668)

Currently TLS for gRPC can only be enabled using the options nested in the tls.grpc configuration stanza.

That leads to a breaking change where the TLS options deprecated in 1.12 cannot be used to enable TLS for gRPC.

This commit updates the logic for determining whether TLS should be used on the public gRPC port: If the 1.12 tls stanza is not specified we default to the original behavior, which is to enable TLS for gRPC if the HTTPS port is set.

The change allows for consul-k8s to continue to use TLS flags compatible with 1.11 until 1.14 is released.

.changelog/14668.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+tls: undo breaking change that prevented setting TLS for gRPC when using config flags available in Consul v1.11.
+```

agent/agent.go

@@ -761,7 +761,7 @@ func (a *Agent) Failed() <-chan struct{} {
 }
 
 func (a *Agent) buildExternalGRPCServer() {
-	a.externalGRPCServer = external.NewServer(a.logger.Named("grpc.external"), a.tlsConfigurator)
+	a.externalGRPCServer = external.NewServer(a.logger.Named("grpc.external"), a.tlsConfigurator, a.config.HTTPSPort > 0)
 }
 
 func (a *Agent) listenAndServeGRPC() error {

agent/config/builder.go

@@ -189,9 +189,10 @@ func newBuilder(opts LoadOpts) (*builder, error) {
 	b.Tail = append(b.Tail, LiteralSource{Name: "flags.values", Config: values})
 	for i, s := range opts.HCL {
 		b.Tail = append(b.Tail, FileSource{
-			Name:   fmt.Sprintf("flags-%d.hcl", i),
-			Format: "hcl",
-			Data:   s,
+			Name:     fmt.Sprintf("flags-%d.hcl", i),
+			Format:   "hcl",
+			Data:     s,
+			FromUser: true,
 		})
 	}
 	b.Tail = append(b.Tail, NonUserSource(), DefaultConsulSource(), OverrideEnterpriseSource(), defaultVersionSource())
@@ -282,7 +283,7 @@ func newSourceFromFile(path string, format string) (Source, error) {
 	if format == "" {
 		format = formatFromFileExtension(path)
 	}
-	return FileSource{Name: path, Data: string(data), Format: format}, nil
+	return FileSource{Name: path, Data: string(data), Format: format, FromUser: true}, nil
 }
 
 // shouldParse file determines whether the file to be read is of a supported extension
@@ -2517,6 +2518,12 @@ func validateAbsoluteURLPath(p string) error {
 func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error) {
 	var c tlsutil.Config
 
+	// This flag indicates whether any of the per-listener configuration was set.
+	// The flag was populated earlier when applying deprecated options.
+	if t.SpecifiedTLSStanza != nil {
+		c.SpecifiedTLSStanza = *t.SpecifiedTLSStanza
+	}
+
 	// Consul makes no outgoing connections to the public gRPC port (internal gRPC
 	// traffic goes through the multiplexed internal RPC port) so return an error
 	// rather than let the user think this setting is going to do anything useful.

agent/config/builder_test.go

@@ -75,10 +75,10 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles(t *testing.T) {
 	require.NoError(t, err)
 
 	expected := []Source{
-		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		FileSource{Name: paths[1], Format: "json", Data: "content b"},
-		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b"},
+		FileSource{Name: paths[0], Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: paths[1], Format: "json", Data: "content b", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "json", Data: "content b", FromUser: true},
 	}
 	require.Equal(t, expected, b.Sources)
 	require.Len(t, b.Warnings, 2)
@@ -91,12 +91,12 @@ func TestNewBuilder_PopulatesSourcesFromConfigFiles_WithConfigFormat(t *testing.
 	require.NoError(t, err)
 
 	expected := []Source{
-		FileSource{Name: paths[0], Format: "hcl", Data: "content a"},
-		FileSource{Name: paths[1], Format: "hcl", Data: "content b"},
-		FileSource{Name: paths[2], Format: "hcl", Data: "content c"},
-		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a"},
-		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b"},
-		FileSource{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c"},
+		FileSource{Name: paths[0], Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: paths[1], Format: "hcl", Data: "content b", FromUser: true},
+		FileSource{Name: paths[2], Format: "hcl", Data: "content c", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "a.hcl"), Format: "hcl", Data: "content a", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "b.json"), Format: "hcl", Data: "content b", FromUser: true},
+		FileSource{Name: filepath.Join(paths[3], "c.yaml"), Format: "hcl", Data: "content c", FromUser: true},
 	}
 	require.Equal(t, expected, b.Sources)
 }

agent/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"reflect"
 	"time"
 
 	"github.com/hashicorp/consul/agent/consul"
@@ -30,6 +31,10 @@ type FileSource struct {
 	Name   string
 	Format string
 	Data   string
+
+	// FromUser indicates whether the the file source was provided by the user.
+	// This distinguishes from synthetic file sources that Consul will generate.
+	FromUser bool
 }
 
 func (f FileSource) Source() string {
@@ -79,7 +84,7 @@ func (f FileSource) Parse() (Config, Metadata, error) {
 		return Config{}, m, err
 	}
 
-	c, warns := applyDeprecatedConfig(&target)
+	c, warns := applyDeprecatedConfig(&target, f.FromUser)
 	m.Unused = md.Unused
 	m.Keys = md.Keys
 	m.Warnings = warns
@@ -870,12 +875,28 @@ type TLSProtocolConfig struct {
 	UseAutoCert          *bool   `mapstructure:"use_auto_cert"`
 }
 
+func (c TLSProtocolConfig) IsZero() bool {
+	v := reflect.ValueOf(c)
+
+	for i := 0; i < v.NumField(); i++ {
+		if !v.Field(i).IsNil() {
+			return false
+		}
+	}
+	return true
+}
+
 type TLS struct {
 	Defaults    TLSProtocolConfig `mapstructure:"defaults"`
 	InternalRPC TLSProtocolConfig `mapstructure:"internal_rpc"`
 	HTTPS       TLSProtocolConfig `mapstructure:"https"`
 	GRPC        TLSProtocolConfig `mapstructure:"grpc"`
 
+	// SpecifiedTLSStanza indicates whether the per-protocol tls stanza from configuration was used.
+	// If unspecified, and TLS is configured, that implies that the deprecated flags were used.
+	// The flag was added exclusively for the 1.13 patch series for backwards compatibility purposes.
+	SpecifiedTLSStanza *bool `mapstructure:"-"`
+
 	// GRPCModifiedByDeprecatedConfig is a flag used to indicate that GRPC was
 	// modified by the deprecated field mapping (as apposed to a user-provided
 	// a grpc stanza). This prevents us from emitting a warning about an
@@ -890,6 +911,11 @@ type TLS struct {
 	GRPCModifiedByDeprecatedConfig *struct{} `mapstructure:"-"`
 }
 
+// ContainsDefaults indicates whether the user-settable values in this type are the defaults.
+func (t *TLS) ContainsDefaults() bool {
+	return t.Defaults.IsZero() && t.InternalRPC.IsZero() && t.HTTPS.IsZero() && t.GRPC.IsZero()
+}
+
 type Peering struct {
 	Enabled *bool `mapstructure:"enabled"`
 }

agent/config/deprecated.go

@@ -70,7 +70,7 @@ type DeprecatedConfig struct {
 	TLSPreferServerCipherSuites *bool `mapstructure:"tls_prefer_server_cipher_suites"`
 }
 
-func applyDeprecatedConfig(d *decodeTarget) (Config, []string) {
+func applyDeprecatedConfig(d *decodeTarget, fromUser bool) (Config, []string) {
 	dep := d.DeprecatedConfig
 	var warns []string
 
@@ -172,15 +172,27 @@ func applyDeprecatedConfig(d *decodeTarget) (Config, []string) {
 		warns = append(warns, deprecationWarning("acl_enable_key_list_policy", "acl.enable_key_list_policy"))
 	}
 
-	warns = append(warns, applyDeprecatedTLSConfig(dep, &d.Config)...)
+	warns = append(warns, applyDeprecatedTLSConfig(dep, &d.Config, fromUser)...)
 
 	return d.Config, warns
 }
 
-func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string {
+func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config, fromUser bool) []string {
 	var warns []string
 
 	tls := &cfg.TLS
+
+	// If the TLS stanza was specified by the user then we set a flag to indicate that.
+	// This check MUST happen before applying the deprecated options below, or else
+	// the tls struct will never be empty.
+	//
+	// This check was added exclusively to the 1.13 patch series for compatibility with
+	// Consul 1.11 style TLS configuration. Consul 1.14 does not require it since 1.12
+	// is the earliest major version supported once 1.14 is released.
+	if fromUser && !tls.ContainsDefaults() {
+		tls.SpecifiedTLSStanza = pBool(true)
+	}
+
 	defaults := &tls.Defaults
 	internalRPC := &tls.InternalRPC
 	https := &tls.HTTPS

agent/config/runtime_test.go

@@ -3051,6 +3051,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		expected: func(rt *RuntimeConfig) {
 			rt.DataDir = dataDir
 			rt.TLS.InternalRPC.VerifyIncoming = true
+			rt.TLS.SpecifiedTLSStanza = true
 			rt.AutoEncryptAllowTLS = true
 			rt.ConnectEnabled = true
 
@@ -3082,6 +3083,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.ConnectEnabled = true
 
 			rt.TLS.InternalRPC.VerifyIncoming = true
+			rt.TLS.SpecifiedTLSStanza = true
 			rt.TLS.GRPC.VerifyIncoming = true
 			rt.TLS.HTTPS.VerifyIncoming = true
 
@@ -3112,6 +3114,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.AutoEncryptAllowTLS = true
 			rt.ConnectEnabled = true
 			rt.TLS.InternalRPC.VerifyIncoming = true
+			rt.TLS.SpecifiedTLSStanza = true
 
 			// server things
 			rt.ServerMode = true
@@ -4509,7 +4512,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 	})
 
-	///////////////////////////////////
+	// /////////////////////////////////
 	// Defaults sanity checks
 
 	run(t, testCase{
@@ -4532,7 +4535,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 		},
 	})
 
-	///////////////////////////////////
+	// /////////////////////////////////
 	// Auto Config related tests
 	run(t, testCase{
 		desc: "auto config and auto encrypt error",
@@ -4767,6 +4770,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.DataDir = dataDir
 			rt.TLS.InternalRPC.VerifyOutgoing = true
 			rt.TLS.AutoTLS = true
+			rt.TLS.SpecifiedTLSStanza = true
 		},
 	})
 
@@ -5025,6 +5029,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.ServerMode = true
 			rt.SkipLeaveOnInt = true
 			rt.TLS.InternalRPC.CertFile = "foo"
+			rt.TLS.SpecifiedTLSStanza = true
 			rt.RPCConfig.EnableStreaming = true
 		},
 	})
@@ -5462,6 +5467,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
+			rt.TLS.SpecifiedTLSStanza = true
 
 			rt.TLS.InternalRPC.CAFile = "internal_rpc_ca_file"
 			rt.TLS.InternalRPC.CAPath = "default_ca_path"
@@ -5510,6 +5516,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
+			rt.TLS.SpecifiedTLSStanza = true
 
 			rt.TLS.InternalRPC.VerifyServerHostname = true
 			rt.TLS.InternalRPC.VerifyOutgoing = true
@@ -5537,6 +5544,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = false
+			rt.TLS.SpecifiedTLSStanza = false // TLS stanza was defined but is empty.
 		},
 	})
 	run(t, testCase{
@@ -5558,6 +5566,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = false
+			rt.TLS.SpecifiedTLSStanza = false // TLS stanza was defined but is empty.
 		},
 	})
 	run(t, testCase{
@@ -5604,6 +5613,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = true
+			rt.TLS.SpecifiedTLSStanza = true
 		},
 	})
 	run(t, testCase{
@@ -5632,6 +5642,37 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.TLS.Domain = "consul."
 			rt.TLS.NodeName = "thehostname"
 			rt.TLS.GRPC.UseAutoCert = false
+			rt.TLS.SpecifiedTLSStanza = true
+		},
+	})
+	run(t, testCase{
+		desc: "SpecifiedTLSStanza is not set if tls stanza was not defined",
+		args: []string{
+			`-data-dir=` + dataDir,
+		},
+		json: []string{`
+			{
+				"cert_file": "foo",
+				"key_file": "bar"
+			}
+		`},
+		hcl: []string{`
+			cert_file = "foo"
+			key_file = "bar"
+		`},
+		expected: func(rt *RuntimeConfig) {
+			rt.DataDir = dataDir
+			rt.TLS.SpecifiedTLSStanza = false
+			rt.TLS.HTTPS.CertFile = "foo"
+			rt.TLS.HTTPS.KeyFile = "bar"
+			rt.TLS.InternalRPC.CertFile = "foo"
+			rt.TLS.InternalRPC.KeyFile = "bar"
+			rt.TLS.GRPC.CertFile = "foo"
+			rt.TLS.GRPC.KeyFile = "bar"
+		},
+		expectedWarnings: []string{
+			"The 'cert_file' field is deprecated. Use the 'tls.defaults.cert_file' field instead.",
+			"The 'key_file' field is deprecated. Use the 'tls.defaults.key_file' field instead.",
 		},
 	})
 }
@@ -5655,9 +5696,10 @@ func (tc testCase) run(format string, dataDir string) func(t *testing.T) {
 
 		for i, data := range tc.source(format) {
 			opts.sources = append(opts.sources, FileSource{
-				Name:   fmt.Sprintf("src-%d.%s", i, format),
-				Format: format,
-				Data:   data,
+				Name:     fmt.Sprintf("src-%d.%s", i, format),
+				Format:   format,
+				Data:     data,
+				FromUser: true,
 			})
 		}
 
@@ -6441,6 +6483,7 @@ func TestLoad_FullConfig(t *testing.T) {
 			ServerName:              "Oerr9n1G",
 			Domain:                  "7W1xXSqd",
 			EnableAgentTLSForChecks: true,
+			SpecifiedTLSStanza:      true,
 		},
 		TaggedAddresses: map[string]string{
 			"7MYgHrYH": "dALJAhLD",