Compare commits
1
Commits
03f126edc1
...
cf0f08586d
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cf0f08586d |
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
Reference in New Issue
Block a user