diff --git a/.changelog/13118.txt b/.changelog/13118.txt new file mode 100644 index 000000000..c2119f9fb --- /dev/null +++ b/.changelog/13118.txt @@ -0,0 +1,3 @@ +```release-note:bug +config: fix backwards compatibility bug where setting the (deprecated) top-level `verify_incoming` option would enable TLS client authentication on the gRPC port +``` diff --git a/agent/config/builder.go b/agent/config/builder.go index 76b41b113..9a814bcb5 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -2522,7 +2522,7 @@ func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error // TLS is only enabled on the gRPC listener if there's an HTTPS port configured // for historic and backwards-compatibility reasons. - if rt.HTTPSPort <= 0 && (t.GRPC != TLSProtocolConfig{}) { + if rt.HTTPSPort <= 0 && (t.GRPC != TLSProtocolConfig{} && t.GRPCModifiedByDeprecatedConfig == nil) { b.warn("tls.grpc was provided but TLS will NOT be enabled on the gRPC listener without an HTTPS listener configured (e.g. via ports.https)") } diff --git a/agent/config/config.go b/agent/config/config.go index affcef601..d78e7098d 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -870,4 +870,17 @@ type TLS struct { InternalRPC TLSProtocolConfig `mapstructure:"internal_rpc"` HTTPS TLSProtocolConfig `mapstructure:"https"` GRPC TLSProtocolConfig `mapstructure:"grpc"` + + // 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 + // ineffectual grpc stanza when we modify GRPC to honor the legacy behaviour + // that setting `verify_incoming = true` at the top-level *does not* enable + // client certificate verification on the gRPC port. + // + // See: applyDeprecatedTLSConfig. + // + // Note: we use a *struct{} here because a simple bool isn't supported by our + // config merging logic. + GRPCModifiedByDeprecatedConfig *struct{} `mapstructure:"-"` } diff --git a/agent/config/deprecated.go b/agent/config/deprecated.go index b27150b51..ab4365708 100644 --- a/agent/config/deprecated.go +++ b/agent/config/deprecated.go @@ -180,9 +180,11 @@ func applyDeprecatedConfig(d *decodeTarget) (Config, []string) { func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string { var warns []string - defaults := &cfg.TLS.Defaults - internalRPC := &cfg.TLS.InternalRPC - https := &cfg.TLS.HTTPS + tls := &cfg.TLS + defaults := &tls.Defaults + internalRPC := &tls.InternalRPC + https := &tls.HTTPS + grpc := &tls.GRPC if v := dep.CAFile; v != nil { if defaults.CAFile == nil { @@ -239,6 +241,16 @@ func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string { if defaults.VerifyIncoming == nil { defaults.VerifyIncoming = v } + + // Prior to Consul 1.12 it was not possible to enable client certificate + // verification on the gRPC port. We must override GRPC.VerifyIncoming to + // prevent it from inheriting Defaults.VerifyIncoming when we've mapped the + // deprecated top-level verify_incoming field. + if grpc.VerifyIncoming == nil { + grpc.VerifyIncoming = pBool(false) + tls.GRPCModifiedByDeprecatedConfig = &struct{}{} + } + warns = append(warns, deprecationWarning("verify_incoming", "tls.defaults.verify_incoming")) } diff --git a/agent/config/deprecated_test.go b/agent/config/deprecated_test.go index 36dd86907..d3febea2b 100644 --- a/agent/config/deprecated_test.go +++ b/agent/config/deprecated_test.go @@ -98,7 +98,7 @@ tls_prefer_server_cipher_suites = true require.False(t, rt.TLS.InternalRPC.VerifyIncoming) require.False(t, rt.TLS.HTTPS.VerifyIncoming) - require.True(t, rt.TLS.GRPC.VerifyIncoming) + require.False(t, rt.TLS.GRPC.VerifyIncoming) require.True(t, rt.TLS.InternalRPC.VerifyOutgoing) require.True(t, rt.TLS.HTTPS.VerifyOutgoing) require.True(t, rt.TLS.InternalRPC.VerifyServerHostname)