From 23a0541f7c0cf80ebc918a4b2844933577210a46 Mon Sep 17 00:00:00 2001 From: MengMengCode <227010654+MengMengCode@users.noreply.github.com> Date: Tue, 4 Aug 2026 05:05:45 +0800 Subject: [PATCH] feat: rename functions and variables for clarity in safehttp package; enhance URL validation tests --- backend/internal/api/images_custom_test.go | 4 +- backend/internal/safehttp/safehttp.go | 41 +++++------ backend/internal/safehttp/safehttp_test.go | 85 +++++++++++++++++++--- 3 files changed, 97 insertions(+), 33 deletions(-) diff --git a/backend/internal/api/images_custom_test.go b/backend/internal/api/images_custom_test.go index 869f77e..bfa3678 100644 --- a/backend/internal/api/images_custom_test.go +++ b/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, diff --git a/backend/internal/safehttp/safehttp.go b/backend/internal/safehttp/safehttp.go index 039a021..42f233a 100644 --- a/backend/internal/safehttp/safehttp.go +++ b/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) { - return nil, fmt.Errorf("download URL resolves to a non-public address") + if addr, err := netip.ParseAddr(parsed.Hostname()); err == nil && !isAllowedDownloadAddress(addr) { + 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) { - return nil, fmt.Errorf("download URL resolves to a non-public address") + if !isAllowedDownloadAddress(addr) { + 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) { - return nil, fmt.Errorf("download host resolves to a non-public address") + if !isAllowedDownloadAddress(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 { - if !address.IsValid() || address.Zone() != "" || !address.IsGlobalUnicast() || address.IsPrivate() || +func isAllowedDownloadAddress(address netip.Addr) bool { + 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 } diff --git a/backend/internal/safehttp/safehttp_test.go b/backend/internal/safehttp/safehttp_test.go index 139a4a5..80d9295 100644 --- a/backend/internal/safehttp/safehttp_test.go +++ b/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, - "100.64.0.1": false, + "10.0.0.1": true, + "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 { - t.Errorf("isPublicAddress(%s) = %v, want %v", raw, actual, expected) + if actual := isAllowedDownloadAddress(netip.MustParseAddr(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{} +}