dockerClient: skip TLS verification if configured in registries

docker/docker_client.go

@@ -17,6 +17,7 @@ import (
 
 	"github.com/containers/image/docker/reference"
 	"github.com/containers/image/pkg/docker/config"
+	"github.com/containers/image/pkg/sysregistriesv2"
 	"github.com/containers/image/pkg/tlsclientconfig"
 	"github.com/containers/image/types"
 	"github.com/docker/distribution/registry/client"
@@ -78,11 +79,13 @@ type bearerToken struct {
 // dockerClient is configuration for dealing with a single Docker registry.
 type dockerClient struct {
 	// The following members are set by newDockerClient and do not change afterwards.
-	sys           *types.SystemContext
-	registry      string
+	sys                   *types.SystemContext
+	registry              string
+	client                *http.Client
+	insecureSkipTLSVerify bool
+	// The following members are not set by newDockerClient and must be set by callers if needed.
 	username      string
 	password      string
-	client        *http.Client
 	signatureBase signatureStorageBase
 	scope         authScope
 	// The following members are detected registry properties:
@@ -194,13 +197,26 @@ func newDockerClientFromRef(sys *types.SystemContext, ref dockerReference, write
 	if err != nil {
 		return nil, err
 	}
-	remoteName := reference.Path(ref.ref)
 
-	return newDockerClientWithDetails(sys, registry, username, password, actions, sigBase, remoteName)
+	client, err := newDockerClient(sys, registry, ref.ref.Name())
+	if err != nil {
+		return nil, err
+	}
+	client.username = username
+	client.password = password
+	client.signatureBase = sigBase
+	client.scope.actions = actions
+	client.scope.remoteName = reference.Path(ref.ref)
+	return client, nil
 }
 
-// newDockerClientWithDetails returns a new dockerClient instance for the given parameters
-func newDockerClientWithDetails(sys *types.SystemContext, registry, username, password, actions string, sigBase signatureStorageBase, remoteName string) (*dockerClient, error) {
+// newDockerClient returns a new dockerClient instance for the given registry
+// and reference.  The reference is used to query the registry configuration
+// and can either be a registry (e.g, "registry.com[:5000]"), a repository
+// (e.g., "registry.com[:5000][/some/namespace]/repo").
+// Please note that newDockerClient does not set all members of dockerClient
+// (e.g., username and password); those must be set by callers if necessary.
+func newDockerClient(sys *types.SystemContext, registry, reference string) (*dockerClient, error) {
 	hostName := registry
 	if registry == dockerHostname {
 		registry = dockerRegistry
@@ -221,33 +237,43 @@ func newDockerClientWithDetails(sys *types.SystemContext, registry, username, pa
 		return nil, err
 	}
 
-	if sys != nil && sys.DockerInsecureSkipTLSVerify {
-		tr.TLSClientConfig.InsecureSkipVerify = true
+	// Check if TLS verification shall be skipped (default=false) which can
+	// either be specified in the sysregistriesv2 configuration or via the
+	// SystemContext, whereas the SystemContext is prioritized.
+	skipVerify := false
+	if sys != nil && sys.DockerInsecureSkipTLSVerify != types.OptionalBoolUndefined {
+		// Only use the SystemContext if the actual value is defined.
+		skipVerify = sys.DockerInsecureSkipTLSVerify == types.OptionalBoolTrue
+	} else {
+		reg, err := sysregistriesv2.FindRegistry(sys, reference)
+		if err != nil {
+			return nil, errors.Wrapf(err, "error loading registries")
+		}
+		if reg != nil {
+			skipVerify = reg.Insecure
+		}
 	}
+	tr.TLSClientConfig.InsecureSkipVerify = skipVerify
 
 	return &dockerClient{
-		sys:           sys,
-		registry:      registry,
-		username:      username,
-		password:      password,
-		client:        &http.Client{Transport: tr},
-		signatureBase: sigBase,
-		scope: authScope{
-			actions:    actions,
-			remoteName: remoteName,
-		},
+		sys:                   sys,
+		registry:              registry,
+		client:                &http.Client{Transport: tr},
+		insecureSkipTLSVerify: skipVerify,
 	}, nil
 }
 
 // CheckAuth validates the credentials by attempting to log into the registry
 // returns an error if an error occcured while making the http request or the status code received was 401
 func CheckAuth(ctx context.Context, sys *types.SystemContext, username, password, registry string) error {
-	newLoginClient, err := newDockerClientWithDetails(sys, registry, username, password, "", nil, "")
+	client, err := newDockerClient(sys, registry, registry)
 	if err != nil {
 		return errors.Wrapf(err, "error creating new docker client")
 	}
+	client.username = username
+	client.password = password
 
-	resp, err := newLoginClient.makeRequest(ctx, "GET", "/v2/", nil, nil, v2Auth)
+	resp, err := client.makeRequest(ctx, "GET", "/v2/", nil, nil, v2Auth)
 	if err != nil {
 		return err
 	}
@@ -299,16 +325,21 @@ func SearchRegistry(ctx context.Context, sys *types.SystemContext, registry, ima
 		return nil, errors.Wrapf(err, "error getting username and password")
 	}
 
-	// The /v2/_catalog endpoint has been disabled for docker.io therefore the call made to that endpoint will fail
-	// So using the v1 hostname for docker.io for simplicity of implementation and the fact that it returns search results
+	// The /v2/_catalog endpoint has been disabled for docker.io therefore
+	// the call made to that endpoint will fail.  So using the v1 hostname
+	// for docker.io for simplicity of implementation and the fact that it
+	// returns search results.
+	hostname := registry
 	if registry == dockerHostname {
-		registry = dockerV1Hostname
+		hostname = dockerV1Hostname
 	}
 
-	client, err := newDockerClientWithDetails(sys, registry, username, password, "", nil, "")
+	client, err := newDockerClient(sys, hostname, registry)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error creating new docker client")
 	}
+	client.username = username
+	client.password = password
 
 	// Only try the v1 search endpoint if the search query is not empty. If it is
 	// empty skip to the v2 endpoint.
@@ -530,7 +561,7 @@ func (c *dockerClient) detectProperties(ctx context.Context) error {
 		return nil
 	}
 	err := ping("https")
-	if err != nil && c.sys != nil && c.sys.DockerInsecureSkipTLSVerify {
+	if err != nil && c.insecureSkipTLSVerify {
 		err = ping("http")
 	}
 	if err != nil {
@@ -554,7 +585,7 @@ func (c *dockerClient) detectProperties(ctx context.Context) error {
 			return true
 		}
 		isV1 := pingV1("https")
-		if !isV1 && c.sys != nil && c.sys.DockerInsecureSkipTLSVerify {
+		if !isV1 && c.insecureSkipTLSVerify {
 			isV1 = pingV1("http")
 		}
 		if isV1 {

pkg/sysregistriesv2/system_registries_v2.go

@@ -3,7 +3,7 @@ package sysregistriesv2
 import (
 	"fmt"
 	"io/ioutil"
-	"net/url"
+	"os"
 	"path/filepath"
 	"strings"
 	"sync"
@@ -82,58 +82,20 @@ func (e *InvalidRegistries) Error() string {
 }
 
 // parseURL parses the input string, performs some sanity checks and returns
-// the sanitized input string.  An error is returned in case parsing fails or
-// or if URI scheme or user is set.
+// the sanitized input string.  An error is returned if the input string is
+// empty or if contains an "http{s,}://" prefix.
 func parseURL(input string) (string, error) {
 	trimmed := strings.TrimRight(input, "/")
 
 	if trimmed == "" {
 		return "", &InvalidRegistries{s: "invalid URL: cannot be empty"}
 	}
 
-	// Ultimately, we expect input of the form example.com[/namespace/…], a prefix
-	// of a fully-expended reference (containers/image/docker/Reference.String()).
-	// c/image/docker/Reference does not currently provide such a parser.
-	// So, we use url.Parse("http://"+trimmed) below to ~verify the format, possibly
-	// letting some invalid input in, trading that off for a simpler parser.
-	//
-	// url.Parse("http://"+trimmed) is, sadly, too permissive, notably for
-	// trimmed == "http://example.com/…", url.Parse("http://http://example.com/…")
-	// is accepted and parsed as
-	// {Scheme: "http", Host: "http:", Path: "//example.com/…"}.
-	//
-	// So, first we do an explicit check for an unwanted scheme prefix:
-
-	// This will parse trimmed=="http://example.com/…" with Scheme: "http".  Perhaps surprisingly,
-	// it also succeeds for the input we want to accept, in different ways:
-	// "example.com" -> {Scheme:"", Opaque:"", Path:"example.com"}
-	// "example.com/repo" -> {Scheme:"", Opaque:"", Path:"example.com/repo"}
-	// "example.com:5000" -> {Scheme:"example.com", Opaque:"5000"}
-	// "example.com:5000/repo" -> {Scheme:"example.com", Opaque:"5000/repo"}
-	uri, err := url.Parse(trimmed)
-	if err != nil {
-		return "", &InvalidRegistries{s: fmt.Sprintf("invalid URL '%s': %v", input, err)}
-	}
-
-	// Check if a URI Scheme is set.
-	// Note that URLs that do not start with a slash after the scheme are
-	// interpreted as `scheme:opaque[?query][#fragment]`; see above for examples.
-	if uri.Scheme != "" && uri.Opaque == "" {
+	if strings.HasPrefix(trimmed, "http://") || strings.HasPrefix(trimmed, "https://") {
 		msg := fmt.Sprintf("invalid URL '%s': URI schemes are not supported", input)
 		return "", &InvalidRegistries{s: msg}
 	}
 
-	uri, err = url.Parse("http://" + trimmed)
-	if err != nil {
-		msg := fmt.Sprintf("invalid URL '%s': sanitized URL did not parse: %v", input, err)
-		return "", &InvalidRegistries{s: msg}
-	}
-
-	if uri.User != nil {
-		msg := fmt.Sprintf("invalid URL '%s': user/password are not supported", trimmed)
-		return "", &InvalidRegistries{s: msg}
-	}
-
 	return trimmed, nil
 }
 
@@ -279,7 +241,18 @@ var configMutex = sync.Mutex{}
 // are synchronized via configMutex.
 var configCache = make(map[string][]Registry)
 
+// InvalidateCache invalidates the registry cache.  This function is meant to be
+// used for long-running processes that need to reload potential changes made to
+// the cached registry config files.
+func InvalidateCache() {
+	configMutex.Lock()
+	defer configMutex.Unlock()
+	configCache = make(map[string][]Registry)
+}
+
 // GetRegistries loads and returns the registries specified in the config.
+// Note the parsed content of registry config files is cached.  For reloading,
+// use `InvalidateCache` and re-call `GetRegistries`.
 func GetRegistries(ctx *types.SystemContext) ([]Registry, error) {
 	configPath := getConfigPath(ctx)
 
@@ -293,6 +266,13 @@ func GetRegistries(ctx *types.SystemContext) ([]Registry, error) {
 	// load the config
 	config, err := loadRegistryConf(configPath)
 	if err != nil {
+		// Return an empty []Registry if we use the default config,
+		// which implies that the config path of the SystemContext
+		// isn't set.  Note: if ctx.SystemRegistriesConfPath points to
+		// the default config, we will still return an error.
+		if os.IsNotExist(err) && (ctx == nil || ctx.SystemRegistriesConfPath == "") {
+			return []Registry{}, nil
+		}
 		return nil, err
 	}
 
@@ -323,23 +303,33 @@ func GetRegistries(ctx *types.SystemContext) ([]Registry, error) {
 
 // FindUnqualifiedSearchRegistries returns all registries that are configured
 // for unqualified image search (i.e., with Registry.Search == true).
-func FindUnqualifiedSearchRegistries(registries []Registry) []Registry {
+func FindUnqualifiedSearchRegistries(ctx *types.SystemContext) ([]Registry, error) {
+	registries, err := GetRegistries(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	unqualified := []Registry{}
 	for _, reg := range registries {
 		if reg.Search {
 			unqualified = append(unqualified, reg)
 		}
 	}
-	return unqualified
+	return unqualified, nil
 }
 
 // FindRegistry returns the Registry with the longest prefix for ref.  If no
 // Registry prefixes the image, nil is returned.
-func FindRegistry(ref string, registries []Registry) *Registry {
+func FindRegistry(ctx *types.SystemContext, ref string) (*Registry, error) {
+	registries, err := GetRegistries(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	reg := Registry{}
 	prefixLen := 0
 	for _, r := range registries {
-		if strings.HasPrefix(ref, r.Prefix) {
+		if strings.HasPrefix(ref, r.Prefix+"/") || ref == r.Prefix {
 			length := len(r.Prefix)
 			if length > prefixLen {
 				reg = r
@@ -348,9 +338,9 @@ func FindRegistry(ref string, registries []Registry) *Registry {
 		}
 	}
 	if prefixLen != 0 {
-		return &reg
+		return &reg, nil
 	}
-	return nil
+	return nil, nil
 }
 
 // Reads the global registry file from the filesystem. Returns a byte array.