From 66e589ab974097d917e36092b1e46496acfba768 Mon Sep 17 00:00:00 2001 From: Ming Luo Date: Sun, 3 May 2020 13:39:51 -0400 Subject: [PATCH 1/4] VerifyPeerCertificate callback to implement skip Hostname verification --- pulsar/internal/connection.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pulsar/internal/connection.go b/pulsar/internal/connection.go index dace305620..39cc75e12f 100644 --- a/pulsar/internal/connection.go +++ b/pulsar/internal/connection.go @@ -713,6 +713,41 @@ func (c *connection) getTLSConfig() (*tls.Config, error) { if c.tlsOptions.ValidateHostname { tlsConfig.ServerName = c.physicalAddr.Hostname() + } else if !tlsConfig.InsecureSkipVerify { + // Solution is credited to https://github.com/golang/go/issues/21971 + // Code is adapted from the original implementation of handshake_client.go at + // https://github.com/golang/go/blob/81555cb4f3521b53f9de4ce15f64b77cc9df61b9/src/crypto/tls/handshake_client.go#L327-L344,%20but%20adapted%20to%20skip%20the%20hostname%20verification + // disable the default verification; use customized VerifyPeerCertificate + tlsConfig.InsecureSkipVerify = true + tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, certChain [][]*x509.Certificate) error { + // If this is the first handshake on a connection, process and + // (optionally) verify the server's certificates. + certs := make([]*x509.Certificate, len(rawCerts)) + for i, asn1Data := range rawCerts { + cert, err := x509.ParseCertificate(asn1Data) + if err != nil { + return fmt.Errorf("tls: failed to parse server certificate error: %s", err.Error()) + } + certs[i] = cert + } + + // Or we could pass c.physicalAddr.Hostname() so this becomes one code path for HostName verification + opts := x509.VerifyOptions{ + Roots: tlsConfig.RootCAs, + CurrentTime: time.Now(), + DNSName: "", // skip hostname verification + Intermediates: x509.NewCertPool(), + } + + for i, cert := range certs { + if i == 0 { + continue + } + opts.Intermediates.AddCert(cert) + } + _, err := certs[0].Verify(opts) + return err + } } cert, err := c.auth.GetTLSCertificate() From 9fd6cddbb52c8f13e743d54bafada8f89f2f8d17 Mon Sep 17 00:00:00 2001 From: Ming Luo Date: Sun, 3 May 2020 16:15:58 -0400 Subject: [PATCH 2/4] fix linting - comment line too long --- pulsar/internal/connection.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pulsar/internal/connection.go b/pulsar/internal/connection.go index 39cc75e12f..9160a4f81b 100644 --- a/pulsar/internal/connection.go +++ b/pulsar/internal/connection.go @@ -716,7 +716,7 @@ func (c *connection) getTLSConfig() (*tls.Config, error) { } else if !tlsConfig.InsecureSkipVerify { // Solution is credited to https://github.com/golang/go/issues/21971 // Code is adapted from the original implementation of handshake_client.go at - // https://github.com/golang/go/blob/81555cb4f3521b53f9de4ce15f64b77cc9df61b9/src/crypto/tls/handshake_client.go#L327-L344,%20but%20adapted%20to%20skip%20the%20hostname%20verification + // https://github.com/golang/go/blob/master/src/crypto/tls/handshake_client.go#L804 // disable the default verification; use customized VerifyPeerCertificate tlsConfig.InsecureSkipVerify = true tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, certChain [][]*x509.Certificate) error { From 0b331e1fa3a6d4e62eaec5cf2ac96c40efc4bb62 Mon Sep 17 00:00:00 2001 From: Ming Luo Date: Mon, 25 May 2020 23:26:46 -0400 Subject: [PATCH 3/4] always set ServerName --- pulsar/internal/connection.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pulsar/internal/connection.go b/pulsar/internal/connection.go index 9160a4f81b..fe91d47ffa 100644 --- a/pulsar/internal/connection.go +++ b/pulsar/internal/connection.go @@ -711,14 +711,13 @@ func (c *connection) getTLSConfig() (*tls.Config, error) { } } - if c.tlsOptions.ValidateHostname { - tlsConfig.ServerName = c.physicalAddr.Hostname() - } else if !tlsConfig.InsecureSkipVerify { + tlsConfig.ServerName = c.physicalAddr.Hostname() + + if tlsConfig.InsecureSkipVerify { // Solution is credited to https://github.com/golang/go/issues/21971 // Code is adapted from the original implementation of handshake_client.go at // https://github.com/golang/go/blob/master/src/crypto/tls/handshake_client.go#L804 // disable the default verification; use customized VerifyPeerCertificate - tlsConfig.InsecureSkipVerify = true tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, certChain [][]*x509.Certificate) error { // If this is the first handshake on a connection, process and // (optionally) verify the server's certificates. From cd2a1035540d9291cb0347bb77b51dc6b9d55e7d Mon Sep 17 00:00:00 2001 From: Ming Luo Date: Tue, 26 May 2020 09:53:08 -0400 Subject: [PATCH 4/4] only verify cert if it exists --- pulsar/client_impl_test.go | 18 ++++++++++++++++++ pulsar/internal/connection.go | 6 +++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pulsar/client_impl_test.go b/pulsar/client_impl_test.go index 8917c46b0f..c94232af94 100644 --- a/pulsar/client_impl_test.go +++ b/pulsar/client_impl_test.go @@ -67,6 +67,24 @@ func TestTLSInsecureConnection(t *testing.T) { client.Close() } +func TestTLSInsecureConnectionWithCerts(t *testing.T) { + client, err := NewClient(ClientOptions{ + URL: serviceURLTLS, + TLSAllowInsecureConnection: true, + TLSTrustCertsFilePath: caCertsPath, + }) + assert.NoError(t, err) + + producer, err := client.CreateProducer(ProducerOptions{ + Topic: newTopicName(), + }) + + assert.NoError(t, err) + assert.NotNil(t, producer) + + client.Close() +} + func TestTLSConnection(t *testing.T) { client, err := NewClient(ClientOptions{ URL: serviceURLTLS, diff --git a/pulsar/internal/connection.go b/pulsar/internal/connection.go index fe91d47ffa..6d022be8ed 100644 --- a/pulsar/internal/connection.go +++ b/pulsar/internal/connection.go @@ -730,7 +730,11 @@ func (c *connection) getTLSConfig() (*tls.Config, error) { certs[i] = cert } - // Or we could pass c.physicalAddr.Hostname() so this becomes one code path for HostName verification + if tlsConfig.RootCAs == nil { + return nil + } + + // Verify certs if they exist but skip ServerName validation opts := x509.VerifyOptions{ Roots: tlsConfig.RootCAs, CurrentTime: time.Now(),