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

Adding min version and cipher suite variables to tls client configuration #217

Merged
merged 6 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
* [ENHANCEMENT] Memberlist: Add cluster label support to memberlist client. #187
* [ENHANCEMENT] Runtimeconfig: Don't unmarshal and merge runtime config yaml files if they haven't changed since last check. #218
* [ENHANCEMENT] ring: DoBatch now differentiates between 4xx and 5xx GRPC errors and keeps track of them separately. It only returns when there is a quorum of either error class. If your errors do not implement `GRPCStatus() *Status` from google.golang.org/grpc/status, then this change does not affect you. #201
* [ENHANCEMENT] Added `<prefix>.tls-min-version` and `<prefix>.tls-cipher-suites` flags to client configurations. #217
* [BUGFIX] spanlogger: Support multiple tenant IDs. #59
* [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85
* [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109
Expand Down
75 changes: 75 additions & 0 deletions crypto/tls/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"crypto/tls"
"crypto/x509"
"flag"
"fmt"
"os"
"strings"

"github.com/pkg/errors"
"google.golang.org/grpc"
Expand All @@ -18,11 +20,20 @@ type ClientConfig struct {
CAPath string `yaml:"tls_ca_path" category:"advanced"`
ServerName string `yaml:"tls_server_name" category:"advanced"`
InsecureSkipVerify bool `yaml:"tls_insecure_skip_verify" category:"advanced"`
CipherSuites string `yaml:"tls_cipher_suites" category:"advanced"`
MinVersion string `yaml:"tls_min_version" category:"advanced"`
}

var (
errKeyMissing = errors.New("certificate given but no key configured")
errCertMissing = errors.New("key given but no certificate configured")

tlsVersions = map[string]uint16{
"VersionTLS10": tls.VersionTLS10,
"VersionTLS11": tls.VersionTLS11,
"VersionTLS12": tls.VersionTLS12,
"VersionTLS13": tls.VersionTLS13,
}
)

// RegisterFlagsWithPrefix registers flags with prefix.
Expand All @@ -32,6 +43,8 @@ func (cfg *ClientConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet)
f.StringVar(&cfg.CAPath, prefix+".tls-ca-path", "", "Path to the CA certificates file to validate server certificate against. If not set, the host's root CA certificates are used.")
f.StringVar(&cfg.ServerName, prefix+".tls-server-name", "", "Override the expected name on the server certificate.")
f.BoolVar(&cfg.InsecureSkipVerify, prefix+".tls-insecure-skip-verify", false, "Skip validating server certificate.")
f.StringVar(&cfg.CipherSuites, prefix+".tls-cipher-suites", "", tlsCipherSuiteHelpText())
f.StringVar(&cfg.MinVersion, prefix+".tls-min-version", "", "Override the default minimum TLS version. Allowed values: VersionTLS10, VersionTLS11, VersionTLS12, VersionTLS13")
}

// GetTLSConfig initialises tls.Config from config options
Expand Down Expand Up @@ -69,6 +82,24 @@ func (cfg *ClientConfig) GetTLSConfig() (*tls.Config, error) {
config.Certificates = []tls.Certificate{clientCert}
}

if cfg.MinVersion != "" {
minVersion, ok := tlsVersions[cfg.MinVersion]
if !ok {
return nil, fmt.Errorf("unknown minimum TLS version: %q", cfg.MinVersion)
}
config.MinVersion = minVersion
}

if cfg.CipherSuites != "" {
cleanedCipherSuiteNames := strings.ReplaceAll(cfg.CipherSuites, " ", "")
cipherSuitesNames := strings.Split(cleanedCipherSuiteNames, ",")
cipherSuites, err := mapCipherNamesToIDs(cipherSuitesNames)
if err != nil {
return nil, err
}
config.CipherSuites = cipherSuites
}

return config, nil
}

Expand All @@ -85,3 +116,47 @@ func (cfg *ClientConfig) GetGRPCDialOptions(enabled bool) ([]grpc.DialOption, er

return []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))}, nil
}

func mapCipherNamesToIDs(cipherSuiteNames []string) ([]uint16, error) {
cipherSuites := []uint16{}
allCipherSuites := tlsCipherSuites()

for _, name := range cipherSuiteNames {
id, ok := allCipherSuites[name]
Red-GV marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
return nil, fmt.Errorf("unsupported cipher suite: %q", name)
}
cipherSuites = append(cipherSuites, id)
}

return cipherSuites, nil
}

func tlsCipherSuites() map[string]uint16 {
cipherSuites := map[string]uint16{}

for _, suite := range tls.CipherSuites() {
cipherSuites[suite.Name] = suite.ID
}
for _, suite := range tls.InsecureCipherSuites() {
cipherSuites[suite.Name] = suite.ID
}

return cipherSuites
}

func tlsCipherSuiteHelpText() string {
text := "Override the default cipher suite list (separated by commas). Allowed values:\n\n"

text += "Secure Ciphers:\n"
for _, suite := range tls.CipherSuites() {
text += fmt.Sprintf("- %s\n", suite.Name)
}

text += "\nInsecure Ciphers:\n"
for _, suite := range tls.InsecureCipherSuites() {
text += fmt.Sprintf("- %s\n", suite.Name)
}

return text
}
173 changes: 173 additions & 0 deletions crypto/tls/tls_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tls

import (
"crypto/tls"
"os"
"path/filepath"
"testing"
Expand Down Expand Up @@ -182,3 +183,175 @@ func TestGetTLSConfig_ServerName(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "myserver.com", tlsConfig.ServerName)
}

func TestGetTLSConfig_MinVersion(t *testing.T) {
type test struct {
desc string
MinVersion string
ExpectedVersion uint16
RequireError bool
}

table := []test{
{
desc: "no version set",
MinVersion: "",
ExpectedVersion: 0,
RequireError: false,
},
{
desc: "TLS v1.0 set",
MinVersion: "VersionTLS10",
ExpectedVersion: tls.VersionTLS10,
RequireError: false,
},
{
desc: "TLS v1.1 set",
MinVersion: "VersionTLS11",
ExpectedVersion: tls.VersionTLS11,
RequireError: false,
},
{
desc: "TLS v1.2 set",
MinVersion: "VersionTLS12",
ExpectedVersion: tls.VersionTLS12,
RequireError: false,
},
{
desc: "TLS v1.3 set",
MinVersion: "VersionTLS13",
ExpectedVersion: tls.VersionTLS13,
RequireError: false,
},
{
desc: "bad TLS version set",
MinVersion: "VersionTLS14",
ExpectedVersion: 0,
RequireError: true,
},
}

for _, tst := range table {
tst := tst
t.Run(tst.desc, func(t *testing.T) {
t.Parallel()

c := &ClientConfig{
MinVersion: tst.MinVersion,
}

tlsConfig, err := c.GetTLSConfig()

if tst.RequireError {
assert.Error(t, err)
assert.Nil(t, tlsConfig)
} else {
assert.NoError(t, err)
assert.Equal(t, tst.ExpectedVersion, tlsConfig.MinVersion)
}
})
}
}

func TestGetTLSConfig_CipherSuites(t *testing.T) {
type test struct {
desc string
CipherSuites string
ExpectedCipherSuites []uint16
RequireError bool
}

cipherSuiteNames := "TLS_RSA_WITH_RC4_128_SHA," +
"TLS_RSA_WITH_3DES_EDE_CBC_SHA," +
"TLS_RSA_WITH_AES_128_CBC_SHA," +
"TLS_RSA_WITH_AES_256_CBC_SHA," +
"TLS_RSA_WITH_AES_128_CBC_SHA256," +
"TLS_RSA_WITH_AES_128_GCM_SHA256," +
"TLS_RSA_WITH_AES_256_GCM_SHA384," +
"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA," +
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA," +
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA," +
"TLS_ECDHE_RSA_WITH_RC4_128_SHA," +
"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA," +
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA," +
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA," +
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256," +
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256," +
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256," +
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256," +
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384," +
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384," +
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256," +
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256," +
"TLS_AES_128_GCM_SHA256," +
"TLS_AES_256_GCM_SHA384," +
"TLS_CHACHA20_POLY1305_SHA256"

table := []test{
{
desc: "no cipher suites set",
CipherSuites: "",
ExpectedCipherSuites: nil,
RequireError: false,
},
{
desc: "all cipher suites set",
CipherSuites: cipherSuiteNames,
ExpectedCipherSuites: []uint16{
tls.TLS_RSA_WITH_RC4_128_SHA,
tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA,
tls.TLS_RSA_WITH_AES_128_CBC_SHA,
tls.TLS_RSA_WITH_AES_256_CBC_SHA,
tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA,
tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256,
tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256,
tls.TLS_AES_128_GCM_SHA256,
tls.TLS_AES_256_GCM_SHA384,
tls.TLS_CHACHA20_POLY1305_SHA256,
},
RequireError: false,
},
{
desc: "bad cipher suites set",
CipherSuites: "TLS_NO_SUITE",
ExpectedCipherSuites: nil,
RequireError: true,
},
}

for _, tst := range table {
tst := tst
t.Run(tst.desc, func(t *testing.T) {
t.Parallel()

c := &ClientConfig{
CipherSuites: tst.CipherSuites,
}

tlsConfig, err := c.GetTLSConfig()

if tst.RequireError {
assert.Error(t, err)
assert.Nil(t, tlsConfig)
} else {
assert.NoError(t, err)
assert.Equal(t, tst.ExpectedCipherSuites, tlsConfig.CipherSuites)
}
})
}
}