config: prevent top-level `verify_incoming` enabling mTLS on gRPC port (#13118)

Fixes #13088

This is a backwards-compatibility bug introduced in 1.12.
This commit is contained in:
Dan Upton 2022-05-18 16:15:57 +01:00 committed by GitHub
parent 34f5f99423
commit 7492357b43
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 33 additions and 5 deletions

3
.changelog/13118.txt Normal file
View File

@ -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
```

View File

@ -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 // TLS is only enabled on the gRPC listener if there's an HTTPS port configured
// for historic and backwards-compatibility reasons. // 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)") 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)")
} }

View File

@ -870,4 +870,17 @@ type TLS struct {
InternalRPC TLSProtocolConfig `mapstructure:"internal_rpc"` InternalRPC TLSProtocolConfig `mapstructure:"internal_rpc"`
HTTPS TLSProtocolConfig `mapstructure:"https"` HTTPS TLSProtocolConfig `mapstructure:"https"`
GRPC TLSProtocolConfig `mapstructure:"grpc"` 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:"-"`
} }

View File

@ -180,9 +180,11 @@ func applyDeprecatedConfig(d *decodeTarget) (Config, []string) {
func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string { func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string {
var warns []string var warns []string
defaults := &cfg.TLS.Defaults tls := &cfg.TLS
internalRPC := &cfg.TLS.InternalRPC defaults := &tls.Defaults
https := &cfg.TLS.HTTPS internalRPC := &tls.InternalRPC
https := &tls.HTTPS
grpc := &tls.GRPC
if v := dep.CAFile; v != nil { if v := dep.CAFile; v != nil {
if defaults.CAFile == nil { if defaults.CAFile == nil {
@ -239,6 +241,16 @@ func applyDeprecatedTLSConfig(dep DeprecatedConfig, cfg *Config) []string {
if defaults.VerifyIncoming == nil { if defaults.VerifyIncoming == nil {
defaults.VerifyIncoming = v 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")) warns = append(warns, deprecationWarning("verify_incoming", "tls.defaults.verify_incoming"))
} }

View File

@ -98,7 +98,7 @@ tls_prefer_server_cipher_suites = true
require.False(t, rt.TLS.InternalRPC.VerifyIncoming) require.False(t, rt.TLS.InternalRPC.VerifyIncoming)
require.False(t, rt.TLS.HTTPS.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.InternalRPC.VerifyOutgoing)
require.True(t, rt.TLS.HTTPS.VerifyOutgoing) require.True(t, rt.TLS.HTTPS.VerifyOutgoing)
require.True(t, rt.TLS.InternalRPC.VerifyServerHostname) require.True(t, rt.TLS.InternalRPC.VerifyServerHostname)