Skip to content

Commit

Permalink
[heartbeat][libbeat][metricbeat][filebeat] Pass TLS options to forwar…
Browse files Browse the repository at this point in the history
…d proxies (elastic#15516)

Fixes a bug where TLS options would not be passed to forward proxies. This fixes a reported issue where x509 client cert auth doesn't work due to the cert parameters not being passed in.

Since this doesn't give us a full dialer we can't easily record TLS certificate expiry with proxies in heartbeat with this approach, so those fields would be missing. I think it makes sense to merge this change as-is and fix that secondary bug in a subsequent PR. I've opened a separate issue ( elastic#15797 ) to cover that.

I've tested this manually against with the following config: https://github.com/elastic/uptime-contrib/tree/master/testing/configs/client-auth , note that you'll need the included client certs in that directory, and will need to alter the YAML to point at their actual path.

I think in this scenario I'm fine with only having unit tests because the complexity of setting of a forward proxy + a custom TLS backend is very high.

Fixes elastic#15524
  • Loading branch information
andrewvc committed Jan 27, 2020
1 parent d8517c6 commit 0e57f6b
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix missing output in dockerlogbeat {pull}15719[15719]
- Fix logging target settings being ignored when Beats are started via systemd or docker. {issue}12024[12024] {pull}15422[15442]
- Do not load dashboards where not available. {pull}15802[15802]
- Fix issue where TLS settings would be ignored when a forward proxy was in use. {pull}15516{15516}
- Update replicaset group to apps/v1 {pull}15854[15802]

*Auditbeat*
Expand Down
1 change: 1 addition & 0 deletions heartbeat/monitors/active/http/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func newRoundTripper(config *Config, tls *transport.TLSConfig) (*http.Transport,
Proxy: proxy,
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
TLSClientConfig: tls.ToConfig(),
DisableKeepAlives: true,
}, nil
}
29 changes: 29 additions & 0 deletions heartbeat/monitors/active/http/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import (
"github.com/elastic/beats/libbeat/beat"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/common/file"
"github.com/elastic/beats/libbeat/outputs/transport"
btesting "github.com/elastic/beats/libbeat/testing"
"github.com/elastic/go-lookslike"
"github.com/elastic/go-lookslike/isdef"
Expand Down Expand Up @@ -498,3 +499,31 @@ func TestRedirect(t *testing.T) {
event.Fields,
)
}

func TestNewRoundTripper(t *testing.T) {
configs := map[string]Config{
"Plain": {Timeout: time.Second},
"With Proxy": {Timeout: time.Second, ProxyURL: "http://localhost:1234"},
}

for name, config := range configs {
t.Run(name, func(t *testing.T) {
transp, err := newRoundTripper(&config, &transport.TLSConfig{})
require.NoError(t, err)

if config.ProxyURL == "" {
require.Nil(t, transp.Proxy)
} else {
require.NotNil(t, transp.Proxy)
}

// It's hard to compare func types in tests
require.NotNil(t, transp.Dial)
require.NotNil(t, transport.TLSDialer)

require.Equal(t, (&transport.TLSConfig{}).ToConfig(), transp.TLSClientConfig)
require.True(t, transp.DisableKeepAlives)
})
}

}
21 changes: 16 additions & 5 deletions libbeat/common/transport/tlscommon/tls_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ type TLSConfig struct {
ClientAuth tls.ClientAuthType
}

// BuildModuleConfig takes the TLSConfig and transform it into a `tls.Config`.
func (c *TLSConfig) BuildModuleConfig(host string) *tls.Config {
// ToConfig generates a tls.Config object. Note, you must use BuildModuleConfig to generate a config with
// ServerName set, use that method for servers with SNI.
func (c *TLSConfig) ToConfig() *tls.Config {
if c == nil {
// use default TLS settings, if config is empty.
return &tls.Config{ServerName: host}
return &tls.Config{}
}

minVersion, maxVersion := extractMinMaxVersion(c.Versions)
Expand All @@ -80,7 +80,6 @@ func (c *TLSConfig) BuildModuleConfig(host string) *tls.Config {
}

return &tls.Config{
ServerName: host,
MinVersion: minVersion,
MaxVersion: maxVersion,
Certificates: c.Certificates,
Expand All @@ -93,3 +92,15 @@ func (c *TLSConfig) BuildModuleConfig(host string) *tls.Config {
ClientAuth: c.ClientAuth,
}
}

// BuildModuleConfig takes the TLSConfig and transform it into a `tls.Config`.
func (c *TLSConfig) BuildModuleConfig(host string) *tls.Config {
if c == nil {
// use default TLS settings, if config is empty.
return &tls.Config{ServerName: host}
}

config := c.ToConfig()
config.ServerName = host
return config
}
5 changes: 3 additions & 2 deletions libbeat/kibana/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ func NewClientWithConfig(config *ClientConfig) (*Client, error) {
Password: password,
HTTP: &http.Client{
Transport: &http.Transport{
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
TLSClientConfig: tlsConfig.ToConfig(),
},
Timeout: config.Timeout,
},
Expand Down
7 changes: 4 additions & 3 deletions libbeat/outputs/elasticsearch/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,10 @@ func NewClient(
Headers: s.Headers,
http: &http.Client{
Transport: &http.Transport{
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
Proxy: proxy,
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
TLSClientConfig: s.TLS.ToConfig(),
Proxy: proxy,
},
Timeout: s.Timeout,
},
Expand Down
7 changes: 4 additions & 3 deletions metricbeat/helper/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,10 @@ func newHTTPFromConfig(config Config, name string, hostData mb.HostData) (*HTTP,
hostData: hostData,
client: &http.Client{
Transport: &http.Transport{
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
Proxy: http.ProxyFromEnvironment,
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
TLSClientConfig: tlsConfig.ToConfig(),
Proxy: http.ProxyFromEnvironment,
},
Timeout: config.Timeout,
},
Expand Down
1 change: 1 addition & 0 deletions x-pack/filebeat/input/httpjson/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ func (in *httpjsonInput) run() error {
Transport: &http.Transport{
Dial: dialer.Dial,
DialTLS: tlsDialer.Dial,
TLSClientConfig: tlsConfig.ToConfig(),
DisableKeepAlives: true,
},
Timeout: in.config.HTTPClientTimeout,
Expand Down

0 comments on commit 0e57f6b

Please sign in to comment.