From eae92fdee6393a50f23e4a4abec66eadcc58192a Mon Sep 17 00:00:00 2001 From: "Benjamin A. Petersen" Date: Wed, 13 Nov 2019 16:28:33 -0500 Subject: [PATCH] Update tls.Config with DefaultCiphers() from library-go - add bash script to test a sample of good ciphers & bad ciphers --- cmd/bridge/main.go | 13 +++++++-- pkg/crypto/crypto.go | 41 +++++++++++++++++++++++++++ pkg/serverconfig/types.go | 1 - test-ciphers.sh | 58 +++++++++++++++++++++++++++++++++++++++ test-prow-e2e.sh | 2 ++ 5 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 pkg/crypto/crypto.go create mode 100755 test-ciphers.sh diff --git a/cmd/bridge/main.go b/cmd/bridge/main.go index 15ba85adf6e..06e4dc9f645 100644 --- a/cmd/bridge/main.go +++ b/cmd/bridge/main.go @@ -6,6 +6,7 @@ import ( "crypto/x509" "flag" "fmt" + "io/ioutil" "net/http" "net/url" @@ -17,6 +18,7 @@ import ( "github.com/openshift/console/pkg/auth" "github.com/openshift/console/pkg/bridge" + "github.com/openshift/console/pkg/crypto" "github.com/openshift/console/pkg/proxy" "github.com/openshift/console/pkg/server" "github.com/openshift/console/pkg/serverconfig" @@ -247,7 +249,10 @@ func main() { if !rootCAs.AppendCertsFromPEM(k8sCertPEM) { log.Fatalf("No CA found for the API server") } - tlsConfig := &tls.Config{RootCAs: rootCAs} + tlsConfig := &tls.Config{ + RootCAs: rootCAs, + CipherSuites: crypto.DefaultCiphers(), + } bearerToken, err := ioutil.ReadFile(k8sInClusterBearerToken) if err != nil { @@ -272,7 +277,10 @@ func main() { if !serviceProxyRootCAs.AppendCertsFromPEM(serviceCertPEM) { log.Fatalf("no CA found for Kubernetes services") } - serviceProxyTLSConfig := &tls.Config{RootCAs: serviceProxyRootCAs} + serviceProxyTLSConfig := &tls.Config{ + RootCAs: serviceProxyRootCAs, + CipherSuites: crypto.DefaultCiphers(), + } srv.PrometheusProxyConfig = &proxy.Config{ TLSClientConfig: serviceProxyTLSConfig, HeaderBlacklist: []string{"Cookie", "X-CSRFToken"}, @@ -304,6 +312,7 @@ func main() { k8sEndpoint = bridge.ValidateFlagIsURL("k8s-mode-off-cluster-endpoint", *fK8sModeOffClusterEndpoint) serviceProxyTLSConfig := &tls.Config{ InsecureSkipVerify: *fK8sModeOffClusterSkipVerifyTLS, + CipherSuites: crypto.DefaultCiphers(), } srv.K8sProxyConfig = &proxy.Config{ TLSClientConfig: serviceProxyTLSConfig, diff --git a/pkg/crypto/crypto.go b/pkg/crypto/crypto.go new file mode 100644 index 00000000000..a8f3081c52c --- /dev/null +++ b/pkg/crypto/crypto.go @@ -0,0 +1,41 @@ +package crypto + +// this file is copied over from: +// https://github.com/openshift/library-go/blob/11013d437d762f00827c7e80d18b0a7b0abc07bd/pkg/crypto/crypto.go#L122 +// we may want to consider importing library-go and using this package +// directly as we would gain any effort maintaining this list. +import ( + "crypto/tls" +) + +func DefaultCiphers() []uint16 { + // HTTP/2 mandates TLS 1.2 or higher with an AEAD cipher + // suite (GCM, Poly1305) and ephemeral key exchange (ECDHE, DHE) for + // perfect forward secrecy. Servers may provide additional cipher + // suites for backwards compatibility with HTTP/1.1 clients. + // See RFC7540, section 9.2 (Use of TLS Features) and Appendix A + // (TLS 1.2 Cipher Suite Black List). + return []uint16{ + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, // required by http/2 + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, // forbidden by http/2, not flagged by http2isBadCipher() in go1.8 + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, // forbidden by http/2, not flagged by http2isBadCipher() in go1.8 + tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, // forbidden by http/2 + tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, // forbidden by http/2 + tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, // forbidden by http/2 + tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, // forbidden by http/2 + tls.TLS_RSA_WITH_AES_128_GCM_SHA256, // forbidden by http/2 + tls.TLS_RSA_WITH_AES_256_GCM_SHA384, // forbidden by http/2 + // the next one is in the intermediate suite, but go1.8 http2isBadCipher() complains when it is included at the recommended index + // because it comes after ciphers forbidden by the http/2 spec + // tls.TLS_RSA_WITH_AES_128_CBC_SHA256, + // tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, // forbidden by http/2, disabled to mitigate SWEET32 attack + // tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, // forbidden by http/2, disabled to mitigate SWEET32 attack + tls.TLS_RSA_WITH_AES_128_CBC_SHA, // forbidden by http/2 + tls.TLS_RSA_WITH_AES_256_CBC_SHA, // forbidden by http/2 + } +} diff --git a/pkg/serverconfig/types.go b/pkg/serverconfig/types.go index a088409637f..14dca623212 100644 --- a/pkg/serverconfig/types.go +++ b/pkg/serverconfig/types.go @@ -1,6 +1,5 @@ package serverconfig - // This file is a copy of the struct within the console operator: // https://github.com/openshift/console-operator/blob/master/pkg/console/subresource/consoleserver/types.go // These structs need to remain in sync. diff --git a/test-ciphers.sh b/test-ciphers.sh new file mode 100755 index 00000000000..519774d82f1 --- /dev/null +++ b/test-ciphers.sh @@ -0,0 +1,58 @@ +#!/bin/bash + +CONSOLE_URL=$(oc get console.config.openshift.io cluster --template '{{.status.consoleURL}}') +if [ -z "${CONSOLE_URL}" ] +then + echo "no console_url, are you connected to a cluster?" + exit 1 +fi + +# need to format the console_url for s_client +CONSOLE_URL_WITHOUT_HTTP=${CONSOLE_URL#"https://"} +SERVER="${CONSOLE_URL_WITHOUT_HTTP}:443" + +ROUTER_CA=$(oc get configmap router-ca -n openshift-config-managed -o json | jq -r '.data["ca-bundle.crt"]') +echo "$ROUTER_CA" > /tmp/router-ca-file.txt + +# CIPHER=ECDHE-ECDSA-CHACHA20-POLY1305 # DENIED +VALID_CIPHER_SAMPLE=( + ECDHE-RSA-AES128-GCM-SHA256 + ECDHE-RSA-AES256-GCM-SHA384 +) + +for CIPHER in "${VALID_CIPHER_SAMPLE[@]}" +do + RESULT=$(openssl s_client -connect "${SERVER}" -cipher "${CIPHER}" -CAfile /tmp/router-ca-file.txt 2>&1) + if [[ $? -eq 0 ]] + then + echo "valid cipher was correctly accepted (${CIPHER})" + else + echo "valid cipher suite was denied (${CIPHER})" + exit 1 + fi +done + + +# ensure we ignore weak ciphers +# CBC (cipher block chaining) are no longer reliable and should not be used +# CBC ciphers use an IV (initialization vector) and a chaining mechanism. +# The chaining mechanism means that a single bit error in a ciphertext block +# will invalidate all previous blocks. The chaining was good in that it hides +# plaintext patterns, but is inferior to other cipher modes. +INVALID_CIPHER_SAMPLE=( + RSA-AES-128-CBC-SHA256 + ECDHE-RSA-3DES-EDE-CBC-SHA # disabled to mitigate SWEET32 attack + RSA-3DES-EDE-CBC-SHA # disabled to mitigate SWEET32 attack +) + +for CIPHER in "${INVALID_CIPHER_SAMPLE[@]}" +do + RESULT=$(openssl s_client -connect "${SERVER}" -cipher "${CIPHER}" -CAfile /tmp/router-ca-file.txt 2>&1) + if [[ $? -eq 0 ]] + then + echo "invalid cipher suite used to connect to console (${CIPHER})" + exit 1 + else + echo "invalid cipher was correctly denied (${CIPHER})" + fi +done \ No newline at end of file diff --git a/test-prow-e2e.sh b/test-prow-e2e.sh index 8aec97a9f58..f6b86a2e951 100755 --- a/test-prow-e2e.sh +++ b/test-prow-e2e.sh @@ -31,3 +31,5 @@ export FORCE_CHROME_BRANCH_BASE="665006" export FORCE_CHROME_BRANCH_SHA256SUM="a1ae2e0950828f991119825f62c24464ab3765aa219d150a94fb782a4c66a744" ./test-gui.sh ${1:-e2e} + +./test-ciphers.sh