From afe199d9a94517b440bafe22b1f0591215ccc1e7 Mon Sep 17 00:00:00 2001 From: Andrew Cholakian Date: Wed, 7 Nov 2018 21:56:36 -0600 Subject: [PATCH 1/2] Add heartbeat test for TLS client cert auth We were missing a test for this specific case. I wrote this hoping to confirm https://github.com/elastic/beats/issues/8979, but actually wound up disproving it. That said, this is still a good test to have, so we should merge it. --- heartbeat/monitors/active/http/http_test.go | 73 +++++++++++++++++-- .../active/http/testdata/client_cert.pem | 14 ++++ .../active/http/testdata/client_key.pem | 7 ++ 3 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 heartbeat/monitors/active/http/testdata/client_cert.pem create mode 100644 heartbeat/monitors/active/http/testdata/client_key.pem diff --git a/heartbeat/monitors/active/http/http_test.go b/heartbeat/monitors/active/http/http_test.go index 9b6fbce738d..f9e6afa8af6 100644 --- a/heartbeat/monitors/active/http/http_test.go +++ b/heartbeat/monitors/active/http/http_test.go @@ -18,13 +18,18 @@ package http import ( + "crypto/tls" "crypto/x509" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "os" + "path" "testing" + "github.com/elastic/beats/libbeat/common/file" + "github.com/stretchr/testify/require" "github.com/elastic/beats/heartbeat/hbtest" @@ -36,19 +41,22 @@ import ( ) func testRequest(t *testing.T, testURL string) beat.Event { - return testTLSRequest(t, testURL, "") + return testTLSRequest(t, testURL, nil) } // testTLSRequest tests the given request. certPath is optional, if given // an empty string no cert will be set. -func testTLSRequest(t *testing.T, testURL string, certPath string) beat.Event { +func testTLSRequest(t *testing.T, testURL string, extraConfig map[string]interface{}) beat.Event { configSrc := map[string]interface{}{ "urls": testURL, "timeout": "1s", } - if certPath != "" { - configSrc["ssl.certificate_authorities"] = certPath + if extraConfig != nil { + for k, v := range extraConfig { + configSrc[k] = v + //configSrc["ssl.certificate_authorities"] = certPath + } } config, err := common.NewConfigFrom(configSrc) @@ -247,8 +255,10 @@ func TestLargeResponse(t *testing.T) { ) } -func TestHTTPSServer(t *testing.T) { - server := httptest.NewTLSServer(hbtest.HelloWorldHandler(http.StatusOK)) +func runHTTPSServerCheck( + t *testing.T, + server *httptest.Server, + reqExtraConfig map[string]interface{}) { port, err := hbtest.ServerPort(server) require.NoError(t, err) @@ -261,7 +271,12 @@ func TestHTTPSServer(t *testing.T) { require.NoError(t, certFile.Close()) defer os.Remove(certFile.Name()) - event := testTLSRequest(t, server.URL, certFile.Name()) + mergedExtraConfig := map[string]interface{}{"ssl.certificate_authorities": certFile.Name()} + for k, v := range reqExtraConfig { + mergedExtraConfig[k] = v + } + + event := testTLSRequest(t, server.URL, mergedExtraConfig) mapvaltest.Test( t, @@ -275,6 +290,50 @@ func TestHTTPSServer(t *testing.T) { ) } +func TestHTTPSServer(t *testing.T) { + server := httptest.NewTLSServer(hbtest.HelloWorldHandler(http.StatusOK)) + + runHTTPSServerCheck(t, server, nil) +} + +func TestHTTPSx509Auth(t *testing.T) { + wd, err := os.Getwd() + require.NoError(t, err) + clientKeyPath := path.Join(wd, "testdata", "client_key.pem") + clientCertPath := path.Join(wd, "testdata", "client_cert.pem") + + certReader, err := file.ReadOpen(clientCertPath) + require.NoError(t, err) + + clientCertBytes, err := ioutil.ReadAll(certReader) + require.NoError(t, err) + + clientCerts := x509.NewCertPool() + certAdded := clientCerts.AppendCertsFromPEM(clientCertBytes) + require.True(t, certAdded) + + tlsConf := &tls.Config{ + ClientAuth: tls.RequireAndVerifyClientCert, + ClientCAs: clientCerts, + MinVersion: tls.VersionTLS12, + } + tlsConf.BuildNameToCertificate() + + server := httptest.NewUnstartedServer(hbtest.HelloWorldHandler(http.StatusOK)) + server.TLS = tlsConf + server.StartTLS() + defer server.Close() + + runHTTPSServerCheck( + t, + server, + map[string]interface{}{ + "ssl.certificate": clientCertPath, + "ssl.key": clientKeyPath, + }, + ) +} + func TestConnRefusedJob(t *testing.T) { ip := "127.0.0.1" port, err := btesting.AvailableTCP4Port() diff --git a/heartbeat/monitors/active/http/testdata/client_cert.pem b/heartbeat/monitors/active/http/testdata/client_cert.pem new file mode 100644 index 00000000000..d7851f1ea28 --- /dev/null +++ b/heartbeat/monitors/active/http/testdata/client_cert.pem @@ -0,0 +1,14 @@ +-----BEGIN CERTIFICATE----- +MIICGzCCAXygAwIBAgIRANIf32fETNS13JB39JYszmwwCgYIKoZIzj0EAwQwEjEQ +MA4GA1UEChMHQWNtZSBDbzAeFw0xODExMDgwMzIxNDlaFw0xOTExMDgwMzIxNDla +MBIxEDAOBgNVBAoTB0FjbWUgQ28wgZswEAYHKoZIzj0CAQYFK4EEACMDgYYABAE+ +n/OJoo7jvetm8zR4lAX2s99fxWF/LiOR1/qTPQgLmLYVUZq1yTZB027GtJGWAqph +kY/n0oNdxS4N9d2JPoaXMgHMGZAXl0A85Q3D5k0xKG/jwaEasTIbTe6UKHed2Zgk +CtEqutG9KwmnqAHCtlia14mgcERpO1eT0A7NRcdtNlcjlKNwMG4wDgYDVR0PAQH/ +BAQDAgKkMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAPBgNVHRMBAf8E +BTADAQH/MCwGA1UdEQQlMCOCCWxvY2FsaG9zdIEWbm9zdWNoYWRkckBleGFtcGxl +Lm5ldDAKBggqhkjOPQQDBAOBjAAwgYgCQgDvHj4Xt5TMqhR4Uavmfa0uOio0FZxL +vGnk3aLj5koJyrQNynntHBcCZ+sPb14J08FWk0j4GPOGroMVud/XTX1BZgJCAc3k +0p+X1r+lt1hkSGrumTY5NRWIGIvJ0gy1AhuZJzXYoPRRdPgnM04vBWniOLHDhmsX +ExbWSt0EY2IiOJc/1GNO +-----END CERTIFICATE----- diff --git a/heartbeat/monitors/active/http/testdata/client_key.pem b/heartbeat/monitors/active/http/testdata/client_key.pem new file mode 100644 index 00000000000..28911a4d6ca --- /dev/null +++ b/heartbeat/monitors/active/http/testdata/client_key.pem @@ -0,0 +1,7 @@ +-----BEGIN EC PRIVATE KEY----- +MIHcAgEBBEIB1YnGgQ42OFGz1rOFlmT97JB52b9/2h1dj85QaBLxX6isSNgnS7yC +VQKAQCudJz+UpqiTNZBQK0goqbD/O47lswagBwYFK4EEACOhgYkDgYYABAE+n/OJ +oo7jvetm8zR4lAX2s99fxWF/LiOR1/qTPQgLmLYVUZq1yTZB027GtJGWAqphkY/n +0oNdxS4N9d2JPoaXMgHMGZAXl0A85Q3D5k0xKG/jwaEasTIbTe6UKHed2ZgkCtEq +utG9KwmnqAHCtlia14mgcERpO1eT0A7NRcdtNlcjlA== +-----END EC PRIVATE KEY----- From aa71a0b7fc5e880844bdf9eea31cd436488572ab Mon Sep 17 00:00:00 2001 From: Andrew Cholakian Date: Thu, 8 Nov 2018 12:31:04 -0600 Subject: [PATCH 2/2] Remove extraneous comment --- heartbeat/monitors/active/http/http_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/heartbeat/monitors/active/http/http_test.go b/heartbeat/monitors/active/http/http_test.go index f9e6afa8af6..e00ddc1dcae 100644 --- a/heartbeat/monitors/active/http/http_test.go +++ b/heartbeat/monitors/active/http/http_test.go @@ -55,7 +55,6 @@ func testTLSRequest(t *testing.T, testURL string, extraConfig map[string]interfa if extraConfig != nil { for k, v := range extraConfig { configSrc[k] = v - //configSrc["ssl.certificate_authorities"] = certPath } }