From 015563ab585e2bec1ab5fe998e1687ee152e58d2 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 17 Apr 2018 00:58:56 -0400 Subject: [PATCH 1/2] distinguish custom dialers in transport cache Kubernetes-commit: d45fbce37985b72ec454a8dbe73d92bf07f8726f --- transport/cache.go | 6 ++++-- transport/cache_test.go | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/transport/cache.go b/transport/cache.go index 7c40848c..83291c57 100644 --- a/transport/cache.go +++ b/transport/cache.go @@ -44,6 +44,7 @@ type tlsCacheKey struct { certData string keyData string serverName string + dial string } func (t tlsCacheKey) String() string { @@ -51,7 +52,7 @@ func (t tlsCacheKey) String() string { if len(t.keyData) > 0 { keyText = "" } - return fmt.Sprintf("insecure:%v, caData:%#v, certData:%#v, keyData:%s, serverName:%s", t.insecure, t.caData, t.certData, keyText, t.serverName) + return fmt.Sprintf("insecure:%v, caData:%#v, certData:%#v, keyData:%s, serverName:%s, dial:%s", t.insecure, t.caData, t.certData, keyText, t.serverName, t.dial) } func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { @@ -75,7 +76,7 @@ func (c *tlsTransportCache) get(config *Config) (http.RoundTripper, error) { return nil, err } // The options didn't require a custom TLS config - if tlsConfig == nil { + if tlsConfig == nil && config.Dial == nil { return http.DefaultTransport, nil } @@ -109,5 +110,6 @@ func tlsConfigKey(c *Config) (tlsCacheKey, error) { certData: string(c.TLS.CertData), keyData: string(c.TLS.KeyData), serverName: c.TLS.ServerName, + dial: fmt.Sprintf("%p", c.Dial), }, nil } diff --git a/transport/cache_test.go b/transport/cache_test.go index 81f428de..d3d14099 100644 --- a/transport/cache_test.go +++ b/transport/cache_test.go @@ -17,6 +17,7 @@ limitations under the License. package transport import ( + "net" "net/http" "testing" ) @@ -53,6 +54,8 @@ func TestTLSConfigKey(t *testing.T) { // Make sure config fields that affect the tls config affect the cache key uniqueConfigurations := map[string]*Config{ "no tls": {}, + "dialer": {Dial: net.Dial}, + "dialer2": {Dial: func(network, address string) (net.Conn, error) { return nil, nil }}, "insecure": {TLS: TLSConfig{Insecure: true}}, "cadata 1": {TLS: TLSConfig{CAData: []byte{1}}}, "cadata 2": {TLS: TLSConfig{CAData: []byte{2}}}, @@ -104,11 +107,6 @@ func TestTLSConfigKey(t *testing.T) { } for nameA, valueA := range uniqueConfigurations { for nameB, valueB := range uniqueConfigurations { - // Don't compare to ourselves - if nameA == nameB { - continue - } - keyA, err := tlsConfigKey(valueA) if err != nil { t.Errorf("Unexpected error for %q: %v", nameA, err) @@ -119,6 +117,15 @@ func TestTLSConfigKey(t *testing.T) { t.Errorf("Unexpected error for %q: %v", nameB, err) continue } + + // Make sure we get the same key on the same config + if nameA == nameB { + if keyA != keyB { + t.Errorf("Expected identical cache keys for %q and %q, got:\n\t%s\n\t%s", nameA, nameB, keyA, keyB) + } + continue + } + if keyA == keyB { t.Errorf("Expected unique cache keys for %q and %q, got:\n\t%s\n\t%s", nameA, nameB, keyA, keyB) continue From d23614d7ea1fa37296618c4f6de13051303798c4 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Tue, 17 Apr 2018 00:59:27 -0400 Subject: [PATCH 2/2] ensure tls server name is used in transport Kubernetes-commit: 6f657424743e93d064f8975a930941ba73f53110 --- transport/transport.go | 2 +- transport/transport_test.go | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/transport/transport.go b/transport/transport.go index 15be0a3e..c2bb7ae5 100644 --- a/transport/transport.go +++ b/transport/transport.go @@ -52,7 +52,7 @@ func New(config *Config) (http.RoundTripper, error) { // TLSConfigFor returns a tls.Config that will provide the transport level security defined // by the provided Config. Will return nil if no transport level security is requested. func TLSConfigFor(c *Config) (*tls.Config, error) { - if !(c.HasCA() || c.HasCertAuth() || c.TLS.Insecure) { + if !(c.HasCA() || c.HasCertAuth() || c.TLS.Insecure || len(c.TLS.ServerName) > 0) { return nil, nil } if c.HasCA() && c.TLS.Insecure { diff --git a/transport/transport_test.go b/transport/transport_test.go index 4d2d78f8..8de75156 100644 --- a/transport/transport_test.go +++ b/transport/transport_test.go @@ -101,6 +101,13 @@ func TestNew(t *testing.T) { Config: &Config{}, }, + "server name": { + TLS: true, + Config: &Config{TLS: TLSConfig{ + ServerName: "foo", + }}, + }, + "ca transport": { TLS: true, Config: &Config{