From c2720d4b086aa39dc81bf88f3bd84142379e6d86 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 21 Sep 2026 17:54:15 +0000 Subject: [PATCH] Hash the plaintext, not the encrypted bytes, in verify --deep (closes #131) Deep verification's final blob-integrity check hashed the encrypted downloaded bytes with a single SHA256 and compared that to the blob's remote name, which is the double SHA256 of the plaintext (blobgen.Writer.Sum256). The two can never be equal, so verify --deep reported every healthy blob as "blob hash mismatch". The per-chunk and blob-existence checks were correct; only this final comparison was wrong. It now hashes the decompressed plaintext as chunk verification streams it and compares its double SHA256 to the blob name. Added a test that backs up a real snapshot, deep-verifies it (which fails before this fix), then flips a byte in one stored blob and confirms deep verification then fails. Model: opus-4-8 --- TODO.md | 10 +++ internal/vaultik/deep_verify_test.go | 108 +++++++++++++++++++++++++++ internal/vaultik/verify.go | 33 ++++---- 3 files changed, 137 insertions(+), 14 deletions(-) create mode 100644 internal/vaultik/deep_verify_test.go diff --git a/TODO.md b/TODO.md index 5896f00..f549799 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,16 @@ release" is exactly the contradiction # Completed Steps +- 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as + corrupt. Its final blob-integrity check hashed the encrypted + downloaded bytes with a single SHA256 and compared that to the blob + ID, which is the double SHA256 of the plaintext, so the two could + never match. It now hashes the decompressed plaintext and compares the + double SHA256. Added a test that backs up a real snapshot, deep-verifies + it, then flips a byte in one stored blob and confirms deep verification + then fails + ([issue #131](https://git.eeqj.de/sneak/vaultik/issues/131)). + - 2026-09-21: Made `snapshot create` VACUUM the per-snapshot metadata database through the `modernc.org/sqlite` driver instead of shelling out to the external `sqlite` command-line binary (issue #120). A diff --git a/internal/vaultik/deep_verify_test.go b/internal/vaultik/deep_verify_test.go new file mode 100644 index 0000000..6c7d782 --- /dev/null +++ b/internal/vaultik/deep_verify_test.go @@ -0,0 +1,108 @@ +package vaultik_test + +import ( + "context" + "io" + "os" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob backs up a real +// snapshot with the on-disk storage backend, runs deep verification on +// it, then flips a byte inside one stored blob and runs deep +// verification again. A healthy snapshot must pass; a corrupted blob +// must fail. The healthy case is the regression guard: deep +// verification used to hash the encrypted blob bytes and compare them +// to the blob's ID (the double SHA256 of the plaintext), so it reported +// every healthy blob as corrupt. +func TestDeepVerifyAcceptsHealthyAndRejectsCorruptBlob(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + fs := afero.NewOsFs() + tempDir := t.TempDir() + + dataDir := filepath.Join(tempDir, "source") + storeDir := filepath.Join(tempDir, "remote") + dbPath := filepath.Join(tempDir, "index.sqlite") + + chunkSize := int64(64 * 1024) + maxBlobSize := int64(512 * 1024) + + // One file large enough to span several chunks within a single blob. + require.NoError(t, fs.MkdirAll(dataDir, 0o755)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(dataDir, "data.bin"), + bytesPattern("deep-", int(chunkSize*3)), 0o644)) + + ctx := context.Background() + + // runFileStorageBackup writes a real snapshot to storeDir and closes + // the source index, so verification runs from remote bytes only. + cfg, storer, snapshotID := runFileStorageBackup( + ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize) + + newVerifier := func() *vaultik.Vaultik { + v := &vaultik.Vaultik{ + Config: cfg, + Storage: storer, + Fs: fs, + Stdout: io.Discard, + Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), + } + v.SetContext(ctx) + + return v + } + + require.NoError(t, + newVerifier().RunDeepVerify(snapshotID, &vaultik.VerifyOptions{Deep: true}), + "deep verify should pass on a healthy snapshot") + + // Flip a byte inside one blob without changing its length, so the + // blob-existence and size checks still pass and verification reaches + // the blob-content stage. + corruptOneBlob(t, fs, filepath.Join(storeDir, "blobs")) + + require.Error(t, + newVerifier().RunDeepVerify(snapshotID, &vaultik.VerifyOptions{Deep: true}), + "deep verify should fail on a corrupted blob") +} + +// corruptOneBlob flips a middle byte of the first blob file found under +// blobsDir, leaving the file length unchanged. +func corruptOneBlob(t *testing.T, fs afero.Fs, blobsDir string) { + t.Helper() + + var blobPath string + + err := afero.Walk(fs, blobsDir, + func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if blobPath == "" && !info.IsDir() { + blobPath = path + } + + return nil + }) + require.NoError(t, err) + require.NotEmpty(t, blobPath, "expected at least one blob on disk") + + data, err := afero.ReadFile(fs, blobPath) + require.NoError(t, err) + require.NotEmpty(t, data) + + data[len(data)/2] ^= 0xff + require.NoError(t, afero.WriteFile(fs, blobPath, data, 0o644)) +} diff --git a/internal/vaultik/verify.go b/internal/vaultik/verify.go index 38be849..46f4be8 100644 --- a/internal/vaultik/verify.go +++ b/internal/vaultik/verify.go @@ -344,12 +344,8 @@ func (v *Vaultik) verifyBlob(blobInfo snapshot.BlobInfo, db *sql.DB) error { return fmt.Errorf("failed to get decryptor: %w", err) } - // Hash the encrypted blob data as it streams through to decryption - blobHasher := sha256.New() - teeReader := io.TeeReader(reader, blobHasher) - - // Decrypt blob (reading through teeReader to hash encrypted data) - decryptedReader, err := decryptor.DecryptStream(teeReader) + // Decrypt blob + decryptedReader, err := decryptor.DecryptStream(reader) if err != nil { return fmt.Errorf("failed to decrypt: %w", err) } @@ -361,12 +357,19 @@ func (v *Vaultik) verifyBlob(blobInfo snapshot.BlobInfo, db *sql.DB) error { } defer decompressor.Close() - chunkCount, err := v.verifyBlobChunks(db, blobInfo.Hash, decompressor) + // A blob's hash — its remote name — is the double SHA256 of its + // decompressed plaintext (see blobgen.Writer.Sum256), not of the + // encrypted bytes. Hash the plaintext as chunk verification streams + // it, then compare on completion. + plaintextHasher := sha256.New() + hashedStream := io.TeeReader(decompressor, plaintextHasher) + + chunkCount, err := v.verifyBlobChunks(db, blobInfo.Hash, hashedStream) if err != nil { return err } - err = v.verifyBlobFinalIntegrity(decompressor, blobHasher, blobInfo.Hash) + err = v.verifyBlobFinalIntegrity(hashedStream, plaintextHasher, blobInfo.Hash) if err != nil { return err } @@ -470,14 +473,13 @@ func (v *Vaultik) verifyBlobChunks( } // verifyBlobFinalIntegrity checks that no trailing data exists in the -// decompressed stream and that the encrypted blob hash matches the -// expected value. +// decompressed stream and that the blob hash matches the expected value. func (v *Vaultik) verifyBlobFinalIntegrity( - decompressor io.Reader, blobHasher hash.Hash, expectedHash string, + plaintext io.Reader, plaintextHasher hash.Hash, expectedHash string, ) error { // Verify no remaining data in blob - if the chunk list is accurate, // the blob should be fully consumed. - remaining, err := io.Copy(io.Discard, decompressor) + remaining, err := io.Copy(io.Discard, plaintext) if err != nil { return fmt.Errorf("failed to check for remaining blob data: %w", err) } @@ -486,8 +488,11 @@ func (v *Vaultik) verifyBlobFinalIntegrity( return fmt.Errorf("%w: %d bytes", errTrailingBlobData, remaining) } - // Verify blob hash matches the encrypted data we downloaded - calculatedBlobHash := hex.EncodeToString(blobHasher.Sum(nil)) + // The blob hash is the double SHA256 of its plaintext content. + firstHash := plaintextHasher.Sum(nil) + secondHash := sha256.Sum256(firstHash) + calculatedBlobHash := hex.EncodeToString(secondHash[:]) + if calculatedBlobHash != expectedHash { return fmt.Errorf("%w: calculated %s, expected %s", errBlobHashMismatch, calculatedBlobHash, expectedHash) -- 2.54.0