From 5917b42e6875f12263c77fba2d9639ba374ca8c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=BCchinger=20Dominic?= Date: Mon, 28 Jun 2021 00:58:04 +0200 Subject: [PATCH] Adds urlutil.CanonicalAddr and use it when comparing chart repository and chart url MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a chart repository was added with https://myhost/chart and the corresponding index.yaml file points to https://myhost:443/helm/somechart-1.0.0.tgz the newly introduced security check in v3.6.1 will mark the two hostname and scheme comparison as different. Canonicalize the hostname by always adding the port, if not defined, based upon the used scheme. http maps to '80' https maps to '443' Signed-off-by: Lüchinger Dominic --- internal/urlutil/urlutil.go | 18 ++++++++++++++++ internal/urlutil/urlutil_test.go | 35 +++++++++++++++++++++++++++++++- pkg/action/install.go | 4 +++- pkg/getter/httpgetter.go | 2 +- 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/internal/urlutil/urlutil.go b/internal/urlutil/urlutil.go index a8cf7398c..b21167b69 100644 --- a/internal/urlutil/urlutil.go +++ b/internal/urlutil/urlutil.go @@ -17,6 +17,7 @@ limitations under the License. package urlutil import ( + "net" "net/url" "path" "path/filepath" @@ -71,3 +72,20 @@ func ExtractHostname(addr string) (string, error) { } return u.Hostname(), nil } + +// From: https://golang.org/src/net/http/transport.go:2712 +var portMap = map[string]string{ + "http": "80", + "https": "443", +} + +// CanonicalAddr returns url.Host but always with a ":port" suffix +// From: https://golang.org/src/net/http/transport.go:2719 without idna to ascii conversion +func CanonicalAddr(url *url.URL) string { + addr := url.Hostname() + port := url.Port() + if port == "" { + port = portMap[url.Scheme] + } + return net.JoinHostPort(addr, port) +} diff --git a/internal/urlutil/urlutil_test.go b/internal/urlutil/urlutil_test.go index 82acc40fe..caabb2f70 100644 --- a/internal/urlutil/urlutil_test.go +++ b/internal/urlutil/urlutil_test.go @@ -16,7 +16,10 @@ limitations under the License. package urlutil -import "testing" +import ( + "net/url" + "testing" +) func TestURLJoin(t *testing.T) { tests := []struct { @@ -79,3 +82,33 @@ func TestExtractHostname(t *testing.T) { } } } + +func TestCanonicalAddr(t *testing.T) { + tests := map[*url.URL]string{ + { + Scheme: "http", + Host: "example.com", + Path: "/", + }: "example.com:80", + { + Scheme: "https", + Host: "example.com", + Path: "/", + }: "example.com:443", + { + Scheme: "https", + Host: "example.com", + Path: "/foo", + }: "example.com:443", + { + Scheme: "https", + Host: "example.com:31337", + Path: "/not/with/a/bang/but/a/whimper", + }: "example.com:31337", + } + for urlInput, expect := range tests { + if got := CanonicalAddr(urlInput); got != expect { + t.Errorf("Got %q, expected %q", got, expect) + } + } +} diff --git a/pkg/action/install.go b/pkg/action/install.go index 25274fcd2..8d7b45f05 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -28,6 +28,8 @@ import ( "text/template" "time" + "helm.sh/helm/v3/internal/urlutil" + "github.com/Masterminds/sprig/v3" "github.com/pkg/errors" v1 "k8s.io/api/core/v1" @@ -688,7 +690,7 @@ func (c *ChartPathOptions) LocateChart(name string, settings *cli.EnvSettings) ( // Host on URL (returned from url.Parse) contains the port if present. // This check ensures credentials are not passed between different // services on different ports. - if c.PassCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) { + if c.PassCredentialsAll || (u1.Scheme == u2.Scheme && urlutil.CanonicalAddr(u1) == urlutil.CanonicalAddr(u2)) { dl.Options = append(dl.Options, getter.WithBasicAuth(c.Username, c.Password)) } else { dl.Options = append(dl.Options, getter.WithBasicAuth("", "")) diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 822abad2e..a41ee6f4a 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -71,7 +71,7 @@ func (g *HTTPGetter) get(href string) (*bytes.Buffer, error) { // Host on URL (returned from url.Parse) contains the port if present. // This check ensures credentials are not passed between different // services on different ports. - if g.opts.passCredentialsAll || (u1.Scheme == u2.Scheme && u1.Host == u2.Host) { + if g.opts.passCredentialsAll || (u1.Scheme == u2.Scheme && urlutil.CanonicalAddr(u1) == urlutil.CanonicalAddr(u2)) { if g.opts.username != "" && g.opts.password != "" { req.SetBasicAuth(g.opts.username, g.opts.password) }