Skip to content

Commit

Permalink
Add verify server hostname to tls default (#17155)
Browse files Browse the repository at this point in the history
  • Loading branch information
fulviodenza authored Jul 10, 2023
1 parent b0a2e33 commit f4b0804
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 23 deletions.
3 changes: 3 additions & 0 deletions .changelog/17155.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
config: Add new `tls.defaults.verify_server_hostname` configuration option. This specifies the default value for any interfaces that support the `verify_server_hostname` option.
```
11 changes: 7 additions & 4 deletions agent/config/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -2653,10 +2653,10 @@ func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error
return c, errors.New("verify_outgoing is not valid in the tls.grpc stanza")
}

// Similarly, only the internal RPC configuration honors VerifyServerHostname
// Similarly, only the internal RPC and defaults configuration honor VerifyServerHostname
// so we call it out here too.
if t.Defaults.VerifyServerHostname != nil || t.GRPC.VerifyServerHostname != nil || t.HTTPS.VerifyServerHostname != nil {
return c, errors.New("verify_server_hostname is only valid in the tls.internal_rpc stanza")
if t.GRPC.VerifyServerHostname != nil || t.HTTPS.VerifyServerHostname != nil {
return c, errors.New("verify_server_hostname is only valid in the tls.defaults and tls.internal_rpc stanzas")
}

// And UseAutoCert right now only applies to external gRPC interface.
Expand Down Expand Up @@ -2706,8 +2706,11 @@ func (b *builder) buildTLSConfig(rt RuntimeConfig, t TLS) (tlsutil.Config, error
}

mapCommon("internal_rpc", t.InternalRPC, &c.InternalRPC)
c.InternalRPC.VerifyServerHostname = boolVal(t.InternalRPC.VerifyServerHostname)

c.InternalRPC.VerifyServerHostname = boolVal(t.Defaults.VerifyServerHostname)
if t.InternalRPC.VerifyServerHostname != nil {
c.InternalRPC.VerifyServerHostname = boolVal(t.InternalRPC.VerifyServerHostname)
}
// Setting only verify_server_hostname is documented to imply verify_outgoing.
// If it doesn't then we risk sending communication over plain TCP when we
// documented it as forcing TLS for RPCs. Enforce this here rather than in
Expand Down
111 changes: 108 additions & 3 deletions agent/config/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2736,7 +2736,44 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
}
}
`},
expectedErr: "verify_server_hostname is only valid in the tls.internal_rpc stanza",
expected: func(rt *RuntimeConfig) {
rt.DataDir = dataDir
rt.TLS.InternalRPC.VerifyServerHostname = true
rt.TLS.InternalRPC.VerifyOutgoing = true
},
})
run(t, testCase{
desc: "verify_server_hostname in the defaults stanza and internal_rpc",
args: []string{
`-data-dir=` + dataDir,
},
hcl: []string{`
tls {
defaults {
verify_server_hostname = false
},
internal_rpc {
verify_server_hostname = true
}
}
`},
json: []string{`
{
"tls": {
"defaults": {
"verify_server_hostname": false
},
"internal_rpc": {
"verify_server_hostname": true
}
}
}
`},
expected: func(rt *RuntimeConfig) {
rt.DataDir = dataDir
rt.TLS.InternalRPC.VerifyServerHostname = true
rt.TLS.InternalRPC.VerifyOutgoing = true
},
})
run(t, testCase{
desc: "verify_server_hostname in the grpc stanza",
Expand All @@ -2759,7 +2796,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
}
}
`},
expectedErr: "verify_server_hostname is only valid in the tls.internal_rpc stanza",
expectedErr: "verify_server_hostname is only valid in the tls.defaults and tls.internal_rpc stanza",
})
run(t, testCase{
desc: "verify_server_hostname in the https stanza",
Expand All @@ -2782,7 +2819,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
}
}
`},
expectedErr: "verify_server_hostname is only valid in the tls.internal_rpc stanza",
expectedErr: "verify_server_hostname is only valid in the tls.defaults and tls.internal_rpc stanza",
})
run(t, testCase{
desc: "translated keys",
Expand Down Expand Up @@ -5723,6 +5760,74 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
rt.TLS.InternalRPC.VerifyOutgoing = true
},
})
run(t, testCase{
desc: "tls.defaults.verify_server_hostname implies tls.internal_rpc.verify_outgoing",
args: []string{
`-data-dir=` + dataDir,
},
json: []string{`
{
"tls": {
"defaults": {
"verify_server_hostname": true
}
}
}
`},
hcl: []string{`
tls {
defaults {
verify_server_hostname = true
}
}
`},
expected: func(rt *RuntimeConfig) {
rt.DataDir = dataDir

rt.TLS.Domain = "consul."
rt.TLS.NodeName = "thehostname"

rt.TLS.InternalRPC.VerifyServerHostname = true
rt.TLS.InternalRPC.VerifyOutgoing = true
},
})
run(t, testCase{
desc: "tls.internal_rpc.verify_server_hostname overwrites tls.defaults.verify_server_hostname",
args: []string{
`-data-dir=` + dataDir,
},
json: []string{`
{
"tls": {
"defaults": {
"verify_server_hostname": false
},
"internal_rpc": {
"verify_server_hostname": true
}
}
}
`},
hcl: []string{`
tls {
defaults {
verify_server_hostname = false
},
internal_rpc {
verify_server_hostname = true
}
}
`},
expected: func(rt *RuntimeConfig) {
rt.DataDir = dataDir

rt.TLS.Domain = "consul."
rt.TLS.NodeName = "thehostname"

rt.TLS.InternalRPC.VerifyServerHostname = true
rt.TLS.InternalRPC.VerifyOutgoing = true
},
})
run(t, testCase{
desc: "tls.grpc.use_auto_cert defaults to false",
args: []string{
Expand Down
15 changes: 8 additions & 7 deletions website/content/docs/agent/config/config-files.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -2094,6 +2094,12 @@ specially crafted certificate signed by the CA can be used to gain full access t
* `TLSv1_2` (default)
* `TLSv1_3`

- `verify_server_hostname` ((#tls_internal_rpc_verify_server_hostname)) When
set to true, Consul verifies the TLS certificate presented by the servers
match the hostname `server.<datacenter>.<domain>`. By default this is false,
and Consul does not verify the hostname of the certificate, only that it
is signed by a trusted CA.

**WARNING: TLS 1.1 and lower are generally considered less secure and
should not be used if possible.**

Expand Down Expand Up @@ -2201,7 +2207,7 @@ specially crafted certificate signed by the CA can be used to gain full access t
only way to enforce that no client can communicate with a server unencrypted
is to also enable `verify_incoming` which requires client certificates too.

- `verify_server_hostname` ((#tls_internal_rpc_verify_server_hostname)) When
- `verify_server_hostname` Overrides [tls.defaults.verify_server_hostname](#tls_defaults_verify_server_hostname). When
set to true, Consul verifies the TLS certificate presented by the servers
match the hostname `server.<datacenter>.<domain>`. By default this is false,
and Consul does not verify the hostname of the certificate, only that it
Expand Down Expand Up @@ -2285,9 +2291,6 @@ tls {
ca_file = "/etc/pki/tls/certs/ca-bundle.crt"
verify_incoming = true
verify_outgoing = true
}

internal_rpc {
verify_server_hostname = true
}
}
Expand Down Expand Up @@ -2316,9 +2319,7 @@ tls {
"cert_file": "/etc/pki/tls/certs/my.crt",
"ca_file": "/etc/pki/tls/certs/ca-bundle.crt",
"verify_incoming": true,
"verify_outgoing": true
},
"internal_rpc": {
"verify_outgoing": true,
"verify_server_hostname": true
}
}
Expand Down
3 changes: 0 additions & 3 deletions website/content/docs/agent/index.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,6 @@ tls {
ca_file = "/consul/config/certs/consul-agent-ca.pem"
cert_file = "/consul/config/certs/dc1-server-consul-0.pem"
key_file = "/consul/config/certs/dc1-server-consul-0-key.pem"
}
internal_rpc {
verify_server_hostname = true
}
}
Expand Down
6 changes: 0 additions & 6 deletions website/content/docs/security/security-models/core.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,6 @@ environment and adapt these configurations accordingly.
ca_file = "consul-agent-ca.pem"
cert_file = "dc1-server-consul-0.pem"
key_file = "dc1-server-consul-0-key.pem"
}
internal_rpc {
verify_server_hostname = true
}
}
Expand All @@ -148,9 +145,6 @@ environment and adapt these configurations accordingly.
verify_incoming = false
verify_outgoing = true
ca_file = "consul-agent-ca.pem"
}
internal_rpc {
verify_server_hostname = true
}
}
Expand Down

0 comments on commit f4b0804

Please sign in to comment.