From ae9cc67b6f819fcb5741e30de9f54a3dd8f2c2ea Mon Sep 17 00:00:00 2001 From: Silvia Mitter Date: Thu, 13 Jun 2019 19:47:40 +0200 Subject: [PATCH] Make client_auth optional by default. (#2283) --- _meta/beat.yml | 9 ++++----- apm-server.docker.yml | 9 ++++----- apm-server.yml | 9 ++++----- beater/beater_test.go | 14 +++++++------- beater/config.go | 13 +++++++++++++ changelogs/7.2.asciidoc | 6 +----- tests/system/test_access.py | 16 ++++++---------- 7 files changed, 39 insertions(+), 37 deletions(-) diff --git a/_meta/beat.yml b/_meta/beat.yml index ca1eb98bc83..1131178ebcb 100644 --- a/_meta/beat.yml +++ b/_meta/beat.yml @@ -65,11 +65,10 @@ apm-server: # Configure curve types for ECDHE based cipher suites #curve_types: [] - # Configure which types of client authentication are supported. - # Options are `none`, `optional`, and `required`. - # It is strongly recommended to always set to `required` - # Default is `required`. - #client_authentication: "required" + # Configure which type of client authentication is supported. + # Options are `none`, `optional`, and `required`. Default is `optional`. + #client_authentication: "optional" + #rum: # To enable real user monitoring (RUM) support set this to true. diff --git a/apm-server.docker.yml b/apm-server.docker.yml index fc9a2936015..8d11884b59c 100644 --- a/apm-server.docker.yml +++ b/apm-server.docker.yml @@ -65,11 +65,10 @@ apm-server: # Configure curve types for ECDHE based cipher suites #curve_types: [] - # Configure which types of client authentication are supported. - # Options are `none`, `optional`, and `required`. - # It is strongly recommended to always set to `required` - # Default is `required`. - #client_authentication: "required" + # Configure which type of client authentication is supported. + # Options are `none`, `optional`, and `required`. Default is `optional`. + #client_authentication: "optional" + #rum: # To enable real user monitoring (RUM) support set this to true. diff --git a/apm-server.yml b/apm-server.yml index ca1eb98bc83..1131178ebcb 100644 --- a/apm-server.yml +++ b/apm-server.yml @@ -65,11 +65,10 @@ apm-server: # Configure curve types for ECDHE based cipher suites #curve_types: [] - # Configure which types of client authentication are supported. - # Options are `none`, `optional`, and `required`. - # It is strongly recommended to always set to `required` - # Default is `required`. - #client_authentication: "required" + # Configure which type of client authentication is supported. + # Options are `none`, `optional`, and `required`. Default is `optional`. + #client_authentication: "optional" + #rum: # To enable real user monitoring (RUM) support set this to true. diff --git a/beater/beater_test.go b/beater/beater_test.go index cca3e189410..a99ce5e5700 100644 --- a/beater/beater_test.go +++ b/beater/beater_test.go @@ -28,12 +28,11 @@ import ( "testing" "time" - "github.com/elastic/beats/libbeat/common/transport/tlscommon" - "github.com/stretchr/testify/assert" "github.com/elastic/beats/libbeat/beat" "github.com/elastic/beats/libbeat/common" + "github.com/elastic/beats/libbeat/common/transport/tlscommon" "github.com/elastic/beats/libbeat/logp" "github.com/elastic/beats/libbeat/outputs" pubs "github.com/elastic/beats/libbeat/publisher" @@ -67,9 +66,10 @@ func TestBeatConfig(t *testing.T) { "capture_personal_data": true, "secret_token": "1234random", "ssl": map[string]interface{}{ - "enabled": true, - "key": "1234key", - "certificate": "1234cert", + "enabled": true, + "key": "1234key", + "certificate": "1234cert", + "client_authentication": "none", }, "expvar": map[string]interface{}{ "enabled": true, @@ -113,7 +113,7 @@ func TestBeatConfig(t *testing.T) { TLS: &tlscommon.ServerConfig{ Enabled: &truthy, Certificate: outputs.CertificateConfig{Certificate: "1234cert", Key: "1234key"}, - ClientAuth: 4}, + ClientAuth: 0}, AugmentEnabled: true, Expvar: &ExpvarConfig{ Enabled: &truthy, @@ -185,7 +185,7 @@ func TestBeatConfig(t *testing.T) { TLS: &tlscommon.ServerConfig{ Enabled: &truthy, Certificate: outputs.CertificateConfig{Certificate: "", Key: ""}, - ClientAuth: 4}, + ClientAuth: 3}, AugmentEnabled: true, Expvar: &ExpvarConfig{ Enabled: &truthy, diff --git a/beater/config.go b/beater/config.go index c34d0127c09..18dff545441 100644 --- a/beater/config.go +++ b/beater/config.go @@ -127,6 +127,19 @@ func (m *Mode) Unpack(s string) error { func newConfig(version string, ucfg *common.Config) (*Config, error) { c := defaultConfig(version) + + if ucfg.HasField("ssl") { + ssl, err := ucfg.Child("ssl", -1) + if err != nil { + return nil, err + } + if !ssl.HasField("client_authentication") { + if err := ucfg.SetString("ssl.client_authentication", -1, "optional"); err != nil { + return nil, err + } + } + } + if err := ucfg.Unpack(c); err != nil { return nil, errors.Wrap(err, "Error processing configuration") } diff --git a/changelogs/7.2.asciidoc b/changelogs/7.2.asciidoc index c7224b49989..ac36b5806da 100644 --- a/changelogs/7.2.asciidoc +++ b/changelogs/7.2.asciidoc @@ -10,10 +10,6 @@ https://github.com/elastic/apm-server/compare/7.1\...7.2[View commits] https://github.com/elastic/apm-server/compare/v7.1.0\...v7.2.0[View commits] -[float] -==== Bug fixes -- Require client authentication by default when `apm-server.ssl.enabled` is true {pull}2224[2224]. - [float] ==== Added - Make stacktrace lineno optional {pull}2105[2105]. @@ -24,4 +20,4 @@ https://github.com/elastic/apm-server/compare/v7.1.0\...v7.2.0[View commits] - Add ephemeral_id attribute in the metadata {pull}2179[2179]. - Add ILM support for APM Server, using fixed policies {pull}2099[2099],{pull}2209[2209]. - Add `setup --index-management` cmd, deprecate `setup --template` cmd {pull}2180[2180],{pull}2099[2099]. -- Support more SSL config options for agent/server communication {pull}2224[2224]. +- Support more SSL config options for agent/server communication {pull}2224[2224],{pull}2281[2281]. diff --git a/tests/system/test_access.py b/tests/system/test_access.py index 5fb01554d40..bde39ed3183 100644 --- a/tests/system/test_access.py +++ b/tests/system/test_access.py @@ -154,9 +154,9 @@ def test_https_no_cert_ok(self): r = self.send_http_request(verify=self.ca_cert) assert r.status_code == 202, r.status_code - @raises(ChunkedEncodingError) def test_http_fails(self): - self.send_http_request(protocol='http') + with self.assertRaises(Exception): + self.send_http_request(protocol='http') @raises(SSLError) def test_https_server_validation_fails(self): @@ -165,8 +165,7 @@ def test_https_server_validation_fails(self): class TestSSLEnabledOptionalClientVerificationTest(TestSecureServerBaseTest): - def ssl_overrides(self): - return {"ssl_client_authentication": "optional"} + # no ssl_overrides necessary as `optional` is default def test_https_no_certificate_ok(self): r = self.send_http_request(verify=self.ca_cert) @@ -184,7 +183,8 @@ def test_https_auth_cert_ok(self): class TestSSLEnabledRequiredClientVerificationTest(TestSecureServerBaseTest): - # no ssl_overrides necessary as `required` is default + def ssl_overrides(self): + return {"ssl_client_authentication": "required"} @raises(SSLError) def test_https_no_cert_fails(self): @@ -202,9 +202,6 @@ def test_https_auth_cert_ok(self): class TestSSLDefaultSupportedProcotolsTest(TestSecureServerBaseTest): - def ssl_overrides(self): - return {"ssl_client_authentication": "none"} - @raises(ssl.SSLError) def test_tls_v1_0(self): self.ssl_connect(protocol=ssl.PROTOCOL_TLSv1) @@ -218,8 +215,7 @@ def test_tls_v1_2(self): class TestSSLSupportedProcotolsTest(TestSecureServerBaseTest): def ssl_overrides(self): - return {"ssl_client_authentication": "none", - "ssl_supported_protocols": ["TLSv1.2"]} + return {"ssl_supported_protocols": ["TLSv1.2"]} @raises(ssl.SSLError) def test_tls_v1_1(self):