Reject a metadata database truncated to the age header and nonce #198

Merged
clawbot merged 1 commits from issue-152-truncated-metadata-db into next 2026-09-22 18:11:58 +02:00
5 changed files with 214 additions and 0 deletions
Showing only changes of commit cf0f08586d - Show all commits
+17
View File
@@ -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
}
+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")
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)
@@ -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) {
@@ -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)
}