diff --git a/internal/database/snapshots.go b/internal/database/snapshots.go index 97bef66..07c8242 100644 --- a/internal/database/snapshots.go +++ b/internal/database/snapshots.go @@ -11,6 +11,18 @@ import ( "sneak.berlin/go/vaultik/internal/types" ) +// Sentinel errors for the single-snapshot invariant that an exported +// per-snapshot metadata database must satisfy. +var ( + // ErrNoSnapshotInDatabase means the metadata database has no snapshot + // row at all. + ErrNoSnapshotInDatabase = errors.New("database contains no snapshot") + // ErrMultipleSnapshotsInDatabase means the metadata database holds + // more than the single snapshot an export is supposed to contain. + ErrMultipleSnapshotsInDatabase = errors.New( + "database contains more than one snapshot") +) + // SnapshotRepository provides access to the snapshots table and its // snapshot_files / snapshot_blobs association tables. type SnapshotRepository struct { @@ -206,6 +218,48 @@ func (r *SnapshotRepository) GetByID( return &snapshot, nil } +// GetOnlySnapshot returns the sole snapshot in an exported per-snapshot +// metadata database. The backup path writes each snapshot's database with +// exactly one snapshot row (see cleanSnapshotDB), so restore and deep +// verify expect exactly one. Zero rows return ErrNoSnapshotInDatabase and +// more than one returns ErrMultipleSnapshotsInDatabase; callers treat +// either as a failed identity check on the downloaded database. +func (r *SnapshotRepository) GetOnlySnapshot(ctx context.Context) (*Snapshot, error) { + query := ` + SELECT id, hostname, vaultik_version, vaultik_git_revision, + started_at, completed_at, file_count, chunk_count, blob_count, + total_size, blob_size, compression_ratio + FROM snapshots + LIMIT 2 + ` + + rows, err := r.db.conn.QueryContext(ctx, query) + if err != nil { + return nil, fmt.Errorf("querying snapshots: %w", err) + } + + defer func() { + err := rows.Close() + if err != nil { + Fatalf("failed to close rows: %v", err) + } + }() + + snapshots, err := r.scanSnapshotRows(rows) + if err != nil { + return nil, err + } + + switch len(snapshots) { + case 1: + return snapshots[0], nil + case 0: + return nil, ErrNoSnapshotInDatabase + default: + return nil, ErrMultipleSnapshotsInDatabase + } +} + // ListRecent returns up to limit snapshots, most recently started first. func (r *SnapshotRepository) ListRecent( ctx context.Context, limit int, diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index 32d51df..9f7f198 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -19,6 +19,7 @@ import ( "sneak.berlin/go/vaultik/internal/blobgen" "sneak.berlin/go/vaultik/internal/database" "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" "sneak.berlin/go/vaultik/internal/types" ) @@ -41,6 +42,8 @@ var ( "restored file has trailing data after its last chunk") errRestoreIncomplete = errors.New( "restore loop ended with files still pending") + errSnapshotDBMismatch = errors.New( + "decrypted database is not the requested snapshot") ) // snapshotDBFilename is the name the decrypted snapshot database is @@ -655,7 +658,53 @@ func (v *Vaultik) downloadSnapshotDB( log.Debug("Decrypted database", "size", ubytes(int64(len(dbData)))) - return v.materializeSnapshotDB(dbData) + db, tempDir, err := v.materializeSnapshotDB(dbData) + if err != nil { + return nil, "", err + } + + // Confirm the decrypted database really is the snapshot named by + // remoteKey before any files are read from it. On mismatch, close the + // database and remove its private directory so nothing is left behind. + err = v.verifySnapshotDBIdentity(db, snapshotID, remoteKey) + if err != nil { + _ = db.Close() + _ = v.Fs.RemoveAll(tempDir) + + return nil, "", err + } + + return db, tempDir, nil +} + +// verifySnapshotDBIdentity confirms the decrypted metadata database really +// is the snapshot named by remoteKey. age decryption proves the database +// is readable, not that the object served at +// metadata//db.zst.age is the snapshot that was requested: an +// attacker who swaps in another valid db.zst.age (which needs no key +// material) would otherwise redirect restore and deep verify to a +// different snapshot's contents. The exported per-snapshot database holds +// exactly one snapshot row, and a snapshot's remote key is derived from +// that row's ID, so the database is the requested one exactly when its +// sole snapshot hashes back to remoteKey. Comparing the requested +// identifier directly would not do: it may be a remote-key prefix a +// recovery host uses in place of a human snapshot ID it cannot know. +func (v *Vaultik) verifySnapshotDBIdentity( + db *database.DB, requested, remoteKey string, +) error { + repos := database.NewRepositories(db) + + snap, err := repos.Snapshots.GetOnlySnapshot(v.ctx) + if err != nil { + return fmt.Errorf("checking identity of database for %s: %w", requested, err) + } + + if snapshot.RemoteSnapshotKey(snap.ID.String()) != remoteKey { + return fmt.Errorf("%w: requested %s but the database is snapshot %s", + errSnapshotDBMismatch, requested, snap.ID) + } + + return nil } // materializeSnapshotDB writes the decrypted snapshot database bytes into diff --git a/internal/vaultik/restore_snapshot_identity_test.go b/internal/vaultik/restore_snapshot_identity_test.go new file mode 100644 index 0000000..5cdedd4 --- /dev/null +++ b/internal/vaultik/restore_snapshot_identity_test.go @@ -0,0 +1,251 @@ +package vaultik_test + +import ( + "context" + "io" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/config" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/snapshot" + "sneak.berlin/go/vaultik/internal/storage" + "sneak.berlin/go/vaultik/internal/ui" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// TestRestoreAndDeepVerifyRejectSwappedDatabase proves that swapping two +// snapshots' encrypted databases on the store is caught. age decryption +// alone proves only that a database is readable; without an identity check +// restore would happily write the wrong snapshot's files and deep verify +// would report success. After the swap, restore and deep verify of A both +// fail, and the error names the snapshot the database actually holds (B). +func TestRestoreAndDeepVerifyRejectSwappedDatabase(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + fs := afero.NewOsFs() + tempDir := t.TempDir() + storeDir := filepath.Join(tempDir, "remote") + + chunkSize := int64(64 * 1024) + + storer, err := storage.NewFileStorer(storeDir) + require.NoError(t, err) + + ctx := context.Background() + + // Two snapshots with different content, backed up into one shared + // store. Different names give them different remote keys, so their + // metadata directories are distinct and can be tampered with alone. + dataA := filepath.Join(tempDir, "srcA") + require.NoError(t, fs.MkdirAll(dataA, 0o755)) + require.NoError(t, afero.WriteFile(fs, filepath.Join(dataA, "a.bin"), + bytesPattern("alpha-", int(chunkSize*2)), 0o644)) + + dataB := filepath.Join(tempDir, "srcB") + require.NoError(t, fs.MkdirAll(dataB, 0o755)) + require.NoError(t, afero.WriteFile(fs, filepath.Join(dataB, "b.bin"), + bytesPattern("beta-", int(chunkSize*2)), 0o644)) + + idA := backupNamedSnapshotToStore(ctx, t, fs, dataA, storer, + filepath.Join(tempDir, "idxA.sqlite"), "alpha") + idB := backupNamedSnapshotToStore(ctx, t, fs, dataB, storer, + filepath.Join(tempDir, "idxB.sqlite"), "beta") + require.NotEqual(t, idA, idB) + + keyA := snapshot.RemoteSnapshotKey(idA) + keyB := snapshot.RemoteSnapshotKey(idB) + require.NotEqual(t, keyA, keyB) + + // Baseline: each snapshot verifies against its own intact metadata. + require.NoError(t, newStoreClient(ctx, t, fs, storer).RunDeepVerify( + idA, &vaultik.VerifyOptions{Deep: true})) + require.NoError(t, newStoreClient(ctx, t, fs, storer).RunDeepVerify( + idB, &vaultik.VerifyOptions{Deep: true})) + + // Swap the two snapshots' encrypted databases on the store. + swapStoreFiles(t, fs, + filepath.Join(storeDir, "metadata", keyA, "db.zst.age"), + filepath.Join(storeDir, "metadata", keyB, "db.zst.age")) + + // Restore of A now decrypts B's database; the identity check must + // reject it and name the snapshot it actually found. + restoreErr := newStoreClient(ctx, t, fs, storer).Restore(&vaultik.RestoreOptions{ + SnapshotID: idA, + TargetDir: filepath.Join(tempDir, "restoreA"), + }) + require.Error(t, restoreErr) + require.ErrorContains(t, restoreErr, idB) + + // Deep verify of A must reject the swapped database for the same reason. + verifyErr := newStoreClient(ctx, t, fs, storer).RunDeepVerify( + idA, &vaultik.VerifyOptions{Deep: true}) + require.Error(t, verifyErr) + require.ErrorContains(t, verifyErr, idB) +} + +// TestDeepVerifyRejectsSwappedDatabaseWithEmptyManifest covers the case the +// issue calls out: swapping in a database whose blob set is empty and +// pairing it with an equally empty manifest. The manifest then agrees with +// the database, so every blob-level check passes and deep verify used to +// report success with zero blobs verified. The identity check rejects it +// before any blob check runs. +func TestDeepVerifyRejectsSwappedDatabaseWithEmptyManifest(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + fs := afero.NewOsFs() + tempDir := t.TempDir() + storeDir := filepath.Join(tempDir, "remote") + + chunkSize := int64(64 * 1024) + + storer, err := storage.NewFileStorer(storeDir) + require.NoError(t, err) + + ctx := context.Background() + + // Snapshot A: real content, so its manifest lists blobs. + dataA := filepath.Join(tempDir, "srcA") + require.NoError(t, fs.MkdirAll(dataA, 0o755)) + require.NoError(t, afero.WriteFile(fs, filepath.Join(dataA, "a.bin"), + bytesPattern("alpha-", int(chunkSize*2)), 0o644)) + idA := backupNamedSnapshotToStore(ctx, t, fs, dataA, storer, + filepath.Join(tempDir, "idxA.sqlite"), "alpha") + + // Snapshot C: a single empty file, so it references no blobs and its + // manifest is empty. This is the database/manifest pair an attacker + // would swap in to make the blob checks vacuously pass. + dataC := filepath.Join(tempDir, "srcC") + require.NoError(t, fs.MkdirAll(dataC, 0o755)) + require.NoError(t, afero.WriteFile(fs, + filepath.Join(dataC, "empty.bin"), []byte{}, 0o644)) + idC := backupNamedSnapshotToStore(ctx, t, fs, dataC, storer, + filepath.Join(tempDir, "idxC.sqlite"), "charlie") + + keyA := snapshot.RemoteSnapshotKey(idA) + keyC := snapshot.RemoteSnapshotKey(idC) + + // Replace A's database and manifest with C's empty pair. + copyStoreFile(t, fs, + filepath.Join(storeDir, "metadata", keyC, "db.zst.age"), + filepath.Join(storeDir, "metadata", keyA, "db.zst.age")) + copyStoreFile(t, fs, + filepath.Join(storeDir, "metadata", keyC, "manifest.json.zst"), + filepath.Join(storeDir, "metadata", keyA, "manifest.json.zst")) + + verifyErr := newStoreClient(ctx, t, fs, storer).RunDeepVerify( + idA, &vaultik.VerifyOptions{Deep: true}) + require.Error(t, verifyErr) + require.ErrorContains(t, verifyErr, idC) +} + +// backupNamedSnapshotToStore backs up dataDir into the shared storer under +// the given snapshot name and returns the human snapshot ID. Two snapshots +// backed up under different names get different remote keys, so their +// metadata directories on the store are distinct. +func backupNamedSnapshotToStore( + ctx context.Context, t *testing.T, fs afero.Fs, + dataDir string, storer storage.Storer, dbPath, name string, +) string { + t.Helper() + + const ( + chunkSize = int64(64 * 1024) + maxBlobSize = int64(512 * 1024) + ) + + cfg := &config.Config{ + AgeRecipients: []string{testAgePublicKey}, + AgeSecretKey: testAgeSecretKey, + CompressionLevel: 3, + Hostname: testHostname, + } + + db, err := database.New(ctx, dbPath) + require.NoError(t, err) + + repos := database.NewRepositories(db) + + sm := snapshot.NewSnapshotManager(snapshot.SnapshotManagerParams{ + Repos: repos, + Storage: storer, + Config: cfg, + }) + sm.SetFilesystem(fs) + + scanner := snapshot.NewScanner(snapshot.ScannerConfig{ + FS: fs, + Storage: storer, + ChunkSize: chunkSize, + MaxBlobSize: maxBlobSize, + CompressionLevel: cfg.CompressionLevel, + AgeRecipients: cfg.AgeRecipients, + Repositories: repos, + }) + + snapshotID, err := sm.CreateSnapshotWithName( + ctx, cfg.Hostname, name, "test-version", "test-git") + require.NoError(t, err) + + _, err = scanner.Scan(ctx, dataDir, snapshotID) + require.NoError(t, err) + + require.NoError(t, sm.CompleteSnapshot(ctx, snapshotID)) + require.NoError(t, sm.ExportSnapshotMetadata(ctx, dbPath, snapshotID)) + require.NoError(t, db.Close()) + + return snapshotID +} + +// newStoreClient builds a Vaultik that reads only from the store: the +// secret key, the storer, and a filesystem, with no local index. This is +// what restore and deep verify need. +func newStoreClient( + ctx context.Context, t *testing.T, fs afero.Fs, storer storage.Storer, +) *vaultik.Vaultik { + t.Helper() + + v := &vaultik.Vaultik{ + Config: &config.Config{ + AgeSecretKey: testAgeSecretKey, + Hostname: testHostname, + }, + Storage: storer, + Fs: fs, + Stdout: io.Discard, + Stderr: io.Discard, + UI: ui.NewWithColor(io.Discard, false), + } + v.SetContext(ctx) + + return v +} + +// swapStoreFiles exchanges the contents of two files on the store. +func swapStoreFiles(t *testing.T, fs afero.Fs, a, b string) { + t.Helper() + + dataA, err := afero.ReadFile(fs, a) + require.NoError(t, err) + + dataB, err := afero.ReadFile(fs, b) + require.NoError(t, err) + + require.NoError(t, afero.WriteFile(fs, a, dataB, 0o644)) + require.NoError(t, afero.WriteFile(fs, b, dataA, 0o644)) +} + +// copyStoreFile overwrites dst with the contents of src on the store. +func copyStoreFile(t *testing.T, fs afero.Fs, src, dst string) { + t.Helper() + + data, err := afero.ReadFile(fs, src) + require.NoError(t, err) + + require.NoError(t, afero.WriteFile(fs, dst, data, 0o644)) +} diff --git a/internal/vaultik/verify.go b/internal/vaultik/verify.go index feea3df..e44f57f 100644 --- a/internal/vaultik/verify.go +++ b/internal/vaultik/verify.go @@ -186,24 +186,9 @@ func (v *Vaultik) loadVerificationData( v.stdoutf("Downloading and decrypting database...\n") } - // Download and decrypt database - dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey) - log.Info("Downloading encrypted database", "path", dbPath) - - dbReader, err := v.Storage.Get(v.ctx, dbPath) + tdb, err := v.downloadVerifiedSnapshotDB(snapshotID, remoteKey, opts, result, identity) if err != nil { - return nil, nil, nil, v.deepVerifyFailure(result, opts, - fmt.Sprintf("failed to download database: %v", err), - fmt.Errorf("failed to download database: %w", err)) - } - - defer func() { _ = dbReader.Close() }() - - tdb, err := v.decryptAndLoadDatabase(dbReader, identity) - if err != nil { - return nil, nil, nil, v.deepVerifyFailure(result, opts, - fmt.Sprintf("failed to decrypt database: %v", err), - fmt.Errorf("failed to decrypt database: %w", err)) + return nil, nil, nil, err } dbBlobs, err := v.getBlobsFromDatabase(tdb.db.Conn()) @@ -232,6 +217,45 @@ func (v *Vaultik) loadVerificationData( return manifest, tdb, dbBlobs, nil } +// downloadVerifiedSnapshotDB downloads and decrypts the snapshot metadata +// database and confirms it really is the snapshot named by remoteKey +// before any of its rows are trusted (see verifySnapshotDBIdentity). On +// any failure it records the failure in result and returns the error the +// caller should propagate; the temp database is closed on a rejected +// identity so nothing is left on disk. +func (v *Vaultik) downloadVerifiedSnapshotDB( + snapshotID, remoteKey string, opts *VerifyOptions, result *VerifyResult, + identity age.Identity, +) (*tempDB, error) { + dbPath := fmt.Sprintf("metadata/%s/db.zst.age", remoteKey) + log.Info("Downloading encrypted database", "path", dbPath) + + dbReader, err := v.Storage.Get(v.ctx, dbPath) + if err != nil { + return nil, v.deepVerifyFailure(result, opts, + fmt.Sprintf("failed to download database: %v", err), + fmt.Errorf("failed to download database: %w", err)) + } + + defer func() { _ = dbReader.Close() }() + + tdb, err := v.decryptAndLoadDatabase(dbReader, identity) + if err != nil { + return nil, v.deepVerifyFailure(result, opts, + fmt.Sprintf("failed to decrypt database: %v", err), + fmt.Errorf("failed to decrypt database: %w", err)) + } + + err = v.verifySnapshotDBIdentity(tdb.db, snapshotID, remoteKey) + if err != nil { + _ = tdb.Close() + + return nil, v.deepVerifyFailure(result, opts, err.Error(), err) + } + + return tdb, nil +} + // runVerificationSteps executes manifest verification, blob existence // check, and deep content verification. func (v *Vaultik) runVerificationSteps(