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

Enforce presence of Certificate Authorities, Certificate file and Key when using LoadTLSServerConfig #12355

Merged
merged 10 commits 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 @@ -107,6 +107,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix goroutine leak on non-explicit finalization of log input. {pull}12164[12164]
- Skipping unparsable log entries from docker json reader {pull}12268[12268]
- 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 @@ -136,6 +137,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Ignore prometheus metrics when their values are NaN or Inf. {pull}12084[12084] {issue}10849[10849]
- In the kibana/stats metricset, only log error (don't also index it) if xpack is enabled. {pull}12265[12265]
- 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