tlscommon: require cert in ServerConfig.Validate (elastic#19584)

* tlscommon: require cert in ServerConfig.Validate

It does not make sense to configure server-side TLS
without specifying a certificate and key pair. Check
that both a certificate and key are configured. We
were previously checking that both or neither were
specified.

CHANGELOG.next.asciidoc

@@ -138,6 +138,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
 - 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*
 

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()
 }
 

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
 	}
 

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)

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
 }