Skip to content

Commit

Permalink
Adding min version and cipher suite variables to tls client configura…
Browse files Browse the repository at this point in the history 
…tion (#217)

* Adding min version and cipher suite variables to tls configuration

* Addressing review comments

* Addressing review comments

* Generating cli flags help text

* Move long cipher suites description to auto-generated doc only

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Co-authored-by: Marco Pracucci <marco@pracucci.com>
  • Loading branch information
@Red-GV @pracucci
Red-GV and pracucci committed Sep 28, 2022
1 parent 04c68bb commit b1b307d
Show file tree
Hide file tree
Showing 3 changed files with 253 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
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
* [ENHANCEMENT] Concurrency: Add LimitedConcurrencySingleFlight to run jobs concurrently and with in-flight deduplication. 214
* [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
Expand Down
79 changes: 79 additions & 0 deletions crypto/tls/tls.go
View file
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" doc:"description_method=GetTLSCipherSuitesLongDescription"`
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,28 @@ 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", "", cfg.GetTLSCipherSuitesShortDescription())
f.StringVar(&cfg.MinVersion, prefix+".tls-min-version", "", "Override the default minimum TLS version. Allowed values: VersionTLS10, VersionTLS11, VersionTLS12, VersionTLS13")
}

func (cfg *ClientConfig) GetTLSCipherSuitesShortDescription() string {
return "Override the default cipher suite list (separated by commas)."
}

func (cfg *ClientConfig) GetTLSCipherSuitesLongDescription() string {
text := cfg.GetTLSCipherSuitesShortDescription() + " 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
}

// GetTLSConfig initialises tls.Config from config options
Expand Down Expand Up @@ -69,6 +102,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 +136,31 @@ 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]
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
}
173 changes: 173 additions & 0 deletions crypto/tls/tls_test.go
View file
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)
}
})
}
}

0 comments on commit b1b307d

Please sign in to comment.