diff --git a/docs/homelab-verification.md b/docs/homelab-verification.md index bd49c25..77e5bb2 100644 --- a/docs/homelab-verification.md +++ b/docs/homelab-verification.md @@ -117,3 +117,27 @@ string `user` field generated by Proxmox 9.1.1, even with no bootstrap errors. Its JSON status distinguishes `status: done`, an empty `errors` list, and `recoverable_errors.DEPRECATED` warnings. Retain and report those warnings; accept a completed bootstrap only after checking its own readiness results. + +## Cloud-image checksum verification + +`cloudImage.checksum` accepts `algorithm:hex-digest`: md5, sha1, sha224, sha256, +sha384, or sha512, with the full digest length required by that algorithm. +Prefer SHA-256 or SHA-512 and obtain the expected digest from a trusted publisher. +Omitting the field leaves downloads unverified. Malformed and unsupported +checksums are rejected before template lookup. + +openctl passes the digest and algorithm to Proxmox's `download-url` API and +requires a successful task `exitstatus` before creating or importing the template. +A stopped task is not necessarily successful. Successful imports carry an +`openctl-image--` provenance tag. A pinned request can reuse a +cached template only when that tag matches. Legacy unverified templates or a +different digest fail closed: choose an unused `cloudImage.templateName` to build +a newly verified template. Do not manually add a provenance tag to bypass this +check. + +The native handler was exercised on Proxmox 9.1.1 with a disposable 1 MiB raw +disk, never booted: a wrong SHA-256 failed before any VM/template creation, the +correct digest imported and cloned successfully, a second VM reused the verified +template without another download, and a changed digest could not use that +cache. Both sandbox VMs, their template, and the downloaded image were removed; +the existing development VM remained running. diff --git a/internal/schema/schemas/proxmox/vm.cue b/internal/schema/schemas/proxmox/vm.cue index 4aa9f22..99af6d9 100644 --- a/internal/schema/schemas/proxmox/vm.cue +++ b/internal/schema/schemas/proxmox/vm.cue @@ -59,7 +59,9 @@ import "openctl.io/schemas/base" url: string // Proxmox storage that will host the downloaded image and template VM disk. storage: string - // Optional checksum to verify the download (e.g. "sha256:abc123"). + // Optional algorithm:hex-digest verified by Proxmox before image import. + // Supports md5, sha1, sha224, sha256 (64 hex characters), sha384, and sha512. + // Cached templates must carry the same verified digest. checksum?: string // Override the auto-generated template name (otherwise derived from URL). templateName?: string diff --git a/pkg/proxmox/client/client.go b/pkg/proxmox/client/client.go index 7d88b22..d625958 100644 --- a/pkg/proxmox/client/client.go +++ b/pkg/proxmox/client/client.go @@ -805,9 +805,9 @@ func (c *Client) GetStorageInfo(ctx context.Context, storage string) (*StorageIn return &result.Data, nil } -// DownloadToStorage downloads a file from URL to Proxmox storage -// Returns the task UPID for tracking progress -func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, filename, contentType string) (string, error) { +// DownloadToStorage starts a storage download, optionally verified by Proxmox. +// It requires a task UPID so callers can check completion before using the file. +func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, filename, contentType string, checksum ImageChecksum) (string, error) { path := fmt.Sprintf("/api2/json/nodes/%s/storage/%s/download-url", node, storage) params := map[string]any{ @@ -815,6 +815,10 @@ func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, file "filename": filename, "content": contentType, } + if checksum.algorithm != "" { + params["checksum-algorithm"] = checksum.algorithm + params["checksum"] = checksum.digest + } debugf("DownloadToStorage: downloading %s to %s:%s/%s", url, storage, contentType, filename) @@ -827,7 +831,10 @@ func (c *Client) DownloadToStorage(ctx context.Context, node, storage, url, file Data string `json:"data"` } if err := json.Unmarshal(resp, &result); err != nil { - return "", nil + return "", fmt.Errorf("failed to parse download task: %w", err) + } + if result.Data == "" { + return "", fmt.Errorf("download response did not contain a task UPID") } return result.Data, nil @@ -909,12 +916,16 @@ func (c *Client) getTaskStatus(ctx context.Context, node, upid string) (string, var result struct { Data struct { - Status string `json:"status"` + Status string `json:"status"` + ExitStatus string `json:"exitstatus"` } `json:"data"` } if err := json.Unmarshal(resp, &result); err != nil { return "", err } + if result.Data.Status == "stopped" && result.Data.ExitStatus != "OK" { + return "", fmt.Errorf("task %s failed: exitstatus %q", upid, result.Data.ExitStatus) + } return result.Data.Status, nil } diff --git a/pkg/proxmox/client/client_test.go b/pkg/proxmox/client/client_test.go index c74d612..d3b3848 100644 --- a/pkg/proxmox/client/client_test.go +++ b/pkg/proxmox/client/client_test.go @@ -605,7 +605,7 @@ func TestDeleteVM_WaitsForDestroyTask(t *testing.T) { json.NewEncoder(w).Encode(map[string]any{"data": "UPID:pve1:0000:qmdestroy:100:root@pam:"}) case strings.HasPrefix(r.URL.Path, "/api2/json/nodes/pve1/tasks/") && strings.HasSuffix(r.URL.Path, "/status"): taskChecked = true - json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"status": "stopped"}}) + json.NewEncoder(w).Encode(map[string]any{"data": map[string]any{"status": "stopped", "exitstatus": "OK"}}) default: http.Error(w, "unexpected "+r.Method+" "+r.URL.Path, http.StatusNotFound) } @@ -874,46 +874,6 @@ func TestAddCloudInitDrive(t *testing.T) { } } -func TestDownloadToStorage(t *testing.T) { - var receivedParams map[string]string - server := mockServer(t, func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/api2/json/nodes/pve1/storage/local/download-url" && r.Method == "POST" { - r.ParseForm() - receivedParams = make(map[string]string) - for k, v := range r.Form { - if len(v) > 0 { - receivedParams[k] = v[0] - } - } - json.NewEncoder(w).Encode(map[string]any{ - "data": "UPID:pve1:00001234:12345678:download:local:root@pam:", - }) - } - }) - defer server.Close() - - c := New(server.URL, "test", "test") - c.httpClient = server.Client() - - upid, err := c.DownloadToStorage(context.Background(), "pve1", "local", "https://example.com/image.img", "image.img", "iso") - if err != nil { - t.Fatalf("DownloadToStorage failed: %v", err) - } - - if upid == "" { - t.Error("expected non-empty UPID") - } - if receivedParams["url"] != "https://example.com/image.img" { - t.Errorf("expected url parameter, got %s", receivedParams["url"]) - } - if receivedParams["filename"] != "image.img" { - t.Errorf("expected filename parameter, got %s", receivedParams["filename"]) - } - if receivedParams["content"] != "iso" { - t.Errorf("expected content=iso, got %s", receivedParams["content"]) - } -} - func TestTruncate(t *testing.T) { tests := []struct { input string diff --git a/pkg/proxmox/client/image_checksum.go b/pkg/proxmox/client/image_checksum.go new file mode 100644 index 0000000..850c1e4 --- /dev/null +++ b/pkg/proxmox/client/image_checksum.go @@ -0,0 +1,61 @@ +package client + +import ( + "fmt" + "strings" +) + +// ImageChecksum is a validated download digest. Its zero value disables checksum +// verification; nonzero values must be constructed with ParseImageChecksum. +type ImageChecksum struct { + algorithm string + digest string +} + +// ParseImageChecksum accepts algorithm:hex-digest for Proxmox's supported +// algorithms. It validates the complete digest and canonicalizes hexadecimal case. +func ParseImageChecksum(value string) (ImageChecksum, error) { + if value == "" { + return ImageChecksum{}, nil + } + algorithm, digest, ok := strings.Cut(value, ":") + if !ok { + return ImageChecksum{}, fmt.Errorf("checksum must use algorithm:hex-digest format") + } + var length int + switch algorithm { + case "md5": + length = 32 + case "sha1": + length = 40 + case "sha224": + length = 56 + case "sha256": + length = 64 + case "sha384": + length = 96 + case "sha512": + length = 128 + default: + return ImageChecksum{}, fmt.Errorf("unsupported checksum algorithm %q", algorithm) + } + if len(digest) != length { + return ImageChecksum{}, fmt.Errorf("%s checksum must contain %d hexadecimal characters", algorithm, length) + } + for i := range len(digest) { + b := digest[i] + if (b < '0' || b > '9') && (b < 'a' || b > 'f') && (b < 'A' || b > 'F') { + return ImageChecksum{}, fmt.Errorf("%s checksum contains a non-hexadecimal character", algorithm) + } + } + return ImageChecksum{algorithm: algorithm, digest: strings.ToLower(digest)}, nil +} + +// TemplateTag records the digest verified before importing a cloud-image template. +// Empty checksums deliberately have no provenance tag. +func (c ImageChecksum) TemplateTag() string { + if c.algorithm == "" { + return "" + } + return "openctl-image-" + c.algorithm + "-" + c.digest +} diff --git a/pkg/proxmox/client/image_checksum_test.go b/pkg/proxmox/client/image_checksum_test.go new file mode 100644 index 0000000..2e50b22 --- /dev/null +++ b/pkg/proxmox/client/image_checksum_test.go @@ -0,0 +1,63 @@ +package client + +import ( + "context" + "encoding/json" + "net/http" + "strings" + "testing" + "time" +) + +func TestParseImageChecksum(t *testing.T) { + for algorithm, size := range map[string]int{"md5": 32, "sha1": 40, "sha224": 56, "sha256": 64, "sha384": 96, "sha512": 128} { + t.Run(algorithm, func(t *testing.T) { + checksum, err := ParseImageChecksum(algorithm + ":" + strings.Repeat("Ab", size/2)) + if err != nil || checksum.algorithm != algorithm || checksum.digest != strings.Repeat("ab", size/2) { + t.Fatalf("valid digest was not normalized: checksum=%+v err=%v", checksum, err) + } + }) + } + for _, value := range []string{"sha256", "sha256:", "sha256:abc123", "sha256:" + strings.Repeat("g", 64), "sha256:" + strings.Repeat("0", 65), "sha256:" + strings.Repeat("0", 63), "blake3:" + strings.Repeat("0", 64), "sha256:" + strings.Repeat("0", 64) + ":extra", " sha256:" + strings.Repeat("0", 64)} { + if _, err := ParseImageChecksum(value); err == nil { + t.Errorf("invalid checksum accepted: %q", value) + } + } + checksum, err := ParseImageChecksum("") + if err != nil || checksum != (ImageChecksum{}) || checksum.TemplateTag() != "" { + t.Fatalf("omitted checksum must disable verification: %+v, %v", checksum, err) + } +} + +func TestWaitForTaskRequiresSuccessfulExit(t *testing.T) { + for _, exitStatus := range []string{"OK", "checksum mismatch", ""} { + t.Run("exit="+exitStatus, func(t *testing.T) { + server := mockServer(t, func(w http.ResponseWriter, r *http.Request) { + json.NewEncoder(w).Encode(map[string]any{"data": map[string]string{"status": "stopped", "exitstatus": exitStatus}}) + }) + defer server.Close() + c := New(server.URL, "test", "test") + c.httpClient = server.Client() + err := c.WaitForTask(context.Background(), "pve1", "UPID:download", time.Second) + if (err == nil) != (exitStatus == "OK") { + t.Fatalf("only a successful task may proceed: exitstatus=%q err=%v", exitStatus, err) + } + }) + } +} + +func TestDownloadRequiresTrackableTask(t *testing.T) { + for _, response := range []string{`{"data":""}`, `{"data":null}`, `not json`} { + t.Run(response, func(t *testing.T) { + server := mockServer(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(response)) + }) + defer server.Close() + c := New(server.URL, "test", "test") + c.httpClient = server.Client() + if _, err := c.DownloadToStorage(context.Background(), "pve1", "local", "https://example.invalid/image.img", "image.qcow2", "import", ImageChecksum{}); err == nil { + t.Fatal("untrackable download would bypass completion and checksum verification") + } + }) + } +} diff --git a/pkg/proxmox/handler/cloud_image_checksum_test.go b/pkg/proxmox/handler/cloud_image_checksum_test.go new file mode 100644 index 0000000..fa893dc --- /dev/null +++ b/pkg/proxmox/handler/cloud_image_checksum_test.go @@ -0,0 +1,132 @@ +package handler + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/openctl/openctl/pkg/protocol" +) + +func TestCloudImageChecksumMismatchBlocksTemplateCreation(t *testing.T) { + image := []byte("downloaded cloud image bytes") + actual := sha256.Sum256(image) + mutations := 0 + exitStatus := "OK" + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api2/json/nodes": + json.NewEncoder(w).Encode(map[string]any{"data": []map[string]string{{"node": "pve1"}}}) + case r.URL.Path == "/api2/json/nodes/pve1/qemu" && r.Method == http.MethodGet: + json.NewEncoder(w).Encode(map[string]any{"data": []any{}}) + case strings.HasSuffix(r.URL.Path, "/download-url"): + if err := r.ParseForm(); err != nil { + t.Error(err) + return + } + if r.Form.Get("checksum-algorithm") == "sha256" && r.Form.Get("checksum") != hex.EncodeToString(actual[:]) { + exitStatus = "checksum mismatch" + } + json.NewEncoder(w).Encode(map[string]any{"data": "UPID:pve1:download"}) + case strings.HasPrefix(r.URL.Path, "/api2/json/nodes/pve1/tasks/"): + json.NewEncoder(w).Encode(map[string]any{"data": map[string]string{"status": "stopped", "exitstatus": exitStatus}}) + default: + if r.Method != http.MethodGet { + mutations++ + } + http.Error(w, "template mutation must not follow a failed checksum", http.StatusBadRequest) + } + })) + defer server.Close() + h := New(&protocol.ProviderConfig{Endpoint: server.URL, Node: "pve1"}) + _, err := h.Handle(context.Background(), checksumCreateRequest("sha256:"+strings.Repeat("0", 64))) + if err == nil || !strings.Contains(err.Error(), "checksum mismatch") || mutations != 0 { + t.Fatalf("mismatched image must fail before template creation: err=%v mutations=%d", err, mutations) + } +} + +func checksumCreateRequest(checksum string) *protocol.Request { + return &protocol.Request{ + Action: protocol.ActionCreate, ResourceType: "VirtualMachine", + Manifest: &protocol.Resource{ + APIVersion: "proxmox.openctl.io/v1", Kind: "VirtualMachine", + Metadata: protocol.ResourceMetadata{Name: "checksum-vm"}, + Spec: map[string]any{ + "node": "pve1", + "cloudImage": map[string]any{"url": "https://example.invalid/noble.img", "storage": "local", "templateName": "checksum-template", "checksum": checksum}, + "startOnCreate": false, + }, + }, + } +} + +func TestCloudImageChecksumCacheRequiresVerifiedDigest(t *testing.T) { + requested := "sha256:" + strings.Repeat("a", 64) + for _, tc := range []struct { + name, tags string + allowClone bool + }{ + {"unverified", "", false}, + {"wrong digest", "openctl-image-sha256-" + strings.Repeat("b", 64), false}, + {"matching digest", "site-tag;openctl-image-sha256-" + strings.Repeat("a", 64) + ";other", true}, + {"partial tag", "openctl-image-sha256-" + strings.Repeat("a", 64) + "-suffix", false}, + } { + t.Run(tc.name, func(t *testing.T) { + clones, downloads := 0, 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/api2/json/nodes": + json.NewEncoder(w).Encode(map[string]any{"data": []map[string]string{{"node": "pve1"}}}) + case r.URL.Path == "/api2/json/nodes/pve1/qemu": + json.NewEncoder(w).Encode(map[string]any{"data": []map[string]any{{"vmid": 9000, "name": "checksum-template", "template": 1}}}) + case r.URL.Path == "/api2/json/nodes/pve1/qemu/9000/config": + json.NewEncoder(w).Encode(map[string]any{"data": map[string]string{"tags": tc.tags}}) + case r.URL.Path == "/api2/json/cluster/nextid": + json.NewEncoder(w).Encode(map[string]any{"data": "200"}) + case r.URL.Path == "/api2/json/nodes/pve1/qemu/9000/clone": + clones++ + json.NewEncoder(w).Encode(map[string]any{"data": ""}) + case r.URL.Path == "/api2/json/nodes/pve1/qemu/200/config": + json.NewEncoder(w).Encode(map[string]any{"data": ""}) + case r.URL.Path == "/api2/json/nodes/pve1/storage": + json.NewEncoder(w).Encode(map[string]any{"data": []any{}}) + case strings.HasSuffix(r.URL.Path, "/download-url"): + downloads++ + http.Error(w, "cache must not redownload", http.StatusBadRequest) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + http.Error(w, "unexpected request", http.StatusBadRequest) + } + })) + defer server.Close() + h := New(&protocol.ProviderConfig{Endpoint: server.URL, Node: "pve1"}) + response, err := h.Handle(context.Background(), checksumCreateRequest(requested)) + if tc.allowClone { + if err != nil || response == nil || response.Status != protocol.StatusSuccess || clones != 1 || downloads != 0 { + t.Fatalf("verified cache must clone without downloading: response=%+v err=%v clones=%d downloads=%d", response, err, clones, downloads) + } + } else if err == nil || clones != 0 || downloads != 0 { + t.Fatalf("unverified cache must be rejected before mutation: err=%v clones=%d downloads=%d", err, clones, downloads) + } + }) + } +} + +func TestCloudImageInvalidChecksumRejectedBeforeCacheLookup(t *testing.T) { + requests := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + http.Error(w, "invalid checksum must not access the cache", http.StatusBadRequest) + })) + defer server.Close() + h := New(&protocol.ProviderConfig{Endpoint: server.URL, Node: "pve1"}) + response, err := h.Handle(context.Background(), checksumCreateRequest("sha256:abc123")) + if err != nil || response == nil || response.Error == nil || response.Error.Code != protocol.ErrorCodeInvalidRequest || requests != 0 { + t.Fatalf("malformed checksum must be an invalid request before API access: response=%+v err=%v requests=%d", response, err, requests) + } +} diff --git a/pkg/proxmox/handler/handler.go b/pkg/proxmox/handler/handler.go index 4f470a2..4641463 100644 --- a/pkg/proxmox/handler/handler.go +++ b/pkg/proxmox/handler/handler.go @@ -477,6 +477,16 @@ func (h *Handler) createVMFromCloudImage(ctx context.Context, name, node string, }, }, nil } + checksum, err := client.ParseImageChecksum(spec.CloudImage.Checksum) + if err != nil { + return &protocol.Response{ + Status: protocol.StatusError, + Error: &protocol.Error{ + Code: protocol.ErrorCodeInvalidRequest, + Message: fmt.Sprintf("invalid cloudImage.checksum: %v", err), + }, + }, nil + } // Determine template name from URL or explicit setting templateName := spec.CloudImage.TemplateName @@ -495,11 +505,28 @@ func (h *Handler) createVMFromCloudImage(ctx context.Context, name, node string, var templateVMID int if existingTemplate != nil { - // Template exists, use it + // A checksum pin must never silently reuse an unverified cached image. + if tag := checksum.TemplateTag(); tag != "" { + config, err := h.client.GetVMConfig(ctx, existingTemplate.Node, existingTemplate.VMID) + if err != nil { + return nil, fmt.Errorf("check cached template checksum: %w", err) + } + tags, _ := config.Raw["tags"].(string) + verified := false + for existingTag := range strings.SplitSeq(tags, ";") { + if existingTag == tag { + verified = true + break + } + } + if !verified { + return nil, fmt.Errorf("cached template %q was not verified with the requested checksum; choose an unused cloudImage.templateName", templateName) + } + } templateVMID = existingTemplate.VMID } else { // Template doesn't exist, create it - templateVMID, err = h.createCloudImageTemplate(ctx, node, templateName, spec) + templateVMID, err = h.createCloudImageTemplate(ctx, node, templateName, spec, checksum) if err != nil { return nil, fmt.Errorf("failed to create cloud image template: %w", err) } @@ -577,7 +604,7 @@ func (h *Handler) createVMFromCloudImage(ctx context.Context, name, node string, } // createCloudImageTemplate downloads a cloud image and creates a template from it -func (h *Handler) createCloudImageTemplate(ctx context.Context, node, templateName string, spec *resources.VMSpec) (int, error) { +func (h *Handler) createCloudImageTemplate(ctx context.Context, node, templateName string, spec *resources.VMSpec, checksum client.ImageChecksum) (int, error) { storage := spec.CloudImage.Storage // Step 1: Download the cloud image to storage @@ -587,26 +614,28 @@ func (h *Handler) createCloudImageTemplate(ctx context.Context, node, templateNa // Proxmox import content type only accepts .qcow2 or .raw extensions // Cloud images often use .img extension but are qcow2 format internally filename = normalizeImageExtension(filename) - upid, err := h.client.DownloadToStorage(ctx, node, storage, spec.CloudImage.URL, filename, "import") + upid, err := h.client.DownloadToStorage(ctx, node, storage, spec.CloudImage.URL, filename, "import", checksum) if err != nil { return 0, fmt.Errorf("failed to download cloud image: %w", err) } // Wait for download to complete - if upid != "" { - if waitErr := h.client.WaitForTask(ctx, node, upid, 30*time.Minute); waitErr != nil { - return 0, fmt.Errorf("download task failed: %w", waitErr) - } + if waitErr := h.client.WaitForTask(ctx, node, upid, 30*time.Minute); waitErr != nil { + return 0, fmt.Errorf("download task failed: %w", waitErr) } // Step 2: Create base VM - nextID, err := h.client.CreateVM(ctx, node, map[string]any{ + params := map[string]any{ "name": templateName, "ostype": "l26", "scsihw": "virtio-scsi-pci", "boot": "order=scsi0", "agent": "1", - }) + } + if tag := checksum.TemplateTag(); tag != "" { + params["tags"] = tag + } + nextID, err := h.client.CreateVM(ctx, node, params) if err != nil { return 0, fmt.Errorf("failed to create template VM: %w", err) } diff --git a/plugins/proxmox/internal/compute/provider.go b/plugins/proxmox/internal/compute/provider.go index f2f9a81..a49cc4c 100644 --- a/plugins/proxmox/internal/compute/provider.go +++ b/plugins/proxmox/internal/compute/provider.go @@ -162,7 +162,7 @@ func (p *ProxmoxProvider) createFromCloudImage(ctx context.Context, node, name s } else { // Download and create template filename := extractFilenameFromURL(spec.CloudImage.URL) - upid, err := p.client.DownloadToStorage(ctx, node, storage, spec.CloudImage.URL, filename, "iso") + upid, err := p.client.DownloadToStorage(ctx, node, storage, spec.CloudImage.URL, filename, "iso", client.ImageChecksum{}) if err != nil { return 0, fmt.Errorf("failed to download cloud image: %w", err) }