diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index b53e558e3..f24f8b5a5 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -26,7 +26,6 @@ import ( "github.com/pkg/errors" "helm.sh/helm/v3/internal/tlsutil" - "helm.sh/helm/v3/internal/urlutil" "helm.sh/helm/v3/internal/version" ) @@ -129,12 +128,6 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { return nil, errors.Wrap(err, "can't create TLS config for client") } - sni, err := urlutil.ExtractHostname(g.opts.url) - if err != nil { - return nil, err - } - tlsConf.ServerName = sni - g.transport.TLSClientConfig = tlsConf } diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index c727d0d7c..b4283b986 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -18,6 +18,7 @@ package getter import ( "fmt" "io" + "io/ioutil" "net/http" "net/http/httptest" "net/url" @@ -331,6 +332,121 @@ func TestDownloadTLS(t *testing.T) { } } +func TestDownloadTLSWithRedirect(t *testing.T) { + cd := "../../testdata" + srv2Resp := "hello" + insecureSkipTLSverify := false + + // Server 2 that will actually fulfil the request. + ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "localhost-crt.pem"), filepath.Join(cd, "key.pem") + tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca, insecureSkipTLSverify) + if err != nil { + t.Fatal(errors.Wrap(err, "can't create TLS config for client")) + } + + tlsSrv2 := httptest.NewUnstartedServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + rw.Header().Set("Content-Type", "text/plain") + rw.Write([]byte(srv2Resp)) + })) + + tlsSrv2.TLS = tlsConf + tlsSrv2.StartTLS() + defer tlsSrv2.Close() + + // Server 1 responds with a redirect to Server 2. + ca, pub, priv = filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem") + tlsConf, err = tlsutil.NewClientTLS(pub, priv, ca, insecureSkipTLSverify) + if err != nil { + t.Fatal(errors.Wrap(err, "can't create TLS config for client")) + } + + tlsSrv1 := httptest.NewUnstartedServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + u, _ := url.ParseRequestURI(tlsSrv2.URL) + + // Make the request using the hostname 'localhost' (to which 'localhost-crt.pem' is issued) + // to verify that a successful TLS connection is made even if the client doesn't specify + // the hostname (SNI) in `tls.Config.ServerName`. By default the hostname is derived from the + // request URL for every request (including redirects). Setting `tls.Config.ServerName` on the + // client just overrides the remote endpoint's hostname. + // See https://golang.org/src/net/http/transport.go#L1505. + u.Host = fmt.Sprintf("localhost:%s", u.Port()) + + http.Redirect(rw, r, u.String(), http.StatusTemporaryRedirect) + })) + + tlsSrv1.TLS = tlsConf + tlsSrv1.StartTLS() + defer tlsSrv1.Close() + + u, _ := url.ParseRequestURI(tlsSrv1.URL) + + t.Run("Test with TLS", func(t *testing.T) { + g, err := NewHTTPGetter( + WithURL(u.String()), + WithTLSClientConfig(pub, priv, ca), + ) + if err != nil { + t.Fatal(err) + } + + buf, err := g.Get(u.String()) + if err != nil { + t.Error(err) + } + + b, err := ioutil.ReadAll(buf) + if err != nil { + t.Error(err) + } + + if string(b) != srv2Resp { + t.Errorf("expected response from Server2 to be '%s', instead got: %s", srv2Resp, string(b)) + } + }) + + t.Run("Test with TLS config being passed along in .Get (see #6635)", func(t *testing.T) { + g, err := NewHTTPGetter() + if err != nil { + t.Fatal(err) + } + + buf, err := g.Get(u.String(), WithURL(u.String()), WithTLSClientConfig(pub, priv, ca)) + if err != nil { + t.Error(err) + } + + b, err := ioutil.ReadAll(buf) + if err != nil { + t.Error(err) + } + + if string(b) != srv2Resp { + t.Errorf("expected response from Server2 to be '%s', instead got: %s", srv2Resp, string(b)) + } + }) + + t.Run("Test with only the CA file (see also #6635)", func(t *testing.T) { + g, err := NewHTTPGetter() + if err != nil { + t.Fatal(err) + } + + buf, err := g.Get(u.String(), WithURL(u.String()), WithTLSClientConfig("", "", ca)) + if err != nil { + t.Error(err) + } + + b, err := ioutil.ReadAll(buf) + if err != nil { + t.Error(err) + } + + if string(b) != srv2Resp { + t.Errorf("expected response from Server2 to be '%s', instead got: %s", srv2Resp, string(b)) + } + }) +} + func TestDownloadInsecureSkipTLSVerify(t *testing.T) { ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) defer ts.Close() @@ -423,9 +539,6 @@ func TestHttpClientInsecureSkipVerify(t *testing.T) { if len(transport.TLSClientConfig.Certificates) <= 0 { t.Fatal("transport.TLSClientConfig.Certificates is not present") } - if transport.TLSClientConfig.ServerName == "" { - t.Fatal("TLSClientConfig.ServerName is blank") - } } func verifyInsecureSkipVerify(t *testing.T, g *HTTPGetter, caseName string, expectedValue bool) *http.Transport { diff --git a/testdata/localhost-cert.pem b/testdata/localhost-cert.pem new file mode 100644 index 000000000..919037cd5 --- /dev/null +++ b/testdata/localhost-cert.pem @@ -0,0 +1,73 @@ +Certificate: + Data: + Version: 3 (0x2) + Serial Number: + 21:73:9a:e7:be:ce:22:31:b5:21:c9:0c:ee:b6:08:1f:37:df:25:bb + Signature Algorithm: sha256WithRSAEncryption + Issuer: C=US, ST=CO, L=Boulder, O=Helm, CN=helm.sh + Validity + Not Before: Mar 25 00:42:21 2021 GMT + Not After : Mar 23 00:42:21 2031 GMT + Subject: C=CA, ST=ON, L=Kitchener, O=Helm, CN=localhost + Subject Public Key Info: + Public Key Algorithm: rsaEncryption + RSA Public-Key: (2048 bit) + Modulus: + 00:c8:89:55:0d:0b:f1:da:e6:c0:70:7d:d3:27:cd: + b8:a8:81:8b:7c:a4:89:e5:d1:b1:78:01:1d:df:44: + 88:0b:fc:d6:81:35:3d:d1:3b:5e:8f:bb:93:b3:7e: + 28:db:ed:ff:a0:13:3a:70:a3:fe:94:6b:0b:fe:fb: + 63:00:b0:cb:dc:81:cd:80:dc:d0:2f:bf:b2:4f:9a: + 81:d4:22:dc:97:c8:8f:27:86:59:91:fa:92:05:75: + c4:cc:6b:f5:a9:6b:74:1e:f5:db:a9:f8:bf:8c:a2: + 25:fd:a0:cc:79:f4:25:57:74:a9:23:9b:e2:b7:22: + 7a:14:7a:3d:ea:f1:7e:32:6b:57:6c:2e:c6:4f:75: + 54:f9:6b:54:d2:ca:eb:54:1c:af:39:15:9b:d0:7c: + 0f:f8:55:51:04:ea:da:fa:7b:8b:63:0f:ac:39:b1: + f6:4b:8e:4e:f6:ea:e9:7b:e6:ba:5e:5a:8e:91:ef: + dc:b1:7d:52:3f:73:83:52:46:83:48:49:ff:f2:2d: + ca:54:f2:36:bb:49:cc:59:99:c0:9e:cf:8e:78:55: + 6c:ed:7d:7e:83:b8:59:2c:7d:f8:1a:81:f0:7d:f5: + 27:f2:db:ae:d4:31:54:38:fe:47:b2:ee:16:20:0f: + f1:db:2d:28:bf:6f:38:eb:11:bb:9a:d4:b2:5a:3a: + 4a:7f + Exponent: 65537 (0x10001) + X509v3 extensions: + X509v3 Subject Alternative Name: + DNS:localhost + Signature Algorithm: sha256WithRSAEncryption + bd:f8:df:36:d9:9e:14:3b:4f:68:b6:d4:40:e0:89:51:e1:a1: + f1:4d:ec:9f:f2:78:e8:f1:4c:45:aa:4b:4a:7c:39:db:b1:9f: + 76:56:5b:d1:7e:46:67:9a:7a:52:f3:f8:3d:26:92:d8:c9:06: + 6e:00:a9:ce:4d:98:24:0a:5a:4b:cc:49:91:9a:ef:ce:77:67: + df:50:d3:66:d1:34:32:aa:17:c8:71:d5:b4:97:b0:a3:a0:9c: + 3b:c4:c2:d6:b6:91:77:4d:68:89:d3:84:c9:6d:42:db:55:96: + 2c:25:40:60:1d:38:41:76:0b:3f:b7:e1:7e:05:82:db:7a:56: + e0:25:ad:34:62:1f:fa:49:18:3e:62:6a:ef:5b:8f:0d:3f:06: + 8a:9b:f7:a7:5f:b3:8e:26:62:5f:92:ab:43:e7:dd:79:90:c8: + 01:09:c3:42:cd:d8:e0:16:17:4f:71:20:18:07:51:b8:60:c1: + 61:3f:76:f1:3e:1e:ad:d5:52:33:27:c3:ef:0f:78:ab:c1:95: + 0e:34:b4:5f:92:54:33:fd:e0:7d:34:27:80:e5:94:a9:2d:db: + 7e:d9:c8:e2:ec:8e:cf:ec:dd:41:6e:d4:c9:2c:2d:a4:eb:63: + a7:4e:62:a7:44:a8:19:e6:7c:47:4f:d2:aa:7f:21:fd:90:a6: + 4c:b4:b3:7a +-----BEGIN CERTIFICATE----- +MIIDRDCCAiygAwIBAgIUIXOa577OIjG1IckM7rYIHzffJbswDQYJKoZIhvcNAQEL +BQAwTTELMAkGA1UEBhMCVVMxCzAJBgNVBAgMAkNPMRAwDgYDVQQHDAdCb3VsZGVy +MQ0wCwYDVQQKDARIZWxtMRAwDgYDVQQDDAdoZWxtLnNoMB4XDTIxMDMyNTAwNDIy +MVoXDTMxMDMyMzAwNDIyMVowUTELMAkGA1UEBhMCQ0ExCzAJBgNVBAgMAk9OMRIw +EAYDVQQHDAlLaXRjaGVuZXIxDTALBgNVBAoMBEhlbG0xEjAQBgNVBAMMCWxvY2Fs +aG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMiJVQ0L8drmwHB9 +0yfNuKiBi3ykieXRsXgBHd9EiAv81oE1PdE7Xo+7k7N+KNvt/6ATOnCj/pRrC/77 +YwCwy9yBzYDc0C+/sk+agdQi3JfIjyeGWZH6kgV1xMxr9alrdB7126n4v4yiJf2g +zHn0JVd0qSOb4rciehR6PerxfjJrV2wuxk91VPlrVNLK61QcrzkVm9B8D/hVUQTq +2vp7i2MPrDmx9kuOTvbq6Xvmul5ajpHv3LF9Uj9zg1JGg0hJ//ItylTyNrtJzFmZ +wJ7PjnhVbO19foO4WSx9+BqB8H31J/LbrtQxVDj+R7LuFiAP8dstKL9vOOsRu5rU +slo6Sn8CAwEAAaMYMBYwFAYDVR0RBA0wC4IJbG9jYWxob3N0MA0GCSqGSIb3DQEB +CwUAA4IBAQC9+N822Z4UO09ottRA4IlR4aHxTeyf8njo8UxFqktKfDnbsZ92VlvR +fkZnmnpS8/g9JpLYyQZuAKnOTZgkClpLzEmRmu/Od2ffUNNm0TQyqhfIcdW0l7Cj +oJw7xMLWtpF3TWiJ04TJbULbVZYsJUBgHThBdgs/t+F+BYLbelbgJa00Yh/6SRg+ +YmrvW48NPwaKm/enX7OOJmJfkqtD5915kMgBCcNCzdjgFhdPcSAYB1G4YMFhP3bx +Ph6t1VIzJ8PvD3irwZUONLRfklQz/eB9NCeA5ZSpLdt+2cji7I7P7N1BbtTJLC2k +62OnTmKnRKgZ5nxHT9KqfyH9kKZMtLN6 +-----END CERTIFICATE----- diff --git a/testdata/openssl.conf b/testdata/openssl.conf index 9b27e445b..be5ff04b7 100644 --- a/testdata/openssl.conf +++ b/testdata/openssl.conf @@ -40,3 +40,7 @@ subjectAltName = @alternate_names [alternate_names] DNS.1 = helm.sh IP.1 = 127.0.0.1 + +# # Used to generate localhost-crt.pem +# [alternate_names] +# DNS.1 = localhost