From aa7e81f2ed16ba051bd59fe23c49359fc1e6c1b9 Mon Sep 17 00:00:00 2001 From: Leonid Bugaev Date: Tue, 22 May 2018 19:38:14 +0300 Subject: [PATCH] Fix certificate pinning though proxy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using proxy DialTLS is not available, instead “VerifyPeerCertificate” should be used. --- cert.go | 33 +++++++ cert_go1.10_test.go | 212 ++++++++++++++++++++++++++++++++++++++++++++ cert_test.go | 149 ------------------------------- helpers_test.go | 95 ++++++++++++++++++++ reverse_proxy.go | 8 +- 5 files changed, 347 insertions(+), 150 deletions(-) create mode 100644 cert_go1.10_test.go diff --git a/cert.go b/cert.go index 1cdec52bd3d..603ecdcead9 100644 --- a/cert.go +++ b/cert.go @@ -95,6 +95,39 @@ func getUpstreamCertificate(host string, spec *APISpec) (cert *tls.Certificate) return certs[0] } +func verifyPeerCertificatePinnedCheck(spec *APISpec, tlsConfig *tls.Config) func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + if (spec == nil || len(spec.PinnedPublicKeys) == 0) && len(config.Global().Security.PinnedPublicKeys) == 0 { + return nil + } + + tlsConfig.InsecureSkipVerify = true + + whitelist := getPinnedPublicKeys("*", spec) + if len(whitelist) == 0 { + return nil + } + + return func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + for _, rawCert := range rawCerts { + cert, _ := x509.ParseCertificate(rawCert) + pub, err := x509.MarshalPKIXPublicKey(cert.PublicKey) + if err != nil { + continue + } + + fingerprint := certs.HexSHA256(pub) + + for _, w := range whitelist { + if w == fingerprint { + return nil + } + } + } + + return errors.New("Certificate public key pinning error. Public keys do not match.") + } +} + func dialTLSPinnedCheck(spec *APISpec, tc *tls.Config) func(network, addr string) (net.Conn, error) { if (spec == nil || len(spec.PinnedPublicKeys) == 0) && len(config.Global().Security.PinnedPublicKeys) == 0 { return nil diff --git a/cert_go1.10_test.go b/cert_go1.10_test.go new file mode 100644 index 00000000000..5d2634637a1 --- /dev/null +++ b/cert_go1.10_test.go @@ -0,0 +1,212 @@ +// +build go1.10 + +package main + +import ( + "crypto/tls" + "crypto/x509" + "encoding/pem" + "net/http" + "net/http/httptest" + "testing" + + "github.com/TykTechnologies/tyk/certs" + "github.com/TykTechnologies/tyk/config" + "github.com/TykTechnologies/tyk/test" +) + +func TestPublicKeyPinning(t *testing.T) { + _, _, _, serverCert := genServerCertificate() + x509Cert, _ := x509.ParseCertificate(serverCert.Certificate[0]) + pubDer, _ := x509.MarshalPKIXPublicKey(x509Cert.PublicKey) + pubPem := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDer}) + pubID, _ := CertificateManager.Add(pubPem, "") + defer CertificateManager.Delete(pubID) + + if pubID != certs.HexSHA256(pubDer) { + t.Error("Certmanager returned wrong pub key fingerprint:", certs.HexSHA256(pubDer), pubID) + } + + upstream := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + })) + upstream.TLS = &tls.Config{ + InsecureSkipVerify: true, + Certificates: []tls.Certificate{serverCert}, + } + + upstream.StartTLS() + defer upstream.Close() + + t.Run("Pub key match", func(t *testing.T) { + globalConf := config.Global() + // For host using pinning, it should ignore standard verification in all cases, e.g setting variable below does nothing + globalConf.ProxySSLInsecureSkipVerify = false + config.SetGlobal(globalConf) + defer resetTestConfig() + + ts := newTykTestServer() + defer ts.Close() + + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.PinnedPublicKeys = map[string]string{"127.0.0.1": pubID} + spec.Proxy.TargetURL = upstream.URL + }) + + ts.Run(t, test.TestCase{Code: 200}) + }) + + t.Run("Pub key not match", func(t *testing.T) { + ts := newTykTestServer() + defer ts.Close() + + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.PinnedPublicKeys = map[string]string{"127.0.0.1": "wrong"} + spec.Proxy.TargetURL = upstream.URL + }) + + ts.Run(t, test.TestCase{Code: 500}) + }) + + t.Run("Global setting", func(t *testing.T) { + globalConf := config.Global() + globalConf.Security.PinnedPublicKeys = map[string]string{"127.0.0.1": "wrong"} + config.SetGlobal(globalConf) + defer resetTestConfig() + + ts := newTykTestServer() + defer ts.Close() + + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + }) + + ts.Run(t, test.TestCase{Code: 500}) + }) + + t.Run("Though proxy", func(t *testing.T) { + _, _, _, proxyCert := genServerCertificate() + proxy := initProxy("https", &tls.Config{ + Certificates: []tls.Certificate{proxyCert}, + }) + + globalConf := config.Global() + globalConf.ProxySSLInsecureSkipVerify = true + config.SetGlobal(globalConf) + defer resetTestConfig() + + defer proxy.Stop() + + ts := newTykTestServer() + defer ts.Close() + + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + spec.Proxy.Transport.ProxyURL = proxy.URL + spec.PinnedPublicKeys = map[string]string{"*": "wrong"} + }) + + ts.Run(t, test.TestCase{Code: 500}) + }) +} + +func TestProxyTransport(t *testing.T) { + upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("test")) + })) + defer upstream.Close() + + globalConf := config.Global() + globalConf.ProxySSLInsecureSkipVerify = true + // force creating new transport on each reque + globalConf.MaxConnTime = -1 + config.SetGlobal(globalConf) + defer resetTestConfig() + + ts := newTykTestServer() + defer ts.Close() + + //matching ciphers + t.Run("Global: Cipher match", func(t *testing.T) { + globalConf.ProxySSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} + config.SetGlobal(globalConf) + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + }) + ts.Run(t, test.TestCase{Path: "/", Code: 200}) + }) + + t.Run("Global: Cipher not match", func(t *testing.T) { + globalConf.ProxySSLCipherSuites = []string{"TLS_RSA_WITH_RC4_128_SHA"} + config.SetGlobal(globalConf) + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + }) + ts.Run(t, test.TestCase{Path: "/", Code: 500}) + }) + + t.Run("API: Cipher override", func(t *testing.T) { + globalConf.ProxySSLCipherSuites = []string{"TLS_RSA_WITH_RC4_128_SHA"} + config.SetGlobal(globalConf) + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} + }) + + ts.Run(t, test.TestCase{Path: "/", Code: 200}) + }) + + t.Run("API: MinTLS not match", func(t *testing.T) { + globalConf.ProxySSLMinVersion = 772 + config.SetGlobal(globalConf) + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} + }) + + ts.Run(t, test.TestCase{Path: "/", Code: 500}) + }) + + t.Run("API: Invalid proxy", func(t *testing.T) { + globalConf.ProxySSLMinVersion = 771 + config.SetGlobal(globalConf) + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} + // Invalid proxy + spec.Proxy.Transport.ProxyURL = upstream.URL + }) + + ts.Run(t, test.TestCase{Path: "/", Code: 500}) + }) + + t.Run("API: Valid proxy", func(t *testing.T) { + _, _, _, proxyCert := genServerCertificate() + proxy := initProxy("https", &tls.Config{ + Certificates: []tls.Certificate{proxyCert}, + }) + defer proxy.Stop() + + buildAndLoadAPI(func(spec *APISpec) { + spec.Proxy.ListenPath = "/" + spec.Proxy.TargetURL = upstream.URL + spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} + // Invalid proxy + spec.Proxy.Transport.ProxyURL = proxy.URL + }) + + client := getTLSClient(nil, nil) + client.Transport = &http.Transport{ + TLSNextProto: make(map[string]func(authority string, c *tls.Conn) http.RoundTripper), + } + ts.Run(t, test.TestCase{Path: "/", Code: 200, Client: client}) + }) +} diff --git a/cert_test.go b/cert_test.go index 9c333c4f132..9bb69e2f829 100644 --- a/cert_test.go +++ b/cert_test.go @@ -5,7 +5,6 @@ import ( "crypto/rand" "crypto/rsa" "crypto/tls" - _ "crypto/tls" "crypto/x509" "encoding/pem" "fmt" @@ -472,78 +471,6 @@ func TestUpstreamMutualTLS(t *testing.T) { }) } -func TestPublicKeyPinning(t *testing.T) { - _, _, _, serverCert := genServerCertificate() - x509Cert, _ := x509.ParseCertificate(serverCert.Certificate[0]) - pubDer, _ := x509.MarshalPKIXPublicKey(x509Cert.PublicKey) - pubPem := pem.EncodeToMemory(&pem.Block{Type: "PUBLIC KEY", Bytes: pubDer}) - pubID, _ := CertificateManager.Add(pubPem, "") - defer CertificateManager.Delete(pubID) - - if pubID != certs.HexSHA256(pubDer) { - t.Error("Certmanager returned wrong pub key fingerprint:", certs.HexSHA256(pubDer), pubID) - } - - upstream := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - })) - upstream.TLS = &tls.Config{ - InsecureSkipVerify: true, - Certificates: []tls.Certificate{serverCert}, - } - - upstream.StartTLS() - defer upstream.Close() - - t.Run("Pub key match", func(t *testing.T) { - globalConf := config.Global() - // For host using pinning, it should ignore standard verification in all cases, e.g setting variable below does nothing - globalConf.ProxySSLInsecureSkipVerify = false - config.SetGlobal(globalConf) - defer resetTestConfig() - - ts := newTykTestServer() - defer ts.Close() - - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.PinnedPublicKeys = map[string]string{"127.0.0.1": pubID} - spec.Proxy.TargetURL = upstream.URL - }) - - ts.Run(t, test.TestCase{Code: 200}) - }) - - t.Run("Pub key not match", func(t *testing.T) { - ts := newTykTestServer() - defer ts.Close() - - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.PinnedPublicKeys = map[string]string{"127.0.0.1": "wrong"} - spec.Proxy.TargetURL = upstream.URL - }) - - ts.Run(t, test.TestCase{Code: 500}) - }) - - t.Run("Global setting", func(t *testing.T) { - globalConf := config.Global() - globalConf.Security.PinnedPublicKeys = map[string]string{"127.0.0.1": "wrong"} - config.SetGlobal(globalConf) - defer resetTestConfig() - - ts := newTykTestServer() - defer ts.Close() - - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.Proxy.TargetURL = upstream.URL - }) - - ts.Run(t, test.TestCase{Code: 500}) - }) -} - func TestKeyWithCertificateTLS(t *testing.T) { _, _, combinedPEM, _ := genServerCertificate() serverCertID, _ := CertificateManager.Add(combinedPEM, "") @@ -683,79 +610,3 @@ func TestCipherSuites(t *testing.T) { ts.Run(t, test.TestCase{Client: client, Path: "/", ErrorMatch: "tls: handshake failure"}) }) } - -func TestProxyTransport(t *testing.T) { - upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("test")) - })) - defer upstream.Close() - - globalConf := config.Global() - globalConf.ProxySSLInsecureSkipVerify = true - // force creating new transport on each reque - globalConf.MaxConnTime = -1 - config.SetGlobal(globalConf) - defer resetTestConfig() - - ts := newTykTestServer() - defer ts.Close() - - //matching ciphers - t.Run("Global: Cipher match", func(t *testing.T) { - globalConf.ProxySSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} - config.SetGlobal(globalConf) - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.Proxy.TargetURL = upstream.URL - }) - ts.Run(t, test.TestCase{Path: "/", Code: 200}) - }) - - t.Run("Global: Cipher not match", func(t *testing.T) { - globalConf.ProxySSLCipherSuites = []string{"TLS_RSA_WITH_RC4_128_SHA"} - config.SetGlobal(globalConf) - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.Proxy.TargetURL = upstream.URL - }) - ts.Run(t, test.TestCase{Path: "/", Code: 500}) - }) - - t.Run("API: Cipher override", func(t *testing.T) { - globalConf.ProxySSLCipherSuites = []string{"TLS_RSA_WITH_RC4_128_SHA"} - config.SetGlobal(globalConf) - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.Proxy.TargetURL = upstream.URL - spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} - }) - - ts.Run(t, test.TestCase{Path: "/", Code: 200}) - }) - - t.Run("API: MinTLS not match", func(t *testing.T) { - globalConf.ProxySSLMinVersion = 772 - config.SetGlobal(globalConf) - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.Proxy.TargetURL = upstream.URL - spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} - }) - - ts.Run(t, test.TestCase{Path: "/", Code: 500}) - }) - - t.Run("API: Proxy", func(t *testing.T) { - globalConf.ProxySSLMinVersion = 771 - config.SetGlobal(globalConf) - buildAndLoadAPI(func(spec *APISpec) { - spec.Proxy.ListenPath = "/" - spec.Proxy.TargetURL = upstream.URL - spec.Proxy.Transport.SSLCipherSuites = []string{"TLS_RSA_WITH_AES_128_CBC_SHA"} - // Invalid proxy - spec.Proxy.Transport.ProxyURL = upstream.URL - }) - - ts.Run(t, test.TestCase{Path: "/", Code: 500}) - }) -} diff --git a/helpers_test.go b/helpers_test.go index 38782e21faf..732ce6f6b41 100644 --- a/helpers_test.go +++ b/helpers_test.go @@ -3,6 +3,7 @@ package main import ( "archive/zip" "context" + "crypto/tls" "encoding/json" "fmt" "io" @@ -630,3 +631,97 @@ func initDNSMock() { }).DialContext, } } + +// Taken from https://medium.com/@mlowicki/http-s-proxy-in-golang-in-less-than-100-lines-of-code-6a51c2f2c38c +type httpProxyHandler struct { + proto string + URL string + server *http.Server + listener net.Listener +} + +func (p *httpProxyHandler) handleTunneling(w http.ResponseWriter, r *http.Request) { + dest_conn, err := net.DialTimeout("tcp", r.Host, 10*time.Second) + if err != nil { + http.Error(w, err.Error(), http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusOK) + hijacker, ok := w.(http.Hijacker) + if !ok { + http.Error(w, "Hijacking not supported", http.StatusInternalServerError) + return + } + client_conn, _, err := hijacker.Hijack() + if err != nil { + http.Error(w, err.Error(), http.StatusServiceUnavailable) + } + go p.transfer(dest_conn, client_conn) + go p.transfer(client_conn, dest_conn) +} + +func (p *httpProxyHandler) transfer(destination io.WriteCloser, source io.ReadCloser) { + defer destination.Close() + defer source.Close() + io.Copy(destination, source) +} +func (p *httpProxyHandler) handleHTTP(w http.ResponseWriter, req *http.Request) { + resp, err := http.DefaultTransport.RoundTrip(req) + if err != nil { + http.Error(w, err.Error(), http.StatusServiceUnavailable) + return + } + defer resp.Body.Close() + p.copyHeader(w.Header(), resp.Header) + w.WriteHeader(resp.StatusCode) + io.Copy(w, resp.Body) +} + +func (p *httpProxyHandler) Stop() error { + return p.server.Close() +} + +func (p *httpProxyHandler) copyHeader(dst, src http.Header) { + for k, vv := range src { + for _, v := range vv { + dst.Add(k, v) + } + } +} + +func initProxy(proto string, tlsConfig *tls.Config) *httpProxyHandler { + proxy := &httpProxyHandler{proto: proto} + + proxy.server = &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodConnect { + proxy.handleTunneling(w, r) + } else { + proxy.handleHTTP(w, r) + } + }), + // Disable HTTP/2. + TLSNextProto: make(map[string]func(*http.Server, *tls.Conn, http.Handler)), + } + + var err error + + switch proto { + case "http": + proxy.listener, err = net.Listen("tcp", ":0") + case "https": + proxy.listener, err = tls.Listen("tcp", ":0", tlsConfig) + default: + log.Fatal("Unsupported proto scheme", proto) + } + + if err != nil { + log.Fatal(err) + } + + proxy.URL = proto + "://" + proxy.listener.Addr().String() + + go proxy.server.Serve(proxy.listener) + + return proxy +} diff --git a/reverse_proxy.go b/reverse_proxy.go index 65911d8829e..d710a9ea597 100644 --- a/reverse_proxy.go +++ b/reverse_proxy.go @@ -438,7 +438,13 @@ func httpTransport(timeOut int, rw http.ResponseWriter, req *http.Request, p *Re transport.TLSClientConfig.InsecureSkipVerify = true } - transport.DialTLS = dialTLSPinnedCheck(p.TykAPISpec, transport.TLSClientConfig) + // When request routed through the proxy `DialTLS` is not used, and only VerifyPeerCertificate is supported + // The reason behind two separate checks is that `DialTLS` supports specifying public keys per hostname, and `VerifyPeerCertificate` only global ones, e.g. `*` + if proxyURL, _ := transport.Proxy(req); proxyURL != nil { + transport.TLSClientConfig.VerifyPeerCertificate = verifyPeerCertificatePinnedCheck(p.TykAPISpec, transport.TLSClientConfig) + } else { + transport.DialTLS = dialTLSPinnedCheck(p.TykAPISpec, transport.TLSClientConfig) + } if p.TykAPISpec.GlobalConfig.ProxySSLMinVersion > 0 { transport.TLSClientConfig.MinVersion = p.TykAPISpec.GlobalConfig.ProxySSLMinVersion