From b5ed2ba5366a2d2a88c88a13d27c9942d0cbc3db Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Fri, 28 Apr 2017 16:15:55 -0700 Subject: [PATCH] Add separate option for verifying incoming HTTPS traffic (#2974) * Add separate option for verifying incoming HTTPS traffic --- api/api_test.go | 132 +++++++++++++----- command/agent/agent.go | 2 +- command/agent/config.go | 16 +++ command/agent/config_test.go | 11 +- command/agent/http.go | 2 +- testutil/server.go | 54 +++---- .../source/docs/agent/options.html.markdown | 28 ++-- 7 files changed, 171 insertions(+), 74 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index df39f8525..083773959 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -256,53 +256,111 @@ func TestSetupTLSConfig(t *testing.T) { func TestClientTLSOptions(t *testing.T) { t.Parallel() - _, s := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + // Start a server that verifies incoming HTTPS connections + _, srvVerify := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { conf.CAFile = "../test/client_certs/rootca.crt" conf.CertFile = "../test/client_certs/server.crt" conf.KeyFile = "../test/client_certs/server.key" - conf.VerifyIncoming = true + conf.VerifyIncomingHTTPS = true }) - defer s.Stop() + defer srvVerify.Stop() - // Set up a client without certs - clientWithoutCerts, err := NewClient(&Config{ - Address: s.HTTPSAddr, - Scheme: "https", - TLSConfig: TLSConfig{ - Address: "consul.test", - CAFile: "../test/client_certs/rootca.crt", - }, + // Start a server without VerifyIncomingHTTPS + _, srvNoVerify := makeClientWithConfig(t, nil, func(conf *testutil.TestServerConfig) { + conf.CAFile = "../test/client_certs/rootca.crt" + conf.CertFile = "../test/client_certs/server.crt" + conf.KeyFile = "../test/client_certs/server.key" + conf.VerifyIncomingHTTPS = false }) - if err != nil { - t.Fatal(err) - } + defer srvNoVerify.Stop() - // Should fail - _, err = clientWithoutCerts.Agent().Self() - if err == nil || !strings.Contains(err.Error(), "bad certificate") { - t.Fatal(err) - } + // Client without a cert + t.Run("client without cert, validation", func(t *testing.T) { + client, err := NewClient(&Config{ + Address: srvVerify.HTTPSAddr, + Scheme: "https", + TLSConfig: TLSConfig{ + Address: "consul.test", + CAFile: "../test/client_certs/rootca.crt", + }, + }) + if err != nil { + t.Fatal(err) + } - // Set up a client with valid certs - clientWithCerts, err := NewClient(&Config{ - Address: s.HTTPSAddr, - Scheme: "https", - TLSConfig: TLSConfig{ - Address: "consul.test", - CAFile: "../test/client_certs/rootca.crt", - CertFile: "../test/client_certs/client.crt", - KeyFile: "../test/client_certs/client.key", - }, + // Should fail + _, err = client.Agent().Self() + if err == nil || !strings.Contains(err.Error(), "bad certificate") { + t.Fatal(err) + } }) - if err != nil { - t.Fatal(err) - } - // Should succeed - _, err = clientWithCerts.Agent().Self() - if err != nil { - t.Fatal(err) - } + // Client with a valid cert + t.Run("client with cert, validation", func(t *testing.T) { + client, err := NewClient(&Config{ + Address: srvVerify.HTTPSAddr, + Scheme: "https", + TLSConfig: TLSConfig{ + Address: "consul.test", + CAFile: "../test/client_certs/rootca.crt", + CertFile: "../test/client_certs/client.crt", + KeyFile: "../test/client_certs/client.key", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Should succeed + _, err = client.Agent().Self() + if err != nil { + t.Fatal(err) + } + }) + + // Client without a cert + t.Run("client without cert, no validation", func(t *testing.T) { + client, err := NewClient(&Config{ + Address: srvNoVerify.HTTPSAddr, + Scheme: "https", + TLSConfig: TLSConfig{ + Address: "consul.test", + CAFile: "../test/client_certs/rootca.crt", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Should succeed + _, err = client.Agent().Self() + if err != nil { + t.Fatal(err) + } + }) + + // Client with a valid cert + t.Run("client with cert, no validation", func(t *testing.T) { + client, err := NewClient(&Config{ + Address: srvNoVerify.HTTPSAddr, + Scheme: "https", + TLSConfig: TLSConfig{ + Address: "consul.test", + CAFile: "../test/client_certs/rootca.crt", + CertFile: "../test/client_certs/client.crt", + KeyFile: "../test/client_certs/client.key", + }, + }) + if err != nil { + t.Fatal(err) + } + + // Should succeed + _, err = client.Agent().Self() + if err != nil { + t.Fatal(err) + } + }) } func TestSetQueryOptions(t *testing.T) { diff --git a/command/agent/agent.go b/command/agent/agent.go index 8988c7cfc..f4198a3a1 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -449,7 +449,7 @@ func (a *Agent) consulConfig() *consul.Config { a.config.Version, a.config.VersionPrerelease, revision) // Copy the TLS configuration - base.VerifyIncoming = a.config.VerifyIncoming + base.VerifyIncoming = a.config.VerifyIncoming || a.config.VerifyIncomingRPC base.VerifyOutgoing = a.config.VerifyOutgoing base.VerifyServerHostname = a.config.VerifyServerHostname base.CAFile = a.config.CAFile diff --git a/command/agent/config.go b/command/agent/config.go index 19e4f8db8..2edd220ff 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -453,6 +453,16 @@ type Config struct { // must match a provided certificate authority. This can be used to force client auth. VerifyIncoming bool `mapstructure:"verify_incoming"` + // VerifyIncomingRPC is used to verify the authenticity of incoming RPC connections. + // This means that TCP requests are forbidden, only allowing for TLS. TLS connections + // must match a provided certificate authority. This can be used to force client auth. + VerifyIncomingRPC bool `mapstructure:"verify_incoming_rpc"` + + // VerifyIncomingHTTPS is used to verify the authenticity of incoming HTTPS connections. + // This means that TCP requests are forbidden, only allowing for TLS. TLS connections + // must match a provided certificate authority. This can be used to force client auth. + VerifyIncomingHTTPS bool `mapstructure:"verify_incoming_https"` + // VerifyOutgoing is used to verify the authenticity of outgoing connections. // This means that TLS requests are used. TLS connections must match a provided // certificate authority. This is used to verify authenticity of server nodes. @@ -1546,6 +1556,12 @@ func MergeConfig(a, b *Config) *Config { if b.VerifyIncoming { result.VerifyIncoming = true } + if b.VerifyIncomingRPC { + result.VerifyIncomingRPC = true + } + if b.VerifyIncomingHTTPS { + result.VerifyIncomingHTTPS = true + } if b.VerifyOutgoing { result.VerifyOutgoing = true } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 22590f77b..82ac4a42c 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -355,7 +355,8 @@ func TestDecodeConfig(t *testing.T) { } // TLS - input = `{"verify_incoming": true, "verify_outgoing": true, "verify_server_hostname": true, "tls_min_version": "tls12", + input = `{"verify_incoming": true, "verify_incoming_rpc": true, "verify_incoming_https": true, + "verify_outgoing": true, "verify_server_hostname": true, "tls_min_version": "tls12", "tls_cipher_suites": "TLS_RSA_WITH_AES_256_CBC_SHA", "tls_prefer_server_cipher_suites": true}` config, err = DecodeConfig(bytes.NewReader([]byte(input))) if err != nil { @@ -366,6 +367,14 @@ func TestDecodeConfig(t *testing.T) { t.Fatalf("bad: %#v", config) } + if config.VerifyIncomingRPC != true { + t.Fatalf("bad: %#v", config) + } + + if config.VerifyIncomingHTTPS != true { + t.Fatalf("bad: %#v", config) + } + if config.VerifyOutgoing != true { t.Fatalf("bad: %#v", config) } diff --git a/command/agent/http.go b/command/agent/http.go index ac38dbcf6..8e1e63dab 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -56,7 +56,7 @@ func NewHTTPServers(agent *Agent, config *Config, logOutput io.Writer) ([]*HTTPS } tlsConf := &tlsutil.Config{ - VerifyIncoming: config.VerifyIncoming, + VerifyIncoming: config.VerifyIncoming || config.VerifyIncomingHTTPS, VerifyOutgoing: config.VerifyOutgoing, CAFile: config.CAFile, CAPath: config.CAPath, diff --git a/testutil/server.go b/testutil/server.go index 3c8894e02..86c91acfe 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -56,32 +56,34 @@ type TestAddressConfig struct { // TestServerConfig is the main server configuration struct. type TestServerConfig struct { - NodeName string `json:"node_name"` - NodeID string `json:"node_id"` - NodeMeta map[string]string `json:"node_meta,omitempty"` - Performance *TestPerformanceConfig `json:"performance,omitempty"` - Bootstrap bool `json:"bootstrap,omitempty"` - Server bool `json:"server,omitempty"` - DataDir string `json:"data_dir,omitempty"` - Datacenter string `json:"datacenter,omitempty"` - DisableCheckpoint bool `json:"disable_update_check"` - LogLevel string `json:"log_level,omitempty"` - Bind string `json:"bind_addr,omitempty"` - Addresses *TestAddressConfig `json:"addresses,omitempty"` - Ports *TestPortConfig `json:"ports,omitempty"` - RaftProtocol int `json:"raft_protocol,omitempty"` - ACLMasterToken string `json:"acl_master_token,omitempty"` - ACLDatacenter string `json:"acl_datacenter,omitempty"` - ACLDefaultPolicy string `json:"acl_default_policy,omitempty"` - ACLEnforceVersion8 bool `json:"acl_enforce_version_8"` - Encrypt string `json:"encrypt,omitempty"` - CAFile string `json:"ca_file,omitempty"` - CertFile string `json:"cert_file,omitempty"` - KeyFile string `json:"key_file,omitempty"` - VerifyIncoming bool `json:"verify_incoming,omitempty"` - VerifyOutgoing bool `json:"verify_outgoing,omitempty"` - Stdout, Stderr io.Writer `json:"-"` - Args []string `json:"-"` + NodeName string `json:"node_name"` + NodeID string `json:"node_id"` + NodeMeta map[string]string `json:"node_meta,omitempty"` + Performance *TestPerformanceConfig `json:"performance,omitempty"` + Bootstrap bool `json:"bootstrap,omitempty"` + Server bool `json:"server,omitempty"` + DataDir string `json:"data_dir,omitempty"` + Datacenter string `json:"datacenter,omitempty"` + DisableCheckpoint bool `json:"disable_update_check"` + LogLevel string `json:"log_level,omitempty"` + Bind string `json:"bind_addr,omitempty"` + Addresses *TestAddressConfig `json:"addresses,omitempty"` + Ports *TestPortConfig `json:"ports,omitempty"` + RaftProtocol int `json:"raft_protocol,omitempty"` + ACLMasterToken string `json:"acl_master_token,omitempty"` + ACLDatacenter string `json:"acl_datacenter,omitempty"` + ACLDefaultPolicy string `json:"acl_default_policy,omitempty"` + ACLEnforceVersion8 bool `json:"acl_enforce_version_8"` + Encrypt string `json:"encrypt,omitempty"` + CAFile string `json:"ca_file,omitempty"` + CertFile string `json:"cert_file,omitempty"` + KeyFile string `json:"key_file,omitempty"` + VerifyIncoming bool `json:"verify_incoming,omitempty"` + VerifyIncomingRPC bool `json:"verify_incoming_rpc,omitempty"` + VerifyIncomingHTTPS bool `json:"verify_incoming_https,omitempty"` + VerifyOutgoing bool `json:"verify_outgoing,omitempty"` + Stdout, Stderr io.Writer `json:"-"` + Args []string `json:"-"` } // ServerConfigCallback is a function interface which can be diff --git a/website/source/docs/agent/options.html.markdown b/website/source/docs/agent/options.html.markdown index 15ece62ed..47336f740 100644 --- a/website/source/docs/agent/options.html.markdown +++ b/website/source/docs/agent/options.html.markdown @@ -1052,18 +1052,30 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass * `verify_incoming` - If set to true, Consul requires that all incoming connections make use of TLS and that the client provides a certificate signed - by the Certificate Authority from the [`ca_file`](#ca_file). By default, this is false, and - Consul will not enforce the use of TLS or verify a client's authenticity. This - applies to both server RPC and to the HTTPS API. To enable the HTTPS API, you - must define an HTTPS port via the [`ports`](#ports) configuration. By default, HTTPS - is disabled. + by a Certificate Authority from the [`ca_file`](#ca_file) or [`ca_path`](#ca_path). + This applies to both server RPC and to the HTTPS API. By default, this is false, and + Consul will not enforce the use of TLS or verify a client's authenticity. + +* `verify_incoming_rpc` - If + set to true, Consul requires that all incoming RPC + connections make use of TLS and that the client provides a certificate signed + by a Certificate Authority from the [`ca_file`](#ca_file) or [`ca_path`](#ca_path). By default, + this is false, and Consul will not enforce the use of TLS or verify a client's authenticity. + +* `verify_incoming_https` - If + set to true, Consul requires that all incoming HTTPS + connections make use of TLS and that the client provides a certificate signed + by a Certificate Authority from the [`ca_file`](#ca_file) or [`ca_path`](#ca_path). By default, + this is false, and Consul will not enforce the use of TLS or verify a client's authenticity. To + enable the HTTPS API, you must define an HTTPS port via the [`ports`](#ports) configuration. By + default, HTTPS is disabled. * `verify_outgoing` - If set to true, Consul requires that all outgoing connections make use of TLS and that the server provides a certificate that is signed by - the Certificate Authority from the [`ca_file`](#ca_file). By default, this is false, and Consul - will not make use of TLS for outgoing connections. This applies to clients and servers - as both will make outgoing connections. + a Certificate Authority from the [`ca_file`](#ca_file) or [`ca_path`](#ca_path). By default, + this is false, and Consul will not make use of TLS for outgoing connections. This applies to clients + and servers as both will make outgoing connections. * `verify_server_hostname` - If set to true, Consul verifies for all outgoing connections that the TLS certificate presented by the servers