From e96bbe83c564dcce7c14f5e0ecd221845136a891 Mon Sep 17 00:00:00 2001 From: Sven Rebhan <36194019+srebhan@users.noreply.github.com> Date: Tue, 9 Aug 2022 13:36:47 +0200 Subject: [PATCH] feat(common.tls): Implement minimum TLS version for clients (#11493) --- CHANGELOG.md | 4 + plugins/common/tls/config.go | 21 +++ plugins/common/tls/config_test.go | 136 ++++++++++++++++++ plugins/common/tls/utils.go | 10 +- plugins/inputs/gnmi/README.md | 3 + plugins/inputs/gnmi/sample.conf | 3 + plugins/inputs/http/README.md | 2 + plugins/inputs/http/sample.conf | 2 + plugins/inputs/http_listener_v2/README.md | 5 + plugins/inputs/http_listener_v2/sample.conf | 3 + .../inputs/jti_openconfig_telemetry/README.md | 6 + .../jti_openconfig_telemetry/sample.conf | 2 + 12 files changed, 196 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55631e893..c4a967b9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog +### BREAKING CHANGES + +- [#11493](https://github.com/influxdata/telegraf/pull/11493) `common.tls` Set default minimum TLS version to v1.2 for security reasons on both server and client connections. This is a change from the previous defaults (TLS v1.0) on the server configuration and might break clients relying on older TLS versions. You can manually revert to older versions on a per-plugin basis using the `tls_min_version` option in the plugins required. + ## v1.23.3 [2022-07-25] ### Bugfixes diff --git a/plugins/common/tls/config.go b/plugins/common/tls/config.go index 558900d07..0f2c40028 100644 --- a/plugins/common/tls/config.go +++ b/plugins/common/tls/config.go @@ -10,12 +10,15 @@ import ( "github.com/influxdata/telegraf/internal/choice" ) +const TLSMinVersionDefault = tls.VersionTLS12 + // ClientConfig represents the standard client TLS config. type ClientConfig struct { TLSCA string `toml:"tls_ca"` TLSCert string `toml:"tls_cert"` TLSKey string `toml:"tls_key"` TLSKeyPwd string `toml:"tls_key_pwd"` + TLSMinVersion string `toml:"tls_min_version"` InsecureSkipVerify bool `toml:"insecure_skip_verify"` ServerName string `toml:"tls_server_name"` @@ -81,6 +84,19 @@ func (c *ClientConfig) TLSConfig() (*tls.Config, error) { } } + // Explicitly and consistently set the minimal accepted version using the + // defined default. We use this setting for both clients and servers + // instead of relying on Golang's default that is different for clients + // and servers and might change over time. + tlsConfig.MinVersion = TLSMinVersionDefault + if c.TLSMinVersion != "" { + version, err := ParseTLSVersion(c.TLSMinVersion) + if err != nil { + return nil, fmt.Errorf("could not parse tls min version %q: %w", c.TLSMinVersion, err) + } + tlsConfig.MinVersion = version + } + if c.ServerName != "" { tlsConfig.ServerName = c.ServerName } @@ -131,6 +147,11 @@ func (c *ServerConfig) TLSConfig() (*tls.Config, error) { tlsConfig.MaxVersion = version } + // Explicitly and consistently set the minimal accepted version using the + // defined default. We use this setting for both clients and servers + // instead of relying on Golang's default that is different for clients + // and servers and might change over time. + tlsConfig.MinVersion = TLSMinVersionDefault if c.TLSMinVersion != "" { version, err := ParseTLSVersion(c.TLSMinVersion) if err != nil { diff --git a/plugins/common/tls/config_test.go b/plugins/common/tls/config_test.go index 123523bb5..32b53b24e 100644 --- a/plugins/common/tls/config_test.go +++ b/plugins/common/tls/config_test.go @@ -1,6 +1,7 @@ package tls_test import ( + cryptotls "crypto/tls" "net/http" "net/http/httptest" "testing" @@ -350,6 +351,141 @@ func TestConnect(t *testing.T) { require.Equal(t, 200, resp.StatusCode) } +func TestConnectClientMinTLSVersion(t *testing.T) { + serverConfig := tls.ServerConfig{ + TLSCert: pki.ServerCertPath(), + TLSKey: pki.ServerKeyPath(), + TLSAllowedCACerts: []string{pki.CACertPath()}, + TLSAllowedDNSNames: []string{"localhost", "127.0.0.1"}, + } + + tests := []struct { + name string + cfg tls.ClientConfig + }{ + { + name: "TLS version default", + cfg: tls.ClientConfig{ + TLSCA: pki.CACertPath(), + TLSCert: pki.ClientCertPath(), + TLSKey: pki.ClientKeyPath(), + }, + }, + { + name: "TLS version 1.0", + cfg: tls.ClientConfig{ + TLSCA: pki.CACertPath(), + TLSCert: pki.ClientCertPath(), + TLSKey: pki.ClientKeyPath(), + TLSMinVersion: "TLS10", + }, + }, + { + name: "TLS version 1.1", + cfg: tls.ClientConfig{ + TLSCA: pki.CACertPath(), + TLSCert: pki.ClientCertPath(), + TLSKey: pki.ClientKeyPath(), + TLSMinVersion: "TLS11", + }, + }, + { + name: "TLS version 1.2", + cfg: tls.ClientConfig{ + TLSCA: pki.CACertPath(), + TLSCert: pki.ClientCertPath(), + TLSKey: pki.ClientKeyPath(), + TLSMinVersion: "TLS12", + }, + }, + { + name: "TLS version 1.3", + cfg: tls.ClientConfig{ + TLSCA: pki.CACertPath(), + TLSCert: pki.ClientCertPath(), + TLSKey: pki.ClientKeyPath(), + TLSMinVersion: "TLS13", + }, + }, + } + + tlsVersions := []uint16{ + cryptotls.VersionTLS10, + cryptotls.VersionTLS11, + cryptotls.VersionTLS12, + cryptotls.VersionTLS13, + } + + tlsVersionNames := []string{ + "TLS 1.0", + "TLS 1.1", + "TLS 1.2", + "TLS 1.3", + } + + for _, tt := range tests { + clientTLSConfig, err := tt.cfg.TLSConfig() + require.NoError(t, err) + + client := http.Client{ + Transport: &http.Transport{ + TLSClientConfig: clientTLSConfig, + }, + Timeout: 1 * time.Second, + } + + clientMinVersion := clientTLSConfig.MinVersion + if tt.cfg.TLSMinVersion == "" { + clientMinVersion = tls.TLSMinVersionDefault + } + + for i, serverTLSMaxVersion := range tlsVersions { + serverVersionName := tlsVersionNames[i] + t.Run(tt.name+" vs "+serverVersionName, func(t *testing.T) { + // Constrain the server's maximum TLS version + serverTLSConfig, err := serverConfig.TLSConfig() + require.NoError(t, err) + serverTLSConfig.MinVersion = cryptotls.VersionTLS10 + serverTLSConfig.MaxVersion = serverTLSMaxVersion + + // Start the server + ts := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + ts.TLS = serverTLSConfig + ts.StartTLS() + + // Do the connection and cleanup + resp, err := client.Get(ts.URL) + ts.Close() + + // Things should fail if the currently tested "serverTLSMaxVersion" + // is below the client minimum version. + if serverTLSMaxVersion < clientMinVersion { + require.ErrorContains(t, err, "tls: protocol version not supported") + } else { + require.NoErrorf(t, err, "server=%v client=%v", serverTLSMaxVersion, clientMinVersion) + require.Equal(t, 200, resp.StatusCode) + resp.Body.Close() + } + }) + } + } +} + +func TestConnectClientInvalidMinTLSVersion(t *testing.T) { + clientConfig := tls.ClientConfig{ + TLSCA: pki.CACertPath(), + TLSCert: pki.ClientCertPath(), + TLSKey: pki.ClientKeyPath(), + TLSMinVersion: "garbage", + } + + _, err := clientConfig.TLSConfig() + expected := `could not parse tls min version "garbage": unsupported version "garbage" (available: TLS10,TLS11,TLS12,TLS13)` + require.EqualError(t, err, expected) +} + func TestConnectWrongDNS(t *testing.T) { clientConfig := tls.ClientConfig{ TLSCA: pki.CACertPath(), diff --git a/plugins/common/tls/utils.go b/plugins/common/tls/utils.go index 65388640f..bc7db89a9 100644 --- a/plugins/common/tls/utils.go +++ b/plugins/common/tls/utils.go @@ -2,6 +2,8 @@ package tls import ( "fmt" + "sort" + "strings" ) // ParseCiphers returns a `[]uint16` by received `[]string` key that represents ciphers from crypto/tls. @@ -26,5 +28,11 @@ func ParseTLSVersion(version string) (uint16, error) { if v, ok := tlsVersionMap[version]; ok { return v, nil } - return 0, fmt.Errorf("unsupported version %q", version) + + var available []string + for n := range tlsVersionMap { + available = append(available, n) + } + sort.Strings(available) + return 0, fmt.Errorf("unsupported version %q (available: %s)", version, strings.Join(available, ",")) } diff --git a/plugins/inputs/gnmi/README.md b/plugins/inputs/gnmi/README.md index 6b91f1d1e..15991e960 100644 --- a/plugins/inputs/gnmi/README.md +++ b/plugins/inputs/gnmi/README.md @@ -32,6 +32,9 @@ It has been optimized to support gNMI telemetry as produced by Cisco IOS XR ## enable client-side TLS and define CA to authenticate the device # enable_tls = true # tls_ca = "/etc/telegraf/ca.pem" + ## Minimal TLS version to accept by the client + # tls_min_version = "TLS12" + ## Use TLS but skip chain & host verification # insecure_skip_verify = true ## define client-side TLS certificate & key to authenticate to the device diff --git a/plugins/inputs/gnmi/sample.conf b/plugins/inputs/gnmi/sample.conf index b10f7e984..b0887e0b1 100644 --- a/plugins/inputs/gnmi/sample.conf +++ b/plugins/inputs/gnmi/sample.conf @@ -16,6 +16,9 @@ ## enable client-side TLS and define CA to authenticate the device # enable_tls = true # tls_ca = "/etc/telegraf/ca.pem" + ## Minimal TLS version to accept by the client + # tls_min_version = "TLS12" + ## Use TLS but skip chain & host verification # insecure_skip_verify = true ## define client-side TLS certificate & key to authenticate to the device diff --git a/plugins/inputs/http/README.md b/plugins/inputs/http/README.md index 32808abec..dbf614128 100644 --- a/plugins/inputs/http/README.md +++ b/plugins/inputs/http/README.md @@ -50,6 +50,8 @@ configuration. # tls_ca = "/etc/telegraf/ca.pem" # tls_cert = "/etc/telegraf/cert.pem" # tls_key = "/etc/telegraf/key.pem" + ## Minimal TLS version to accept by the client + # tls_min_version = "TLS12" ## Use TLS but skip chain & host verification # insecure_skip_verify = false diff --git a/plugins/inputs/http/sample.conf b/plugins/inputs/http/sample.conf index 9c38fb9b0..18d702d07 100644 --- a/plugins/inputs/http/sample.conf +++ b/plugins/inputs/http/sample.conf @@ -39,6 +39,8 @@ # tls_ca = "/etc/telegraf/ca.pem" # tls_cert = "/etc/telegraf/cert.pem" # tls_key = "/etc/telegraf/key.pem" + ## Minimal TLS version to accept by the client + # tls_min_version = "TLS12" ## Use TLS but skip chain & host verification # insecure_skip_verify = false diff --git a/plugins/inputs/http_listener_v2/README.md b/plugins/inputs/http_listener_v2/README.md index fefacae42..9f7abea70 100644 --- a/plugins/inputs/http_listener_v2/README.md +++ b/plugins/inputs/http_listener_v2/README.md @@ -49,6 +49,9 @@ InfluxDB it is recommended to use [`influxdb_listener`][influxdb_listener] or # tls_cert = "/etc/telegraf/cert.pem" # tls_key = "/etc/telegraf/key.pem" + ## Minimal TLS version accepted by the server + # tls_min_version = "TLS12" + ## Optional username and password to accept for HTTP basic authentication. ## You probably want to make sure you have TLS configured above for this. # basic_username = "foobar" @@ -71,6 +74,8 @@ InfluxDB it is recommended to use [`influxdb_listener`][influxdb_listener] or Metrics are collected from the part of the request specified by the `data_source` param and are parsed depending on the value of `data_format`. +## Example Output + ## Troubleshooting Send Line Protocol: diff --git a/plugins/inputs/http_listener_v2/sample.conf b/plugins/inputs/http_listener_v2/sample.conf index 102c90e77..e9a0b8155 100644 --- a/plugins/inputs/http_listener_v2/sample.conf +++ b/plugins/inputs/http_listener_v2/sample.conf @@ -33,6 +33,9 @@ # tls_cert = "/etc/telegraf/cert.pem" # tls_key = "/etc/telegraf/key.pem" + ## Minimal TLS version accepted by the server + # tls_min_version = "TLS12" + ## Optional username and password to accept for HTTP basic authentication. ## You probably want to make sure you have TLS configured above for this. # basic_username = "foobar" diff --git a/plugins/inputs/jti_openconfig_telemetry/README.md b/plugins/inputs/jti_openconfig_telemetry/README.md index c325b2305..a638c0cbb 100644 --- a/plugins/inputs/jti_openconfig_telemetry/README.md +++ b/plugins/inputs/jti_openconfig_telemetry/README.md @@ -50,6 +50,8 @@ from listed sensors using Junos Telemetry Interface. Refer to # tls_ca = "/etc/telegraf/ca.pem" # tls_cert = "/etc/telegraf/cert.pem" # tls_key = "/etc/telegraf/key.pem" + ## Minimal TLS version to accept by the client + # tls_min_version = "TLS12" ## Use TLS but skip chain & host verification # insecure_skip_verify = false @@ -65,3 +67,7 @@ from listed sensors using Junos Telemetry Interface. Refer to - All measurements are tagged appropriately using the identifier information in incoming data + +## Example Output + +## Metrics diff --git a/plugins/inputs/jti_openconfig_telemetry/sample.conf b/plugins/inputs/jti_openconfig_telemetry/sample.conf index 1f9872a13..2dcda3d73 100644 --- a/plugins/inputs/jti_openconfig_telemetry/sample.conf +++ b/plugins/inputs/jti_openconfig_telemetry/sample.conf @@ -38,6 +38,8 @@ # tls_ca = "/etc/telegraf/ca.pem" # tls_cert = "/etc/telegraf/cert.pem" # tls_key = "/etc/telegraf/key.pem" + ## Minimal TLS version to accept by the client + # tls_min_version = "TLS12" ## Use TLS but skip chain & host verification # insecure_skip_verify = false