Skip to content

When certificate_authorities is configured for ServerConfig, we now set client auth to required #12584

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 17, 2019
Merged

When certificate_authorities is configured for ServerConfig, we now set client auth to required #12584

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
551d474
When `certificate_authorities` is configured for ServerConfig, we now…
ph Jun 17, 2019
1c27e2b
make sure we point to the right PR
ph Jun 17, 2019
55a6a84
Update filebeat.yml
ph Jun 17, 2019
5485881
update doc
ph Jun 17, 2019
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
3 changes: 2 additions & 1 deletion CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Skipping unparsable log entries from docker json reader {pull}12268[12268]
- Parse timezone in PostgreSQL logs as part of the timestamp {pull}12338[12338]
- Require client_auth by default when ssl is enabled for tcp input {pull}12333[12333]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should also be removed with the fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, since this PR change that logic it make sense to make sure that the changelog is clear.

- Require certificate authorities, certificate file, and key when SSL is enabled for the TCP input. {pull}12355[12355]
Copy link
Contributor Author

@ph ph Jun 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was not correctly removed from a previous revert.

- Load correct pipelines when system module is configured in modules.d. {pull}12340[12340]
- Fix timezone offset parsing in system/syslog. {pull}12529[12529]
- When TLS is configured for the TCP input and a `certificate_authorities` is configured we now default to `required` for the `client_authentication`. {pull}12584[12584]

*Heartbeat*

Expand Down Expand Up @@ -131,6 +131,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix an issue listing all processes when run under Windows as a non-privileged user. {issue}12301[12301] {pull}12475[12475]
- Require client_auth by default when ssl is enabled for module http metricset server{pull}12333[12333]
- The `elasticsearch/index_summary` metricset gracefully handles an empty Elasticsearch cluster when `xpack.enabled: true` is set. {pull}12489[12489] {issue}12487[12487]
- When TLS is configured for the http metricset and a `certificate_authorities` is configured we now default to `required` for the `client_authentication`. {pull}12584[12584]

*Packetbeat*

Expand Down
6 changes: 4 additions & 2 deletions filebeat/_meta/common.reference.inputs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,8 @@ filebeat.inputs:
#ssl.curve_types: []

# Configure what types of client authentication are supported. Valid options
# are `none`, `optional`, and `required`. Default is required.
# are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
# default to `required` otherwise it will be set to `none`.
#ssl.client_authentication: "required"

#------------------------------ Syslog input --------------------------------
Expand Down Expand Up @@ -352,7 +353,8 @@ filebeat.inputs:
#ssl.curve_types: []

# Configure what types of client authentication are supported. Valid options
# are `none`, `optional`, and `required`. Default is required.
# are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
# default to `required` otherwise it will be set to `none`.
#ssl.client_authentication: "required"

#------------------------------ Docker input --------------------------------
Expand Down
6 changes: 4 additions & 2 deletions filebeat/filebeat.reference.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,8 @@ filebeat.inputs:
#ssl.curve_types: []

# Configure what types of client authentication are supported. Valid options
# are `none`, `optional`, and `required`. Default is required.
# are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
# default to `required` otherwise it will be set to `none`.
#ssl.client_authentication: "required"

#------------------------------ Syslog input --------------------------------
Expand Down Expand Up @@ -729,7 +730,8 @@ filebeat.inputs:
#ssl.curve_types: []

# Configure what types of client authentication are supported. Valid options
# are `none`, `optional`, and `required`. Default is required.
# are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
# default to `required` otherwise it will be set to `none`.
#ssl.client_authentication: "required"

#------------------------------ Docker input --------------------------------
Expand Down
9 changes: 7 additions & 2 deletions libbeat/common/transport/tlscommon/server_config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,14 @@ 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) {
const clientAuthKey = "client_authentication"
const ca = "certificate_authorities"

// When we have explicitely defined the `certificate_authorities` in the configuration we default
// to `required` for the `client_authentication`, when CA is not defined we should set to `none`.
if cfg.HasField(ca) && !cfg.HasField(clientAuthKey) {
cfg.SetString(clientAuthKey, -1, "required")
}
type serverCfg ServerConfig
Expand Down
68 changes: 48 additions & 20 deletions libbeat/common/transport/tlscommon/tls_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,26 +170,54 @@ func TestApplyWithConfig(t *testing.T) {
}

func TestServerConfigDefaults(t *testing.T) {
var c ServerConfig
config := common.MustNewConfigFrom([]byte(``))
err := config.Unpack(&c)
require.NoError(t, err)
tmp, err := LoadTLSServerConfig(&c)
require.NoError(t, err)

cfg := tmp.BuildModuleConfig("")

assert.NotNil(t, cfg)
// values not set by default
assert.Len(t, cfg.Certificates, 0)
assert.Nil(t, cfg.ClientCAs)
assert.Len(t, cfg.CipherSuites, 0)
assert.Len(t, cfg.CurvePreferences, 0)
// values set by default
assert.Equal(t, false, cfg.InsecureSkipVerify)
assert.Equal(t, int(tls.VersionTLS11), int(cfg.MinVersion))
assert.Equal(t, int(tls.VersionTLS12), int(cfg.MaxVersion))
assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth)
t.Run("when CA is not explicitly set", func(t *testing.T) {
var c ServerConfig
config := common.MustNewConfigFrom([]byte(``))
err := config.Unpack(&c)
require.NoError(t, err)
tmp, err := LoadTLSServerConfig(&c)
require.NoError(t, err)

cfg := tmp.BuildModuleConfig("")

assert.NotNil(t, cfg)
// values not set by default
assert.Len(t, cfg.Certificates, 0)
assert.Nil(t, cfg.ClientCAs)
assert.Len(t, cfg.CipherSuites, 0)
assert.Len(t, cfg.CurvePreferences, 0)
// values set by default
assert.Equal(t, false, cfg.InsecureSkipVerify)
assert.Equal(t, int(tls.VersionTLS11), int(cfg.MinVersion))
assert.Equal(t, int(tls.VersionTLS12), int(cfg.MaxVersion))
assert.Equal(t, tls.NoClientCert, cfg.ClientAuth)
})
t.Run("when CA is explicitly set", func(t *testing.T) {

yamlStr := `
certificate_authorities: [ca_test.pem]
`
var c ServerConfig
config, err := common.NewConfigWithYAML([]byte(yamlStr), "")
err = config.Unpack(&c)
require.NoError(t, err)
tmp, err := LoadTLSServerConfig(&c)
require.NoError(t, err)

cfg := tmp.BuildModuleConfig("")

assert.NotNil(t, cfg)
// values not set by default
assert.Len(t, cfg.Certificates, 0)
assert.NotNil(t, cfg.ClientCAs)
assert.Len(t, cfg.CipherSuites, 0)
assert.Len(t, cfg.CurvePreferences, 0)
// values set by default
assert.Equal(t, false, cfg.InsecureSkipVerify)
assert.Equal(t, int(tls.VersionTLS11), int(cfg.MinVersion))
assert.Equal(t, int(tls.VersionTLS12), int(cfg.MaxVersion))
assert.Equal(t, tls.RequireAndVerifyClientCert, cfg.ClientAuth)
})
}

func TestApplyWithServerConfig(t *testing.T) {
Expand Down
3 changes: 2 additions & 1 deletion libbeat/docs/shared-ssl-config.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ ifeval::["{beatname_lc}" == "filebeat"]
==== `client_authentication`

This configures what types of client authentication are supported. The valid options
are `none`, `optional`, and `required`. The default value is required.
are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
default to `required` otherwise it will be set to `none`.

NOTE: This option is only valid with the TCP or the Syslog input.

Expand Down
6 changes: 4 additions & 2 deletions x-pack/filebeat/filebeat.reference.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,8 @@ filebeat.inputs:
#ssl.curve_types: []

# Configure what types of client authentication are supported. Valid options
# are `none`, `optional`, and `required`. Default is required.
# are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
# default to `required` otherwise it will be set to `none`.
#ssl.client_authentication: "required"

#------------------------------ Syslog input --------------------------------
Expand Down Expand Up @@ -848,7 +849,8 @@ filebeat.inputs:
#ssl.curve_types: []

# Configure what types of client authentication are supported. Valid options
# are `none`, `optional`, and `required`. Default is required.
# are `none`, `optional`, and `required`. When `certificate_authorities` is set it will
# default to `required` otherwise it will be set to `none`.
#ssl.client_authentication: "required"

#------------------------------ Docker input --------------------------------
Expand Down