Cherry-pick #12584 to 7.0: When certificate_authorities is configur…

…ed for ServerConfig, we now set client auth to `required` (#12589) * When `certificate_authorities` is configured for ServerConfig, we now set client auth to `required` (#12584) * When `certificate_authorities` is configured for ServerConfig, we now set client auth to `required` When a CA is explicitly set in the configuration options we now default the client authentication to required. (cherry picked from commit 7f99982)
elastic · Jun 18, 2019 · aad4daf · aad4daf
1 parent c08a5e3
commit aad4daf
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 29 deletions.
diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc
@@ -40,6 +40,7 @@ https://github.com/elastic/beats/compare/v7.0.0...7.0[Check the HEAD diff]
 - Fix memory leak in Filebeat pipeline acker. {pull}12063[12063]
 - Fix goroutine leak on non-explicit finalization of log input. {pull}12164[12164]
 - Require client_auth by default when ssl is enabled for tcp input {pull}12333[12333]
+- When TLS is configured for the TCP input and a `certificate_authorities` is configured we now default to `required` for the `client_authentication`. {pull}12584[12584]
 
 *Heartbeat*
 
@@ -52,6 +53,7 @@ https://github.com/elastic/beats/compare/v7.0.0...7.0[Check the HEAD diff]
 - Change some field type from scaled_float to long in aws module. {pull}11982[11982]
 - Ignore prometheus metrics when their values are NaN or Inf. {pull}12084[12084] {issue}10849[10849]
 - Require client_auth by default when ssl is enabled for module http metricset server{pull}12333[12333]
+- When TLS is configured for the http metricset and a `certificate_authorities` is configured we now default to `required` for the `client_authentication`. {pull}12584[12584]
 
 *Packetbeat*
 

diff --git a/filebeat/_meta/common.reference.inputs.yml b/filebeat/_meta/common.reference.inputs.yml
@@ -285,7 +285,8 @@ filebeat.inputs:
   #ssl.curve_types: []
 
   # Configure what types of client authentication are supported. Valid options
-  # are `none`, `optional`, and `required`. Default is required.
+  # are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+  # default to `required` otherwise it will be set to `none`.
   #ssl.client_authentication: "required"
 
 #------------------------------ Syslog input --------------------------------
@@ -344,7 +345,8 @@ filebeat.inputs:
     #ssl.curve_types: []
 
     # Configure what types of client authentication are supported. Valid options
-    # are `none`, `optional`, and `required`. Default is required.
+    # are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+    # default to `required` otherwise it will be set to `none`.
     #ssl.client_authentication: "required"
 
 #------------------------------ Docker input --------------------------------

diff --git a/filebeat/filebeat.reference.yml b/filebeat/filebeat.reference.yml
@@ -680,7 +680,8 @@ filebeat.inputs:
   #ssl.curve_types: []
 
   # Configure what types of client authentication are supported. Valid options
-  # are `none`, `optional`, and `required`. Default is required.
+  # are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+  # default to `required` otherwise it will be set to `none`.
   #ssl.client_authentication: "required"
 
 #------------------------------ Syslog input --------------------------------
@@ -739,7 +740,8 @@ filebeat.inputs:
     #ssl.curve_types: []
 
     # Configure what types of client authentication are supported. Valid options
-    # are `none`, `optional`, and `required`. Default is required.
+    # are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+    # default to `required` otherwise it will be set to `none`.
     #ssl.client_authentication: "required"
 
 #------------------------------ Docker input --------------------------------

diff --git a/libbeat/common/transport/tlscommon/server_config.go b/libbeat/common/transport/tlscommon/server_config.go
@@ -91,9 +91,14 @@ func LoadTLSServerConfig(config *ServerConfig) (*TLSConfig, error) {
 	}, nil
 }
 
+// Unpack unpacks the TLS Server configuration.
 func (c *ServerConfig) Unpack(cfg common.Config) error {
-	clientAuthKey := "client_authentication"
-	if !cfg.HasField(clientAuthKey) {
+	const clientAuthKey = "client_authentication"
+	const ca = "certificate_authorities"
+
+	// When we have explicitely defined the `certificate_authorities` in the configuration we default
+	// to `required` for the `client_authentication`, when CA is not defined we should set to `none`.
+	if cfg.HasField(ca) && !cfg.HasField(clientAuthKey) {
 		cfg.SetString(clientAuthKey, -1, "required")
 	}
 	type serverCfg ServerConfig

diff --git a/libbeat/common/transport/tlscommon/tls_test.go b/libbeat/common/transport/tlscommon/tls_test.go
@@ -167,26 +167,54 @@ func TestApplyWithConfig(t *testing.T) {
 }
 
 func TestServerConfigDefaults(t *testing.T) {
-	var c ServerConfig
-	config := common.MustNewConfigFrom([]byte(``))
-	err := config.Unpack(&c)
-	require.NoError(t, err)
-	tmp, err := LoadTLSServerConfig(&c)
-	require.NoError(t, err)
-
-	cfg := tmp.BuildModuleConfig("")
-
-	assert.NotNil(t, cfg)
-	// values not set by default
-	assert.Len(t, cfg.Certificates, 0)
-	assert.Nil(t, cfg.ClientCAs)
-	assert.Len(t, cfg.CipherSuites, 0)
-	assert.Len(t, cfg.CurvePreferences, 0)
-	// values set by default
-	assert.Equal(t, false, cfg.InsecureSkipVerify)
-	assert.Equal(t, int(tls.VersionTLS11), int(cfg.MinVersion))
-	assert.Equal(t, int(tls.VersionTLS12), int(cfg.MaxVersion))
-	assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth)
+	t.Run("when CA is not explicitly set", func(t *testing.T) {
+		var c ServerConfig
+		config := common.MustNewConfigFrom([]byte(``))
+		err := config.Unpack(&c)
+		require.NoError(t, err)
+		tmp, err := LoadTLSServerConfig(&c)
+		require.NoError(t, err)
+
+		cfg := tmp.BuildModuleConfig("")
+
+		assert.NotNil(t, cfg)
+		// values not set by default
+		assert.Len(t, cfg.Certificates, 0)
+		assert.Nil(t, cfg.ClientCAs)
+		assert.Len(t, cfg.CipherSuites, 0)
+		assert.Len(t, cfg.CurvePreferences, 0)
+		// values set by default
+		assert.Equal(t, false, cfg.InsecureSkipVerify)
+		assert.Equal(t, int(tls.VersionTLS11), int(cfg.MinVersion))
+		assert.Equal(t, int(tls.VersionTLS12), int(cfg.MaxVersion))
+		assert.Equal(t, tls.NoClientCert, cfg.ClientAuth)
+	})
+	t.Run("when CA is explicitly set", func(t *testing.T) {
+
+		yamlStr := `
+    certificate_authorities: [ca_test.pem]
+`
+		var c ServerConfig
+		config, err := common.NewConfigWithYAML([]byte(yamlStr), "")
+		err = config.Unpack(&c)
+		require.NoError(t, err)
+		tmp, err := LoadTLSServerConfig(&c)
+		require.NoError(t, err)
+
+		cfg := tmp.BuildModuleConfig("")
+
+		assert.NotNil(t, cfg)
+		// values not set by default
+		assert.Len(t, cfg.Certificates, 0)
+		assert.NotNil(t, cfg.ClientCAs)
+		assert.Len(t, cfg.CipherSuites, 0)
+		assert.Len(t, cfg.CurvePreferences, 0)
+		// values set by default
+		assert.Equal(t, false, cfg.InsecureSkipVerify)
+		assert.Equal(t, int(tls.VersionTLS11), int(cfg.MinVersion))
+		assert.Equal(t, int(tls.VersionTLS12), int(cfg.MaxVersion))
+		assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth)
+	})
 }
 
 func TestApplyWithServerConfig(t *testing.T) {

diff --git a/libbeat/docs/shared-ssl-config.asciidoc b/libbeat/docs/shared-ssl-config.asciidoc
@@ -224,7 +224,8 @@ ifeval::["{beatname_lc}" == "filebeat"]
 ==== `client_authentication`
 
 This configures what types of client authentication are supported. The valid options
-are `none`, `optional`, and `required`. The default value is required.
+are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+default to `required` otherwise it will be set to `none`.
 
 NOTE: This option is only valid with the TCP or the Syslog input.
 

diff --git a/x-pack/filebeat/filebeat.reference.yml b/x-pack/filebeat/filebeat.reference.yml
@@ -722,7 +722,8 @@ filebeat.inputs:
   #ssl.curve_types: []
 
   # Configure what types of client authentication are supported. Valid options
-  # are `none`, `optional`, and `required`. Default is required.
+  # are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+  # default to `required` otherwise it will be set to `none`.
   #ssl.client_authentication: "required"
 
 #------------------------------ Syslog input --------------------------------
@@ -781,7 +782,8 @@ filebeat.inputs:
     #ssl.curve_types: []
 
     # Configure what types of client authentication are supported. Valid options
-    # are `none`, `optional`, and `required`. Default is required.
+    # are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
+    # default to `required` otherwise it will be set to `none`.
     #ssl.client_authentication: "required"
 
 #------------------------------ Docker input --------------------------------