Reject a metadata database truncated to the age header and nonce (closes #152)
check / check (push) Successful in 2m5s
check / check (pull_request) Successful in 2m28s

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
This commit was merged in pull request #198.
This commit is contained in:
2026-09-22 18:11:57 +02:00
parent 1244c9e48d
commit c3bec7d3aa
5 changed files with 214 additions and 0 deletions
+17
View File
@@ -2,6 +2,7 @@ package blobgen
import ( import (
"crypto/sha256" "crypto/sha256"
"errors"
"fmt" "fmt"
"hash" "hash"
"io" "io"
@@ -56,6 +57,22 @@ func (r *Reader) Read(p []byte) (int, error) {
n, err := r.teeReader.Read(p) n, err := r.teeReader.Read(p)
r.bytesRead += int64(n) 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 return n, err
} }
+70
View File
@@ -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)
}
+9
View File
@@ -48,6 +48,10 @@ var (
"restore loop ended with files still pending") "restore loop ended with files still pending")
errSnapshotDBMismatch = errors.New( errSnapshotDBMismatch = errors.New(
"decrypted database is not the requested snapshot") "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 // 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)) 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) db, err := database.OpenReadOnly(v.ctx, dbPath)
if err != nil { if err != nil {
return nil, "", fmt.Errorf("opening restore database: %w", err) return nil, "", fmt.Errorf("opening restore database: %w", err)
@@ -7,8 +7,10 @@ import (
"path/filepath" "path/filepath"
"testing" "testing"
"filippo.io/age"
"github.com/spf13/afero" "github.com/spf13/afero"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"sneak.berlin/go/vaultik/internal/blobgen"
"sneak.berlin/go/vaultik/internal/database" "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") 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 // TestMaterializeSnapshotDBRemovesDirOnOpenFailure proves a failed open
// leaves no temp directory behind. // leaves no temp directory behind.
func TestMaterializeSnapshotDBRemovesDirOnOpenFailure(t *testing.T) { func TestMaterializeSnapshotDBRemovesDirOnOpenFailure(t *testing.T) {
@@ -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)
}