Fix acquisition ZIP integrity checks

acquisition/acquisition_test.go

@@ -1,7 +1,9 @@
 package acquisition
 
 import (
+	"archive/zip"
 	"encoding/json"
+	"io"
 	"os"
 	"path/filepath"
 	"testing"
@@ -83,3 +85,69 @@ func TestStoreSecurelyUsesCurrentWorkingDirectory(t *testing.T) {
 		t.Fatalf("storage path still exists or returned unexpected error: %v", err)
 	}
 }
+
+func TestCreateZipFileCreatesReadableArchive(t *testing.T) {
+	sourcePath := t.TempDir()
+	if err := os.Mkdir(filepath.Join(sourcePath, "nested"), 0o755); err != nil {
+		t.Fatalf("Mkdir(nested) error = %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(sourcePath, "nested", "data.txt"), []byte("evidence"), 0o600); err != nil {
+		t.Fatalf("WriteFile(data.txt) error = %v", err)
+	}
+
+	zipPath := filepath.Join(t.TempDir(), "acquisition.zip")
+	if err := createZipFile(sourcePath, zipPath); err != nil {
+		t.Fatalf("createZipFile() error = %v", err)
+	}
+
+	files := readZipFiles(t, zipPath)
+	if files["nested/data.txt"] != "evidence" {
+		t.Fatalf("nested/data.txt = %q, want evidence", files["nested/data.txt"])
+	}
+}
+
+func TestCreateZipFileSkipsArchiveInsideSourceDirectory(t *testing.T) {
+	sourcePath := t.TempDir()
+	if err := os.WriteFile(filepath.Join(sourcePath, "data.txt"), []byte("evidence"), 0o600); err != nil {
+		t.Fatalf("WriteFile(data.txt) error = %v", err)
+	}
+
+	zipPath := filepath.Join(sourcePath, "acquisition.zip")
+	if err := createZipFile(sourcePath, zipPath); err != nil {
+		t.Fatalf("createZipFile() error = %v", err)
+	}
+
+	files := readZipFiles(t, zipPath)
+	if files["data.txt"] != "evidence" {
+		t.Fatalf("data.txt = %q, want evidence", files["data.txt"])
+	}
+	if _, ok := files["acquisition.zip"]; ok {
+		t.Fatal("archive contains itself")
+	}
+}
+
+func readZipFiles(t *testing.T, zipPath string) map[string]string {
+	t.Helper()
+
+	reader, err := zip.OpenReader(zipPath)
+	if err != nil {
+		t.Fatalf("OpenReader(%q) error = %v", zipPath, err)
+	}
+	defer reader.Close()
+
+	files := make(map[string]string)
+	for _, file := range reader.File {
+		readCloser, err := file.Open()
+		if err != nil {
+			t.Fatalf("Open(%q) error = %v", file.Name, err)
+		}
+		content, err := io.ReadAll(readCloser)
+		readCloser.Close()
+		if err != nil {
+			t.Fatalf("ReadAll(%q) error = %v", file.Name, err)
+		}
+		files[file.Name] = string(content)
+	}
+
+	return files
+}

acquisition/secure.go

@@ -18,22 +18,100 @@ import (
 )
 
 func createZipFile(sourceDir, zipPath string) error {
-	zipFile, err := os.Create(zipPath)
+	sourceAbs, err := filepath.Abs(sourceDir)
+	if err != nil {
+		return fmt.Errorf("failed to resolve source directory: %v", err)
+	}
+	zipAbs, err := filepath.Abs(zipPath)
+	if err != nil {
+		return fmt.Errorf("failed to resolve ZIP path: %v", err)
+	}
+
+	zipFile, err := os.OpenFile(zipPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
 	if err != nil {
 		return fmt.Errorf("failed to create ZIP file: %v", err)
 	}
-	defer zipFile.Close()
 
 	zipWriter := zip.NewWriter(zipFile)
-	defer zipWriter.Close()
 
-	// Use AddFS to add the entire directory
-	fsys := os.DirFS(sourceDir)
-	err = zipWriter.AddFS(fsys)
+	err = filepath.WalkDir(sourceDir, func(filePath string, dirEntry os.DirEntry, walkErr error) error {
+		if walkErr != nil {
+			return walkErr
+		}
+		if dirEntry.IsDir() {
+			return nil
+		}
+
+		fileAbs, err := filepath.Abs(filePath)
+		if err != nil {
+			return err
+		}
+		if fileAbs == zipAbs {
+			return nil
+		}
+
+		fileInfo, err := dirEntry.Info()
+		if err != nil {
+			return err
+		}
+		if !fileInfo.Mode().IsRegular() {
+			return nil
+		}
+
+		relPath, err := filepath.Rel(sourceAbs, fileAbs)
+		if err != nil {
+			return err
+		}
+		header, err := zip.FileInfoHeader(fileInfo)
+		if err != nil {
+			return err
+		}
+		header.Name = filepath.ToSlash(relPath)
+		header.Method = zip.Deflate
+
+		writer, err := zipWriter.CreateHeader(header)
+		if err != nil {
+			return err
+		}
+
+		file, err := os.Open(filePath)
+		if err != nil {
+			return err
+		}
+
+		_, copyErr := io.Copy(writer, file)
+		closeErr := file.Close()
+		if copyErr != nil {
+			return copyErr
+		}
+		return closeErr
+	})
 	if err != nil {
+		_ = zipWriter.Close()
+		_ = zipFile.Close()
 		return fmt.Errorf("failed to add directory to ZIP: %v", err)
 	}
 
+	if err := zipWriter.Close(); err != nil {
+		_ = zipFile.Close()
+		return fmt.Errorf("failed to close ZIP writer: %v", err)
+	}
+	if err := zipFile.Sync(); err != nil {
+		_ = zipFile.Close()
+		return fmt.Errorf("failed to sync ZIP file: %v", err)
+	}
+	if err := zipFile.Close(); err != nil {
+		return fmt.Errorf("failed to close ZIP file: %v", err)
+	}
+
+	reader, err := zip.OpenReader(zipPath)
+	if err != nil {
+		return fmt.Errorf("failed to verify ZIP file: %v", err)
+	}
+	if err := reader.Close(); err != nil {
+		return fmt.Errorf("failed to close ZIP verification reader: %v", err)
+	}
+
 	return nil
 }
 
@@ -65,6 +143,7 @@ func (a *Acquisition) StoreSecurely() error {
 
 	err = createZipFile(a.StoragePath, zipFilePath)
 	if err != nil {
+		_ = os.Remove(zipFilePath)
 		return err
 	}
 
@@ -85,34 +164,57 @@ func (a *Acquisition) StoreSecurely() error {
 	if err != nil {
 		return err
 	}
-	defer zipFile.Close()
+	zipFileClosed := false
+	defer func() {
+		if !zipFileClosed {
+			zipFile.Close()
+		}
+	}()
 
 	encFileName := fmt.Sprintf("%s.age", zipFileName)
 	encFilePath := filepath.Join(cwd, encFileName)
-	encFile, err := os.OpenFile(encFilePath, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0o600)
+	tmpEncFilePath := encFilePath + ".tmp"
+	encFile, err := os.OpenFile(tmpEncFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o600)
 	if err != nil {
 		return fmt.Errorf("unable to create encrypted file: %v", err)
 	}
-	defer encFile.Close()
+	defer os.Remove(tmpEncFilePath)
 
 	w, err := age.Encrypt(encFile, recipient)
 	if err != nil {
+		encFile.Close()
 		return fmt.Errorf("failed to create encrypted file: %v", err)
 	}
 
 	_, err = io.Copy(w, zipFile)
 	if err != nil {
+		w.Close()
+		encFile.Close()
 		return fmt.Errorf("failed to write to encrypted file: %v", err)
 	}
 
 	if err := w.Close(); err != nil {
+		encFile.Close()
 		return fmt.Errorf("failed to close encrypted file: %v", err)
 	}
+	if err := encFile.Sync(); err != nil {
+		encFile.Close()
+		return fmt.Errorf("failed to sync encrypted file: %v", err)
+	}
+	if err := encFile.Close(); err != nil {
+		return fmt.Errorf("failed to close encrypted output file: %v", err)
+	}
+	if err := os.Rename(tmpEncFilePath, encFilePath); err != nil {
+		return fmt.Errorf("failed to move encrypted file into place: %v", err)
+	}
 
 	log.Infof("Acquisition successfully encrypted at %s", encFilePath)
 
 	// TODO: we should securely wipe the files.
-	zipFile.Close()
+	if err := zipFile.Close(); err != nil {
+		return fmt.Errorf("failed to close the unencrypted compressed archive: %v", err)
+	}
+	zipFileClosed = true
 	err = os.Remove(zipFilePath)
 	if err != nil {
 		return fmt.Errorf("failed to delete the unencrypted compressed archive: %v", err)