From 28088db8bd2b2aa619162e3a69c1ed105d104cd5 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Tue, 7 Jul 2020 19:35:21 +0800 Subject: [PATCH] Cherry-pick #19584 to 7.x: tlscommon: require cert in ServerConfig.Validate (#19692) --- CHANGELOG.next.asciidoc | 1 + .../common/transport/tlscommon/server_config.go | 8 ++++++++ libbeat/common/transport/tlscommon/tls.go | 15 +++++---------- libbeat/common/transport/tlscommon/tls_test.go | 9 ++++++++- libbeat/common/transport/tlscommon/types.go | 8 ++++---- libbeat/outputs/tls.go | 4 ++-- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 518576e7257..412d2ccc9b1 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -159,6 +159,7 @@ field. You can revert this change by configuring tags for the module and omittin - Fix redis key setting not allowing upper case characters. {pull}18854[18854] {issue}18640[18640] - Fix config reload metrics (`libbeat.config.module.start/stops/running`). {pull}19168[19168] - Fix metrics hints builder to avoid wrong container metadata usage when port is not exposed {pull}18979[18979] +- Server-side TLS config now validates certificate and key are both specified {pull}19584[19584] *Auditbeat* diff --git a/libbeat/common/transport/tlscommon/server_config.go b/libbeat/common/transport/tlscommon/server_config.go index 3cea793eaab..866d6e3c28c 100644 --- a/libbeat/common/transport/tlscommon/server_config.go +++ b/libbeat/common/transport/tlscommon/server_config.go @@ -113,6 +113,14 @@ func (c *ServerConfig) Unpack(cfg common.Config) error { // Validate values the TLSConfig struct making sure certificate sure we have both a certificate and // a key. func (c *ServerConfig) Validate() error { + if c.IsEnabled() { + // c.Certificate.Validate() ensures that both a certificate and key + // are specified, or neither are specified. For server-side TLS we + // require both to be specified. + if c.Certificate.Certificate == "" { + return ErrCertificateUnspecified + } + } return c.Certificate.Validate() } diff --git a/libbeat/common/transport/tlscommon/tls.go b/libbeat/common/transport/tlscommon/tls.go index 67661c89bd4..5dea417637e 100644 --- a/libbeat/common/transport/tlscommon/tls.go +++ b/libbeat/common/transport/tlscommon/tls.go @@ -33,18 +33,13 @@ const logSelector = "tls" // LoadCertificate will load a certificate from disk and return a tls.Certificate or error func LoadCertificate(config *CertificateConfig) (*tls.Certificate, error) { + if err := config.Validate(); err != nil { + return nil, err + } + certificate := config.Certificate key := config.Key - - hasCertificate := certificate != "" - hasKey := key != "" - - switch { - case hasCertificate && !hasKey: - return nil, ErrCertificateNoKey - case !hasCertificate && hasKey: - return nil, ErrKeyNoCertificate - case !hasCertificate && !hasKey: + if certificate == "" { return nil, nil } diff --git a/libbeat/common/transport/tlscommon/tls_test.go b/libbeat/common/transport/tlscommon/tls_test.go index 94e74a49b92..b89308494e0 100644 --- a/libbeat/common/transport/tlscommon/tls_test.go +++ b/libbeat/common/transport/tlscommon/tls_test.go @@ -172,9 +172,13 @@ func TestApplyWithConfig(t *testing.T) { func TestServerConfigDefaults(t *testing.T) { t.Run("when CA is not explicitly set", func(t *testing.T) { var c ServerConfig - config := common.MustNewConfigFrom([]byte(``)) + config := common.MustNewConfigFrom(` +certificate: mycert.pem +key: mykey.pem +`) err := config.Unpack(&c) require.NoError(t, err) + c.Certificate = CertificateConfig{} // prevent reading non-existent files tmp, err := LoadTLSServerConfig(&c) require.NoError(t, err) @@ -196,10 +200,13 @@ func TestServerConfigDefaults(t *testing.T) { yamlStr := ` certificate_authorities: [ca_test.pem] + certificate: mycert.pem + key: mykey.pem ` var c ServerConfig config, err := common.NewConfigWithYAML([]byte(yamlStr), "") err = config.Unpack(&c) + c.Certificate = CertificateConfig{} // prevent reading non-existent files require.NoError(t, err) tmp, err := LoadTLSServerConfig(&c) require.NoError(t, err) diff --git a/libbeat/common/transport/tlscommon/types.go b/libbeat/common/transport/tlscommon/types.go index 93cdf95464e..3c14f1f1ca9 100644 --- a/libbeat/common/transport/tlscommon/types.go +++ b/libbeat/common/transport/tlscommon/types.go @@ -29,10 +29,10 @@ var ( ErrNotACertificate = errors.New("file is not a certificate") // ErrCertificateNoKey indicate a configuration error with missing key file - ErrCertificateNoKey = errors.New("key file not configured") + ErrKeyUnspecified = errors.New("key file not configured") // ErrKeyNoCertificate indicate a configuration error with missing certificate file - ErrKeyNoCertificate = errors.New("certificate file not configured") + ErrCertificateUnspecified = errors.New("certificate file not configured") ) var tlsCipherSuites = map[string]tlsCipherSuite{ @@ -261,9 +261,9 @@ func (c *CertificateConfig) Validate() error { switch { case hasCertificate && !hasKey: - return ErrCertificateNoKey + return ErrKeyUnspecified case !hasCertificate && hasKey: - return ErrKeyNoCertificate + return ErrCertificateUnspecified } return nil } diff --git a/libbeat/outputs/tls.go b/libbeat/outputs/tls.go index 907af8a79fa..c1f3fd9f077 100644 --- a/libbeat/outputs/tls.go +++ b/libbeat/outputs/tls.go @@ -29,10 +29,10 @@ var ( ErrNotACertificate = tlscommon.ErrNotACertificate // ErrCertificateNoKey indicate a configuration error with missing key file - ErrCertificateNoKey = tlscommon.ErrCertificateNoKey + ErrCertificateNoKey = tlscommon.ErrKeyUnspecified // ErrKeyNoCertificate indicate a configuration error with missing certificate file - ErrKeyNoCertificate = tlscommon.ErrKeyNoCertificate + ErrKeyNoCertificate = tlscommon.ErrCertificateUnspecified ) // TLSConfig defines config file options for TLS clients.