From 08593c8dd6e4b05b1855296f6d7c6247de1d9e6a Mon Sep 17 00:00:00 2001 From: Andrew Block Date: Mon, 21 Nov 2022 11:21:55 -0600 Subject: [PATCH] Added support for insecure OCI registries Signed-off-by: Andrew Block --- cmd/helm/push.go | 9 ++++++--- internal/tlsutil/tls.go | 6 ++++-- internal/tlsutil/tlsutil_test.go | 7 ++++--- pkg/action/install.go | 4 ++-- pkg/action/pull.go | 5 +++-- pkg/action/push.go | 24 ++++++++++++++++-------- pkg/getter/httpgetter.go | 4 ++-- pkg/getter/httpgetter_test.go | 3 ++- pkg/getter/ocigetter.go | 4 ++-- pkg/pusher/ocipusher.go | 4 ++-- pkg/pusher/ocipusher_test.go | 2 ++ pkg/pusher/pusher.go | 16 ++++++++++++---- pkg/registry/client_test.go | 2 +- pkg/registry/client_tls_test.go | 2 +- pkg/registry/util.go | 4 ++-- pkg/registry/utils_test.go | 10 +++++----- pkg/repo/repotest/server.go | 4 +++- 17 files changed, 69 insertions(+), 41 deletions(-) diff --git a/cmd/helm/push.go b/cmd/helm/push.go index ac6659fa30f..dffe8f4eadf 100644 --- a/cmd/helm/push.go +++ b/cmd/helm/push.go @@ -35,9 +35,10 @@ it will also be uploaded. ` type registryPushOptions struct { - certFile string - keyFile string - caFile string + certFile string + keyFile string + caFile string + insecureSkipTLSverify bool } func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { @@ -70,6 +71,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { remote := args[1] client := action.NewPushWithOpts(action.WithPushConfig(cfg), action.WithTLSClientConfig(o.certFile, o.keyFile, o.caFile), + action.WithInsecureSkipTLSVerify(o.insecureSkipTLSverify), action.WithPushOptWriter(out)) client.Settings = settings output, err := client.Run(chartRef, remote) @@ -85,6 +87,7 @@ func newPushCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.StringVar(&o.certFile, "cert-file", "", "identify registry client using this SSL certificate file") f.StringVar(&o.keyFile, "key-file", "", "identify registry client using this SSL key file") f.StringVar(&o.caFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") + f.BoolVar(&o.insecureSkipTLSverify, "insecure-skip-tls-verify", false, "skip tls certificate checks for the chart upload") return cmd } diff --git a/internal/tlsutil/tls.go b/internal/tlsutil/tls.go index ed7795dbeae..28c70f9db65 100644 --- a/internal/tlsutil/tls.go +++ b/internal/tlsutil/tls.go @@ -25,8 +25,10 @@ import ( ) // NewClientTLS returns tls.Config appropriate for client auth. -func NewClientTLS(certFile, keyFile, caFile string) (*tls.Config, error) { - config := tls.Config{} +func NewClientTLS(certFile, keyFile, caFile string, insecureSkipTLSverify bool) (*tls.Config, error) { + config := tls.Config{ + InsecureSkipVerify: insecureSkipTLSverify, + } if certFile != "" && keyFile != "" { cert, err := CertFromFilePair(certFile, keyFile) diff --git a/internal/tlsutil/tlsutil_test.go b/internal/tlsutil/tlsutil_test.go index e660c030c6c..e31a873d34c 100644 --- a/internal/tlsutil/tlsutil_test.go +++ b/internal/tlsutil/tlsutil_test.go @@ -65,8 +65,9 @@ func TestNewClientTLS(t *testing.T) { certFile := testfile(t, testCertFile) keyFile := testfile(t, testKeyFile) caCertFile := testfile(t, testCaCertFile) + insecureSkipTLSverify := false - cfg, err := NewClientTLS(certFile, keyFile, caCertFile) + cfg, err := NewClientTLS(certFile, keyFile, caCertFile, insecureSkipTLSverify) if err != nil { t.Error(err) } @@ -81,7 +82,7 @@ func TestNewClientTLS(t *testing.T) { t.Fatalf("mismatch tls RootCAs, expecting non-nil") } - cfg, err = NewClientTLS("", "", caCertFile) + cfg, err = NewClientTLS("", "", caCertFile, insecureSkipTLSverify) if err != nil { t.Error(err) } @@ -96,7 +97,7 @@ func TestNewClientTLS(t *testing.T) { t.Fatalf("mismatch tls RootCAs, expecting non-nil") } - cfg, err = NewClientTLS(certFile, keyFile, "") + cfg, err = NewClientTLS(certFile, keyFile, "", insecureSkipTLSverify) if err != nil { t.Error(err) } diff --git a/pkg/action/install.go b/pkg/action/install.go index 4d716bec77a..bc228e0c0e3 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -680,9 +680,9 @@ func (c *ChartPathOptions) LocateChart(name string, out io.Writer, settings *cli // If there is no registry client and the name is in an OCI registry return // an error and a lookup will not occur. if registry.IsOCI(name) { - if (c.CertFile != "" && c.KeyFile != "") || c.CaFile != "" { + if (c.CertFile != "" && c.KeyFile != "") || c.CaFile != "" || c.InsecureSkipTLSverify { registryClient, err := registry.NewRegistryClientWithTLS(out, c.CertFile, c.KeyFile, c.CaFile, - settings.RegistryConfig, settings.Debug) + c.InsecureSkipTLSverify, settings.RegistryConfig, settings.Debug) if err != nil { return "", err } diff --git a/pkg/action/pull.go b/pkg/action/pull.go index bcd93705d02..102e4a58881 100644 --- a/pkg/action/pull.go +++ b/pkg/action/pull.go @@ -104,9 +104,9 @@ func (p *Pull) Run(chartRef string) (string, error) { if registry.IsOCI(chartRef) { // Provide a tls enabled client for the pull command if the user has // specified the cert file or key file or ca file. - if (p.ChartPathOptions.CertFile != "" && p.ChartPathOptions.KeyFile != "") || p.ChartPathOptions.CaFile != "" { + if (p.ChartPathOptions.CertFile != "" && p.ChartPathOptions.KeyFile != "") || p.ChartPathOptions.CaFile != "" || p.ChartPathOptions.InsecureSkipTLSverify { registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.ChartPathOptions.CertFile, p.ChartPathOptions.KeyFile, p.ChartPathOptions.CaFile, - p.Settings.RegistryConfig, p.Settings.Debug) + p.ChartPathOptions.InsecureSkipTLSverify, p.Settings.RegistryConfig, p.Settings.Debug) if err != nil { return out.String(), err } @@ -114,6 +114,7 @@ func (p *Pull) Run(chartRef string) (string, error) { } c.Options = append(c.Options, getter.WithRegistryClient(p.cfg.RegistryClient)) + c.RegistryClient = p.cfg.RegistryClient } if p.Verify { diff --git a/pkg/action/push.go b/pkg/action/push.go index f6e7a6aaaf8..21f6fb94702 100644 --- a/pkg/action/push.go +++ b/pkg/action/push.go @@ -30,12 +30,13 @@ import ( // // It provides the implementation of 'helm push'. type Push struct { - Settings *cli.EnvSettings - cfg *Configuration - certFile string - keyFile string - caFile string - out io.Writer + Settings *cli.EnvSettings + cfg *Configuration + certFile string + keyFile string + caFile string + insecureSkipTLSverify bool + out io.Writer } // PushOpt is a type of function that sets options for a push action. @@ -57,6 +58,13 @@ func WithTLSClientConfig(certFile, keyFile, caFile string) PushOpt { } } +// WithInsecureSkipTLSVerify determines if a TLS Certificate will be checked +func WithInsecureSkipTLSVerify(insecureSkipTLSVerify bool) PushOpt { + return func(p *Push) { + p.insecureSkipTLSverify = insecureSkipTLSVerify + } +} + // WithOptWriter sets the registryOut field on the push configuration object. func WithPushOptWriter(out io.Writer) PushOpt { return func(p *Push) { @@ -88,9 +96,9 @@ func (p *Push) Run(chartRef string, remote string) (string, error) { if registry.IsOCI(remote) { // Provide a tls enabled client for the pull command if the user has // specified the cert file or key file or ca file. - if (p.certFile != "" && p.keyFile != "") || p.caFile != "" { + if (p.certFile != "" && p.keyFile != "") || p.caFile != "" || p.insecureSkipTLSverify { registryClient, err := registry.NewRegistryClientWithTLS(p.out, p.certFile, p.keyFile, p.caFile, - p.Settings.RegistryConfig, p.Settings.Debug) + p.insecureSkipTLSverify, p.Settings.RegistryConfig, p.Settings.Debug) if err != nil { return out.String(), err } diff --git a/pkg/getter/httpgetter.go b/pkg/getter/httpgetter.go index 081bf059eb5..b53e558e35d 100644 --- a/pkg/getter/httpgetter.go +++ b/pkg/getter/httpgetter.go @@ -123,8 +123,8 @@ func (g *HTTPGetter) httpClient() (*http.Client, error) { } }) - if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { - tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile) + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS { + tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile, g.opts.insecureSkipVerifyTLS) if err != nil { return nil, errors.Wrap(err, "can't create TLS config for client") } diff --git a/pkg/getter/httpgetter_test.go b/pkg/getter/httpgetter_test.go index 94baef3b8eb..47276351139 100644 --- a/pkg/getter/httpgetter_test.go +++ b/pkg/getter/httpgetter_test.go @@ -285,9 +285,10 @@ func TestDownload(t *testing.T) { func TestDownloadTLS(t *testing.T) { cd := "../../testdata" ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem") + insecureSkipTLSverify := false tlsSrv := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) - tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca) + tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca, insecureSkipTLSverify) if err != nil { t.Fatal(errors.Wrap(err, "can't create TLS config for client")) } diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index d64ca0fb240..1705fca91db 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -122,8 +122,8 @@ func (g *OCIGetter) newRegistryClient() (*registry.Client, error) { } }) - if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" { - tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile) + if (g.opts.certFile != "" && g.opts.keyFile != "") || g.opts.caFile != "" || g.opts.insecureSkipVerifyTLS { + tlsConf, err := tlsutil.NewClientTLS(g.opts.certFile, g.opts.keyFile, g.opts.caFile, g.opts.insecureSkipVerifyTLS) if err != nil { return nil, fmt.Errorf("can't create TLS config for client: %w", err) } diff --git a/pkg/pusher/ocipusher.go b/pkg/pusher/ocipusher.go index a431577af20..61414169833 100644 --- a/pkg/pusher/ocipusher.go +++ b/pkg/pusher/ocipusher.go @@ -106,8 +106,8 @@ func NewOCIPusher(ops ...Option) (Pusher, error) { } func (pusher *OCIPusher) newRegistryClient() (*registry.Client, error) { - if (pusher.opts.certFile != "" && pusher.opts.keyFile != "") || pusher.opts.caFile != "" { - tlsConf, err := tlsutil.NewClientTLS(pusher.opts.certFile, pusher.opts.keyFile, pusher.opts.caFile) + if (pusher.opts.certFile != "" && pusher.opts.keyFile != "") || pusher.opts.caFile != "" || pusher.opts.insecureSkipTLSverify { + tlsConf, err := tlsutil.NewClientTLS(pusher.opts.certFile, pusher.opts.keyFile, pusher.opts.caFile, pusher.opts.insecureSkipTLSverify) if err != nil { return nil, errors.Wrap(err, "can't create TLS config for client") } diff --git a/pkg/pusher/ocipusher_test.go b/pkg/pusher/ocipusher_test.go index d718cec6d56..9390710a06d 100644 --- a/pkg/pusher/ocipusher_test.go +++ b/pkg/pusher/ocipusher_test.go @@ -35,10 +35,12 @@ func TestNewOCIPusher(t *testing.T) { cd := "../../testdata" join := filepath.Join ca, pub, priv := join(cd, "rootca.crt"), join(cd, "crt.pem"), join(cd, "key.pem") + insecureSkipTLSverify := false // Test with options p, err = NewOCIPusher( WithTLSClientConfig(pub, priv, ca), + WithInsecureSkipTLSVerify(insecureSkipTLSverify), ) if err != nil { t.Fatal(err) diff --git a/pkg/pusher/pusher.go b/pkg/pusher/pusher.go index 86a5ebf907b..e325ce49829 100644 --- a/pkg/pusher/pusher.go +++ b/pkg/pusher/pusher.go @@ -27,10 +27,11 @@ import ( // // Pushers may or may not ignore these parameters as they are passed in. type options struct { - registryClient *registry.Client - certFile string - keyFile string - caFile string + registryClient *registry.Client + certFile string + keyFile string + caFile string + insecureSkipTLSverify bool } // Option allows specifying various settings configurable by the user for overriding the defaults @@ -53,6 +54,13 @@ func WithTLSClientConfig(certFile, keyFile, caFile string) Option { } } +// WithInsecureSkipTLSVerify determines if a TLS Certificate will be checked +func WithInsecureSkipTLSVerify(insecureSkipTLSVerify bool) Option { + return func(opts *options) { + opts.insecureSkipTLSverify = insecureSkipTLSVerify + } +} + // Pusher is an interface to support upload to the specified URL. type Pusher interface { // Push file content by url string diff --git a/pkg/registry/client_test.go b/pkg/registry/client_test.go index 4be6a1cd4bd..3bb4a991b80 100644 --- a/pkg/registry/client_test.go +++ b/pkg/registry/client_test.go @@ -31,7 +31,7 @@ type RegistryClientTestSuite struct { func (suite *RegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, false) + dockerRegistry := setup(&suite.TestSuite, false, false) // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/client_tls_test.go b/pkg/registry/client_tls_test.go index 3be0419bf37..c4bb31c254a 100644 --- a/pkg/registry/client_tls_test.go +++ b/pkg/registry/client_tls_test.go @@ -29,7 +29,7 @@ type TLSRegistryClientTestSuite struct { func (suite *TLSRegistryClientTestSuite) SetupSuite() { // init test client - dockerRegistry := setup(&suite.TestSuite, true) + dockerRegistry := setup(&suite.TestSuite, true, false) // Start Docker registry go dockerRegistry.ListenAndServe() diff --git a/pkg/registry/util.go b/pkg/registry/util.go index 025b145ba2c..2a27543780e 100644 --- a/pkg/registry/util.go +++ b/pkg/registry/util.go @@ -133,8 +133,8 @@ func parseReference(raw string) (registry.Reference, error) { } // NewRegistryClientWithTLS is a helper function to create a new registry client with TLS enabled. -func NewRegistryClientWithTLS(out io.Writer, certFile, keyFile, caFile string, registryConfig string, debug bool) (*Client, error) { - tlsConf, err := tlsutil.NewClientTLS(certFile, keyFile, caFile) +func NewRegistryClientWithTLS(out io.Writer, certFile, keyFile, caFile string, insecureSkipTLSverify bool, registryConfig string, debug bool) (*Client, error) { + tlsConf, err := tlsutil.NewClientTLS(certFile, keyFile, caFile, insecureSkipTLSverify) if err != nil { return nil, fmt.Errorf("can't create TLS config for client: %s", err) } diff --git a/pkg/registry/utils_test.go b/pkg/registry/utils_test.go index abe9d649f76..7cd33879331 100644 --- a/pkg/registry/utils_test.go +++ b/pkg/registry/utils_test.go @@ -66,7 +66,7 @@ type TestSuite struct { RegistryClient *Client } -func setup(suite *TestSuite, secure bool) *registry.Registry { +func setup(suite *TestSuite, tlsEnabled bool, insecure bool) *registry.Registry { suite.WorkspaceDir = testWorkspaceDir os.RemoveAll(suite.WorkspaceDir) os.Mkdir(suite.WorkspaceDir, 0700) @@ -79,9 +79,9 @@ func setup(suite *TestSuite, secure bool) *registry.Registry { credentialsFile := filepath.Join(suite.WorkspaceDir, CredentialsFileBasename) // init test client - if secure { + if tlsEnabled { var tlsConf *tls.Config - tlsConf, err = tlsutil.NewClientTLS(tlsCert, tlsKey, tlsCA) + tlsConf, err = tlsutil.NewClientTLS(tlsCert, tlsKey, tlsCA, insecure) httpClient := &http.Client{ Transport: &http.Transport{ TLSClientConfig: tlsConf, @@ -117,7 +117,7 @@ func setup(suite *TestSuite, secure bool) *registry.Registry { config := &configuration.Configuration{} port, err := freeport.GetFreePort() suite.Nil(err, "no error finding free port for test registry") - if secure { + if tlsEnabled { // docker has "MatchLocalhost is a host match function which returns true for // localhost, and is used to enforce http for localhost requests." // That function does not handle matching of ip addresses in octal, @@ -138,7 +138,7 @@ func setup(suite *TestSuite, secure bool) *registry.Registry { } // config tls - if secure { + if tlsEnabled { // TLS config // this set tlsConf.ClientAuth = tls.RequireAndVerifyClientCert in the // server tls config diff --git a/pkg/repo/repotest/server.go b/pkg/repo/repotest/server.go index 90ad3d856c9..5753dbeb948 100644 --- a/pkg/repo/repotest/server.go +++ b/pkg/repo/repotest/server.go @@ -360,6 +360,7 @@ func (s *Server) Start() { func (s *Server) StartTLS() { cd := "../../testdata" ca, pub, priv := filepath.Join(cd, "rootca.crt"), filepath.Join(cd, "crt.pem"), filepath.Join(cd, "key.pem") + insecure := false s.srv = httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if s.middleware != nil { @@ -367,7 +368,7 @@ func (s *Server) StartTLS() { } http.FileServer(http.Dir(s.Root())).ServeHTTP(w, r) })) - tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca) + tlsConf, err := tlsutil.NewClientTLS(pub, priv, ca, insecure) if err != nil { panic(err) } @@ -400,6 +401,7 @@ func (s *Server) Stop() { // URL returns the URL of the server. // // Example: +// // http://localhost:1776 func (s *Server) URL() string { return s.srv.URL