tlscommon: require cert in ServerConfig.Validate (#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
 }