From 4a5b352c18e6e8c7dca10f446adad89ecc4feb5b Mon Sep 17 00:00:00 2001 From: Matt Keeler Date: Fri, 19 Jun 2020 16:38:14 -0400 Subject: [PATCH] Require enabling TLS to enable Auto Config (#8159) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the servers they must have a certificate. On the clients they just have to set verify_outgoing to true to attempt TLS connections for RPCs. Eventually we may relax these restrictions but right now all of the settings we push down (acl tokens, acl related settings, certificates, gossip key) are sensitive and shouldn’t be transmitted over an unencrypted connection. Our guides and docs should recoommend verify_server_hostname on the clients as well. Another reason to do this is weird things happen when making an insecure RPC when TLS is not enabled. Basically it tries TLS anyways. We should probably fix that to make it clearer what is going on. --- agent/auto-config/auto_config_test.go | 9 +-- agent/config/builder.go | 12 ++++ agent/config/runtime_test.go | 83 ++++++++++++++++++++++++--- 3 files changed, 91 insertions(+), 13 deletions(-) diff --git a/agent/auto-config/auto_config_test.go b/agent/auto-config/auto_config_test.go index 47e1abaf52..fa15169432 100644 --- a/agent/auto-config/auto_config_test.go +++ b/agent/auto-config/auto_config_test.go @@ -193,7 +193,8 @@ func TestInitialConfiguration_cancelled(t *testing.T) { cfgFile := filepath.Join(configDir, "test.json") require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{ "primary_datacenter": "primary", - "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]} + "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]}, + "verify_outgoing": true }`), 0600)) builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile) @@ -227,7 +228,7 @@ func TestInitialConfiguration_restored(t *testing.T) { cfgFile := filepath.Join(configDir, "test.json") require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{ - "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]} + "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]}, "verify_outgoing": true }`), 0600)) builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile) @@ -261,7 +262,7 @@ func TestInitialConfiguration_success(t *testing.T) { cfgFile := filepath.Join(configDir, "test.json") require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{ - "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]} + "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["127.0.0.1:8300"]}, "verify_outgoing": true }`), 0600)) builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile) @@ -313,7 +314,7 @@ func TestInitialConfiguration_retries(t *testing.T) { cfgFile := filepath.Join(configDir, "test.json") require.NoError(t, ioutil.WriteFile(cfgFile, []byte(`{ - "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["198.18.0.1", "198.18.0.2:8398", "198.18.0.3:8399", "127.0.0.1:1234"]} + "auto_config": {"enabled": true, "intro_token": "blarg", "server_addresses": ["198.18.0.1", "198.18.0.2:8398", "198.18.0.3:8399", "127.0.0.1:1234"]}, "verify_outgoing": true }`), 0600)) builderOpts.ConfigFiles = append(builderOpts.ConfigFiles, cfgFile) diff --git a/agent/config/builder.go b/agent/config/builder.go index 8a6471fcc7..54b7659605 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -1997,6 +1997,12 @@ func (b *Builder) validateAutoConfig(rt RuntimeConfig) error { return nil } + // Right now we require TLS as everything we are going to transmit via auto-config is sensitive. Signed Certificates, Tokens + // and other encryption keys. This must be transmitted over a secure connection so we don't allow doing otherwise. + if !rt.VerifyOutgoing { + return fmt.Errorf("auto_config.enabled cannot be set without configuring TLS for server communications") + } + // Auto Config doesn't currently support configuring servers if rt.ServerMode { return fmt.Errorf("auto_config.enabled cannot be set to true for server agents.") @@ -2030,6 +2036,12 @@ func (b *Builder) validateAutoConfigAuthorizer(rt RuntimeConfig) error { return fmt.Errorf("auto_config.authorization.enabled cannot be set to true for client agents") } + // Right now we require TLS as everything we are going to transmit via auto-config is sensitive. Signed Certificates, Tokens + // and other encryption keys. This must be transmitted over a secure connection so we don't allow doing otherwise. + if rt.CertFile == "" { + return fmt.Errorf("auto_config.authorization.enabled cannot be set without providing a TLS certificate for the server") + } + // build out the validator to ensure that the given configuration was valid null := hclog.NewNullLogger() validator, err := ssoauth.NewValidator(null, &authz.AuthMethod) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index cd323799f1..b49ee9b676 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -3814,6 +3814,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { intro_token = "blah" server_addresses = ["198.18.0.1"] } + verify_outgoing = true `}, json: []string{` { @@ -3822,11 +3823,60 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { "enabled": true, "intro_token": "blah", "server_addresses": ["198.18.0.1"] - } + }, + "verify_outgoing": true }`}, err: "auto_config.enabled cannot be set to true for server agents", }, + { + desc: "auto config tls not enabled", + args: []string{ + `-data-dir=` + dataDir, + }, + hcl: []string{` + auto_config { + enabled = true + server_addresses = ["198.18.0.1"] + intro_token = "foo" + } + `}, + json: []string{` + { + "auto_config": { + "enabled": true, + "server_addresses": ["198.18.0.1"], + "intro_token": "foo" + } + }`}, + err: "auto_config.enabled cannot be set without configuring TLS for server communications", + }, + + { + desc: "auto config server tls not enabled", + args: []string{ + `-data-dir=` + dataDir, + }, + hcl: []string{` + server = true + auto_config { + authorization { + enabled = true + } + } + `}, + json: []string{` + { + "server": true, + "auto_config": { + "authorization": { + "enabled": true + } + } + }`}, + err: "auto_config.authorization.enabled cannot be set without providing a TLS certificate for the server", + }, + { desc: "auto config no intro token", args: []string{ @@ -3836,15 +3886,16 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { auto_config { enabled = true server_addresses = ["198.18.0.1"] - } + verify_outgoing = true `}, json: []string{` { "auto_config": { "enabled": true, "server_addresses": ["198.18.0.1"] - } + }, + "verify_outgoing": true }`}, err: "One of auto_config.intro_token, auto_config.intro_token_file or the CONSUL_INTRO_TOKEN environment variable must be set to enable auto_config", }, @@ -3859,13 +3910,15 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { enabled = true intro_token = "blah" } + verify_outgoing = true `}, json: []string{` { "auto_config": { "enabled": true, "intro_token": "blah" - } + }, + "verify_outgoing": true }`}, err: "auto_config.enabled is set without providing a list of addresses", }, @@ -3884,6 +3937,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { dns_sans = ["foo"] ip_sans = ["invalid", "127.0.0.1"] } + verify_outgoing = true `}, json: []string{` { @@ -3894,7 +3948,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { "server_addresses": ["198.18.0.1"], "dns_sans": ["foo"], "ip_sans": ["invalid", "127.0.0.1"] - } + }, + "verify_outgoing": true }`}, warns: []string{ "Cannot parse ip \"invalid\" from auto_config.ip_sans", @@ -3908,6 +3963,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.AutoConfig.DNSSANs = []string{"foo"} rt.AutoConfig.IPSANs = []net.IP{net.IPv4(127, 0, 0, 1)} rt.DataDir = dataDir + rt.VerifyOutgoing = true }, }, @@ -3946,6 +4002,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { enabled = true } } + cert_file = "foo" `}, json: []string{` { @@ -3953,7 +4010,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { "authorization": { "enabled": true } - } + }, + "cert_file": "foo" }`}, err: `auto_config.authorization.static has invalid configuration: exactly one of 'JWTValidationPubKeys', 'JWKSURL', or 'OIDCDiscoveryURL' must be set for type "jwt"`, }, @@ -3974,6 +4032,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { } } } + cert_file = "foo" `}, json: []string{` { @@ -3985,7 +4044,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { "oidc_discovery_url": "https://fake.uri.local" } } - } + }, + "cert_file": "foo" }`}, err: `auto_config.authorization.static has invalid configuration: exactly one of 'JWTValidationPubKeys', 'JWKSURL', or 'OIDCDiscoveryURL' must be set for type "jwt"`, }, @@ -4008,6 +4068,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { } } } + cert_file = "foo" `}, json: []string{` { @@ -4021,7 +4082,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { ] } } - } + }, + "cert_file": "foo" }`}, err: `auto_config.authorization.static.claim_assertion "values.node == ${node}" is invalid: Selector "values" is not valid`, }, @@ -4046,6 +4108,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { } } } + cert_file = "foo" `}, json: []string{` { @@ -4062,7 +4125,8 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { } } } - } + }, + "cert_file": "foo" }`}, patch: func(rt *RuntimeConfig) { rt.AutoConfig.Authorizer.Enabled = true @@ -4075,6 +4139,7 @@ func TestConfigFlagsAndEdgecases(t *testing.T) { rt.LeaveOnTerm = false rt.ServerMode = true rt.SkipLeaveOnInt = true + rt.CertFile = "foo" }, }, }