feat: rename functions and variables for clarity in safehttp package; enhance URL validation tests

backend/internal/api/images_custom_test.go

@@ -84,12 +84,12 @@ func TestCustomLXCImageCreateRejectsInvalidSource(t *testing.T) {
 	}
 }
 
-func TestCustomImageCreateRejectsPrivateNetworkSource(t *testing.T) {
+func TestCustomImageCreateRejectsMetadataSource(t *testing.T) {
 	for _, imageType := range []string{"lxc", "kvm"} {
 		t.Run(imageType, func(t *testing.T) {
 			payload := map[string]string{
 				"type":        imageType,
-				"name":        "Private Network Source",
+				"name":        "Metadata Source",
 				"distro":      "ubuntu",
 				"release":     "noble",
 				"arch":        runtime.GOARCH,

backend/internal/safehttp/safehttp.go

@@ -14,17 +14,13 @@ import (
 
 const maxRedirects = 10
 
-var blockedPrefixes = []netip.Prefix{
+var blockedDownloadPrefixes = []netip.Prefix{
 	netip.MustParsePrefix("0.0.0.0/8"),
-	netip.MustParsePrefix("10.0.0.0/8"),
-	netip.MustParsePrefix("100.64.0.0/10"),
 	netip.MustParsePrefix("127.0.0.0/8"),
 	netip.MustParsePrefix("169.254.0.0/16"),
-	netip.MustParsePrefix("172.16.0.0/12"),
 	netip.MustParsePrefix("192.0.0.0/24"),
 	netip.MustParsePrefix("192.0.2.0/24"),
 	netip.MustParsePrefix("192.88.99.0/24"),
-	netip.MustParsePrefix("192.168.0.0/16"),
 	netip.MustParsePrefix("198.18.0.0/15"),
 	netip.MustParsePrefix("198.51.100.0/24"),
 	netip.MustParsePrefix("203.0.113.0/24"),
@@ -40,7 +36,6 @@ var blockedPrefixes = []netip.Prefix{
 	netip.MustParsePrefix("2001:db8::/32"),
 	netip.MustParsePrefix("2001:20::/28"),
 	netip.MustParsePrefix("2002::/16"),
-	netip.MustParsePrefix("fc00::/7"),
 	netip.MustParsePrefix("fec0::/10"),
 	netip.MustParsePrefix("fe80::/10"),
 	netip.MustParsePrefix("ff00::/8"),
@@ -74,13 +69,15 @@ func ValidateURL(rawURL string) (*url.URL, error) {
 			return nil, fmt.Errorf("download URL contains an invalid port")
 		}
 	}
-	if addr, err := netip.ParseAddr(parsed.Hostname()); err == nil && !isPublicAddress(addr) {
+	if addr, err := netip.ParseAddr(parsed.Hostname()); err == nil && !isAllowedDownloadAddress(addr) {
-		return nil, fmt.Errorf("download URL resolves to a non-public address")
+		return nil, fmt.Errorf("download URL resolves to a blocked address")
 	}
 	return parsed, nil
 }
 
-// Get retrieves a resource only when every resolved destination is public.
+// Get retrieves a resource only when every resolved destination is safe for
+// image downloads. Private network image mirrors are allowed; loopback,
+// link-local, metadata, multicast and reserved destinations remain blocked.
 func Get(ctx context.Context, rawURL, userAgent string, timeout time.Duration) (*http.Response, error) {
 	parsed, err := ValidateURL(rawURL)
 	if err != nil {
@@ -98,7 +95,7 @@ func Get(ctx context.Context, rawURL, userAgent string, timeout time.Duration) (
 
 	client := &http.Client{
 		Timeout:   timeout,
-		Transport: publicTransport(net.DefaultResolver),
+		Transport: restrictedTransport(net.DefaultResolver),
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			if len(via) >= maxRedirects {
 				return fmt.Errorf("too many redirects")
@@ -118,12 +115,12 @@ func Get(ctx context.Context, rawURL, userAgent string, timeout time.Duration) (
 	}
 
 	// All URL components, redirects, DNS answers and dial destinations are
-	// constrained above and in publicTransport.
+	// constrained above and in restrictedTransport.
 	// lgtm[go/request-forgery]
 	return client.Do(request)
 }
 
-func publicTransport(resolver *net.Resolver) *http.Transport {
+func restrictedTransport(resolver *net.Resolver) *http.Transport {
 	dialer := &net.Dialer{
 		Timeout:   30 * time.Second,
 		KeepAlive: 30 * time.Second,
@@ -135,7 +132,7 @@ func publicTransport(resolver *net.Resolver) *http.Transport {
 			if err != nil {
 				return nil, fmt.Errorf("invalid download destination: %v", err)
 			}
-			addresses, err := resolvePublicHost(ctx, resolver, host)
+			addresses, err := resolveAllowedHost(ctx, resolver, host)
 			if err != nil {
 				return nil, err
 			}
@@ -159,11 +156,11 @@ func publicTransport(resolver *net.Resolver) *http.Transport {
 }
 
 func validateHost(ctx context.Context, resolver *net.Resolver, host string) error {
-	_, err := resolvePublicHost(ctx, resolver, host)
+	_, err := resolveAllowedHost(ctx, resolver, host)
 	return err
 }
 
-func resolvePublicHost(ctx context.Context, resolver *net.Resolver, host string) ([]netip.Addr, error) {
+func resolveAllowedHost(ctx context.Context, resolver *net.Resolver, host string) ([]netip.Addr, error) {
 	host = strings.TrimSpace(strings.TrimSuffix(host, "."))
 	if host == "" {
 		return nil, fmt.Errorf("download URL host is empty")
@@ -174,8 +171,8 @@ func resolvePublicHost(ctx context.Context, resolver *net.Resolver, host string)
 
 	if addr, err := netip.ParseAddr(host); err == nil {
 		addr = addr.Unmap()
-		if !isPublicAddress(addr) {
+		if !isAllowedDownloadAddress(addr) {
-			return nil, fmt.Errorf("download URL resolves to a non-public address")
+			return nil, fmt.Errorf("download URL resolves to a blocked address")
 		}
 		return []netip.Addr{addr}, nil
 	}
@@ -190,22 +187,22 @@ func resolvePublicHost(ctx context.Context, resolver *net.Resolver, host string)
 	result := make([]netip.Addr, 0, len(addresses))
 	for _, address := range addresses {
 		address = address.Unmap()
-		if !isPublicAddress(address) {
+		if !isAllowedDownloadAddress(address) {
-			return nil, fmt.Errorf("download host resolves to a non-public address")
+			return nil, fmt.Errorf("download host resolves to a blocked address")
 		}
 		result = append(result, address)
 	}
 	return result, nil
 }
 
-func isPublicAddress(address netip.Addr) bool {
+func isAllowedDownloadAddress(address netip.Addr) bool {
-	if !address.IsValid() || address.Zone() != "" || !address.IsGlobalUnicast() || address.IsPrivate() ||
+	if !address.IsValid() || address.Zone() != "" || !address.IsGlobalUnicast() ||
 		address.IsLoopback() || address.IsLinkLocalUnicast() || address.IsLinkLocalMulticast() ||
 		address.IsMulticast() || address.IsUnspecified() {
 		return false
 	}
 	address = address.Unmap()
-	for _, prefix := range blockedPrefixes {
+	for _, prefix := range blockedDownloadPrefixes {
 		if prefix.Contains(address) {
 			return false
 		}

backend/internal/safehttp/safehttp_test.go

@@ -2,6 +2,9 @@ package safehttp
 
 import (
 	"context"
+	"io"
+	"net"
+	"net/http"
 	"net/netip"
 	"testing"
 	"time"
@@ -15,9 +18,6 @@ func TestValidateURLRejectsUnsafeDestinations(t *testing.T) {
 		"http://127.0.0.1/image",
 		"http://[::1]/image",
 		"http://169.254.169.254/latest/meta-data",
-		"http://10.0.0.1/image",
-		"http://192.168.1.10/image",
-		"http://100.64.0.1/image",
 		"http://example.com:99999/image",
 	} {
 		if _, err := ValidateURL(rawURL); err == nil {
@@ -26,6 +26,21 @@ func TestValidateURLRejectsUnsafeDestinations(t *testing.T) {
 	}
 }
 
+func TestValidateURLAcceptsPrivateNetworkMirror(t *testing.T) {
+	t.Parallel()
+	for _, rawURL := range []string{
+		"http://10.0.0.10/images/rootfs.tar.xz",
+		"http://172.16.20.30:8080/images/vm.qcow2",
+		"https://192.168.1.10/image.iso",
+		"http://100.64.0.10/image.qcow2",
+		"http://[fd00::10]/rootfs.tar.xz",
+	} {
+		if _, err := ValidateURL(rawURL); err != nil {
+			t.Errorf("ValidateURL(%q) returned error: %v", rawURL, err)
+		}
+	}
+}
+
 func TestValidateURLAcceptsPublicHTTPURL(t *testing.T) {
 	t.Parallel()
 	parsed, err := ValidateURL("https://example.com/images/rootfs.tar.xz?variant=default")
@@ -37,29 +52,31 @@ func TestValidateURLAcceptsPublicHTTPURL(t *testing.T) {
 	}
 }
 
-func TestIsPublicAddress(t *testing.T) {
+func TestIsAllowedDownloadAddress(t *testing.T) {
 	t.Parallel()
 	tests := map[string]bool{
 		"8.8.8.8":              true,
 		"1.1.1.1":              true,
 		"2606:4700:4700::1111": true,
 		"127.0.0.1":            false,
-		"10.0.0.1":             false,
+		"10.0.0.1":             true,
-		"100.64.0.1":           false,
+		"100.64.0.1":           true,
+		"172.16.0.1":           true,
+		"192.168.1.1":          true,
 		"169.254.169.254":      false,
 		"192.0.2.1":            false,
 		"198.18.0.1":           false,
 		"::1":                  false,
 		"64:ff9b::127.0.0.1":   false,
 		"2002:7f00:1::1":       false,
-		"fc00::1":              false,
+		"fc00::1":              true,
 		"fec0::1":              false,
 		"fe80::1":              false,
 		"2001:db8::1":          false,
 	}
 	for raw, expected := range tests {
-		if actual := isPublicAddress(netip.MustParseAddr(raw)); actual != expected {
+		if actual := isAllowedDownloadAddress(netip.MustParseAddr(raw)); actual != expected {
-			t.Errorf("isPublicAddress(%s) = %v, want %v", raw, actual, expected)
+			t.Errorf("isAllowedDownloadAddress(%s) = %v, want %v", raw, actual, expected)
 		}
 	}
 }
@@ -72,3 +89,53 @@ func TestGetRejectsLoopbackBeforeRequest(t *testing.T) {
 		t.Fatal("Get accepted a loopback destination")
 	}
 }
+
+func TestGetAllowsPrivateNetworkMirror(t *testing.T) {
+	privateIP := privateInterfaceIPv4(t)
+	listener, err := net.Listen("tcp4", net.JoinHostPort(privateIP.String(), "0"))
+	if err != nil {
+		t.Fatalf("failed to listen on private interface: %v", err)
+	}
+	defer listener.Close()
+
+	server := &http.Server{Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = io.WriteString(w, "private mirror ok")
+	})}
+	go func() { _ = server.Serve(listener) }()
+	defer server.Close()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	response, err := Get(ctx, "http://"+listener.Addr().String()+"/image", "test", 5*time.Second)
+	if err != nil {
+		t.Fatalf("Get rejected private mirror: %v", err)
+	}
+	defer response.Body.Close()
+	body, err := io.ReadAll(response.Body)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if string(body) != "private mirror ok" {
+		t.Fatalf("body = %q, want private mirror response", body)
+	}
+}
+
+func privateInterfaceIPv4(t *testing.T) netip.Addr {
+	t.Helper()
+	addresses, err := net.InterfaceAddrs()
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, rawAddress := range addresses {
+		prefix, err := netip.ParsePrefix(rawAddress.String())
+		if err != nil {
+			continue
+		}
+		address := prefix.Addr().Unmap()
+		if address.Is4() && address.IsPrivate() && isAllowedDownloadAddress(address) {
+			return address
+		}
+	}
+	t.Skip("no private IPv4 interface is available")
+	return netip.Addr{}
+}