From 73b281a27c748210f64e4e83b23312d23afb4593 Mon Sep 17 00:00:00 2001 From: Kyle McCullough Date: Thu, 3 Nov 2016 15:17:30 -0500 Subject: [PATCH] Add setting to skip ssl certificate verification for HTTP checks (#1984) * http check: add setting to skip ssl certificate verification * update http check documentation * fix typo in documentation * Add TLSSkipVerify to agent api --- api/agent.go | 1 + command/agent/agent.go | 13 +- command/agent/check.go | 24 +++- command/agent/check_test.go | 127 ++++++++++++++++++ command/agent/config.go | 3 + command/agent/config_test.go | 39 ++++++ .../source/docs/agent/checks.html.markdown | 5 +- .../docs/agent/http/agent.html.markdown | 11 +- 8 files changed, 207 insertions(+), 16 deletions(-) diff --git a/api/agent.go b/api/agent.go index 87a6c10016..41d8bc0b1f 100644 --- a/api/agent.go +++ b/api/agent.go @@ -73,6 +73,7 @@ type AgentServiceCheck struct { HTTP string `json:",omitempty"` TCP string `json:",omitempty"` Status string `json:",omitempty"` + TLSSkipVerify string `json:",omitempty"` // In Consul 0.7 and later, checks that are associated with a service // may also contain this optional DeregisterCriticalServiceAfter field, diff --git a/command/agent/agent.go b/command/agent/agent.go index fe83b4c90a..a79454e085 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -982,12 +982,13 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *CheckType, persist } http := &CheckHTTP{ - Notify: &a.state, - CheckID: check.CheckID, - HTTP: chkType.HTTP, - Interval: chkType.Interval, - Timeout: chkType.Timeout, - Logger: a.logger, + Notify: &a.state, + CheckID: check.CheckID, + HTTP: chkType.HTTP, + Interval: chkType.Interval, + Timeout: chkType.Timeout, + Logger: a.logger, + TLSSkipVerify: chkType.TLSSkipVerify, } http.Start() a.checkHTTPs[check.CheckID] = http diff --git a/command/agent/check.go b/command/agent/check.go index 7bf67e9a7e..6564a39fd7 100644 --- a/command/agent/check.go +++ b/command/agent/check.go @@ -1,6 +1,7 @@ package agent import ( + "crypto/tls" "fmt" "io" "log" @@ -47,6 +48,7 @@ type CheckType struct { Interval time.Duration DockerContainerID string Shell string + TLSSkipVerify bool Timeout time.Duration TTL time.Duration @@ -340,12 +342,13 @@ type persistedCheckState struct { // The check is critical if the response code is anything else // or if the request returns an error type CheckHTTP struct { - Notify CheckNotifier - CheckID types.CheckID - HTTP string - Interval time.Duration - Timeout time.Duration - Logger *log.Logger + Notify CheckNotifier + CheckID types.CheckID + HTTP string + Interval time.Duration + Timeout time.Duration + Logger *log.Logger + TLSSkipVerify bool httpClient *http.Client stop bool @@ -365,6 +368,15 @@ func (c *CheckHTTP) Start() { trans := cleanhttp.DefaultTransport() trans.DisableKeepAlives = true + // Skip SSL certificate verification if TLSSkipVerify is true + if trans.TLSClientConfig == nil { + trans.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: c.TLSSkipVerify, + } + } else { + trans.TLSClientConfig.InsecureSkipVerify = c.TLSSkipVerify + } + // Create the HTTP client. c.httpClient = &http.Client{ Timeout: 10 * time.Second, diff --git a/command/agent/check_test.go b/command/agent/check_test.go index ef0a9fbd37..ddd7656c4a 100644 --- a/command/agent/check_test.go +++ b/command/agent/check_test.go @@ -228,6 +228,19 @@ func mockHTTPServer(responseCode int) *httptest.Server { return httptest.NewServer(mux) } +func mockTLSHTTPServer(responseCode int) *httptest.Server { + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + // Body larger than 4k limit + body := bytes.Repeat([]byte{'a'}, 2*CheckBufSize) + w.WriteHeader(responseCode) + w.Write(body) + return + }) + + return httptest.NewTLSServer(mux) +} + func expectHTTPStatus(t *testing.T, url string, status string) { mock := &MockNotify{ state: make(map[types.CheckID]string), @@ -381,6 +394,120 @@ func TestCheckHTTP_disablesKeepAlives(t *testing.T) { } } +func TestCheckHTTP_TLSSkipVerify_defaultFalse(t *testing.T) { + check := &CheckHTTP{ + CheckID: "foo", + HTTP: "https://foo.bar/baz", + Interval: 10 * time.Second, + Logger: log.New(os.Stderr, "", log.LstdFlags), + } + + check.Start() + defer check.Stop() + + if check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { + t.Fatalf("should default to false") + } +} + +func TestCheckHTTP_TLSSkipVerify_true_pass(t *testing.T) { + server := mockTLSHTTPServer(200) + defer server.Close() + + mock := &MockNotify{ + state: make(map[types.CheckID]string), + updates: make(map[types.CheckID]int), + output: make(map[types.CheckID]string), + } + + check := &CheckHTTP{ + Notify: mock, + CheckID: types.CheckID("skipverify_true"), + HTTP: server.URL, + Interval: 5 * time.Millisecond, + Logger: log.New(os.Stderr, "", log.LstdFlags), + TLSSkipVerify: true, + } + + check.Start() + defer check.Stop() + time.Sleep(50 * time.Millisecond) + if !check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { + t.Fatalf("should be true") + } + + if mock.state["skipverify_true"] != structs.HealthPassing { + t.Fatalf("should be passing %v", mock.state) + } +} + +func TestCheckHTTP_TLSSkipVerify_true_fail(t *testing.T) { + server := mockTLSHTTPServer(500) + defer server.Close() + + mock := &MockNotify{ + state: make(map[types.CheckID]string), + updates: make(map[types.CheckID]int), + output: make(map[types.CheckID]string), + } + + check := &CheckHTTP{ + Notify: mock, + CheckID: types.CheckID("skipverify_true"), + HTTP: server.URL, + Interval: 5 * time.Millisecond, + Logger: log.New(os.Stderr, "", log.LstdFlags), + TLSSkipVerify: true, + } + check.Start() + defer check.Stop() + time.Sleep(50 * time.Millisecond) + + if !check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { + t.Fatalf("should be true") + } + + if mock.state["skipverify_true"] != structs.HealthCritical { + t.Fatalf("should be critical %v", mock.state) + } +} + +func TestCheckHTTP_TLSSkipVerify_false(t *testing.T) { + server := mockTLSHTTPServer(200) + defer server.Close() + + mock := &MockNotify{ + state: make(map[types.CheckID]string), + updates: make(map[types.CheckID]int), + output: make(map[types.CheckID]string), + } + + check := &CheckHTTP{ + Notify: mock, + CheckID: types.CheckID("skipverify_false"), + HTTP: server.URL, + Interval: 100 * time.Millisecond, + Logger: log.New(os.Stderr, "", log.LstdFlags), + TLSSkipVerify: false, + } + + check.Start() + defer check.Stop() + time.Sleep(150 * time.Millisecond) + if check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { + t.Fatalf("should be false") + } + + // This should fail due to an invalid SSL cert + if mock.state["skipverify_false"] != structs.HealthCritical { + t.Fatalf("should be critical %v", mock.state) + } + + if !strings.Contains(mock.output["skipverify_false"], "certificate signed by unknown authority") { + t.Fatalf("should fail with certificate error %v", mock.output) + } +} + func mockTCPServer(network string) net.Listener { var ( addr string diff --git a/command/agent/config.go b/command/agent/config.go index d346abe8af..bc0a1fa6fd 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1066,6 +1066,9 @@ func FixupCheckType(raw interface{}) error { case "docker_container_id": rawMap["DockerContainerID"] = v delete(rawMap, k) + case "tls_skip_verify": + rawMap["TLSSkipVerify"] = v + delete(rawMap, k) } } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index c966cc18ec..804b405237 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -1175,6 +1175,23 @@ func TestDecodeConfig_Checks(t *testing.T) { "interval": "10s", "timeout": "100ms", "service_id": "elasticsearch" + }, + { + "id": "chk5", + "name": "service:sslservice", + "HTTP": "https://sslservice/status", + "interval": "10s", + "timeout": "100ms", + "service_id": "sslservice" + }, + { + "id": "chk6", + "name": "service:insecure-sslservice", + "HTTP": "https://insecure-sslservice/status", + "interval": "10s", + "timeout": "100ms", + "service_id": "insecure-sslservice", + "tls_skip_verify": true } ] }` @@ -1221,6 +1238,28 @@ func TestDecodeConfig_Checks(t *testing.T) { Timeout: 100 * time.Millisecond, }, }, + &CheckDefinition{ + ID: "chk5", + Name: "service:sslservice", + ServiceID: "sslservice", + CheckType: CheckType{ + HTTP: "https://sslservice/status", + Interval: 10 * time.Second, + Timeout: 100 * time.Millisecond, + TLSSkipVerify: false, + }, + }, + &CheckDefinition{ + ID: "chk6", + Name: "service:insecure-sslservice", + ServiceID: "insecure-sslservice", + CheckType: CheckType{ + HTTP: "https://insecure-sslservice/status", + Interval: 10 * time.Second, + Timeout: 100 * time.Millisecond, + TLSSkipVerify: true, + }, + }, }, } diff --git a/website/source/docs/agent/checks.html.markdown b/website/source/docs/agent/checks.html.markdown index e6d2bd7a7c..cc64e761da 100644 --- a/website/source/docs/agent/checks.html.markdown +++ b/website/source/docs/agent/checks.html.markdown @@ -34,7 +34,10 @@ There are five different kinds of checks: with a request timeout equal to the check interval, with a max of 10 seconds. It is possible to configure a custom HTTP check timeout value by specifying the `timeout` field in the check definition. The output of the check is - limited to roughly 4K. Responses larger than this will be truncated. + limited to roughly 4K. Responses larger than this will be truncated. HTTP checks + also support SSL. By default, a valid SSL certificate is expected. Certificate + verification can be turned off by setting the `tls_skip_verify` field to `true` + in the check definition. * TCP + Interval - These checks make an TCP connection attempt every Interval (e.g. every 30 seconds) to the specified IP/hostname and port. If no hostname diff --git a/website/source/docs/agent/http/agent.html.markdown b/website/source/docs/agent/http/agent.html.markdown index 9c6dbc3966..539f973df1 100644 --- a/website/source/docs/agent/http/agent.html.markdown +++ b/website/source/docs/agent/http/agent.html.markdown @@ -250,7 +250,8 @@ body must look like: "HTTP": "http://example.com", "TCP": "example.com:22", "Interval": "10s", - "TTL": "15s" + "TTL": "15s", + "TLSSkipVerify": true } ``` @@ -282,9 +283,13 @@ evaluate the script every `Interval` in the given container using the specified An `HTTP` check will perform an HTTP GET request against the value of `HTTP` (expected to be a URL) every `Interval`. If the response is any `2xx` code, the check is `passing`. If the response is `429 Too Many Requests`, the check is `warning`. Otherwise, the check -is `critical`. +is `critical`. HTTP checks also support SSL. By default, a valid SSL certificate is expected. +Certificate verification can be controlled using the `TLSSkipVerify`. -An `TCP` check will perform an TCP connection attempt against the value of `TCP` +If `TLSSkipVerify` is set to `true`, certificate verification will be disabled. By default, +certificate verification is enabled. + +A `TCP` check will perform an TCP connection attempt against the value of `TCP` (expected to be an IP/hostname and port combination) every `Interval`. If the connection attempt is successful, the check is `passing`. If the connection attempt is unsuccessful, the check is `critical`. In the case of a hostname