From cf0f08586d4becf872b2a3d8905be5243bd806cc Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 22 Sep 2026 15:12:48 +0000 Subject: [PATCH] Reject a metadata database truncated to the age header and nonce (closes #152) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An object holding just the age header and its 16-byte nonce decrypts without error: the truncated read surfaces as io.ErrUnexpectedEOF at the age layer, which the zstd decoder maps to a clean EOF at frame start. blobgen then reported zero bytes and no error, so a truncated stream was indistinguishable from a valid empty one, and the metadata database export slipped through — restore built a fresh schema on the empty file and reported success. blobgen.Reader.Read now, on EOF, reads once more from the age reader and surfaces io.ErrUnexpectedEOF unless that read is (0, io.EOF), the state a genuine end leaves. downloadSnapshotDB additionally rejects a zero-length decrypted database before any schema is built. Model: opus-4-8 --- internal/blobgen/reader.go | 17 ++++ internal/blobgen/truncation_test.go | 70 +++++++++++++++ internal/vaultik/restore.go | 9 ++ internal/vaultik/restore_snapshotdb_test.go | 33 +++++++ internal/vaultik/restore_truncated_db_test.go | 85 +++++++++++++++++++ 5 files changed, 214 insertions(+) create mode 100644 internal/blobgen/truncation_test.go create mode 100644 internal/vaultik/restore_truncated_db_test.go diff --git a/internal/blobgen/reader.go b/internal/blobgen/reader.go index 7c6ebeb..27bad18 100644 --- a/internal/blobgen/reader.go +++ b/internal/blobgen/reader.go @@ -2,6 +2,7 @@ package blobgen import ( "crypto/sha256" + "errors" "fmt" "hash" "io" @@ -56,6 +57,22 @@ func (r *Reader) Read(p []byte) (int, error) { n, err := r.teeReader.Read(p) r.bytesRead += int64(n) + // When the ciphertext is cut right after the age header plus its + // 16-byte nonce, the age reader's first read fails with + // io.ErrUnexpectedEOF, and the zstd decoder maps that to a clean + // io.EOF at frame start. That makes a truncated stream look like a + // valid empty one. Distinguish the two: on EOF, read once more from + // the age reader. A genuine end leaves it at (0, io.EOF); a truncated + // stream leaves its stored io.ErrUnexpectedEOF, which we surface. + if errors.Is(err, io.EOF) { + var probe [1]byte + + m, ageErr := r.decryptor.Read(probe[:]) + if m != 0 || !errors.Is(ageErr, io.EOF) { + return n, io.ErrUnexpectedEOF + } + } + return n, err } diff --git a/internal/blobgen/truncation_test.go b/internal/blobgen/truncation_test.go new file mode 100644 index 0000000..d21dcfd --- /dev/null +++ b/internal/blobgen/truncation_test.go @@ -0,0 +1,70 @@ +package blobgen_test + +import ( + "bytes" + "io" + "testing" + + "filippo.io/age" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/blobgen" +) + +// TestReaderRejectsHeaderNonceTruncation guards against a stream cut right +// after the age header plus its 16-byte nonce. age.Decrypt still succeeds on +// such an object, and the zstd decoder maps the age reader's +// io.ErrUnexpectedEOF to a clean io.EOF at frame start, so without the extra +// check the truncated stream would read as a valid empty one. Reading it must +// now fail. +func TestReaderRejectsHeaderNonceTruncation(t *testing.T) { + t.Parallel() + + identity, err := age.GenerateX25519Identity() + require.NoError(t, err) + + // Encrypting empty plaintext yields header + nonce(16) + a single + // 16-byte final chunk tag. Dropping the trailing tag leaves exactly the + // age header plus its nonce — the truncation point that triggers the bug. + var full bytes.Buffer + + w, err := age.Encrypt(&full, identity.Recipient()) + require.NoError(t, err) + require.NoError(t, w.Close()) + + truncated := full.Bytes()[:full.Len()-16] + + reader, err := blobgen.NewReader(bytes.NewReader(truncated), identity) + require.NoError(t, err) + + defer func() { _ = reader.Close() }() + + _, err = io.ReadAll(reader) + require.Error(t, err) + require.ErrorIs(t, err, io.ErrUnexpectedEOF) +} + +// TestReaderReadsGenuinelyEmptyBlob confirms the truncation check does not +// reject a legitimately empty payload: a blob written with no data must round +// trip back to zero bytes with no error. +func TestReaderReadsGenuinelyEmptyBlob(t *testing.T) { + t.Parallel() + + identity, err := age.GenerateX25519Identity() + require.NoError(t, err) + + var encrypted bytes.Buffer + + writer, err := blobgen.NewWriter( + &encrypted, 3, []string{identity.Recipient().String()}) + require.NoError(t, err) + require.NoError(t, writer.Close()) + + reader, err := blobgen.NewReader(bytes.NewReader(encrypted.Bytes()), identity) + require.NoError(t, err) + + defer func() { _ = reader.Close() }() + + data, err := io.ReadAll(reader) + require.NoError(t, err) + require.Empty(t, data) +} diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index ca5b29b..118a68b 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -48,6 +48,10 @@ var ( "restore loop ended with files still pending") errSnapshotDBMismatch = errors.New( "decrypted database is not the requested snapshot") + // errEmptySnapshotDB is returned when the decrypted metadata database has + // zero length, which happens when the object was truncated or replaced + // with an empty payload. Rejected before any schema is built on it. + errEmptySnapshotDB = errors.New("decrypted snapshot database is empty") ) // snapshotDBFilename is the name the decrypted snapshot database is @@ -758,6 +762,11 @@ func (v *Vaultik) materializeSnapshotDB( log.Debug("Created restore database", "path", dbPath, "size", ubytes(written)) + // Reject an empty database before OpenReadOnly builds a schema on it. + if written == 0 { + return nil, "", errEmptySnapshotDB + } + db, err := database.OpenReadOnly(v.ctx, dbPath) if err != nil { return nil, "", fmt.Errorf("opening restore database: %w", err) diff --git a/internal/vaultik/restore_snapshotdb_test.go b/internal/vaultik/restore_snapshotdb_test.go index c1e1605..efb61ad 100644 --- a/internal/vaultik/restore_snapshotdb_test.go +++ b/internal/vaultik/restore_snapshotdb_test.go @@ -7,8 +7,10 @@ import ( "path/filepath" "testing" + "filippo.io/age" "github.com/spf13/afero" "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/blobgen" "sneak.berlin/go/vaultik/internal/database" ) @@ -56,6 +58,37 @@ func TestMaterializeSnapshotDBPrivateDir(t *testing.T) { require.Error(t, err, "materialized snapshot database must be read-only") } +// TestMaterializeSnapshotDBRejectsCompleteEmptyStream proves the written == 0 +// guard rejects a genuinely empty but complete metadata object: a real age +// header, nonce, and final tag encrypting zero plaintext bytes. The truncation +// case is stopped earlier by the reader (io.ErrUnexpectedEOF) and never reaches +// this branch, so it needs its own input. This complete stream decrypts to zero +// bytes with a clean EOF, passes the reader, and must be refused as empty rather +// than accepted as a valid zero-table database. Reverting the guard lets the +// empty file open as a fresh schema and the test fails. +func TestMaterializeSnapshotDBRejectsCompleteEmptyStream(t *testing.T) { + identity, err := age.GenerateX25519Identity() + require.NoError(t, err) + + var stream bytes.Buffer + + w, err := age.Encrypt(&stream, identity.Recipient()) + require.NoError(t, err) + require.NoError(t, w.Close()) + + blobReader, err := blobgen.NewReader(bytes.NewReader(stream.Bytes()), identity) + require.NoError(t, err) + + t.Cleanup(func() { _ = blobReader.Close() }) + + t.Setenv("TMPDIR", t.TempDir()) + + v := &Vaultik{ctx: context.Background(), Fs: afero.NewOsFs()} + + _, _, err = v.materializeSnapshotDB(blobReader) + require.ErrorIs(t, err, errEmptySnapshotDB) +} + // TestMaterializeSnapshotDBRemovesDirOnOpenFailure proves a failed open // leaves no temp directory behind. func TestMaterializeSnapshotDBRemovesDirOnOpenFailure(t *testing.T) { diff --git a/internal/vaultik/restore_truncated_db_test.go b/internal/vaultik/restore_truncated_db_test.go new file mode 100644 index 0000000..aeedd79 --- /dev/null +++ b/internal/vaultik/restore_truncated_db_test.go @@ -0,0 +1,85 @@ +package vaultik_test + +import ( + "bytes" + "context" + "io" + "path/filepath" + "testing" + + "filippo.io/age" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestRestoreRejectsTruncatedMetadataDB backs up a real tree, then replaces +// the snapshot's db.zst.age with a stream cut right after the age header and +// its 16-byte nonce. age.Decrypt still accepts such an object and the zstd +// decoder turns the truncated read into a clean EOF, so before the fix restore +// built a fresh empty schema and reported success. Restore must now fail with +// io.ErrUnexpectedEOF, the error the reader raises for a truncated object. +// Asserting that specific error pins the reader fix: without it the truncation +// yields an empty database, which the identity check rejects for an unrelated +// reason, and this test would pass anyway. +func TestRestoreRejectsTruncatedMetadataDB(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") + restoreDir := filepath.Join(tempDir, "restored") + dbPath := filepath.Join(tempDir, "index.sqlite") + + chunkSize := int64(64 * 1024) + maxBlobSize := int64(512 * 1024) + + setupE2ESourceTree(t, fs, dataDir, chunkSize) + + ctx := context.Background() + + cfg, storer, snapshotID := runFileStorageBackup( + ctx, t, fs, dataDir, storeDir, dbPath, chunkSize, maxBlobSize) + + // Encrypting empty plaintext to the snapshot recipient yields + // header + nonce(16) + a single 16-byte final chunk tag. Dropping the + // trailing tag leaves exactly the age header plus its nonce — the + // truncation an attacker can write over metadata without any key. + recipient, err := age.ParseX25519Recipient(testAgePublicKey) + require.NoError(t, err) + + var full bytes.Buffer + + w, err := age.Encrypt(&full, recipient) + require.NoError(t, err) + require.NoError(t, w.Close()) + + truncated := full.Bytes()[:full.Len()-16] + + dbKeyPath := filepath.Join(storeDir, "metadata", + snapshot.RemoteSnapshotKey(snapshotID), "db.zst.age") + require.NoError(t, afero.WriteFile(fs, dbKeyPath, truncated, 0o644)) + + restoreVaultik := &vaultik.Vaultik{ + Config: cfg, + Storage: storer, + Fs: fs, + Stdout: io.Discard, + Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), + } + restoreVaultik.SetContext(ctx) + + err = restoreVaultik.Restore(&vaultik.RestoreOptions{ + SnapshotID: snapshotID, + TargetDir: restoreDir, + Verify: true, + }) + require.ErrorIs(t, err, io.ErrUnexpectedEOF) +} -- 2.54.0