diff --git a/pkg/server/api.go b/pkg/server/api.go index 480950b976..4397ae85ea 100644 --- a/pkg/server/api.go +++ b/pkg/server/api.go @@ -60,14 +60,7 @@ func NewAPIServer(a *APIHandler, p int, is bool, c, k string) *APIServer { // Serve launches the API Server. func (a *APIServer) Serve() { - mcs := &http.Server{ - Addr: fmt.Sprintf(":%v", a.port), - Handler: a.handler, - // We don't want to allow 1.1 as that's old. This was flagged in a security audit. - TLSConfig: &tls.Config{ - MinVersion: tls.VersionTLS12, - }, - } + mcs := getHTTPServerCfg(fmt.Sprintf(":%v", a.port), a.handler) glog.Infof("Launching server on %s", mcs.Addr) if a.insecure { @@ -340,3 +333,63 @@ func (h *defaultHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusMethodNotAllowed) return } + +// getHTTPServerCfg returns the basic HTTP Server +func getHTTPServerCfg(addr string, handler http.Handler) *http.Server { + return &http.Server{ + Addr: addr, + Handler: handler, + // CVE-2016-2183: disable http/2, which by definition, requires insecure ciphers + // Per https://golang.org/src/net/http/doc.go this is the runtime method of disabling HTTP/2 + TLSNextProto: make(map[string]func(*http.Server, *tls.Conn, http.Handler)), + // We don't want to allow 1.1 as that's old. This was flagged in a security audit. + TLSConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + CipherSuites: cipherOrder(), + }, + } + +} + +// Disable insecure cipher suites for CVE-2016-2183 +// cipherOrder returns an ordered list of Ciphers that are considered secure +// Deprecated ciphers are not returned. +func cipherOrder() []uint16 { + var first []uint16 + var second []uint16 + + allowable := func(c *tls.CipherSuite) bool { + // Disallow block ciphers using straight SHA1 + // See: https://tools.ietf.org/html/rfc7540#appendix-A + if strings.HasSuffix(c.Name, "CBC_SHA") { + return false + } + // 3DES is considered insecure + if strings.Contains(c.Name, "3DES") { + return false + } + return true + } + + for _, c := range tls.CipherSuites() { + for _, v := range c.SupportedVersions { + if v == tls.VersionTLS13 { + first = append(first, c.ID) + } + if v == tls.VersionTLS12 && allowable(c) { + inFirst := false + for _, id := range first { + if c.ID == id { + inFirst = true + break + } + } + if !inFirst { + second = append(second, c.ID) + } + } + } + } + + return append(first, second...) +} diff --git a/pkg/server/api_test.go b/pkg/server/api_test.go index 687d46521b..8343671219 100644 --- a/pkg/server/api_test.go +++ b/pkg/server/api_test.go @@ -1,15 +1,19 @@ package server import ( + "crypto/tls" "fmt" "io/ioutil" + "log" "net/http" "net/http/httptest" + "strings" "testing" "github.com/coreos/go-semver/semver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/net/http2" "k8s.io/apimachinery/pkg/runtime" ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" @@ -560,6 +564,143 @@ func TestAPIServer(t *testing.T) { } } +func TestAPIServerCiphers(t *testing.T) { + certData, err := tls.X509KeyPair( + []byte(`-----BEGIN CERTIFICATE----- +MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw +DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow +EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d +7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B +5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr +BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1 +NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l +Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc +6MF9+Yw1Yy0t +-----END CERTIFICATE-----`), + []byte(`-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49 +AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q +EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA== +-----END EC PRIVATE KEY-----`), + ) + if err != nil { + t.Fatalf("failed to create test x509 cert") + } + + ts := getHTTPServerCfg( + fmt.Sprintf(":%d", 8088), + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintln(w, "Hello, client") + }), + ) + + // Configure the TLS testing to allow + ts.TLSConfig.Certificates = []tls.Certificate{certData} + ts.TLSConfig.InsecureSkipVerify = true + + go func(t *testing.T) { + if err := ts.ListenAndServeTLS("", ""); err != http.ErrServerClosed { + log.Fatal("error creating test server") + } + }(t) + defer ts.Close() + + type testCase struct { + desc string + wantErr bool + errString string + client *http.Client + ciphers []uint16 + } + + tests := []testCase{ + { + desc: "ensure base TLS configuration", + wantErr: false, + client: &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, // test server certificate is not trusted. + }, + }, + }, + }, + { + desc: "http/2 should fail", + wantErr: true, + errString: "http2: unexpected ALPN protocol", + client: &http.Client{ + Transport: &http2.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: true, // test server certificate is not trusted. + }, + }, + }, + }, + { + desc: "TLS1.1 should fail", + wantErr: true, + errString: "protocol version not supported", + client: &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + MaxVersion: tls.VersionTLS11, + InsecureSkipVerify: true, // test server certificate is not trusted. + }, + }, + }, + }, + { + desc: "TLS1.2 should work", + wantErr: false, + client: &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + MaxVersion: tls.VersionTLS12, + InsecureSkipVerify: true, // test server certificate is not trusted. + }, + }, + }, + }, + } + + // Test that all known insecure ciphers are straight up refused. + for _, c := range tls.CipherSuites() { + if c.Insecure || strings.HasSuffix(c.Name, "CBC_SHA") || strings.HasSuffix(c.Name, "DES") { + tests = append( + tests, + testCase{ + desc: fmt.Sprintf("refuse insecure ciphers %v", c.Name), + ciphers: []uint16{c.ID}, + client: &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + CipherSuites: []uint16{c.ID}, + InsecureSkipVerify: true, // test server certificate is not trusted. + }, + }, + }, + }) + } + } + + for idx, c := range tests { + t.Run(fmt.Sprintf("case#%d: %s", idx, c.desc), func(t *testing.T) { + resp, err := c.client.Get("https://localhost:8088") + for _, insecure := range c.ciphers { + if insecure == resp.TLS.CipherSuite { + t.Fatalf("failed to %s:", c.desc) + } + } + if c.wantErr { + if !strings.Contains(fmt.Sprintf("%v", err), c.errString) { + t.Fatalf("want: %s\n got: %v", c.errString, err) + } + } + }) + } +} + func checkStatus(t *testing.T, response *http.Response, expected int) { if response.StatusCode != expected { t.Errorf("expected response status %d, received %d", expected, response.StatusCode)