Merge pull request #48999 from thaJeztah/no_dnslookup

registry: isCIDRMatch: avoid performing DNS lookups if not needed
moby · Dec 3, 2024 · 612b853 · 612b853
2 parents efa041a + 28a700b
commit 612b853
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 42 deletions.
diff --git a/registry/config.go b/registry/config.go
@@ -58,9 +58,6 @@ var (
 	emptyServiceConfig, _ = newServiceConfig(ServiceOptions{})
 	validHostPortRegex    = regexp.MustCompile(`^` + reference.DomainRegexp.String() + `$`)
 
-	// for mocking in unit tests
-	lookupIP = net.LookupIP
-
 	// certsDir is used to override defaultCertsDir.
 	certsDir string
 )
@@ -285,30 +282,37 @@ func (config *serviceConfig) isSecureIndex(indexName string) bool {
 	return !isCIDRMatch(config.InsecureRegistryCIDRs, indexName)
 }
 
+// for mocking in unit tests.
+var lookupIP = net.LookupIP
+
 // isCIDRMatch returns true if URLHost matches an element of cidrs. URLHost is a URL.Host (`host:port` or `host`)
 // where the `host` part can be either a domain name or an IP address. If it is a domain name, then it will be
 // resolved to IP addresses for matching. If resolution fails, false is returned.
 func isCIDRMatch(cidrs []*registry.NetIPNet, URLHost string) bool {
+	if len(cidrs) == 0 {
+		return false
+	}
+
 	host, _, err := net.SplitHostPort(URLHost)
 	if err != nil {
-		// Assume URLHost is of the form `host` without the port and go on.
+		// Assume URLHost is a host without port and go on.
 		host = URLHost
 	}
 
-	addrs, err := lookupIP(host)
-	if err != nil {
-		ip := net.ParseIP(host)
-		if ip != nil {
-			addrs = []net.IP{ip}
+	var addresses []net.IP
+	if ip := net.ParseIP(host); ip != nil {
+		// Host is an IP-address.
+		addresses = append(addresses, ip)
+	} else {
+		// Try to resolve the host's IP-address.
+		addresses, err = lookupIP(host)
+		if err != nil {
+			// We failed to resolve the host; assume there's no match.
+			return false
 		}
-
-		// if ip == nil, then `host` is neither an IP nor it could be looked up,
-		// either because the index is unreachable, or because the index is behind an HTTP proxy.
-		// So, len(addrs) == 0 and we're not aborting.
 	}
 
-	// Try CIDR notation only if addrs has any elements, i.e. if `host`'s IP could be determined.
-	for _, addr := range addrs {
+	for _, addr := range addresses {
 		for _, ipnet := range cidrs {
 			// check if the addr falls in the subnet
 			if (*net.IPNet)(ipnet).Contains(addr) {

diff --git a/registry/registry_mock_test.go b/registry/registry_mock_test.go
@@ -3,9 +3,7 @@ package registry // import "github.com/docker/docker/registry"
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"io"
-	"net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -32,31 +30,6 @@ func init() {
 
 	testHTTPServer = httptest.NewServer(handlerAccessLog(r))
 	testHTTPSServer = httptest.NewTLSServer(handlerAccessLog(r))
-
-	// override net.LookupIP
-	lookupIP = func(host string) ([]net.IP, error) {
-		if host == "127.0.0.1" {
-			// I believe in future Go versions this will fail, so let's fix it later
-			return net.LookupIP(host)
-		}
-		mockHosts := map[string][]net.IP{
-			"":            {net.ParseIP("0.0.0.0")},
-			"localhost":   {net.ParseIP("127.0.0.1"), net.ParseIP("::1")},
-			"example.com": {net.ParseIP("42.42.42.42")},
-			"other.com":   {net.ParseIP("43.43.43.43")},
-		}
-		for h, addrs := range mockHosts {
-			if host == h {
-				return addrs, nil
-			}
-			for _, addr := range addrs {
-				if addr.String() == host {
-					return []net.IP{addr}, nil
-				}
-			}
-		}
-		return nil, errors.New("lookup: no such host")
-	}
 }
 
 func handlerAccessLog(handler http.Handler) http.Handler {

diff --git a/registry/registry_test.go b/registry/registry_test.go
@@ -1,6 +1,8 @@
 package registry // import "github.com/docker/docker/registry"
 
 import (
+	"errors"
+	"net"
 	"testing"
 
 	"github.com/distribution/reference"
@@ -9,6 +11,29 @@ import (
 	is "gotest.tools/v3/assert/cmp"
 )
 
+// overrideLookupIP overrides net.LookupIP for testing.
+func overrideLookupIP(t *testing.T) {
+	t.Helper()
+	restoreLookup := lookupIP
+
+	// override net.LookupIP
+	lookupIP = func(host string) ([]net.IP, error) {
+		mockHosts := map[string][]net.IP{
+			"":            {net.ParseIP("0.0.0.0")},
+			"localhost":   {net.ParseIP("127.0.0.1"), net.ParseIP("::1")},
+			"example.com": {net.ParseIP("42.42.42.42")},
+			"other.com":   {net.ParseIP("43.43.43.43")},
+		}
+		if addrs, ok := mockHosts[host]; ok {
+			return addrs, nil
+		}
+		return nil, errors.New("lookup: no such host")
+	}
+	t.Cleanup(func() {
+		lookupIP = restoreLookup
+	})
+}
+
 func TestParseRepositoryInfo(t *testing.T) {
 	type staticRepositoryInfo struct {
 		Index         *registry.IndexInfo
@@ -242,6 +267,7 @@ func TestParseRepositoryInfo(t *testing.T) {
 }
 
 func TestNewIndexInfo(t *testing.T) {
+	overrideLookupIP(t)
 	testIndexInfo := func(config *serviceConfig, expectedIndexInfos map[string]*registry.IndexInfo) {
 		for indexName, expectedIndexInfo := range expectedIndexInfos {
 			index, err := newIndexInfo(config, indexName)
@@ -415,6 +441,7 @@ func TestMirrorEndpointLookup(t *testing.T) {
 }
 
 func TestAllowNondistributableArtifacts(t *testing.T) {
+	overrideLookupIP(t)
 	tests := []struct {
 		addr       string
 		registries []string
@@ -460,6 +487,7 @@ func TestAllowNondistributableArtifacts(t *testing.T) {
 }
 
 func TestIsSecureIndex(t *testing.T) {
+	overrideLookupIP(t)
 	tests := []struct {
 		addr               string
 		insecureRegistries []string