Skip to content

Commit

Permalink
Cherry-pick #19584 to 7.x: tlscommon: require cert in ServerConfig.Va…
Browse files Browse the repository at this point in the history
…lidate (#19692)
  • Loading branch information
axw authored Jul 7, 2020
1 parent 2be60b2 commit 28088db
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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*

Expand Down
8 changes: 8 additions & 0 deletions libbeat/common/transport/tlscommon/server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
15 changes: 5 additions & 10 deletions libbeat/common/transport/tlscommon/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
9 changes: 8 additions & 1 deletion libbeat/common/transport/tlscommon/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions libbeat/common/transport/tlscommon/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
4 changes: 2 additions & 2 deletions libbeat/outputs/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit 28088db

Please sign in to comment.