Merge pull request #18 from google/fix-mitm-sans-sni

mitm: tls.Config should have empty Certificates to force execution of GetCertificate.
google · Aug 12, 2015 · 30c9e83 · 30c9e83
2 parents f12d9b7 + 74e6a27
commit 30c9e83
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 33 deletions.
diff --git a/mitm/mitm.go b/mitm/mitm.go
@@ -162,13 +162,6 @@ func (c *Config) SetOrganization(org string) {
 // the SNI extension in the TLS ClientHello.
 func (c *Config) TLS() *tls.Config {
 	return &tls.Config{
-		Certificates: []tls.Certificate{
-			{
-				Certificate: [][]byte{c.ca.Raw},
-				PrivateKey:  c.capriv,
-				Leaf:        c.ca,
-			},
-		},
 		GetCertificate: func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) {
 			if clientHello.ServerName == "" {
 				return nil, errors.New("mitm: SNI not provided, failed to build certificate")
@@ -184,19 +177,13 @@ func (c *Config) TLS() *tls.Config {
 // using SNI from the connection, or fall back to the provided hostname.
 func (c *Config) TLSForHost(hostname string) *tls.Config {
 	return &tls.Config{
-		Certificates: []tls.Certificate{
-			{
-				Certificate: [][]byte{c.ca.Raw},
-				PrivateKey:  c.capriv,
-				Leaf:        c.ca,
-			},
-		},
 		GetCertificate: func(clientHello *tls.ClientHelloInfo) (*tls.Certificate, error) {
-			if clientHello.ServerName == "" {
-				return c.cert(hostname)
+			host := clientHello.ServerName
+			if host == "" {
+				host = hostname
 			}
 
-			return c.cert(clientHello.ServerName)
+			return c.cert(host)
 		},
 		NextProtos: []string{"http/1.1"},
 	}
@@ -267,7 +254,7 @@ func (c *Config) cert(hostname string) (*tls.Certificate, error) {
 	}
 
 	tlsc = &tls.Certificate{
-		Certificate: [][]byte{raw},
+		Certificate: [][]byte{raw, c.ca.Raw},
 		PrivateKey:  c.priv,
 		Leaf:        x509c,
 	}

diff --git a/mitm/mitm_test.go b/mitm/mitm_test.go
@@ -43,12 +43,6 @@ func TestMITM(t *testing.T) {
 	if got := conf.NextProtos; !reflect.DeepEqual(got, protos) {
 		t.Errorf("conf.NextProtos: got %v, want %v", got, protos)
 	}
-	if got, want := len(conf.Certificates), 1; got != want {
-		t.Fatalf("len(conf.Certificates): got %d, want %d", got, want)
-	}
-	if !conf.Certificates[0].Leaf.IsCA {
-		t.Error("conf.Certificates[0].Leaf.IsCA: got false, want true")
-	}
 
 	// Simulate a TLS connection without SNI.
 	clientHello := &tls.ClientHelloInfo{
@@ -76,12 +70,6 @@ func TestMITM(t *testing.T) {
 	if got := conf.NextProtos; !reflect.DeepEqual(got, protos) {
 		t.Errorf("conf.NextProtos: got %v, want %v", got, protos)
 	}
-	if got, want := len(conf.Certificates), 1; got != want {
-		t.Fatalf("len(conf.Certificates): got %d, want %d", got, want)
-	}
-	if !conf.Certificates[0].Leaf.IsCA {
-		t.Error("conf.Certificates[0].Leaf.IsCA: got false, want true")
-	}
 
 	// Set SNI, takes precendence over host.
 	clientHello.ServerName = "google.com"
@@ -119,7 +107,7 @@ func TestCert(t *testing.T) {
 		t.Fatalf("NewConfig(): got %v, want no error", err)
 	}
 
-	tlsc, err := c.cert("example.com:8080")
+	tlsc, err := c.cert("example.com")
 	if err != nil {
 		t.Fatalf("c.cert(%q): got %v, want no error", "example.com:8080", err)
 	}
@@ -142,6 +130,10 @@ func TestCert(t *testing.T) {
 	if got, want := x509c.Subject.CommonName, "example.com"; got != want {
 		t.Errorf("X509c.Subject.CommonName: got %q, want %q", got, want)
 	}
+	if err := x509c.VerifyHostname("example.com"); err != nil {
+		t.Errorf("x509c.VerifyHostname(%q): got %v, want no error", "example.com", err)
+	}
+
 	if got, want := x509c.Subject.Organization, []string{"Martian Proxy"}; !reflect.DeepEqual(got, want) {
 		t.Errorf("x509c.Subject.Organization: got %v, want %v", got, want)
 	}

diff --git a/proxy_test.go b/proxy_test.go
@@ -411,10 +411,11 @@ func TestIntegrationConnect(t *testing.T) {
 		t.Fatalf("mitm.NewConfig(): got %v, want no error", err)
 	}
 
-	tl, err := tls.Listen("tcp", "[::1]:0", mc.TLS())
+	tl, err := net.Listen("tcp", "[::1]:0")
 	if err != nil {
 		t.Fatalf("tls.Listen(): got %v, want no error", err)
 	}
+	tl = tls.NewListener(tl, mc.TLS())
 
 	go http.Serve(tl, http.HandlerFunc(
 		func(rw http.ResponseWriter, req *http.Request) {
@@ -803,10 +804,14 @@ func TestIntegrationTransparentMITM(t *testing.T) {
 
 	// Start TLS listener with config that will generate certificates based on
 	// SNI from connection.
-	l, err := tls.Listen("tcp", "[::1]:0", mc.TLS())
+	//
+	// BUG: tls.Listen will not accept a tls.Config where Certificates is empty,
+	// even though it is supported by tls.Server when GetCertificate is not nil.
+	l, err := net.Listen("tcp", "[::1]:0")
 	if err != nil {
 		t.Fatalf("net.Listen(): got %v, want no error", err)
 	}
+	l = tls.NewListener(l, mc.TLS())
 
 	p := NewProxy()
 	defer p.Close()