Reject a metadata database truncated to the age header and nonce #198
@@ -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
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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")
|
"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)
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user