From 935c7b6a0873ec63501dc3a5cfbd711d24148a01 Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Mon, 5 Oct 2026 23:46:50 +0200 Subject: [PATCH] Phase 2: checksum audit (occ backup:verify) Repository::verify(): every blob a snapshot's files and database dump reference must be in a freshly re-downloaded, re-authenticated index (never trusted from the local cache, same reasoning as prune()) and have its pack still present on the location -- shallow, cheap, no blob content read, catches a deleted pack, a tampered/rolled-back index, or a database manifest that went missing (reported as a failure, not silently skipped). With $deep, each blob is also downloaded and decrypted, same as loadBlob() already does on every real read (AEAD authentication tag, content hash against its blob id), done here deliberately for everything in the snapshot rather than waiting for it to come up during an actual restore. A blob referenced by more than one file is only checked once; pack presence is cached per pack, not re-checked per blob. occ backup:verify [--deep], meant to run periodically (cron, a sample of recent snapshots), per PLAN.md's "Verificatie: periodiek checksums controleren, steekproef-restore". The lease covering the audit is refreshed roughly every half of its TTL while blobs are checked, rather than relying on a single long-lived lease like the other new phase-2 commands do. Verified against a real S3-backed 1.4 GiB / 603-blob snapshot (shallow: instant; --deep: ~28s, fully decrypted) in addition to the new unit tests and the roundtrip.sh addition (shallow and deep on a snapshot with real, varied file sizes, now covered on every CI matrix combination). Local review (Codex) on the first pass found three real issues, all fixed: a missing database manifest was silently skipped instead of reported; shallow verification trusted the (possibly stale or unauthenticated) local index cache instead of re-checking the authenticated index on the location; and the lease had no refresh, so a --deep audit of a very large snapshot could outrun its TTL and run unprotected against a concurrent prune. --- appinfo/info.xml | 1 + lib/Command/Verify.php | 71 +++++++++++++++++++++ lib/Repository/Repository.php | 77 ++++++++++++++++++++++ lib/Service/VerifyService.php | 39 ++++++++++++ tests/Unit/Repository/VerifyTest.php | 95 ++++++++++++++++++++++++++++ tests/integration/roundtrip.sh | 5 ++ 6 files changed, 288 insertions(+) create mode 100644 lib/Command/Verify.php create mode 100644 lib/Service/VerifyService.php create mode 100644 tests/Unit/Repository/VerifyTest.php diff --git a/appinfo/info.xml b/appinfo/info.xml index 99cc4e4..90b064f 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -51,6 +51,7 @@ relying on this for anything.]]> OCA\NgBackup\Command\RestoreUser OCA\NgBackup\Command\KeyImport OCA\NgBackup\Command\RestoreFull + OCA\NgBackup\Command\Verify OCA\NgBackup\Settings\Admin diff --git a/lib/Command/Verify.php b/lib/Command/Verify.php new file mode 100644 index 0000000..bae03fb --- /dev/null +++ b/lib/Command/Verify.php @@ -0,0 +1,71 @@ +setName('backup:verify') + ->setDescription('Checksum audit of a snapshot: every blob it needs must be in the (freshly re-checked) index and its pack present') + ->addArgument('target', InputArgument::REQUIRED, 'Location name or id') + ->addArgument('snapshot', InputArgument::REQUIRED, 'Snapshot id (see backup:list)') + ->addOption('deep', null, InputOption::VALUE_NONE, 'Also download and decrypt every blob (verifies its authentication tag and content hash, not just that it is listed); much slower') + ->setHelp(<<<'HELP' +Without --deep this re-downloads and re-authenticates the index (never trusting the local cache) +and confirms every blob the snapshot needs is listed and its pack is present -- fast, no blob +content is read, so it cannot by itself catch a truncated or bit-rotted pack. With --deep every +blob is also downloaded and decrypted, which fails loudly on exactly that even if the index still +lists it -- the same verification an actual restore would do, run here deliberately instead of +waiting to find out during one. + +Run this periodically (cron, picking a sample of recent snapshots) rather than only right before +you'd need to rely on a restore. + +Examples: + occ backup:verify offsite 537bf7e9c1a4... + occ backup:verify offsite 537bf7e9c1a4... --deep +HELP); + } + + protected function execute(InputInterface $input, OutputInterface $output): int { + try { + $target = $this->targets->get($input->getArgument('target')); + $result = $this->verify->verify($target, (string)$input->getArgument('snapshot'), (bool)$input->getOption('deep')); + } catch (\Throwable $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; + } + $output->writeln(sprintf('%d files, %d blobs checked%s.', $result['filesChecked'], $result['blobsChecked'], + $input->getOption('deep') ? sprintf(' (%.1f MiB decrypted)', $result['bytesChecked'] / 1048576) : '')); + if ($result['missing'] !== []) { + $output->writeln('' . count($result['missing']) . ' blob(s) missing from the index: ' . implode(', ', $result['missing'])); + } + if ($result['failed'] !== []) { + $output->writeln('' . count($result['failed']) . ' blob(s) failed verification (corrupted or tampered): ' . implode(', ', $result['failed'])); + } + if ($result['missing'] === [] && $result['failed'] === []) { + $output->writeln('OK'); + return 0; + } + return 1; + } +} diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index ad65baf..2d83758 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -662,6 +662,83 @@ final class Repository { return $this->streamEntries($snapshotId); } + /** + * Checksum audit of a snapshot: every blob it references (files and the database dump, when + * the snapshot has one) must be in the index, freshly re-downloaded and re-authenticated from + * the location rather than trusted from the local cache (same reasoning as prune()), and its + * pack must still be present. With $deep, each blob is also downloaded and decrypted, which + * verifies its AEAD authentication tag and its content hash against the blob id (loadBlob() + * already does both on every real read -- this just does it for everything in the snapshot, + * deliberately, rather than relying on it to come up during a later restore). A blob + * referenced by more than one file is only checked once; a missing database manifest is a + * failure, not something to silently skip over. + * + * @param ?callable $heartbeat called after every blob (shallow) or every blob read (deep); + * callers use it to keep a time-boxed lease alive during a large audit + * @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list, failed:list} + */ + public function verify(string $snapshotId, bool $deep = false, ?callable $heartbeat = null): array { + $heartbeat ??= static function (): void { + }; + $this->index->load(true); + $stats = ['filesChecked' => 0, 'blobsChecked' => 0, 'bytesChecked' => 0, 'missing' => [], 'failed' => []]; + $seen = []; + $packSeen = []; // pack id => exists on the backend; avoids one exists() call per blob + foreach ($this->streamEntries($snapshotId) as $entry) { + $stats['filesChecked']++; + foreach ($entry['b'] as $id) { + if (isset($seen[$id])) { + continue; + } + $seen[$id] = true; + $this->verifyBlob($id, $deep, $stats, $packSeen); + $heartbeat(); + } + } + $meta = $this->snapshotMeta($snapshotId); + if (isset($meta['db']) && is_string($meta['db'])) { + if (!$this->backend->exists($meta['db'])) { + $stats['missing'][] = $meta['db']; + } else { + $dbManifest = json_decode($this->getObject($meta['db']), true, 512, JSON_THROW_ON_ERROR); + foreach ($dbManifest['tables'] as $info) { + foreach ($info['blobs'] as $id) { + if (isset($seen[$id])) { + continue; + } + $seen[$id] = true; + $this->verifyBlob($id, $deep, $stats, $packSeen); + $heartbeat(); + } + } + } + } + return $stats; + } + + /** @param array $packSeen */ + private function verifyBlob(string $id, bool $deep, array &$stats, array &$packSeen): void { + if (!$this->index->has($id)) { + $stats['missing'][] = $id; + return; + } + [$pack] = $this->index->get($id); + $packExists = $packSeen[$pack] ??= $this->backend->exists('packs/' . substr($pack, 0, 2) . '/' . $pack); + if (!$packExists) { + $stats['missing'][] = $id; + return; + } + $stats['blobsChecked']++; + if (!$deep) { + return; + } + try { + $stats['bytesChecked'] += strlen($this->loadBlob($id)); + } catch (\Throwable) { + $stats['failed'][] = $id; + } + } + /** The meta record (first line) of a snapshot. */ public function snapshotMeta(string $snapshotId): array { $plain = fopen('php://temp/maxmemory:' . (4 * 1048576), 'w+b'); diff --git a/lib/Service/VerifyService.php b/lib/Service/VerifyService.php new file mode 100644 index 0000000..ef3d893 --- /dev/null +++ b/lib/Service/VerifyService.php @@ -0,0 +1,39 @@ +, failed:list} */ + public function verify(Target $target, string $snapshotId, bool $deep = false): array { + $ttl = $deep ? 600 : 300; + try { + return $this->leases->with(PruneService::lockKey($target), LeaseService::SHARED, $ttl, function (callable $refresh) use ($target, $snapshotId, $deep, $ttl) { + $last = time(); + $heartbeat = function () use ($refresh, $ttl, &$last): void { + if (time() - $last >= intdiv($ttl, 2)) { + $refresh(); + $last = time(); + } + }; + return $this->targets->repository($target)->verify($snapshotId, $deep, $heartbeat); + }); + } catch (LockedException) { + throw new \RuntimeException('A cleanup of ' . $target->getName() . ' is in progress; try again later'); + } + } +} diff --git a/tests/Unit/Repository/VerifyTest.php b/tests/Unit/Repository/VerifyTest.php new file mode 100644 index 0000000..7b9ed77 --- /dev/null +++ b/tests/Unit/Repository/VerifyTest.php @@ -0,0 +1,95 @@ +root = $this->tempDir(); + $this->backend = new LocalBackend($this->root); + $this->keys = KeyRing::generate(); + $this->src = $this->tempDir(); + file_put_contents("$this->src/a", random_bytes(100000)); + file_put_contents("$this->src/b", random_bytes(100000)); + } + + private function open(): Repository { + return Repository::openWithKey($this->backend, $this->keys, null, null); + } + + public function testShallowVerifyPassesOnAHealthyRepository(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); + $s = $repo->backupDirectory($this->src); + + $result = $this->open()->verify($s['snapshot']); + + $this->assertSame(2, $result['filesChecked']); + $this->assertGreaterThanOrEqual(1, $result['blobsChecked']); + $this->assertSame(0, $result['bytesChecked']); // not downloaded without --deep + $this->assertSame([], $result['missing']); + $this->assertSame([], $result['failed']); + } + + public function testDeepVerifyDownloadsAndCountsBytes(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); + $s = $repo->backupDirectory($this->src); + + $result = $this->open()->verify($s['snapshot'], deep: true); + + $this->assertSame(200000, $result['bytesChecked']); + $this->assertSame([], $result['missing']); + $this->assertSame([], $result['failed']); + } + + public function testShallowVerifyDetectsAMissingPack(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); + $s = $repo->backupDirectory($this->src); + foreach ($this->backend->list('packs') as $pack) { + $this->backend->delete($pack); + } + + $result = $this->open()->verify($s['snapshot']); + + $this->assertNotSame([], $result['missing']); + } + + public function testVerifyFailsOnAMissingDatabaseManifest(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); + $repo->writeSnapshot('snap-no-db', ['time' => gmdate('c'), 'db' => 'db/does-not-exist'], []); + + $result = $this->open()->verify('snap-no-db'); + + $this->assertSame(['db/does-not-exist'], $result['missing']); + } + + public function testDeepVerifyDetectsATamperedPack(): void { + $repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw')); + $s = $repo->backupDirectory($this->src); + $packs = $this->backend->list('packs'); + $this->assertNotEmpty($packs); + $path = $this->root . '/' . $packs[0]; + $bytes = file_get_contents($path); + // Flip a byte well inside the file, away from any header, so decryption's own + // authentication tag catches it (this must fail loudly, not just "do nothing"). + $bytes[(int)(strlen($bytes) / 2)] = chr(~ord($bytes[(int)(strlen($bytes) / 2)]) & 0xFF); + file_put_contents($path, $bytes); + + $result = $this->open()->verify($s['snapshot'], deep: true); + + $this->assertNotSame([], $result['failed']); + } +} diff --git a/tests/integration/roundtrip.sh b/tests/integration/roundtrip.sh index b94ce2a..3b76b80 100755 --- a/tests/integration/roundtrip.sh +++ b/tests/integration/roundtrip.sh @@ -49,6 +49,11 @@ SUM_AFTER=$($X sh -c "cd $DATA/$U/files && find . -type f -print0 | sort -z | xa $X sh -c "ls $DATA/$U/files_trashbin/files/ | grep -q '^extra.txt'" || fail "extra file not in trash" ok "restore new-folder and replace: files identical, extra file in the trash" +# Checksum audit: shallow (index presence) and deep (download, decrypt, verify every blob). +occ backup:verify ci "$SNAP" | grep -q "^OK$" || fail "shallow verify" +occ backup:verify ci "$SNAP" --deep | grep -q "^OK$" || fail "deep verify" +ok "shallow and deep checksum audit" + # Database dump/restore round trip (all tables, checksums, sequences on PostgreSQL). $X php -d memory_limit=512M "$APPS/ng_backup/spikes/03-db-dump.php" /tmp/ngb-rt-db | grep "ALL PASS" >/dev/null || fail "database round trip" ok "database dump and restore"