Skip to content

Commit

Permalink
common/tls: Allow specifying SNI hostnames (influxdata#7897)
Browse files Browse the repository at this point in the history
* tls_config: Allow specifying SNI hostnames

Add a new configration field `tls_server_name` that allows specifying
the server name that'll be sent in the ClientHello when telegraf makes
a request to TLS servers. This allows checking against load balancers
responding to specific hostnames that otherwise wouldn't resolve to
their addresses.

Add the setting to the documentation of common TLS options, as well as
to the http_response plugin.

Fixes influxdata#7598.

* Adjust the x509_cert to allow usage of tls_server_name

This plugin has been using ServerName previously, and will have to
deal with the new setting, too: Extract the server-name choosing into
a method & add a test to ensure we choose the right value (and error
under the right circumstances). Also document that the two settings
are mutually exclusive.

* Improve documentation on what we try to accomplish in the nil return

Also get rid of the TODO, as I am fairly certain this behavior is the
correct one.

* Remove unused struct field in tests
  • Loading branch information
antifuchs authored Dec 23, 2020
1 parent 8450965 commit 2f82d30
Show file tree
Hide file tree
Showing 9 changed files with 128 additions and 17 deletions.
2 changes: 2 additions & 0 deletions docs/TLS.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ For client TLS support we have the following options:
# tls_key = "/etc/telegraf/key.pem"
## Skip TLS verification.
# insecure_skip_verify = false
## Send the specified TLS server name via SNI.
# tls_server_name = "foo.example.com"
```

### Server Configuration
Expand Down
18 changes: 13 additions & 5 deletions plugins/common/tls/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ type ClientConfig struct {
TLSCert string `toml:"tls_cert"`
TLSKey string `toml:"tls_key"`
InsecureSkipVerify bool `toml:"insecure_skip_verify"`
ServerName string `toml:"tls_server_name"`

// Deprecated in 1.7; use TLS variables above
SSLCA string `toml:"ssl_ca"`
Expand Down Expand Up @@ -45,11 +46,14 @@ func (c *ClientConfig) TLSConfig() (*tls.Config, error) {
c.TLSKey = c.SSLKey
}

// TODO: return default tls.Config; plugins should not call if they don't
// want TLS, this will require using another option to determine. In the
// case of an HTTP plugin, you could use `https`. Other plugins may need
// the dedicated option `TLSEnable`.
if c.TLSCA == "" && c.TLSKey == "" && c.TLSCert == "" && !c.InsecureSkipVerify {
// This check returns a nil (aka, "use the default")
// tls.Config if no field is set that would have an effect on
// a TLS connection. That is, any of:
// * client certificate settings,
// * peer certificate authorities,
// * disabled security, or
// * an SNI server name.
if c.TLSCA == "" && c.TLSKey == "" && c.TLSCert == "" && !c.InsecureSkipVerify && c.ServerName == "" {
return nil, nil
}

Expand All @@ -73,6 +77,10 @@ func (c *ClientConfig) TLSConfig() (*tls.Config, error) {
}
}

if c.ServerName != "" {
tlsConfig.ServerName = c.ServerName
}

return tlsConfig, nil
}

Expand Down
8 changes: 8 additions & 0 deletions plugins/common/tls/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ func TestClientConfig(t *testing.T) {
SSLKey: pki.ClientKeyPath(),
},
},
{
name: "set SNI server name",
client: tls.ClientConfig{
ServerName: "foo.example.com",
},
expNil: false,
expErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion plugins/inputs/http_response/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ This input plugin checks HTTP/HTTPS connections.
# tls_key = "/etc/telegraf/key.pem"
## Use TLS but skip chain & host verification
# insecure_skip_verify = false
## Use the given name as the SNI server name on each URL
# tls_server_name = ""

## HTTP Request Headers (all values must be strings)
# [inputs.http_response.headers]
Expand Down Expand Up @@ -91,7 +93,7 @@ This input plugin checks HTTP/HTTPS connections.
- response_string_match (int, 0 = mismatch / body read error, 1 = match)
- response_status_code_match (int, 0 = mismatch, 1 = match)
- http_response_code (int, response status code)
- result_type (string, deprecated in 1.6: use `result` tag and `result_code` field)
- result_type (string, deprecated in 1.6: use `result` tag and `result_code` field)
- result_code (int, [see below](#result--result_code))

#### `result` / `result_code`
Expand Down
4 changes: 2 additions & 2 deletions plugins/inputs/http_response/http_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ var sampleConfig = `
# {'fake':'data'}
# '''
## Optional name of the field that will contain the body of the response.
## By default it is set to an empty String indicating that the body's content won't be added
## Optional name of the field that will contain the body of the response.
## By default it is set to an empty String indicating that the body's content won't be added
# response_body_field = ''
## Maximum allowed HTTP response body size in bytes.
Expand Down
38 changes: 38 additions & 0 deletions plugins/inputs/http_response/http_response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/influxdata/telegraf"
"github.com/influxdata/telegraf/internal"
"github.com/influxdata/telegraf/plugins/common/tls"
"github.com/influxdata/telegraf/testutil"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -1266,3 +1267,40 @@ func TestStatusCodeAndStringMatchFail(t *testing.T) {
}
checkOutput(t, &acc, expectedFields, expectedTags, nil, nil)
}

func TestSNI(t *testing.T) {
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
assert.Equal(t, "super-special-hostname.example.com", r.TLS.ServerName)
w.WriteHeader(http.StatusOK)
}))
defer ts.Close()

h := &HTTPResponse{
Log: testutil.Logger{},
URLs: []string{ts.URL + "/good"},
Method: "GET",
ResponseTimeout: internal.Duration{Duration: time.Second * 20},
ClientConfig: tls.ClientConfig{
InsecureSkipVerify: true,
ServerName: "super-special-hostname.example.com",
},
}
var acc testutil.Accumulator
err := h.Gather(&acc)
require.NoError(t, err)
expectedFields := map[string]interface{}{
"http_response_code": http.StatusOK,
"result_type": "success",
"result_code": 0,
"response_time": nil,
"content_length": nil,
}
expectedTags := map[string]interface{}{
"server": nil,
"method": "GET",
"status_code": "200",
"result": "success",
}
absentFields := []string{"response_string_match"}
checkOutput(t, &acc, expectedFields, expectedTags, absentFields, nil)
}
5 changes: 4 additions & 1 deletion plugins/inputs/x509_cert/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,17 @@ file or network connection.
## Timeout for SSL connection
# timeout = "5s"

## Pass a different name into the TLS request (Server Name Indication)
## Pass a different name into the TLS request (Server Name Indication).
## This is synonymous with tls_server_name, and only one of the two
## options may be specified at one time.
## example: server_name = "myhost.example.org"
# server_name = "myhost.example.org"

## Optional TLS Config
# tls_ca = "/etc/telegraf/ca.pem"
# tls_cert = "/etc/telegraf/cert.pem"
# tls_key = "/etc/telegraf/key.pem"
# tls_server_name = "myhost.example.org"
```


Expand Down
28 changes: 20 additions & 8 deletions plugins/inputs/x509_cert/x509_cert.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,19 @@ func (c *X509Cert) locationToURL(location string) (*url.URL, error) {
return u, nil
}

func (c *X509Cert) serverName(u *url.URL) (string, error) {
if c.tlsCfg.ServerName != "" {
if c.ServerName != "" {
return "", fmt.Errorf("both server_name (%q) and tls_server_name (%q) are set, but they are mutually exclusive", c.ServerName, c.tlsCfg.ServerName)
}
return c.tlsCfg.ServerName, nil
}
if c.ServerName != "" {
return c.ServerName, nil
}
return u.Hostname(), nil
}

func (c *X509Cert) getCert(u *url.URL, timeout time.Duration) ([]*x509.Certificate, error) {
switch u.Scheme {
case "https":
Expand All @@ -87,11 +100,11 @@ func (c *X509Cert) getCert(u *url.URL, timeout time.Duration) ([]*x509.Certifica
}
defer ipConn.Close()

if c.ServerName == "" {
c.tlsCfg.ServerName = u.Hostname()
} else {
c.tlsCfg.ServerName = c.ServerName
serverName, err := c.serverName(u)
if err != nil {
return nil, err
}
c.tlsCfg.ServerName = serverName

c.tlsCfg.InsecureSkipVerify = true
conn := tls.Client(ipConn, c.tlsCfg)
Expand Down Expand Up @@ -218,10 +231,9 @@ func (c *X509Cert) Gather(acc telegraf.Accumulator) error {
KeyUsages: []x509.ExtKeyUsage{x509.ExtKeyUsageAny},
}
if i == 0 {
if c.ServerName == "" {
opts.DNSName = u.Hostname()
} else {
opts.DNSName = c.ServerName
opts.DNSName, err = c.serverName(u)
if err != nil {
return err
}
for j, cert := range certs {
if j != 0 {
Expand Down
38 changes: 38 additions & 0 deletions plugins/inputs/x509_cert/x509_cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io/ioutil"
"math/big"
"net/url"
"os"
"path/filepath"
"runtime"
Expand All @@ -17,6 +18,7 @@ import (

"github.com/influxdata/telegraf"
"github.com/influxdata/telegraf/internal"
_tls "github.com/influxdata/telegraf/plugins/common/tls"
"github.com/influxdata/telegraf/testutil"
)

Expand Down Expand Up @@ -347,3 +349,39 @@ func TestGatherCert(t *testing.T) {

assert.True(t, acc.HasMeasurement("x509_cert"))
}

func TestServerName(t *testing.T) {
tests := []struct {
name string
fromTLS string
fromCfg string
url string
expected string
err bool
}{
{name: "in cfg", fromCfg: "example.com", url: "https://other.example.com", expected: "example.com"},
{name: "in tls", fromTLS: "example.com", url: "https://other.example.com", expected: "example.com"},
{name: "from URL", url: "https://other.example.com", expected: "other.example.com"},
{name: "errors", fromCfg: "otherex.com", fromTLS: "example.com", url: "https://other.example.com", err: true},
}

for _, elt := range tests {
test := elt
t.Run(test.name, func(t *testing.T) {
sc := &X509Cert{
ServerName: test.fromCfg,
ClientConfig: _tls.ClientConfig{ServerName: test.fromTLS},
}
sc.Init()
u, err := url.Parse(test.url)
require.NoError(t, err)
actual, err := sc.serverName(u)
if test.err {
assert.Error(t, err)
} else {
assert.NoError(t, err)
}
assert.Equal(t, test.expected, actual)
})
}
}

0 comments on commit 2f82d30

Please sign in to comment.