Reject a metadata database truncated to the age header and nonce (closes #152)
check / check (pull_request) Successful in 2m33s
check / check (pull_request) Successful in 2m33s
An object holding exactly the age header plus its 16-byte nonce decrypts without error: age.Decrypt succeeds, and on the first read the age stream's io.ErrUnexpectedEOF is mapped by the zstd decoder to a clean io.EOF at frame start. blobgen.Reader.Read then reported zero bytes and no error, so a truncated stream was indistinguishable from a valid empty one. Blobs are caught by the Close-time hash check, but the metadata database export was not: restore built a fresh schema on the empty file and exited reporting 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 behind, so an empty payload still round-trips to empty with no error. downloadSnapshotDB additionally rejects a zero-length decrypted database before OpenReadOnly applies a schema. On next the decrypted-DB identity check already rejects an empty database, so the restore-level test guards the required end-to-end property; the blobgen tests are the ones that fail without the reader change. Model: opus-4-8
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
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.
|
||||
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.Error(t, err)
|
||||
}
|
||||
Reference in New Issue
Block a user