From 56b122c50179c7e928f4ff22b801421da1ef8dcc Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Tue, 25 Apr 2017 23:29:43 +0100 Subject: [PATCH 1/8] Add verification options to TLS config struct --- nomad/structs/config/tls.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go index aea3cc4df..48a7ef021 100644 --- a/nomad/structs/config/tls.go +++ b/nomad/structs/config/tls.go @@ -28,6 +28,12 @@ type TLSConfig struct { // KeyFile is used to provide a TLS key that is used for serving TLS connections. // Must be provided to serve TLS connections. KeyFile string `mapstructure:"key_file"` + + // VerifyIncoming + VerifyIncoming bool `mapstructure:"verify_incoming"` + + // VerifyOutgoing + VerifyOutgoing bool `mapstructure:"verify_outgoing"` } // Merge is used to merge two TLS configs together @@ -52,6 +58,12 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig { if b.KeyFile != "" { result.KeyFile = b.KeyFile } + if b.VerifyIncoming { + result.VerifyIncoming = true + } + if b.VerifyOutgoing { + result.VerifyOutgoing = true + } return &result } From 3070d5ab9d3243526451328b90420262e41bea21 Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Tue, 25 Apr 2017 23:33:12 +0100 Subject: [PATCH 2/8] Copy TLSConfig verification flags in server create --- command/agent/http.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index 8dbfca78e..70fef37db 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -65,8 +65,8 @@ func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) { // If TLS is enabled, wrap the listener with a TLS listener if config.TLSConfig.EnableHTTP { tlsConf := &tlsutil.Config{ - VerifyIncoming: false, - VerifyOutgoing: true, + VerifyIncoming: config.TLSConfig.VerifyIncoming, + VerifyOutgoing: config.TLSConfig.VerifyOutgoing, VerifyServerHostname: config.TLSConfig.VerifyServerHostname, CAFile: config.TLSConfig.CAFile, CertFile: config.TLSConfig.CertFile, From 1e6694c5c18f64de5d3c02a0a6bc41231b8e6073 Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Tue, 25 Apr 2017 23:35:47 +0100 Subject: [PATCH 3/8] Verification options allowed in TLS config --- command/agent/config_parse.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index 43e9e5b22..da14f6fd1 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -689,6 +689,8 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error { "ca_file", "cert_file", "key_file", + "verify_incoming", + "verify_outgoing", } if err := checkHCLKeys(listVal, valid); err != nil { From c948d2ee27f453b0bc5759c00517280451e09814 Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Wed, 26 Apr 2017 18:58:19 +0100 Subject: [PATCH 4/8] apply gofmt --- nomad/structs/config/tls.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go index 48a7ef021..694400e61 100644 --- a/nomad/structs/config/tls.go +++ b/nomad/structs/config/tls.go @@ -28,10 +28,10 @@ type TLSConfig struct { // KeyFile is used to provide a TLS key that is used for serving TLS connections. // Must be provided to serve TLS connections. KeyFile string `mapstructure:"key_file"` - + // VerifyIncoming VerifyIncoming bool `mapstructure:"verify_incoming"` - + // VerifyOutgoing VerifyOutgoing bool `mapstructure:"verify_outgoing"` } From 829d9e60b90fbbdfbd44ee7e2d8f4a49da8b8bae Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Wed, 26 Apr 2017 21:13:54 +0100 Subject: [PATCH 5/8] fix config parse test --- command/agent/config-test-fixtures/basic.hcl | 2 ++ command/agent/config_parse_test.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/command/agent/config-test-fixtures/basic.hcl b/command/agent/config-test-fixtures/basic.hcl index cd741295d..65c0b874e 100644 --- a/command/agent/config-test-fixtures/basic.hcl +++ b/command/agent/config-test-fixtures/basic.hcl @@ -138,4 +138,6 @@ tls { ca_file = "foo" cert_file = "bar" key_file = "pipe" + verify_incoming = true + verify_outgoing = true } diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index 19fccdf64..cec5cbef2 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -154,6 +154,8 @@ func TestConfig_Parse(t *testing.T) { CAFile: "foo", CertFile: "bar", KeyFile: "pipe", + VerifyIncoming: true, + VerifyOutgoing: true, }, HTTPAPIResponseHeaders: map[string]string{ "Access-Control-Allow-Origin": "*", From 1b8a1614ca81de89100b259116570236c7d64309 Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Fri, 28 Apr 2017 10:45:09 +0100 Subject: [PATCH 6/8] reduce to one configuration option There should be just one option, verify_https_client, which controls incoming and outgoing validation for the HTTPS wrapper --- command/agent/config-test-fixtures/basic.hcl | 3 +-- command/agent/config_parse.go | 3 +-- command/agent/http.go | 4 ++-- nomad/structs/config/tls.go | 15 ++++----------- 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/command/agent/config-test-fixtures/basic.hcl b/command/agent/config-test-fixtures/basic.hcl index 65c0b874e..8d4880a7d 100644 --- a/command/agent/config-test-fixtures/basic.hcl +++ b/command/agent/config-test-fixtures/basic.hcl @@ -138,6 +138,5 @@ tls { ca_file = "foo" cert_file = "bar" key_file = "pipe" - verify_incoming = true - verify_outgoing = true + verify_https_client = true } diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go index da14f6fd1..403f5b75b 100644 --- a/command/agent/config_parse.go +++ b/command/agent/config_parse.go @@ -689,8 +689,7 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error { "ca_file", "cert_file", "key_file", - "verify_incoming", - "verify_outgoing", + "verify_https_client", } if err := checkHCLKeys(listVal, valid); err != nil { diff --git a/command/agent/http.go b/command/agent/http.go index 70fef37db..787f02e66 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -65,8 +65,8 @@ func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) { // If TLS is enabled, wrap the listener with a TLS listener if config.TLSConfig.EnableHTTP { tlsConf := &tlsutil.Config{ - VerifyIncoming: config.TLSConfig.VerifyIncoming, - VerifyOutgoing: config.TLSConfig.VerifyOutgoing, + VerifyIncoming: config.TLSConfig.VerifyHTTPSClient, + VerifyOutgoing: config.TLSConfig.VerifyHTTPSClient, VerifyServerHostname: config.TLSConfig.VerifyServerHostname, CAFile: config.TLSConfig.CAFile, CertFile: config.TLSConfig.CertFile, diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go index 694400e61..2baa76a07 100644 --- a/nomad/structs/config/tls.go +++ b/nomad/structs/config/tls.go @@ -29,11 +29,8 @@ type TLSConfig struct { // Must be provided to serve TLS connections. KeyFile string `mapstructure:"key_file"` - // VerifyIncoming - VerifyIncoming bool `mapstructure:"verify_incoming"` - - // VerifyOutgoing - VerifyOutgoing bool `mapstructure:"verify_outgoing"` + // Verify connections to the HTTPS API + VerifyHTTPSClient bool `mapstructure:"verify_https_client"` } // Merge is used to merge two TLS configs together @@ -58,12 +55,8 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig { if b.KeyFile != "" { result.KeyFile = b.KeyFile } - if b.VerifyIncoming { - result.VerifyIncoming = true + if b.VerifyHTTPSClient { + result.VerifyHTTPSClient = true } - if b.VerifyOutgoing { - result.VerifyOutgoing = true - } - return &result } From 642fbd2f5624b1c39cdd334f9aeb160bc4002643 Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Sat, 29 Apr 2017 08:26:12 +0100 Subject: [PATCH 7/8] address feedback --- command/agent/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/agent/http.go b/command/agent/http.go index 787f02e66..bdae5ee33 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -66,7 +66,7 @@ func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) { if config.TLSConfig.EnableHTTP { tlsConf := &tlsutil.Config{ VerifyIncoming: config.TLSConfig.VerifyHTTPSClient, - VerifyOutgoing: config.TLSConfig.VerifyHTTPSClient, + VerifyOutgoing: true, VerifyServerHostname: config.TLSConfig.VerifyServerHostname, CAFile: config.TLSConfig.CAFile, CertFile: config.TLSConfig.CertFile, From f64f37317e18eacf45e483b92d2e06b790ecb4e5 Mon Sep 17 00:00:00 2001 From: Pete Wildsmith Date: Sun, 30 Apr 2017 15:40:04 +0100 Subject: [PATCH 8/8] update test --- command/agent/config_parse_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go index cec5cbef2..9774b0db3 100644 --- a/command/agent/config_parse_test.go +++ b/command/agent/config_parse_test.go @@ -154,8 +154,7 @@ func TestConfig_Parse(t *testing.T) { CAFile: "foo", CertFile: "bar", KeyFile: "pipe", - VerifyIncoming: true, - VerifyOutgoing: true, + VerifyHTTPSClient: true, }, HTTPAPIResponseHeaders: map[string]string{ "Access-Control-Allow-Origin": "*",