From 90e575cd9c44463b4df91cdf960b940f7f6c44ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20Min=C3=A1=C5=99?= Date: Mon, 31 Jul 2017 14:01:55 +0200 Subject: [PATCH] image-pruner: Reenable registry-url validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Michal Minář --- pkg/image/apis/image/helper.go | 42 +++++++++++++++ pkg/image/apis/image/helper_test.go | 82 +++++++++++++++++++++++++++++ pkg/oc/admin/prune/images.go | 31 +++++++---- 3 files changed, 144 insertions(+), 11 deletions(-) diff --git a/pkg/image/apis/image/helper.go b/pkg/image/apis/image/helper.go index 2100045d5229..b334a2b10d32 100644 --- a/pkg/image/apis/image/helper.go +++ b/pkg/image/apis/image/helper.go @@ -41,6 +41,10 @@ const ( ImportRegistryNotAllowed = "registry is not allowed for import" ) +var errNoRegistryURLPathAllowed = fmt.Errorf("no path after [:] is allowed") +var errNoRegistryURLQueryAllowed = fmt.Errorf("no query arguments are allowed after [:]") +var errRegistryURLHostEmpty = fmt.Errorf("no host name specified") + // DefaultRegistry returns the default Docker registry (host or host:port), or false if it is not available. type DefaultRegistry interface { DefaultRegistry() (string, bool) @@ -1161,3 +1165,41 @@ func (tagref TagReference) HasAnnotationTag(searchTag string) bool { } return false } + +// ValidateRegistryURL returns error if the given input is not a valid registry URL. The url may be prefixed +// with http:// or https:// schema. It may not contain any path or query after the host:[port]. +func ValidateRegistryURL(registryURL string) error { + var ( + u *url.URL + err error + parts = strings.SplitN(registryURL, "://", 2) + ) + + switch len(parts) { + case 2: + u, err = url.Parse(registryURL) + if err != nil { + return err + } + switch u.Scheme { + case "http", "https": + default: + return fmt.Errorf("unsupported scheme: %s", u.Scheme) + } + case 1: + u, err = url.Parse("https://" + registryURL) + if err != nil { + return err + } + } + if len(u.Path) > 0 && u.Path != "/" { + return errNoRegistryURLPathAllowed + } + if len(u.RawQuery) > 0 { + return errNoRegistryURLQueryAllowed + } + if len(u.Host) == 0 { + return errRegistryURLHostEmpty + } + return nil +} diff --git a/pkg/image/apis/image/helper_test.go b/pkg/image/apis/image/helper_test.go index e55e0e862bda..fadaec195bd5 100644 --- a/pkg/image/apis/image/helper_test.go +++ b/pkg/image/apis/image/helper_test.go @@ -1834,3 +1834,85 @@ func TestDockerImageReferenceForImage(t *testing.T) { t.Errorf("expected failure for unknown image") } } + +func TestValidateRegistryURL(t *testing.T) { + for _, tc := range []struct { + input string + expectedError bool + expectedErrorString string + }{ + {input: "172.30.30.30:5000"}, + {input: ":5000"}, + {input: "[fd12:3456:789a:1::1]:80/"}, + {input: "[fd12:3456:789a:1::1]:80"}, + {input: "http://172.30.30.30:5000"}, + {input: "http://[fd12:3456:789a:1::1]:5000/"}, + {input: "http://[fd12:3456:789a:1::1]:5000"}, + {input: "http://registry.org:5000"}, + {input: "https://172.30.30.30:5000"}, + {input: "https://:80/"}, + {input: "https://[fd12:3456:789a:1::1]/"}, + {input: "https://[fd12:3456:789a:1::1]"}, + {input: "https://[fd12:3456:789a:1::1]:5000/"}, + {input: "https://[fd12:3456:789a:1::1]:5000"}, + {input: "https://registry.org/"}, + {input: "https://registry.org"}, + {input: "localhost/"}, + {input: "localhost"}, + {input: "localhost:80"}, + {input: "registry.org/"}, + {input: "registry.org"}, + {input: "registry.org:5000"}, + + { + input: "httpss://registry.org", + expectedErrorString: "unsupported scheme: httpss", + }, + { + input: "ftp://registry.org", + expectedErrorString: "unsupported scheme: ftp", + }, + { + input: "http://registry.org://", + expectedErrorString: errNoRegistryURLPathAllowed.Error(), + }, + { + input: "http://registry.org/path", + expectedErrorString: errNoRegistryURLPathAllowed.Error(), + }, + { + input: "[fd12:3456:789a:1::1", + expectedError: true, + }, + { + input: "bad url", + expectedError: true, + }, + { + input: "/registry.org", + expectedErrorString: errNoRegistryURLPathAllowed.Error(), + }, + { + input: "https:///", + expectedErrorString: errRegistryURLHostEmpty.Error(), + }, + { + input: "http://registry.org?parm=arg", + expectedErrorString: errNoRegistryURLQueryAllowed.Error(), + }, + } { + + err := ValidateRegistryURL(tc.input) + if err != nil { + if len(tc.expectedErrorString) > 0 && err.Error() != tc.expectedErrorString { + t.Errorf("[%s] unexpected error string: %q != %q", tc.input, err.Error(), tc.expectedErrorString) + } else if len(tc.expectedErrorString) == 0 && !tc.expectedError { + t.Errorf("[%s] unexpected error: %q", tc.input, err.Error()) + } + } else if len(tc.expectedErrorString) > 0 { + t.Errorf("[%s] got non-error while expecting %q", tc.input, tc.expectedErrorString) + } else if tc.expectedError { + t.Errorf("[%s] got unexpected non-error", tc.input) + } + } +} diff --git a/pkg/oc/admin/prune/images.go b/pkg/oc/admin/prune/images.go index 0b216c9cc0d7..8a45675663b7 100644 --- a/pkg/oc/admin/prune/images.go +++ b/pkg/oc/admin/prune/images.go @@ -59,9 +59,10 @@ var ( specified: 1. --force-insecure is given - 2. user's config allows for insecure connection (the user logged in to the cluster with - --insecure-skip-tls-verify or allowed for insecure connection) - 3. registry url is a private or link-local address`) + 2. provided registry-url is prefixed with http:// + 3. registry url is a private or link-local address + 4. user's config allows for insecure connection (the user logged in to the cluster with + --insecure-skip-tls-verify or allowed for insecure connection)`) imagesExample = templates.Examples(` # See, what the prune command would delete if only images more than an hour old and obsoleted @@ -76,7 +77,13 @@ var ( %[1]s %[2]s --prune-over-size-limit # To actually perform the prune operation, the confirm flag must be appended - %[1]s %[2]s --prune-over-size-limit --confirm`) + %[1]s %[2]s --prune-over-size-limit --confirm + + # Force the insecure http protocol with the particular registry host name + %[1]s %[2]s --registry-url=http://registry.example.org --confirm + + # Force a secure connection with a custom certificate authority to the particular registry host name + %[1]s %[2]s --registry-url=registry.example.org --certificate-authority=/path/to/custom/ca.crt --confirm`) ) var ( @@ -134,7 +141,7 @@ func NewCmdPruneImages(f *clientcmd.Factory, parentName, name string, out io.Wri cmd.Flags().IntVar(opts.KeepTagRevisions, "keep-tag-revisions", *opts.KeepTagRevisions, "Specify the number of image revisions for a tag in an image stream that will be preserved.") cmd.Flags().BoolVar(opts.PruneOverSizeLimit, "prune-over-size-limit", *opts.PruneOverSizeLimit, "Specify if images which are exceeding LimitRanges (see 'openshift.io/Image'), specified in the same namespace, should be considered for pruning. This flag cannot be combined with --keep-younger-than nor --keep-tag-revisions.") cmd.Flags().StringVar(&opts.CABundle, "certificate-authority", opts.CABundle, "The path to a certificate authority bundle to use when communicating with the managed Docker registries. Defaults to the certificate authority data from the current user's config file. It cannot be used together with --force-insecure.") - cmd.Flags().StringVar(&opts.RegistryUrlOverride, "registry-url", opts.RegistryUrlOverride, "The address to use when contacting the registry, instead of using the default value. This is useful if you can't resolve or reach the registry (e.g.; the default is a cluster-internal URL) but you do have an alternative route that works.") + cmd.Flags().StringVar(&opts.RegistryUrlOverride, "registry-url", opts.RegistryUrlOverride, "The address to use when contacting the registry, instead of using the default value. This is useful if you can't resolve or reach the registry (e.g.; the default is a cluster-internal URL) but you do have an alternative route that works. Particular transport protocol can be enforced using '://' prefix.") cmd.Flags().BoolVar(&opts.ForceInsecure, "force-insecure", opts.ForceInsecure, "If true, allow an insecure connection to the docker registry that is hosted via HTTP or has an invalid HTTPS certificate. Whenever possible, use --certificate-authority instead of this dangerous option.") return cmd @@ -195,14 +202,15 @@ func (o PruneImagesOptions) Validate() error { if o.KeepTagRevisions != nil && *o.KeepTagRevisions < 0 { return fmt.Errorf("--keep-tag-revisions must be greater than or equal to 0") } - // golang validation tighten and our code actually expects this to be scheme-less - // TODO figure out how to validate - // if _, err := url.Parse(o.RegistryUrlOverride); err != nil { - // return fmt.Errorf("invalid --registry-url flag: %v", err) - // } + if err := imageapi.ValidateRegistryURL(o.RegistryUrlOverride); len(o.RegistryUrlOverride) > 0 && err != nil { + return fmt.Errorf("invalid --registry-url flag: %v", err) + } if o.ForceInsecure && len(o.CABundle) > 0 { return fmt.Errorf("--certificate-authority cannot be specified with --force-insecure") } + if len(o.CABundle) > 0 && strings.HasPrefix(o.RegistryUrlOverride, "http://") { + return fmt.Errorf("--cerificate-authority cannot be specified for insecure http protocol") + } return nil } @@ -278,7 +286,8 @@ func (o PruneImagesOptions) Run() error { insecure := o.ForceInsecure if !insecure && len(o.CABundle) == 0 { - insecure = o.ClientConfig.TLSClientConfig.Insecure || netutils.IsPrivateAddress(registryHost) + insecure = o.ClientConfig.TLSClientConfig.Insecure || netutils.IsPrivateAddress(registryHost) || + strings.HasPrefix(registryHost, "http://") } registryClient, err = getRegistryClient(o.ClientConfig, o.CABundle, insecure)