From 0a4fa5fa3c58e6bfb369d212f1c529b160ca6f9d Mon Sep 17 00:00:00 2001 From: Pier-Hugues Pellerin Date: Wed, 29 May 2019 22:40:26 -0400 Subject: [PATCH] Cherry-pick #12355 to 7.2: Enforce presence of Certificate Authorities, Certificate file and Key when using LoadTLSServerConfig (#12365) * Enforce presence of Certificate Authorities, Certificate file and Key when using LoadTLSServerConfig (cherry picked from commit 8589309ac42d6a4ad031424a12e11e59980b87aa) --- CHANGELOG.next.asciidoc | 2 + .../transport/tlscommon/server_config.go | 7 +++ .../common/transport/tlscommon/tls_test.go | 61 +++++++++++++++++-- 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index dc4bda8fcbae..aa53f05de800 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -104,6 +104,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix goroutine leak caused on initialization failures of log input. {pull}12125[12125] - 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] +- Require certificate authorities, certificate file, and key when SSL is enabled for the TCP input. {pull}12355[12355] *Heartbeat* @@ -131,6 +132,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d - Fix direction of incoming IPv6 sockets. {pull}12248[12248] - 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] +- Require certificate authorities, certificate file, and key when SSL is enabled for module http metricset server. {pull}12355[12355] *Packetbeat* diff --git a/libbeat/common/transport/tlscommon/server_config.go b/libbeat/common/transport/tlscommon/server_config.go index 79d4722049f4..2228d12a819a 100644 --- a/libbeat/common/transport/tlscommon/server_config.go +++ b/libbeat/common/transport/tlscommon/server_config.go @@ -19,6 +19,7 @@ package tlscommon import ( "crypto/tls" + "errors" "github.com/joeshaw/multierror" @@ -91,6 +92,7 @@ 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) { @@ -101,6 +103,11 @@ func (c *ServerConfig) Unpack(cfg common.Config) error { if err := cfg.Unpack(&sCfg); err != nil { return err } + + if sCfg.VerificationMode != VerifyNone && len(sCfg.CAs) == 0 { + return errors.New("certificate_authorities not configured") + } + *c = ServerConfig(sCfg) return nil } diff --git a/libbeat/common/transport/tlscommon/tls_test.go b/libbeat/common/transport/tlscommon/tls_test.go index b4b1f1dd093a..608607641814 100644 --- a/libbeat/common/transport/tlscommon/tls_test.go +++ b/libbeat/common/transport/tlscommon/tls_test.go @@ -167,9 +167,15 @@ func TestApplyWithConfig(t *testing.T) { } func TestServerConfigDefaults(t *testing.T) { + yamlStr := ` + certificate: ca_test.pem + key: ca_test.key + certificate_authorities: [ca_test.pem] + ` var c ServerConfig - config := common.MustNewConfigFrom([]byte(``)) - err := config.Unpack(&c) + config, err := common.NewConfigWithYAML([]byte(yamlStr), "") + require.NoError(t, err) + err = config.Unpack(&c) require.NoError(t, err) tmp, err := LoadTLSServerConfig(&c) require.NoError(t, err) @@ -178,8 +184,8 @@ func TestServerConfigDefaults(t *testing.T) { assert.NotNil(t, cfg) // values not set by default - assert.Len(t, cfg.Certificates, 0) - assert.Nil(t, cfg.ClientCAs) + assert.Len(t, cfg.Certificates, 1) + assert.NotNil(t, cfg.ClientCAs) assert.Len(t, cfg.CipherSuites, 0) assert.Len(t, cfg.CurvePreferences, 0) // values set by default @@ -189,6 +195,53 @@ func TestServerConfigDefaults(t *testing.T) { assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth) } +func TestServerConfigSkipCACertificateAndKeyWhenVerifyNone(t *testing.T) { + yamlStr := ` + verification_mode: none + ` + var c ServerConfig + config, err := common.NewConfigWithYAML([]byte(yamlStr), "") + require.NoError(t, err) + err = config.Unpack(&c) + require.NoError(t, err) +} + +func TestServerConfigEnsureCA(t *testing.T) { + yamlStr := ` + certificate: ca_test.pem + key: ca_test.key + ` + var c ServerConfig + config, err := common.NewConfigWithYAML([]byte(yamlStr), "") + require.NoError(t, err) + err = config.Unpack(&c) + require.Error(t, err) +} + +func TestServerConfigCertificateKey(t *testing.T) { + yamlStr := ` + certificate: ca_test.pem + certificate_authorities: [ca_test.pem] + ` + var c ServerConfig + config, err := common.NewConfigWithYAML([]byte(yamlStr), "") + require.NoError(t, err) + err = config.Unpack(&c) + require.Error(t, err) +} + +func TestServerConfigCertificate(t *testing.T) { + yamlStr := ` + key: ca_test.key + certificate_authorities: [ca_test.pem] + ` + var c ServerConfig + config, err := common.NewConfigWithYAML([]byte(yamlStr), "") + require.NoError(t, err) + err = config.Unpack(&c) + require.Error(t, err) +} + func TestApplyWithServerConfig(t *testing.T) { yamlStr := ` certificate: ca_test.pem