Skip to content

Commit

Permalink
Print warning when 'tls_cipher_suites' includes blacklisted cipher su…
Browse files Browse the repository at this point in the history
…ites (#6300)

* Implemented a warning when tls_cipher_suites includes only cipher suites which are not supprted by the HTTP/2 spec

* Added test for cipher suites

* Added hard fail on startup when all defined cipher suites are blacklisted. Added warning when some ciphers are blacklisted.

* Replaced hard failure with warning. Removed bad cipher util function and replaced it by external library.

* Added missing dependency. Fixed renaming of package name.
  • Loading branch information
michelvocks authored Mar 1, 2019
1 parent 25c12e4 commit 9617832
Show file tree
Hide file tree
Showing 10 changed files with 791 additions and 24 deletions.
29 changes: 29 additions & 0 deletions command/server/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/hashicorp/vault/helper/proxyutil"
"github.com/hashicorp/vault/helper/reload"
"github.com/hashicorp/vault/helper/tlsutil"
"github.com/jefferai/isbadcipher"
"github.com/mitchellh/cli"
)

Expand Down Expand Up @@ -140,6 +141,34 @@ PASSPHRASECORRECT:
if err != nil {
return nil, nil, nil, errwrap.Wrapf("invalid value for 'tls_cipher_suites': {{err}}", err)
}

// HTTP/2 with TLS 1.2 blacklists several cipher suites.
// https://tools.ietf.org/html/rfc7540#appendix-A
//
// Since the CLI (net/http) automatically uses HTTP/2 with TLS 1.2,
// we check here if all or some specified cipher suites are blacklisted.
badCiphers := []string{}
for _, cipher := range ciphers {
if isbadcipher.IsBadCipher(cipher) {
// Get the name of the current cipher.
cipherStr, err := tlsutil.GetCipherName(cipher)
if err != nil {
return nil, nil, nil, errwrap.Wrapf("invalid value for 'tls_cipher_suites': {{err}}", err)
}
badCiphers = append(badCiphers, cipherStr)
}
}
if len(badCiphers) == len(ciphers) {
ui.Warn(`WARNING! All cipher suites defined by 'tls_cipher_suites' are blacklisted by the
HTTP/2 specification. HTTP/2 communication with TLS 1.2 will not work as intended
and Vault will be unavailable via the CLI.
Please see https://tools.ietf.org/html/rfc7540#appendix-A for further information.`)
} else if len(badCiphers) > 0 {
ui.Warn(fmt.Sprintf(`WARNING! The following cipher suites defined by 'tls_cipher_suites' are
blacklisted by the HTTP/2 specification:
%v
Please see https://tools.ietf.org/html/rfc7540#appendix-A for further information.`, badCiphers))
}
tlsConf.CipherSuites = ciphers
}
if v, ok := config["tls_prefer_server_cipher_suites"]; ok {
Expand Down
61 changes: 37 additions & 24 deletions helper/tlsutil/tlsutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,36 @@ var TLSLookup = map[string]uint16{
"tls12": tls.VersionTLS12,
}

// cipherMap maps the cipher suite names to the internal cipher suite code.
var cipherMap = map[string]uint16{
"TLS_RSA_WITH_RC4_128_SHA": tls.TLS_RSA_WITH_RC4_128_SHA,
"TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA,
"TLS_RSA_WITH_AES_128_CBC_SHA": tls.TLS_RSA_WITH_AES_128_CBC_SHA,
"TLS_RSA_WITH_AES_256_CBC_SHA": tls.TLS_RSA_WITH_AES_256_CBC_SHA,
"TLS_RSA_WITH_AES_128_CBC_SHA256": tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
"TLS_RSA_WITH_AES_128_GCM_SHA256": tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
"TLS_RSA_WITH_AES_256_GCM_SHA384": tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
"TLS_ECDHE_RSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA,
"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
}

// ParseCiphers parse ciphersuites from the comma-separated string into recognized slice
func ParseCiphers(cipherStr string) ([]uint16, error) {
suites := []uint16{}
ciphers := strutil.ParseStringSlice(cipherStr, ",")
cipherMap := map[string]uint16{
"TLS_RSA_WITH_RC4_128_SHA": tls.TLS_RSA_WITH_RC4_128_SHA,
"TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA,
"TLS_RSA_WITH_AES_128_CBC_SHA": tls.TLS_RSA_WITH_AES_128_CBC_SHA,
"TLS_RSA_WITH_AES_256_CBC_SHA": tls.TLS_RSA_WITH_AES_256_CBC_SHA,
"TLS_RSA_WITH_AES_128_CBC_SHA256": tls.TLS_RSA_WITH_AES_128_CBC_SHA256,
"TLS_RSA_WITH_AES_128_GCM_SHA256": tls.TLS_RSA_WITH_AES_128_GCM_SHA256,
"TLS_RSA_WITH_AES_256_GCM_SHA384": tls.TLS_RSA_WITH_AES_256_GCM_SHA384,
"TLS_ECDHE_ECDSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA,
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA,
"TLS_ECDHE_RSA_WITH_RC4_128_SHA": tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA,
"TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA,
"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA,
"TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256,
"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
"TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256": tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256,
"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256": tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256,
"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384,
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
"TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305,
"TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305,
}
for _, cipher := range ciphers {
if v, ok := cipherMap[cipher]; ok {
suites = append(suites, v)
Expand All @@ -52,3 +54,14 @@ func ParseCiphers(cipherStr string) ([]uint16, error) {

return suites, nil
}

// GetCipherName returns the name of a given cipher suite code or an error if the
// given cipher is unsupported.
func GetCipherName(cipher uint16) (string, error) {
for cipherStr, cipherCode := range cipherMap {
if cipherCode == cipher {
return cipherStr, nil
}
}
return "", fmt.Errorf("unsupported cipher %d", cipher)
}
18 changes: 18 additions & 0 deletions helper/tlsutil/tlsutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,21 @@ func TestParseCiphers(t *testing.T) {
t.Fatal("cipher order is not preserved")
}
}

func TestGetCipherName(t *testing.T) {
testOkCipherStr := "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA"
testOkCipher := tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
cipherStr, err := GetCipherName(testOkCipher)
if err != nil {
t.Fatal(err)
}
if cipherStr != testOkCipherStr {
t.Fatalf("cipher string should be %s but is %s", testOkCipherStr, cipherStr)
}

var testBadCipher uint16 = 0xC022
cipherStr, err = GetCipherName(testBadCipher)
if err == nil {
t.Fatal("should fail on unsupported cipher 0xC022")
}
}
3 changes: 3 additions & 0 deletions vendor/github.com/jefferai/isbadcipher/AUTHORS

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions vendor/github.com/jefferai/isbadcipher/CONTRIBUTORS

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions vendor/github.com/jefferai/isbadcipher/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions vendor/github.com/jefferai/isbadcipher/PATENTS

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions vendor/github.com/jefferai/isbadcipher/README.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 9617832

Please sign in to comment.