diff --git a/backend/internal/api/images.go b/backend/internal/api/images.go index 2c5e39d..f6be322 100644 --- a/backend/internal/api/images.go +++ b/backend/internal/api/images.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "net/http" - "net/url" "os" "os/exec" "path/filepath" @@ -20,6 +19,7 @@ import ( "clicd/internal/config" "clicd/internal/kvm" "clicd/internal/lxc" + "clicd/internal/safehttp" ) // ImageInfo represents a template image with its download/enable status. @@ -187,51 +187,66 @@ func cleanupOldImageDownloadErrors() { } } -// isImageDownloaded checks if the LXC download cache exists for a template. -func isImageDownloaded(distro, release, arch string) bool { - downloaded, _ := imageDownloadedInfo(distro, release, arch) - return downloaded -} - // imageDownloadedInfo returns whether the image is downloaded and its total size in bytes. -func imageDownloadedInfo(distro, release, arch string) (bool, int64) { - cachePath := filepath.Join("/var/cache/lxc/download", distro, release, arch) +func imageDownloadedInfo(templateID string) (bool, int64) { + cachePath, ok := officialLXCImageCachePath(templateID) + if !ok { + return false, 0 + } info, err := os.Stat(cachePath) if err != nil || !info.IsDir() { return false, 0 } - // Check directly for rootfs.tar.xz (some LXC versions store it here) - if fi, err := os.Stat(filepath.Join(cachePath, "rootfs.tar.xz")); err == nil { - return true, fi.Size() - } - if fi, err := os.Stat(filepath.Join(cachePath, "meta.tar.xz")); err == nil { - return true, fi.Size() - } - // Check one level deeper (LXC uses variant subdirectories like "default") - entries, err := os.ReadDir(cachePath) - if err != nil { - return false, 0 - } - for _, entry := range entries { - if !entry.IsDir() { - continue - } - subPath := filepath.Join(cachePath, entry.Name()) - if fi, err := os.Stat(filepath.Join(subPath, "rootfs.tar.xz")); err == nil { - return true, fi.Size() - } - if fi, err := os.Stat(filepath.Join(subPath, "meta.tar.xz")); err == nil { - return true, fi.Size() + for _, candidate := range []string{ + filepath.Join(cachePath, "rootfs.tar.xz"), + filepath.Join(cachePath, "meta.tar.xz"), + filepath.Join(cachePath, "default", "rootfs.tar.xz"), + filepath.Join(cachePath, "default", "meta.tar.xz"), + } { + if fileInfo, err := os.Stat(candidate); err == nil && !fileInfo.IsDir() { + return true, fileInfo.Size() } } return false, 0 } +func officialLXCImageCachePath(templateID string) (string, bool) { + arch := "amd64" + if runtime.GOARCH == "arm64" { + arch = "arm64" + } + base := "/var/cache/lxc/download" + switch templateID { + case "ubuntu-noble": + return filepath.Join(base, "ubuntu", "noble", arch), true + case "ubuntu-jammy": + return filepath.Join(base, "ubuntu", "jammy", arch), true + case "debian-trixie": + return filepath.Join(base, "debian", "trixie", arch), true + case "debian-bookworm": + return filepath.Join(base, "debian", "bookworm", arch), true + case "debian-bullseye": + return filepath.Join(base, "debian", "bullseye", arch), true + case "alpine-3.21": + return filepath.Join(base, "alpine", "3.21", arch), true + case "centos-9-stream": + return filepath.Join(base, "centos", "9-Stream", arch), true + case "archlinux-current": + return filepath.Join(base, "archlinux", "current", arch), true + case "fedora-44": + return filepath.Join(base, "fedora", "44", arch), true + case "rockylinux-10": + return filepath.Join(base, "rockylinux", "10", arch), true + default: + return "", false + } +} + func lxcTemplateDownloadedInfo(template lxc.Template) (bool, int64) { if template.Custom { return lxc.CustomImageDownloadedInfo(template.ID) } - return imageDownloadedInfo(template.Distro, template.Release, template.Arch) + return imageDownloadedInfo(template.ID) } // getEnabledImageSet returns the set of enabled image IDs. @@ -272,7 +287,7 @@ func HandleImages(w http.ResponseWriter, r *http.Request) { if kvmAvailable { kvmImages = kvm.GetImages() } - images := make([]ImageInfo, 0, len(templates)+len(kvmImages)) + images := make([]ImageInfo, 0, len(templates)) for _, t := range templates { dl := imageDownloadInfo(t.ID) downloaded, size := lxcTemplateDownloadedInfo(t) @@ -437,9 +452,8 @@ func handleCustomKVMImageCreate(w http.ResponseWriter, r *http.Request) { jsonResponse(w, http.StatusBadRequest, APIResponse{Success: false, Message: "url must not exceed 4096 characters"}) return } - parsedURL, err := url.ParseRequestURI(req.URL) - if err != nil || parsedURL.Host == "" || (parsedURL.Scheme != "https" && parsedURL.Scheme != "http") || parsedURL.User != nil { - jsonResponse(w, http.StatusBadRequest, APIResponse{Success: false, Message: "url must be a valid HTTP or HTTPS download URL without credentials"}) + if _, err := safehttp.ValidateURL(req.URL); err != nil { + jsonResponse(w, http.StatusBadRequest, APIResponse{Success: false, Message: err.Error()}) return } if req.SHA256 != "" && !sha256Pattern.MatchString(req.SHA256) { @@ -991,7 +1005,11 @@ func HandleImageDelete(w http.ResponseWriter, r *http.Request) { } // Remove cache directory - cachePath := filepath.Join("/var/cache/lxc/download", tmpl.Distro, tmpl.Release, tmpl.Arch) + cachePath, ok := officialLXCImageCachePath(tmpl.ID) + if !ok { + jsonResponse(w, http.StatusBadRequest, APIResponse{Success: false, Message: "Template cache path is not managed by CLICD"}) + return + } if err := os.RemoveAll(cachePath); err != nil { jsonResponse(w, http.StatusInternalServerError, APIResponse{ Success: false, diff --git a/backend/internal/api/images_custom_test.go b/backend/internal/api/images_custom_test.go index 41fadeb..869f77e 100644 --- a/backend/internal/api/images_custom_test.go +++ b/backend/internal/api/images_custom_test.go @@ -5,7 +5,9 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "path/filepath" "runtime" + "strings" "testing" ) @@ -81,3 +83,51 @@ func TestCustomLXCImageCreateRejectsInvalidSource(t *testing.T) { t.Fatalf("status = %d, want %d; body=%s", response.Code, http.StatusBadRequest, response.Body.String()) } } + +func TestCustomImageCreateRejectsPrivateNetworkSource(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", + "distro": "ubuntu", + "release": "noble", + "arch": runtime.GOARCH, + "url": "http://169.254.169.254/latest/meta-data", + "provisioner": "linux-cloud-init", + } + body, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + request := httptest.NewRequest(http.MethodPost, "/api/images/custom", bytes.NewReader(body)) + response := httptest.NewRecorder() + + HandleCustomKVMImages(response, request) + + if response.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d; body=%s", response.Code, http.StatusBadRequest, response.Body.String()) + } + }) + } +} + +func TestOfficialLXCImageCachePathUsesAllowlist(t *testing.T) { + cachePath, ok := officialLXCImageCachePath("debian-trixie") + if !ok { + t.Fatal("known template cache path was rejected") + } + normalized := filepath.ToSlash(cachePath) + if !strings.Contains(normalized, "/debian/trixie/") { + t.Fatalf("cache path = %q, want Debian trixie path", cachePath) + } + for _, templateID := range []string{ + "../../../etc", + "custom-lxc-attacker", + "debian-trixie/../../etc", + } { + if cachePath, ok := officialLXCImageCachePath(templateID); ok || cachePath != "" { + t.Fatalf("officialLXCImageCachePath(%q) = %q, %v; want rejection", templateID, cachePath, ok) + } + } +} diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 923a413..f352087 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -583,7 +583,7 @@ func (c *Container) NormalizeNetworkAssignments() bool { c.PublicIPv4s = filteredIPv4 seenIPv6 := map[string]bool{} - filteredIPv6 := make([]IPv6Assignment, 0, len(c.IPv6Addresses)+1) + filteredIPv6 := make([]IPv6Assignment, 0, len(c.IPv6Addresses)) for _, item := range c.IPv6Addresses { item.Address = strings.TrimSpace(item.Address) item.Interface = strings.TrimSpace(item.Interface) diff --git a/backend/internal/kvm/kvm.go b/backend/internal/kvm/kvm.go index a4fca0e..4599676 100644 --- a/backend/internal/kvm/kvm.go +++ b/backend/internal/kvm/kvm.go @@ -29,6 +29,7 @@ import ( "clicd/internal/config" "clicd/internal/lxc" + "clicd/internal/safehttp" "golang.org/x/crypto/ssh" ) @@ -255,26 +256,8 @@ func downloadFile(ctx context.Context, url, target string, progress DownloadProg } func downloadFileWithValidator(ctx context.Context, url, target string, validate downloadResponseValidator, progress DownloadProgressFunc) error { - client := http.Client{ - Timeout: 30 * time.Minute, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - if len(via) >= 10 { - return fmt.Errorf("too many redirects") - } - // Copy User-Agent on redirect - if ua := via[0].Header.Get("User-Agent"); ua != "" { - req.Header.Set("User-Agent", ua) - } - return nil - }, - } - req, err := http.NewRequestWithContext(ctx, "GET", url, nil) - if err != nil { - return err - } - // Windows UA needed for Microsoft download servers - req.Header.Set("User-Agent", "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36") - resp, err := client.Do(req) + const userAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" + resp, err := safehttp.Get(ctx, url, userAgent, 30*time.Minute) if err != nil { return err } @@ -2223,7 +2206,7 @@ runcmd: // Build static address block (IPv4 + IPv6) ipv4s = normalizeKVMIPv4List(ipv4s) addressBlock := "" - addressLines := make([]string, 0, len(ipv4s)+len(ipv6s)) + addressLines := make([]string, 0, len(ipv4s)) for _, ipv4 := range ipv4s { addressLines = append(addressLines, fmt.Sprintf(" - %s/32", ipv4)) } diff --git a/backend/internal/lxc/custom_images.go b/backend/internal/lxc/custom_images.go index 73a85a7..1afb4ff 100644 --- a/backend/internal/lxc/custom_images.go +++ b/backend/internal/lxc/custom_images.go @@ -7,13 +7,14 @@ import ( "encoding/hex" "fmt" "io" - "net/http" "os" "os/exec" "path" "path/filepath" "strings" "time" + + "clicd/internal/safehttp" ) type CustomImageDownloadProgress struct { @@ -91,21 +92,7 @@ func DownloadCustomImageWithProgress(ctx context.Context, template Template, pro } func downloadCustomRootfs(ctx context.Context, sourceURL, target string, progress CustomImageDownloadProgressFunc) error { - client := http.Client{ - Timeout: 30 * time.Minute, - CheckRedirect: func(req *http.Request, via []*http.Request) error { - if len(via) >= 10 { - return fmt.Errorf("too many redirects") - } - return nil - }, - } - request, err := http.NewRequestWithContext(ctx, http.MethodGet, sourceURL, nil) - if err != nil { - return err - } - request.Header.Set("User-Agent", "CLICD/1.0 LXC image downloader") - response, err := client.Do(request) + response, err := safehttp.Get(ctx, sourceURL, "CLICD/1.0 LXC image downloader", 30*time.Minute) if err != nil { return err } diff --git a/backend/internal/lxc/ipv6.go b/backend/internal/lxc/ipv6.go index 2f2c550..f98690f 100644 --- a/backend/internal/lxc/ipv6.go +++ b/backend/internal/lxc/ipv6.go @@ -1591,7 +1591,7 @@ func (m *Manager) applyIPv6Config(lxcName string, ipv6s ...string) error { return fmt.Errorf("failed to read container config: %v", err) } lines := strings.Split(string(data), "\n") - next := make([]string, 0, len(lines)+4) + next := make([]string, 0, len(lines)) for _, line := range lines { trimmed := strings.TrimSpace(line) if strings.Contains(trimmed, "# clicd managed: public IPv6") || diff --git a/backend/internal/lxc/lxc.go b/backend/internal/lxc/lxc.go index e00da2b..b9321a2 100644 --- a/backend/internal/lxc/lxc.go +++ b/backend/internal/lxc/lxc.go @@ -410,7 +410,7 @@ func (cfg *ContainerConfig) NormalizeCreateNATMappings() error { } func (cfg ContainerConfig) RequestedNATHostPorts() []int { - ports := make([]int, 0, len(cfg.NATPortMappings)+1) + ports := make([]int, 0, len(cfg.NATPortMappings)) if cfg.ManagementPort > 0 { ports = append(ports, cfg.ManagementPort) } diff --git a/backend/internal/lxc/portmap.go b/backend/internal/lxc/portmap.go index a2b5b46..208d22d 100644 --- a/backend/internal/lxc/portmap.go +++ b/backend/internal/lxc/portmap.go @@ -837,7 +837,7 @@ func createNATReservationOwner(name string) string { } func createNATReservationMappings(cfg ContainerConfig, managementPort int) []config.PortMapping { - reservations := make([]config.PortMapping, 0, len(cfg.NATPortMappings)+1) + reservations := make([]config.PortMapping, 0, len(cfg.NATPortMappings)) if managementPort > 0 { reservations = append(reservations, config.PortMapping{HostPort: managementPort, Protocol: "tcp"}) } diff --git a/backend/internal/safehttp/safehttp.go b/backend/internal/safehttp/safehttp.go new file mode 100644 index 0000000..039a021 --- /dev/null +++ b/backend/internal/safehttp/safehttp.go @@ -0,0 +1,214 @@ +package safehttp + +import ( + "context" + "fmt" + "net" + "net/http" + "net/netip" + "net/url" + "strconv" + "strings" + "time" +) + +const maxRedirects = 10 + +var blockedPrefixes = []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"), + netip.MustParsePrefix("224.0.0.0/4"), + netip.MustParsePrefix("240.0.0.0/4"), + netip.MustParsePrefix("::/128"), + netip.MustParsePrefix("::1/128"), + netip.MustParsePrefix("64:ff9b::/96"), + netip.MustParsePrefix("64:ff9b:1::/48"), + netip.MustParsePrefix("100::/64"), + netip.MustParsePrefix("2001::/32"), + netip.MustParsePrefix("2001:2::/48"), + 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"), +} + +// ValidateURL performs the URL checks that do not require DNS. Host addresses +// are checked again after resolution and immediately before every connection. +func ValidateURL(rawURL string) (*url.URL, error) { + if len(rawURL) == 0 || len(rawURL) > 4096 { + return nil, fmt.Errorf("download URL must be between 1 and 4096 characters") + } + parsed, err := url.ParseRequestURI(rawURL) + if err != nil { + return nil, fmt.Errorf("invalid download URL: %v", err) + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return nil, fmt.Errorf("download URL must use HTTP or HTTPS") + } + if parsed.Host == "" || parsed.Hostname() == "" { + return nil, fmt.Errorf("download URL must include a host") + } + if parsed.User != nil { + return nil, fmt.Errorf("download URL must not include credentials") + } + if parsed.Fragment != "" { + return nil, fmt.Errorf("download URL must not include a fragment") + } + if port := parsed.Port(); port != "" { + value, err := strconv.Atoi(port) + if err != nil || value < 1 || value > 65535 { + 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") + } + return parsed, nil +} + +// Get retrieves a resource only when every resolved destination is public. +func Get(ctx context.Context, rawURL, userAgent string, timeout time.Duration) (*http.Response, error) { + parsed, err := ValidateURL(rawURL) + if err != nil { + return nil, err + } + if err := validateHost(ctx, net.DefaultResolver, parsed.Hostname()); err != nil { + return nil, err + } + + request, err := http.NewRequestWithContext(ctx, http.MethodGet, parsed.String(), nil) + if err != nil { + return nil, err + } + request.Header.Set("User-Agent", userAgent) + + client := &http.Client{ + Timeout: timeout, + Transport: publicTransport(net.DefaultResolver), + CheckRedirect: func(req *http.Request, via []*http.Request) error { + if len(via) >= maxRedirects { + return fmt.Errorf("too many redirects") + } + redirect, err := ValidateURL(req.URL.String()) + if err != nil { + return err + } + if err := validateHost(req.Context(), net.DefaultResolver, redirect.Hostname()); err != nil { + return err + } + if len(via) > 0 { + req.Header.Set("User-Agent", via[0].Header.Get("User-Agent")) + } + return nil + }, + } + + // All URL components, redirects, DNS answers and dial destinations are + // constrained above and in publicTransport. + // lgtm[go/request-forgery] + return client.Do(request) +} + +func publicTransport(resolver *net.Resolver) *http.Transport { + dialer := &net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + } + return &http.Transport{ + Proxy: nil, + DialContext: func(ctx context.Context, network, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil { + return nil, fmt.Errorf("invalid download destination: %v", err) + } + addresses, err := resolvePublicHost(ctx, resolver, host) + if err != nil { + return nil, err + } + var lastErr error + for _, addr := range addresses { + conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(addr.String(), port)) + if err == nil { + return conn, nil + } + lastErr = err + } + if lastErr == nil { + lastErr = fmt.Errorf("host has no usable public addresses") + } + return nil, lastErr + }, + ForceAttemptHTTP2: true, + TLSHandshakeTimeout: 30 * time.Second, + IdleConnTimeout: 90 * time.Second, + } +} + +func validateHost(ctx context.Context, resolver *net.Resolver, host string) error { + _, err := resolvePublicHost(ctx, resolver, host) + return err +} + +func resolvePublicHost(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") + } + if strings.EqualFold(host, "localhost") || strings.HasSuffix(strings.ToLower(host), ".localhost") { + return nil, fmt.Errorf("download URL host is not public") + } + + 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") + } + return []netip.Addr{addr}, nil + } + + addresses, err := resolver.LookupNetIP(ctx, "ip", host) + if err != nil { + return nil, fmt.Errorf("failed to resolve download host: %v", err) + } + if len(addresses) == 0 { + return nil, fmt.Errorf("download host has no IP addresses") + } + 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") + } + result = append(result, address) + } + return result, nil +} + +func isPublicAddress(address netip.Addr) bool { + if !address.IsValid() || address.Zone() != "" || !address.IsGlobalUnicast() || address.IsPrivate() || + address.IsLoopback() || address.IsLinkLocalUnicast() || address.IsLinkLocalMulticast() || + address.IsMulticast() || address.IsUnspecified() { + return false + } + address = address.Unmap() + for _, prefix := range blockedPrefixes { + if prefix.Contains(address) { + return false + } + } + return true +} diff --git a/backend/internal/safehttp/safehttp_test.go b/backend/internal/safehttp/safehttp_test.go new file mode 100644 index 0000000..139a4a5 --- /dev/null +++ b/backend/internal/safehttp/safehttp_test.go @@ -0,0 +1,74 @@ +package safehttp + +import ( + "context" + "net/netip" + "testing" + "time" +) + +func TestValidateURLRejectsUnsafeDestinations(t *testing.T) { + t.Parallel() + for _, rawURL := range []string{ + "file:///etc/passwd", + "http://user:pass@example.com/image", + "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 { + t.Fatalf("ValidateURL(%q) succeeded, want rejection", rawURL) + } + } +} + +func TestValidateURLAcceptsPublicHTTPURL(t *testing.T) { + t.Parallel() + parsed, err := ValidateURL("https://example.com/images/rootfs.tar.xz?variant=default") + if err != nil { + t.Fatalf("ValidateURL returned error: %v", err) + } + if parsed.Hostname() != "example.com" { + t.Fatalf("hostname = %q, want example.com", parsed.Hostname()) + } +} + +func TestIsPublicAddress(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, + "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, + "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) + } + } +} + +func TestGetRejectsLoopbackBeforeRequest(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + if _, err := Get(ctx, "http://127.0.0.1:1/image", "test", time.Second); err == nil { + t.Fatal("Get accepted a loopback destination") + } +}