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

Add minimum and maximum tls version support in tls config #3567

Merged
merged 4 commits into from
Mar 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions pkg/config/tlscfg/ciphersuites.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ import (
"fmt"
)

// https://pkg.go.dev/crypto/tls#pkg-constants
var versions = map[string]uint16{
"1.0": tls.VersionTLS10,
"1.1": tls.VersionTLS11,
"1.2": tls.VersionTLS12,
"1.3": tls.VersionTLS13,
}

func allCiphers() map[string]uint16 {
acceptedCiphers := make(map[string]uint16)
for _, suite := range tls.CipherSuites() {
Expand All @@ -40,3 +48,11 @@ func CipherSuiteNamesToIDs(cipherNames []string) ([]uint16, error) {
}
return ciphersIDs, nil
}

// VersionNameToID returns the version ID from version name
func VersionNameToID(versionName string) (uint16, error) {
if version, ok := versions[versionName]; ok {
return version, nil
}
return 0, fmt.Errorf("unknown tls version %q", versionName)
}
33 changes: 32 additions & 1 deletion pkg/config/tlscfg/ciphersuites_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
"testing"
)

func TestTLSCipherSuites(t *testing.T) {
func TestCipherSuiteNamesToIDs(t *testing.T) {
tests := []struct {
flag []string
expected []uint16
Expand Down Expand Up @@ -68,3 +68,34 @@ func TestTLSCipherSuites(t *testing.T) {
}
}
}

func TestVersionNameToID(t *testing.T) {
tests := []struct {
flag string
expected uint16
expectedError bool
}{
{
// Happy case
flag: "1.1",
expected: tls.VersionTLS11,
expectedError: false,
},
{
// Invalid flag
flag: "Invalid",
expected: 0,
expectedError: true,
},
}

for i, test := range tests {
uIntFlag, err := VersionNameToID(test.flag)
if !reflect.DeepEqual(uIntFlag, test.expected) {
t.Errorf("%d: expected %+v, got %+v", i, test.expected, uIntFlag)
}
if test.expectedError && err == nil {
t.Errorf("%d: expecting error, got %+v", i, err)
}
}
}
6 changes: 6 additions & 0 deletions pkg/config/tlscfg/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ const (
tlsClientCA = tlsPrefix + ".client-ca"
tlsSkipHostVerify = tlsPrefix + ".skip-host-verify"
tlsCipherSuites = tlsPrefix + ".cipher-suites"
tlsMinVersion = tlsPrefix + ".min-version"
tlsMaxVersion = tlsPrefix + ".max-version"
)

// ClientFlagsConfig describes which CLI flags for TLS client should be generated.
Expand Down Expand Up @@ -60,6 +62,8 @@ func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) {
flags.String(c.Prefix+tlsKey, "", "Path to a TLS Private Key file, used to identify this server to clients")
flags.String(c.Prefix+tlsClientCA, "", "Path to a TLS CA (Certification Authority) file used to verify certificates presented by clients (if unset, all clients are permitted)")
flags.String(c.Prefix+tlsCipherSuites, "", "Comma-separated list of cipher suites for the server, values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants).")
flags.String(c.Prefix+tlsMinVersion, "", "Minimum TLS version supported (Possible values: 1.0, 1.1, 1.2, 1.3)")
flags.String(c.Prefix+tlsMaxVersion, "", "Maximum TLS version supported (Possible values: 1.0, 1.1, 1.2, 1.3)")
}

// InitFromViper creates tls.Config populated with values retrieved from Viper.
Expand All @@ -82,6 +86,8 @@ func (c ServerFlagsConfig) InitFromViper(v *viper.Viper) Options {
p.KeyPath = v.GetString(c.Prefix + tlsKey)
p.ClientCAPath = v.GetString(c.Prefix + tlsClientCA)
p.CipherSuites = strings.Split(stripWhiteSpace(v.GetString(c.Prefix+tlsCipherSuites)), ",")
p.MinVersion = v.GetString(c.Prefix + tlsMinVersion)
p.MaxVersion = v.GetString(c.Prefix + tlsMaxVersion)
return p
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/config/tlscfg/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ func TestServerFlags(t *testing.T) {
"--prefix.tls.cert=cert-file",
"--prefix.tls.key=key-file",
"--prefix.tls.cipher-suites=TLS_AES_256_GCM_SHA384,TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
"--prefix.tls.min-version=1.2",
"--prefix.tls.max-version=1.3",
}

tests := []struct {
Expand Down Expand Up @@ -109,6 +111,8 @@ func TestServerFlags(t *testing.T) {
KeyPath: "key-file",
ClientCAPath: test.file,
CipherSuites: []string{"TLS_AES_256_GCM_SHA384", "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"},
MinVersion: "1.2",
MaxVersion: "1.3",
}, tlsOpts)
})
}
Expand Down
26 changes: 26 additions & 0 deletions pkg/config/tlscfg/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type Options struct {
ServerName string `mapstructure:"server_name"` // only for client-side TLS config
ClientCAPath string `mapstructure:"client_ca"` // only for server-side TLS config for client auth
CipherSuites []string `mapstructure:"cipher_suites"`
MinVersion string `mapstructure:"min_version"`
MaxVersion string `mapstructure:"max_version"`
SkipHostVerify bool `mapstructure:"skip_host_verify"`
certWatcher *certWatcher `mapstructure:"-"`
}
Expand All @@ -42,6 +44,8 @@ var systemCertPool = x509.SystemCertPool // to allow overriding in unit test

// Config loads TLS certificates and returns a TLS Config.
func (p *Options) Config(logger *zap.Logger) (*tls.Config, error) {
var minVersionId, maxVersionId uint16

certPool, err := p.loadCertPool()
if err != nil {
return nil, fmt.Errorf("failed to load CA CertPool: %w", err)
Expand All @@ -52,12 +56,34 @@ func (p *Options) Config(logger *zap.Logger) (*tls.Config, error) {
return nil, fmt.Errorf("failed to get cipher suite ids from cipher suite names: %w", err)
}

if p.MinVersion != "" {
minVersionId, err = VersionNameToID(p.MinVersion)
if err != nil {
return nil, fmt.Errorf("failed to get minimum tls version: %w", err)
}
}

if p.MaxVersion != "" {
maxVersionId, err = VersionNameToID(p.MaxVersion)
if err != nil {
return nil, fmt.Errorf("failed to get maximum tls version: %w", err)
}
}

if p.MinVersion != "" && p.MaxVersion != "" {
if minVersionId > maxVersionId {
return nil, fmt.Errorf("minimum tls version can't be greater than maximum tls version")
}
}

// #nosec G402
tlsCfg := &tls.Config{
RootCAs: certPool,
ServerName: p.ServerName,
InsecureSkipVerify: p.SkipHostVerify,
CipherSuites: cipherSuiteIds,
MinVersion: minVersionId,
MaxVersion: maxVersionId,
}

if p.ClientCAPath != "" {
Expand Down
28 changes: 25 additions & 3 deletions pkg/config/tlscfg/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,25 +109,47 @@ func TestOptionsToConfig(t *testing.T) {
expectError: "failed to load CA",
},
{
name: "should fail with invalid Client CA pool",
name: "should fail with invalid TLS Client CA pool",
options: Options{
ClientCAPath: testCertKeyLocation + "/bad-CA-cert.txt",
},
expectError: "failed to parse CA",
},
{
name: "should pass with valid Client CA pool",
name: "should pass with valid TLS Client CA pool",
options: Options{
ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
},
},
{
name: "should fail with invalid Cipher Suite",
name: "should fail with invalid TLS Cipher Suite",
options: Options{
CipherSuites: []string{"TLS_INVALID_CIPHER_SUITE"},
},
expectError: "failed to get cipher suite ids from cipher suite names: cipher suite TLS_INVALID_CIPHER_SUITE not supported or doesn't exist",
},
{
name: "should fail with invalid TLS Min Version",
options: Options{
MinVersion: "Invalid",
},
expectError: "failed to get minimum tls version",
},
{
name: "should fail with invalid TLS Max Version",
options: Options{
MaxVersion: "Invalid",
},
expectError: "failed to get maximum tls version",
},
{
name: "should fail with TLS Min Version greater than TLS Max Version error",
options: Options{
MinVersion: "1.2",
MaxVersion: "1.1",
},
expectError: "minimum tls version can't be greater than maximum tls version",
},
}

for _, test := range tests {
Expand Down