Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry-pick #12355 to 7.2: Enforce presence of Certificate Authorities, Certificate file and Key when using LoadTLSServerConfig #12365

Merged
merged 1 commit into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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*

Expand Down Expand Up @@ -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*

Expand Down
7 changes: 7 additions & 0 deletions libbeat/common/transport/tlscommon/server_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package tlscommon

import (
"crypto/tls"
"errors"

"github.com/joeshaw/multierror"

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