From 9718da4410e558da9da518a6286e25cb4442d3d0 Mon Sep 17 00:00:00 2001 From: Steven Falken Date: Sun, 13 May 2018 21:29:28 +0200 Subject: [PATCH 1/3] Add possibility to install a cert store Add the possibility to install a certificate storage to cache certificates; this can make goproxy faster. --- ctx.go | 11 ++++++-- https.go | 24 ++++++++++++---- proxy.go | 1 + proxy_test.go | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++ signer.go | 4 +-- signer_test.go | 2 +- 6 files changed, 106 insertions(+), 11 deletions(-) diff --git a/ctx.go b/ctx.go index 70b4cf0f..27f38894 100644 --- a/ctx.go +++ b/ctx.go @@ -1,6 +1,7 @@ package goproxy import ( + "crypto/tls" "net/http" "regexp" ) @@ -19,14 +20,20 @@ type ProxyCtx struct { // call of RespHandler UserData interface{} // Will connect a request to a response - Session int64 - proxy *ProxyHttpServer + Session int64 + certStore CertStorage + proxy *ProxyHttpServer } type RoundTripper interface { RoundTrip(req *http.Request, ctx *ProxyCtx) (*http.Response, error) } +type CertStorage interface { + Store(hostname string, cert *tls.Certificate) + Load(hostname string) *tls.Certificate +} + type RoundTripperFunc func(req *http.Request, ctx *ProxyCtx) (*http.Response, error) func (f RoundTripperFunc) RoundTrip(req *http.Request, ctx *ProxyCtx) (*http.Response, error) { diff --git a/https.go b/https.go index 5498f8b7..ce40b2d4 100644 --- a/https.go +++ b/https.go @@ -65,7 +65,7 @@ func (proxy *ProxyHttpServer) connectDial(network, addr string) (c net.Conn, err } func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request) { - ctx := &ProxyCtx{Req: r, Session: atomic.AddInt64(&proxy.sess, 1), proxy: proxy} + ctx := &ProxyCtx{Req: r, Session: atomic.AddInt64(&proxy.sess, 1), proxy: proxy, certStore: proxy.CertStore} hij, ok := w.(http.Hijacker) if !ok { @@ -408,14 +408,26 @@ func (proxy *ProxyHttpServer) NewConnectDialToProxyWithHandler(https_proxy strin func TLSConfigFromCA(ca *tls.Certificate) func(host string, ctx *ProxyCtx) (*tls.Config, error) { return func(host string, ctx *ProxyCtx) (*tls.Config, error) { + var err error + var cert *tls.Certificate + + hostname := stripPort(host) config := *defaultTLSConfig ctx.Logf("signing for %s", stripPort(host)) - cert, err := signHost(*ca, []string{stripPort(host)}) - if err != nil { - ctx.Warnf("Cannot sign host certificate with provided CA: %s", err) - return nil, err + if ctx.certStore != nil { + cert = ctx.certStore.Load(hostname) + } + if cert == nil { + cert, err = signHost(*ca, []string{hostname}) + if err != nil { + ctx.Warnf("Cannot sign host certificate with provided CA: %s", err) + return nil, err + } + if ctx.certStore != nil { + ctx.certStore.Store(hostname, cert) + } } - config.Certificates = append(config.Certificates, cert) + config.Certificates = append(config.Certificates, *cert) return &config, nil } } diff --git a/proxy.go b/proxy.go index 93359000..29b415ac 100644 --- a/proxy.go +++ b/proxy.go @@ -29,6 +29,7 @@ type ProxyHttpServer struct { // ConnectDial will be used to create TCP connections for CONNECT requests // if nil Tr.Dial will be used ConnectDial func(network string, addr string) (net.Conn, error) + CertStore CertStorage } var hasPort = regexp.MustCompile(`:\d+$`) diff --git a/proxy_test.go b/proxy_test.go index 8117517f..c8c68d83 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -766,6 +766,81 @@ func TestHasGoproxyCA(t *testing.T) { } } +type TestCertStorage struct { + certs map[string]*tls.Certificate + loads int + stores int +} + +func (tcs *TestCertStorage) Store(hostname string, cert *tls.Certificate) { + tcs.stores++ + tcs.certs[hostname] = cert +} + +func (tcs *TestCertStorage) Load(hostname string) *tls.Certificate { + tcs.loads++ + return tcs.certs[hostname] +} + +func (tcs *TestCertStorage) statLoads() int { + return tcs.loads +} + +func (tcs *TestCertStorage) statStores() int { + return tcs.stores +} + +func newTestCertStorage() *TestCertStorage { + tcs := &TestCertStorage{} + tcs.certs = make(map[string]*tls.Certificate) + + return tcs +} + +func TestProxyWithCertStorage(t *testing.T) { + tcs := newTestCertStorage() + t.Logf("TestProxyWithCertStorage started") + proxy := goproxy.NewProxyHttpServer() + proxy.CertStore = tcs + proxy.OnRequest().HandleConnect(goproxy.AlwaysMitm) + proxy.OnRequest().DoFunc(func(req *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) { + req.URL.Path = "/bobo" + return req, nil + }) + + s := httptest.NewServer(proxy) + + proxyUrl, _ := url.Parse(s.URL) + goproxyCA := x509.NewCertPool() + goproxyCA.AddCert(goproxy.GoproxyCa.Leaf) + + tr := &http.Transport{TLSClientConfig: &tls.Config{RootCAs: goproxyCA}, Proxy: http.ProxyURL(proxyUrl)} + client := &http.Client{Transport: tr} + + if resp := string(getOrFail(https.URL+"/bobo", client, t)); resp != "bobo" { + t.Error("Wrong response when mitm", resp, "expected bobo") + } + + if tcs.statLoads() != 1 { + t.Fatal("TestCertStorage.Load has not been called") + } + if tcs.statStores() != 1 { + t.Fatal("TestCertStorage.Store has not been called") + } + + // Another round - this time the certificate can be loaded + if resp := string(getOrFail(https.URL+"/bobo", client, t)); resp != "bobo" { + t.Error("Wrong response when mitm", resp, "expected bobo") + } + + if tcs.statLoads() != 2 { + t.Fatal("TestCertStorage.Load has not been called") + } + if tcs.statStores() != 1 { + t.Fatal("TestCertStorage.Store has not been called") + } +} + func TestHttpsMitmURLRewrite(t *testing.T) { scheme := "https" diff --git a/signer.go b/signer.go index c01ec76d..11a98de4 100644 --- a/signer.go +++ b/signer.go @@ -32,7 +32,7 @@ func hashSortedBigInt(lst []string) *big.Int { var goproxySignerVersion = ":goroxy1" -func signHost(ca tls.Certificate, hosts []string) (cert tls.Certificate, err error) { +func signHost(ca tls.Certificate, hosts []string) (cert *tls.Certificate, err error) { var x509ca *x509.Certificate // Use the provided ca and not the global GoproxyCa for certificate generation. @@ -81,7 +81,7 @@ func signHost(ca tls.Certificate, hosts []string) (cert tls.Certificate, err err if derBytes, err = x509.CreateCertificate(&csprng, &template, x509ca, &certpriv.PublicKey, ca.PrivateKey); err != nil { return } - return tls.Certificate{ + return &tls.Certificate{ Certificate: [][]byte{derBytes, ca.Certificate[0]}, PrivateKey: certpriv, }, nil diff --git a/signer_test.go b/signer_test.go index d0e24d29..3d568514 100644 --- a/signer_test.go +++ b/signer_test.go @@ -45,7 +45,7 @@ func TestSingerTls(t *testing.T) { expected := "key verifies with Go" server := httptest.NewUnstartedServer(ConstantHanlder(expected)) defer server.Close() - server.TLS = &tls.Config{Certificates: []tls.Certificate{cert, GoproxyCa}} + server.TLS = &tls.Config{Certificates: []tls.Certificate{*cert, GoproxyCa}} server.TLS.BuildNameToCertificate() server.StartTLS() certpool := x509.NewCertPool() From 8a57b51e514fc074f577feeffff8e90a345898b2 Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Sat, 30 Mar 2019 18:53:38 -0500 Subject: [PATCH 2/3] Switch CertStorage interface to single function Rather than having Store() and Load() as separate functions, Fetch() is provided with the key and a function that returns the value to cache, and it handles the store and load internally. A simple storage implementation may mimic the previous behaviour, but the problem with this is many concurrent requests will result in the certificate being generated many times in parallel. An alternative implementation of the certificate storage might lock on the key when generating the certificate, meaning only one certificate will be generated and other clients will wait until it exists. --- ctx.go | 3 +-- https.go | 22 ++++++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/ctx.go b/ctx.go index 27f38894..86162deb 100644 --- a/ctx.go +++ b/ctx.go @@ -30,8 +30,7 @@ type RoundTripper interface { } type CertStorage interface { - Store(hostname string, cert *tls.Certificate) - Load(hostname string) *tls.Certificate + Fetch(hostname string, gen func() (*tls.Certificate, error)) (*tls.Certificate, error) } type RoundTripperFunc func(req *http.Request, ctx *ProxyCtx) (*http.Response, error) diff --git a/https.go b/https.go index ce40b2d4..12de7511 100644 --- a/https.go +++ b/https.go @@ -414,19 +414,21 @@ func TLSConfigFromCA(ca *tls.Certificate) func(host string, ctx *ProxyCtx) (*tls hostname := stripPort(host) config := *defaultTLSConfig ctx.Logf("signing for %s", stripPort(host)) + + genCert := func() (*tls.Certificate, error) { + return signHost(*ca, []string{hostname}) + } if ctx.certStore != nil { - cert = ctx.certStore.Load(hostname) + cert, err = ctx.certStore.Fetch(hostname, genCert) + } else { + cert, err = genCert() } - if cert == nil { - cert, err = signHost(*ca, []string{hostname}) - if err != nil { - ctx.Warnf("Cannot sign host certificate with provided CA: %s", err) - return nil, err - } - if ctx.certStore != nil { - ctx.certStore.Store(hostname, cert) - } + + if err != nil { + ctx.Warnf("Cannot sign host certificate with provided CA: %s", err) + return nil, err } + config.Certificates = append(config.Certificates, *cert) return &config, nil } From 305c124ce887af28af8efe59d631a384222dbdec Mon Sep 17 00:00:00 2001 From: Harry Marr Date: Sat, 30 Mar 2019 19:11:14 -0500 Subject: [PATCH 3/3] Test new cert storage interface --- proxy_test.go | 56 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/proxy_test.go b/proxy_test.go index c8c68d83..2b120e26 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -6,6 +6,7 @@ import ( "crypto/tls" "crypto/x509" "encoding/base64" + "fmt" "image" "io" "io/ioutil" @@ -19,7 +20,7 @@ import ( "testing" "github.com/elazarl/goproxy" - "github.com/elazarl/goproxy/ext/image" + goproxy_image "github.com/elazarl/goproxy/ext/image" ) var acceptAllCerts = &tls.Config{InsecureSkipVerify: true} @@ -768,26 +769,35 @@ func TestHasGoproxyCA(t *testing.T) { type TestCertStorage struct { certs map[string]*tls.Certificate - loads int - stores int + hits int + misses int } -func (tcs *TestCertStorage) Store(hostname string, cert *tls.Certificate) { - tcs.stores++ - tcs.certs[hostname] = cert -} - -func (tcs *TestCertStorage) Load(hostname string) *tls.Certificate { - tcs.loads++ - return tcs.certs[hostname] +func (tcs *TestCertStorage) Fetch(hostname string, gen func() (*tls.Certificate, error)) (*tls.Certificate, error) { + var cert *tls.Certificate + var err error + cert, ok := tcs.certs[hostname] + if ok { + fmt.Printf("hit %v\n", cert == nil) + tcs.hits++ + } else { + cert, err = gen() + if err != nil { + return nil, err + } + fmt.Printf("miss %v\n", cert == nil) + tcs.certs[hostname] = cert + tcs.misses++ + } + return cert, err } -func (tcs *TestCertStorage) statLoads() int { - return tcs.loads +func (tcs *TestCertStorage) statHits() int { + return tcs.hits } -func (tcs *TestCertStorage) statStores() int { - return tcs.stores +func (tcs *TestCertStorage) statMisses() int { + return tcs.misses } func newTestCertStorage() *TestCertStorage { @@ -821,11 +831,11 @@ func TestProxyWithCertStorage(t *testing.T) { t.Error("Wrong response when mitm", resp, "expected bobo") } - if tcs.statLoads() != 1 { - t.Fatal("TestCertStorage.Load has not been called") + if tcs.statHits() != 0 { + t.Fatalf("Expected 0 cache hits, got %d", tcs.statHits()) } - if tcs.statStores() != 1 { - t.Fatal("TestCertStorage.Store has not been called") + if tcs.statMisses() != 1 { + t.Fatalf("Expected 1 cache miss, got %d", tcs.statMisses()) } // Another round - this time the certificate can be loaded @@ -833,11 +843,11 @@ func TestProxyWithCertStorage(t *testing.T) { t.Error("Wrong response when mitm", resp, "expected bobo") } - if tcs.statLoads() != 2 { - t.Fatal("TestCertStorage.Load has not been called") + if tcs.statHits() != 1 { + t.Fatalf("Expected 1 cache hit, got %d", tcs.statHits()) } - if tcs.statStores() != 1 { - t.Fatal("TestCertStorage.Store has not been called") + if tcs.statMisses() != 1 { + t.Fatalf("Expected 1 cache miss, got %d", tcs.statMisses()) } }