Merge pull request #4565 from hashicorp/b-compare-cert-alg

Error if TLS Certificate signature algorithm isn't supported in cipher suites
This commit is contained in:
Chelsea Komlo
2018-08-15 16:09:46 -04:00
committed by GitHub
5 changed files with 188 additions and 34 deletions

View File

@@ -844,7 +844,7 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error {
return err
}
if _, err := tlsutil.ParseCiphers(tlsConfig.TLSCipherSuites); err != nil {
if _, err := tlsutil.ParseCiphers(&tlsConfig); err != nil {
return err
}

View File

@@ -286,7 +286,19 @@ func TestConfig_Parse(t *testing.T) {
if (err != nil) != tc.Err {
t.Fatalf("file: %s\n\n%s", tc.File, err)
}
require.EqualValues(actual, tc.Result)
//panic(fmt.Sprintf("first: %+v \n second: %+v", actual.TLSConfig, tc.Result.TLSConfig))
require.EqualValues(removeHelperAttributes(actual), tc.Result)
})
}
}
// In order to compare the Config struct after parsing, and from generating what
// is expected in the test, we need to remove helper attributes that are
// instantiated in the process of parsing the configuration
func removeHelperAttributes(c *Config) *Config {
if c.TLSConfig != nil {
c.TLSConfig.KeyLoader = nil
}
return c
}

View File

@@ -1,6 +1,8 @@
package tlsutil
import (
"crypto/ecdsa"
"crypto/rsa"
"crypto/tls"
"crypto/x509"
"encoding/pem"
@@ -41,6 +43,36 @@ var supportedTLSCiphers = map[string]uint16{
"TLS_RSA_WITH_AES_256_CBC_SHA": tls.TLS_RSA_WITH_AES_256_CBC_SHA,
}
// signatureAlgorithm is the string representation of a signing algorithm
type signatureAlgorithm string
const (
rsaStringRepr signatureAlgorithm = "RSA"
ecdsaStringRepr signatureAlgorithm = "ECDSA"
)
// supportedCipherSignatures is the supported cipher suites with their
// corresponding signature algorithm
var supportedCipherSignatures = map[string]signatureAlgorithm{
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305": rsaStringRepr,
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305": ecdsaStringRepr,
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256": rsaStringRepr,
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256": ecdsaStringRepr,
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384": rsaStringRepr,
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": ecdsaStringRepr,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256": rsaStringRepr,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA": rsaStringRepr,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256": ecdsaStringRepr,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA": ecdsaStringRepr,
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": rsaStringRepr,
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA": ecdsaStringRepr,
"TLS_RSA_WITH_AES_128_GCM_SHA256": rsaStringRepr,
"TLS_RSA_WITH_AES_256_GCM_SHA384": rsaStringRepr,
"TLS_RSA_WITH_AES_128_CBC_SHA256": rsaStringRepr,
"TLS_RSA_WITH_AES_128_CBC_SHA": rsaStringRepr,
"TLS_RSA_WITH_AES_256_CBC_SHA": rsaStringRepr,
}
// defaultTLSCiphers are the TLS Ciphers that are supported by default
var defaultTLSCiphers = []string{
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
@@ -125,7 +157,7 @@ type Config struct {
}
func NewTLSConfiguration(newConf *config.TLSConfig, verifyIncoming, verifyOutgoing bool) (*Config, error) {
ciphers, err := ParseCiphers(newConf.TLSCipherSuites)
ciphers, err := ParseCiphers(newConf)
if err != nil {
return nil, err
}
@@ -385,19 +417,19 @@ func (c *Config) IncomingTLSConfig() (*tls.Config, error) {
// ParseCiphers parses ciphersuites from the comma-separated string into
// recognized slice
func ParseCiphers(cipherStr string) ([]uint16, error) {
func ParseCiphers(tlsConfig *config.TLSConfig) ([]uint16, error) {
suites := []uint16{}
cipherStr = strings.TrimSpace(cipherStr)
cipherStr := strings.TrimSpace(tlsConfig.TLSCipherSuites)
var ciphers []string
var parsedCiphers []string
if cipherStr == "" {
ciphers = defaultTLSCiphers
parsedCiphers = defaultTLSCiphers
} else {
ciphers = strings.Split(cipherStr, ",")
parsedCiphers = strings.Split(tlsConfig.TLSCipherSuites, ",")
}
for _, cipher := range ciphers {
for _, cipher := range parsedCiphers {
c, ok := supportedTLSCiphers[cipher]
if !ok {
return suites, fmt.Errorf("unsupported TLS cipher %q", cipher)
@@ -405,7 +437,54 @@ func ParseCiphers(cipherStr string) ([]uint16, error) {
suites = append(suites, c)
}
return suites, nil
// Ensure that the specified cipher suite list is supported by the TLS
// Certificate signature algorithm. This is a check for user error, where a
// TLS certificate could support RSA but a user has configured a cipher suite
// list of ciphers where only ECDSA is supported.
keyLoader := tlsConfig.GetKeyLoader()
// Ensure that the keypair has been loaded before continuing
keyLoader.LoadKeyPair(tlsConfig.CertFile, tlsConfig.KeyFile)
if keyLoader.GetCertificate() != nil {
supportedSignatureAlgorithm, err := getSignatureAlgorithm(keyLoader.GetCertificate())
if err != nil {
return []uint16{}, err
}
for _, cipher := range parsedCiphers {
if supportedCipherSignatures[cipher] == supportedSignatureAlgorithm {
// Positive case, return the matched cipher suites as the signature
// algorithm is also supported
return suites, nil
}
}
// Negative case, if this is reached it means that none of the specified
// cipher suites signature algorithms match the signature algorithm
// for the certificate.
return []uint16{}, fmt.Errorf("Specified cipher suites don't support the certificate signature algorithm %s, consider adding more cipher suites to match this signature algorithm.", supportedSignatureAlgorithm)
}
// Default in case this function is called but TLS is not actually configured
// This is only reached if the TLS certificate is nil
return []uint16{}, nil
}
// getSignatureAlgorithm returns the signature algorithm for a TLS certificate
// This is determined by examining the type of the certificate's public key,
// as Golang doesn't expose a more straightforward API which returns this
// type
func getSignatureAlgorithm(tlsCert *tls.Certificate) (signatureAlgorithm, error) {
privKey := tlsCert.PrivateKey
switch privKey.(type) {
case *rsa.PrivateKey:
return rsaStringRepr, nil
case *ecdsa.PrivateKey:
return ecdsaStringRepr, nil
default:
return "", fmt.Errorf("Unsupported signature algorithm %T; RSA and ECDSA only are supported.", privKey)
}
}
// ParseMinVersion parses the specified minimum TLS version for the Nomad agent

View File

@@ -644,28 +644,35 @@ func TestConfig_IncomingTLS_TLSCipherSuites(t *testing.T) {
}
}
// This test relies on the fact that the specified certificate has an ECDSA
// signature algorithm
func TestConfig_ParseCiphers_Valid(t *testing.T) {
require := require.New(t)
validCiphers := strings.Join([]string{
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
"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_AES_128_CBC_SHA256",
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
"TLS_RSA_WITH_AES_128_GCM_SHA256",
"TLS_RSA_WITH_AES_256_GCM_SHA384",
"TLS_RSA_WITH_AES_128_CBC_SHA256",
"TLS_RSA_WITH_AES_128_CBC_SHA",
"TLS_RSA_WITH_AES_256_CBC_SHA",
}, ",")
tlsConfig := &config.TLSConfig{
CertFile: foocert,
KeyFile: fookey,
KeyLoader: &config.KeyLoader{},
TLSCipherSuites: strings.Join([]string{
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
"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_AES_128_CBC_SHA256",
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256",
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA",
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
"TLS_RSA_WITH_AES_128_GCM_SHA256",
"TLS_RSA_WITH_AES_256_GCM_SHA384",
"TLS_RSA_WITH_AES_128_CBC_SHA256",
"TLS_RSA_WITH_AES_128_CBC_SHA",
"TLS_RSA_WITH_AES_256_CBC_SHA",
}, ","),
}
expectedCiphers := []uint16{
tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
@@ -687,11 +694,13 @@ func TestConfig_ParseCiphers_Valid(t *testing.T) {
tls.TLS_RSA_WITH_AES_256_CBC_SHA,
}
parsedCiphers, err := ParseCiphers(validCiphers)
parsedCiphers, err := ParseCiphers(tlsConfig)
require.Nil(err)
require.Equal(parsedCiphers, expectedCiphers)
}
// This test relies on the fact that the specified certificate has an ECDSA
// signature algorithm
func TestConfig_ParseCiphers_Default(t *testing.T) {
require := require.New(t)
@@ -708,11 +717,18 @@ func TestConfig_ParseCiphers_Default(t *testing.T) {
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
}
parsedCiphers, err := ParseCiphers("")
empty := &config.TLSConfig{
CertFile: foocert,
KeyFile: fookey,
KeyLoader: &config.KeyLoader{},
}
parsedCiphers, err := ParseCiphers(empty)
require.Nil(err)
require.Equal(parsedCiphers, expectedCiphers)
}
// This test relies on the fact that the specified certificate has an ECDSA
// signature algorithm
func TestConfig_ParseCiphers_Invalid(t *testing.T) {
require := require.New(t)
@@ -722,13 +738,51 @@ func TestConfig_ParseCiphers_Invalid(t *testing.T) {
}
for _, cipher := range invalidCiphers {
parsedCiphers, err := ParseCiphers(cipher)
tlsConfig := &config.TLSConfig{
TLSCipherSuites: cipher,
CertFile: foocert,
KeyFile: fookey,
KeyLoader: &config.KeyLoader{},
}
parsedCiphers, err := ParseCiphers(tlsConfig)
require.NotNil(err)
require.Equal(fmt.Sprintf("unsupported TLS cipher %q", cipher), err.Error())
require.Equal(0, len(parsedCiphers))
}
}
// This test relies on the fact that the specified certificate has an ECDSA
// signature algorithm
func TestConfig_ParseCiphers_SupportedSignature(t *testing.T) {
require := require.New(t)
// Supported signature
{
tlsConfig := &config.TLSConfig{
TLSCipherSuites: "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305",
CertFile: foocert,
KeyFile: fookey,
KeyLoader: &config.KeyLoader{},
}
parsedCiphers, err := ParseCiphers(tlsConfig)
require.Nil(err)
require.Equal(1, len(parsedCiphers))
}
// Unsupported signature
{
tlsConfig := &config.TLSConfig{
TLSCipherSuites: "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305",
CertFile: foocert,
KeyFile: fookey,
KeyLoader: &config.KeyLoader{},
}
parsedCiphers, err := ParseCiphers(tlsConfig)
require.NotNil(err)
require.Equal(0, len(parsedCiphers))
}
}
func TestConfig_ParseMinVersion_Valid(t *testing.T) {
require := require.New(t)
@@ -769,7 +823,10 @@ func TestConfig_NewTLSConfiguration(t *testing.T) {
require := require.New(t)
conf := &config.TLSConfig{
TLSCipherSuites: "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
TLSCipherSuites: "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256",
CertFile: foocert,
KeyFile: fookey,
KeyLoader: &config.KeyLoader{},
}
tlsConf, err := NewTLSConfiguration(conf, true, true)
@@ -778,7 +835,7 @@ func TestConfig_NewTLSConfiguration(t *testing.T) {
require.True(tlsConf.VerifyOutgoing)
expectedCiphers := []uint16{
tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
}
require.Equal(tlsConf.CipherSuites, expectedCiphers)

View File

@@ -97,6 +97,12 @@ func (k *KeyLoader) LoadKeyPair(certFile, keyFile string) (*tls.Certificate, err
return k.certificate, nil
}
func (k *KeyLoader) GetCertificate() *tls.Certificate {
k.cacheLock.Lock()
defer k.cacheLock.Unlock()
return k.certificate
}
// GetOutgoingCertificate fetches the currently-loaded certificate when
// accepting a TLS connection. This currently does not consider information in
// the ClientHello and only returns the certificate that was last loaded.