diff --git a/agent/agent.go b/agent/agent.go index 8c8d71b642..2ebdd1b761 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1716,16 +1716,33 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *structs.CheckType, chkType.Interval = checks.MinInterval } + // We re-use the API client's TLS structure since it + // closely aligns with Consul's internal configuration. + tlsConfig := &api.TLSConfig{ + InsecureSkipVerify: chkType.TLSSkipVerify, + } + if a.config.EnableAgentTLSForChecks { + tlsConfig.Address = a.config.ServerName + tlsConfig.KeyFile = a.config.KeyFile + tlsConfig.CertFile = a.config.CertFile + tlsConfig.CAFile = a.config.CAFile + tlsConfig.CAPath = a.config.CAPath + } + tlsClientConfig, err := api.SetupTLSConfig(tlsConfig) + if err != nil { + return fmt.Errorf("Failed to set up TLS: %v", err) + } + http := &checks.CheckHTTP{ - Notify: a.State, - CheckID: check.CheckID, - HTTP: chkType.HTTP, - Header: chkType.Header, - Method: chkType.Method, - Interval: chkType.Interval, - Timeout: chkType.Timeout, - Logger: a.logger, - TLSSkipVerify: chkType.TLSSkipVerify, + Notify: a.State, + CheckID: check.CheckID, + HTTP: chkType.HTTP, + Header: chkType.Header, + Method: chkType.Method, + Interval: chkType.Interval, + Timeout: chkType.Timeout, + Logger: a.logger, + TLSClientConfig: tlsClientConfig, } http.Start() a.checkHTTPs[check.CheckID] = http diff --git a/agent/agent_test.go b/agent/agent_test.go index 84eec1057b..3807227d6e 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -6,6 +6,8 @@ import ( "fmt" "io/ioutil" "net" + "net/http" + "net/http/httptest" "os" "path/filepath" "reflect" @@ -17,6 +19,7 @@ import ( "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" "github.com/hashicorp/consul/testutil" + "github.com/hashicorp/consul/testutil/retry" "github.com/hashicorp/consul/types" uuid "github.com/hashicorp/go-uuid" "github.com/pascaldekloe/goe/verify" @@ -793,6 +796,112 @@ func TestAgent_RemoveCheck(t *testing.T) { } } +func TestAgent_HTTPCheck_TLSSkipVerify(t *testing.T) { + t.Parallel() + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "GOOD") + }) + server := httptest.NewTLSServer(handler) + defer server.Close() + + a := NewTestAgent(t.Name(), "") + defer a.Shutdown() + + health := &structs.HealthCheck{ + Node: "foo", + CheckID: "tls", + Name: "tls check", + Status: api.HealthCritical, + } + chk := &structs.CheckType{ + HTTP: server.URL, + Interval: 20 * time.Millisecond, + TLSSkipVerify: true, + } + + err := a.AddCheck(health, chk, false, "") + if err != nil { + t.Fatalf("err: %v", err) + } + + retry.Run(t, func(r *retry.R) { + status := a.State.Checks()["tls"] + if status.Status != api.HealthPassing { + r.Fatalf("bad: %v", status.Status) + } + if !strings.Contains(status.Output, "GOOD") { + r.Fatalf("bad: %v", status.Output) + } + }) + +} + +func TestAgent_HTTPCheck_EnableAgentTLSForChecks(t *testing.T) { + t.Parallel() + + run := func(t *testing.T, ca string) { + a := &TestAgent{ + Name: t.Name(), + UseTLS: true, + HCL: ` + enable_agent_tls_for_checks = true + + verify_incoming = true + server_name = "consul.test" + key_file = "../test/client_certs/server.key" + cert_file = "../test/client_certs/server.crt" + ` + ca, + } + a.Start() + defer a.Shutdown() + + health := &structs.HealthCheck{ + Node: "foo", + CheckID: "tls", + Name: "tls check", + Status: api.HealthCritical, + } + + url := fmt.Sprintf("https://%s/v1/agent/self", a.srv.ln.Addr().String()) + chk := &structs.CheckType{ + HTTP: url, + Interval: 20 * time.Millisecond, + } + + err := a.AddCheck(health, chk, false, "") + if err != nil { + t.Fatalf("err: %v", err) + } + + retry.Run(t, func(r *retry.R) { + status := a.State.Checks()["tls"] + if status.Status != api.HealthPassing { + r.Fatalf("bad: %v", status.Status) + } + if !strings.Contains(status.Output, "200 OK") { + r.Fatalf("bad: %v", status.Output) + } + }) + } + + // We need to test both methods of passing the CA info to ensure that + // we propagate all the fields correctly. All the other fields are + // covered by the HCL in the test run function. + tests := []struct { + desc string + config string + }{ + {"ca_file", `ca_file = "../test/client_certs/rootca.crt"`}, + {"ca_path", `ca_path = "../test/client_certs/path"`}, + } + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + run(t, tt.config) + }) + } +} + func TestAgent_updateTTLCheck(t *testing.T) { t.Parallel() a := NewTestAgent(t.Name(), "") diff --git a/agent/checks/check.go b/agent/checks/check.go index 87e2c904b8..821a03dca3 100644 --- a/agent/checks/check.go +++ b/agent/checks/check.go @@ -292,15 +292,15 @@ func (c *CheckTTL) SetStatus(status, output string) { // 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 - Header map[string][]string - Method string - Interval time.Duration - Timeout time.Duration - Logger *log.Logger - TLSSkipVerify bool + Notify CheckNotifier + CheckID types.CheckID + HTTP string + Header map[string][]string + Method string + Interval time.Duration + Timeout time.Duration + Logger *log.Logger + TLSClientConfig *tls.Config httpClient *http.Client stop bool @@ -320,14 +320,8 @@ 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 - } + // Take on the supplied TLS client config. + trans.TLSClientConfig = c.TLSClientConfig // Create the HTTP client. c.httpClient = &http.Client{ diff --git a/agent/checks/check_test.go b/agent/checks/check_test.go index 4a652b09e0..1d2e1f3a36 100644 --- a/agent/checks/check_test.go +++ b/agent/checks/check_test.go @@ -368,23 +368,6 @@ func TestCheckHTTP_disablesKeepAlives(t *testing.T) { } } -func TestCheckHTTP_TLSSkipVerify_defaultFalse(t *testing.T) { - t.Parallel() - check := &CheckHTTP{ - CheckID: "foo", - HTTP: "https://foo.bar/baz", - Interval: 10 * time.Second, - Logger: log.New(ioutil.Discard, uniqueID(), log.LstdFlags), - } - - check.Start() - defer check.Stop() - - if check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { - t.Fatalf("should default to false") - } -} - func largeBodyHandler(code int) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // Body larger than 4k limit @@ -394,31 +377,36 @@ func largeBodyHandler(code int) http.Handler { }) } -func TestCheckHTTP_TLSSkipVerify_true_pass(t *testing.T) { +func TestCheckHTTP_TLS_SkipVerify(t *testing.T) { t.Parallel() server := httptest.NewTLSServer(largeBodyHandler(200)) defer server.Close() - notif := mock.NewNotify() + tlsConfig := &api.TLSConfig{ + InsecureSkipVerify: true, + } + tlsClientConfig, err := api.SetupTLSConfig(tlsConfig) + if err != nil { + t.Fatalf("err: %v", err) + } + notif := mock.NewNotify() check := &CheckHTTP{ - Notify: notif, - CheckID: types.CheckID("skipverify_true"), - HTTP: server.URL, - Interval: 25 * time.Millisecond, - Logger: log.New(ioutil.Discard, uniqueID(), log.LstdFlags), - TLSSkipVerify: true, + Notify: notif, + CheckID: types.CheckID("skipverify_true"), + HTTP: server.URL, + Interval: 25 * time.Millisecond, + Logger: log.New(ioutil.Discard, uniqueID(), log.LstdFlags), + TLSClientConfig: tlsClientConfig, } check.Start() defer check.Stop() - // give check some time to execute - time.Sleep(200 * time.Millisecond) - if !check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { t.Fatalf("should be true") } + retry.Run(t, func(r *retry.R) { if got, want := notif.State("skipverify_true"), api.HealthPassing; got != want { r.Fatalf("got state %q want %q", got, want) @@ -426,56 +414,33 @@ func TestCheckHTTP_TLSSkipVerify_true_pass(t *testing.T) { }) } -func TestCheckHTTP_TLSSkipVerify_true_fail(t *testing.T) { - t.Parallel() - server := httptest.NewTLSServer(largeBodyHandler(500)) - defer server.Close() - - notif := mock.NewNotify() - - check := &CheckHTTP{ - Notify: notif, - CheckID: types.CheckID("skipverify_true"), - HTTP: server.URL, - Interval: 5 * time.Millisecond, - Logger: log.New(ioutil.Discard, uniqueID(), log.LstdFlags), - TLSSkipVerify: true, - } - check.Start() - defer check.Stop() - - if !check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { - t.Fatalf("should be true") - } - retry.Run(t, func(r *retry.R) { - if got, want := notif.State("skipverify_true"), api.HealthCritical; got != want { - r.Fatalf("got state %q want %q", got, want) - } - }) -} - -func TestCheckHTTP_TLSSkipVerify_false(t *testing.T) { +func TestCheckHTTP_TLS_BadVerify(t *testing.T) { t.Parallel() server := httptest.NewTLSServer(largeBodyHandler(200)) defer server.Close() - notif := mock.NewNotify() + tlsClientConfig, err := api.SetupTLSConfig(&api.TLSConfig{}) + if err != nil { + t.Fatalf("err: %v", err) + } + notif := mock.NewNotify() check := &CheckHTTP{ - Notify: notif, - CheckID: types.CheckID("skipverify_false"), - HTTP: server.URL, - Interval: 100 * time.Millisecond, - Logger: log.New(ioutil.Discard, uniqueID(), log.LstdFlags), - TLSSkipVerify: false, + Notify: notif, + CheckID: types.CheckID("skipverify_false"), + HTTP: server.URL, + Interval: 100 * time.Millisecond, + Logger: log.New(ioutil.Discard, uniqueID(), log.LstdFlags), + TLSClientConfig: tlsClientConfig, } check.Start() defer check.Stop() if check.httpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify { - t.Fatalf("should be false") + t.Fatalf("should default to false") } + retry.Run(t, func(r *retry.R) { // This should fail due to an invalid SSL cert if got, want := notif.State("skipverify_false"), api.HealthCritical; got != want { diff --git a/agent/config/builder.go b/agent/config/builder.go index 869be6b660..9740282843 100644 --- a/agent/config/builder.go +++ b/agent/config/builder.go @@ -620,6 +620,7 @@ func (b *Builder) Build() (rt RuntimeConfig, err error) { DisableRemoteExec: b.boolVal(c.DisableRemoteExec), DisableUpdateCheck: b.boolVal(c.DisableUpdateCheck), DiscardCheckOutput: b.boolVal(c.DiscardCheckOutput), + EnableAgentTLSForChecks: b.boolVal(c.EnableAgentTLSForChecks), EnableDebug: b.boolVal(c.EnableDebug), EnableScriptChecks: b.boolVal(c.EnableScriptChecks), EnableSyslog: b.boolVal(c.EnableSyslog), diff --git a/agent/config/config.go b/agent/config/config.go index d79773fd55..8fad60cbcb 100644 --- a/agent/config/config.go +++ b/agent/config/config.go @@ -175,6 +175,7 @@ type Config struct { DisableUpdateCheck *bool `json:"disable_update_check,omitempty" hcl:"disable_update_check" mapstructure:"disable_update_check"` DiscardCheckOutput *bool `json:"discard_check_output" hcl:"discard_check_output" mapstructure:"discard_check_output"` EnableACLReplication *bool `json:"enable_acl_replication,omitempty" hcl:"enable_acl_replication" mapstructure:"enable_acl_replication"` + EnableAgentTLSForChecks *bool `json:"enable_agent_tls_for_checks,omitempty" hcl:"enable_agent_tls_for_checks" mapstructure:"enable_agent_tls_for_checks"` EnableDebug *bool `json:"enable_debug,omitempty" hcl:"enable_debug" mapstructure:"enable_debug"` EnableScriptChecks *bool `json:"enable_script_checks,omitempty" hcl:"enable_script_checks" mapstructure:"enable_script_checks"` EnableSyslog *bool `json:"enable_syslog,omitempty" hcl:"enable_syslog" mapstructure:"enable_syslog"` diff --git a/agent/config/runtime.go b/agent/config/runtime.go index 8f1549e551..b2da426d0c 100644 --- a/agent/config/runtime.go +++ b/agent/config/runtime.go @@ -677,6 +677,13 @@ type RuntimeConfig struct { // todo(fs): rename to ACLEnableReplication EnableACLReplication bool + // EnableAgentTLSForChecks is used to apply the agent's TLS settings in + // order to configure the HTTP client used for health checks. Enabling + // this allows HTTP checks to present a client certificate and verify + // the server using the same TLS configuration as the agent (CA, cert, + // and key). + EnableAgentTLSForChecks bool + // EnableDebug is used to enable various debugging features. // // hcl: enable_debug = (true|false) diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index e14b8ade07..03389cd286 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -2234,6 +2234,7 @@ func TestFullConfig(t *testing.T) { "udp_answer_limit": 29909 }, "enable_acl_replication": true, + "enable_agent_tls_for_checks": true, "enable_debug": true, "enable_script_checks": true, "enable_syslog": true, @@ -2668,6 +2669,7 @@ func TestFullConfig(t *testing.T) { udp_answer_limit = 29909 } enable_acl_replication = true + enable_agent_tls_for_checks = true enable_debug = true enable_script_checks = true enable_syslog = true @@ -3238,6 +3240,7 @@ func TestFullConfig(t *testing.T) { DisableUpdateCheck: true, DiscardCheckOutput: true, EnableACLReplication: true, + EnableAgentTLSForChecks: true, EnableDebug: true, EnableScriptChecks: true, EnableSyslog: true, @@ -3915,6 +3918,7 @@ func TestSanitize(t *testing.T) { "DisableUpdateCheck": false, "DiscardCheckOutput": false, "EnableACLReplication": false, + "EnableAgentTLSForChecks": false, "EnableDebug": false, "EnableScriptChecks": false, "EnableSyslog": false, diff --git a/api/api.go b/api/api.go index e7e6e8023a..7db12b49d7 100644 --- a/api/api.go +++ b/api/api.go @@ -377,12 +377,14 @@ func SetupTLSConfig(tlsConfig *TLSConfig) (*tls.Config, error) { tlsClientConfig.Certificates = []tls.Certificate{tlsCert} } - rootConfig := &rootcerts.Config{ - CAFile: tlsConfig.CAFile, - CAPath: tlsConfig.CAPath, - } - if err := rootcerts.ConfigureTLS(tlsClientConfig, rootConfig); err != nil { - return nil, err + if tlsConfig.CAFile != "" || tlsConfig.CAPath != "" { + rootConfig := &rootcerts.Config{ + CAFile: tlsConfig.CAFile, + CAPath: tlsConfig.CAPath, + } + if err := rootcerts.ConfigureTLS(tlsClientConfig, rootConfig); err != nil { + return nil, err + } } return tlsClientConfig, nil diff --git a/test/client_certs/path/rootca.crt b/test/client_certs/path/rootca.crt new file mode 100644 index 0000000000..e6c1c238a6 --- /dev/null +++ b/test/client_certs/path/rootca.crt @@ -0,0 +1,31 @@ +-----BEGIN CERTIFICATE----- +MIIFXTCCA0WgAwIBAgIJAKkYXwqUpHWIMA0GCSqGSIb3DQEBCwUAMEUxCzAJBgNV +BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX +aWRnaXRzIFB0eSBMdGQwHhcNMTcwNDE0MTkwNjIwWhcNMjIwNDE0MTkwNjIwWjBF +MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50 +ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIICIjANBgkqhkiG9w0BAQEFAAOCAg8AMIIC +CgKCAgEA0pMZRBhBHaYG1FttDzt+rctrM8NtslfbAbHflM4ly7mibCmrnh/sGYSP +Y/QcrGnCWrjXspUM0dxtmXGmXKj4yRpKhVYw16saDcn2t55dxMw23uEhgfrigtYS +/MoC78OaQryRGvYdxF5unrybBrXDrVfxA8ycYBOV04piS7NlN1/m3TkiNrqkl1up +paPscfFaY59yz3Lgq2vs8U1SLtph8ALwjJcz8O3BbBUQuiZYNuiLLnkRroxT3oSp +Zbdna2aXtdD/H9JJUoJLjoZp6x5fNOtc+5vF8QZ/jKeJDsxqwf4VT4Z6t8sUMFtO +YAR3QEV7wtOhWWfFVzdlCsKZhD8ryemlYSMJ7wfmXUeoiElSPRfvIHUTcJFx37VR +V6LmOzs1gjMpGZxGLk/GtaCbDlyCaT/hae8bqtRhngBy5bvGxmdSBQd0fjYRr2CP +Pz+Gzx3yQx8yb9VV7dTI3H4LWaTjuy2WooITzy3xY4bOvp4UwusFgbIdywwCpDFJ +zzy3FaqoMx06v5D3I9JCanlXk5FErA0GX45pPM4x3FWZfVwrXZtomBFDC6qlRMpH +FH8A/KrbpolwiDklSWUxHbfz32gpzf3kLW2nOVpnZPyAgSYudjSFGW00jRomCXOy +EOgQj/w+M3hKosvVqIE8IQgfMPNRCK9hn11gVqUvkdcYvo/2jUMCAwEAAaNQME4w +HQYDVR0OBBYEFMnRHZ6zGzu/N5MdNv4HKPP5rMqOMB8GA1UdIwQYMBaAFMnRHZ6z +Gzu/N5MdNv4HKPP5rMqOMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQELBQADggIB +AFPWJZiQTupGr5B14PGPkiesyL1VCkO9D7nr6dXgQwAUTIdDuo9+nZg1HzOd1evY +tRWGTAYOsyGNLBIjdhvTZcZz625i1zXFRzkpu0iOUoaH77/XVGkAKyaeYc3oIe9i +bRke37/NQokmM3Eko+M/KoAk31osDxlmdQTOVqqoZN+ag6xsrpN6XUmhu3DNfw1j +xud6RMH6J+SamG/vUg+GyqKZ9WpKNcLliVPJgOfX1XI3wstOIqkhlw/qveIgIfPv +aUVf+rykdgqYMMQvR6qx0k5iHeqj+F1cZRp3P0Uao0hoxi+udgj0X3F7/s/Cbr9c +TPZrIlhicZgli9UOwrjZ4B4mEZ4aD8yDbYO3TZe2DnuhPI7uDFG8lvCuLYu8V/lA +0yRoBaJf5Z1IXI4190Ww6bmxYV/n5EAFi6o46hWRUiYROtKEcP9Rmabodgp+Jxw6 +fwzcYqUXTOXR8/gAFPxt5oEfhZ8VJ4nlB9PFTjDi7Gbz+2MnmJokqkxLAR7//FEz +Rdmyfl+CvnPZ4TXLk77tuhf3Os9zHSTLobBdDivrTOpc0LdYw5l9yN9s93bG2TBr +2T0aKInqAduReLE2nhkYCdlY+dbjbELEiLicqSaEiwr9WbaWuLMoy4tDY52pYgTR +lEclafi+O3Y35hEE6VAfZvM1TeR3gvnnQf4ThqIxkRVl +-----END CERTIFICATE----- diff --git a/website/source/docs/agent/options.html.md b/website/source/docs/agent/options.html.md index 59a81da2c9..24dbedf2c7 100644 --- a/website/source/docs/agent/options.html.md +++ b/website/source/docs/agent/options.html.md @@ -954,6 +954,11 @@ Consul will not enable TLS for the HTTP API unless the `https` port has been ass and then introduce the token using the [agent token API](/api/agent.html#update-acl-tokens) on each server. See [`acl_replication_token`](#acl_replication_token) for more details. +* `enable_agent_tls_for_checks` + When set, uses a subset of the agent's TLS configuration (`key_file`, `cert_file`, `ca_file`, `ca_path`, and + `server_name`) to set up the HTTP client for HTTP health checks. This allows services requiring 2-way TLS to + be checked using the agent's credentials. This was added in Consul 1.0.1 and defaults to false. + * `enable_debug` When set, enables some additional debugging features. Currently, this is only used to set the runtime profiling HTTP endpoints.