Ensembia/nextcloud-ng_backup
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 17 Wiki Activity

Verify: report read errors as unreadable, not as corrupted

Browse Source 
verifyBlob() caught every exception from loading a blob and counted it as
a failed (corrupted or tampered) blob, so a transient network error during
a deep verify over WebDAV looked like data corruption. Read the range
separately, retry it, and report a read that keeps failing as unreadable
with its error; only a blob that was read but does not decrypt or match
its id counts as failed.
This commit is contained in:
Dillard Blom
2026-10-08 00:53:31 +02:00
parent 695d972785
commit fe7ad9b4b5
5 changed files with 111 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+5
View File
@@ -14,6 +14,11 @@ instead carried by the app description, the git tag, the release, and this file.
when the login was more than 30 minutes old: the form did not ask for password confirmation.
- Settings page: the "Backup and restore" / "Settings" tabs line up with the sections below them.
- Backups: the runs table scrolls horizontally on narrow screens instead of running off the page.
- `occ backup:verify --deep`: a blob that could not be read (network or storage error) was reported
as "corrupted or tampered". Reads are now retried, and a read that keeps failing is reported
separately as unreadable, with the error; only a blob that was read but fails decryption or its
hash check counts as corrupted. Found on a Hetzner Storage Box over WebDAV, where the blob
turned out to be intact.
## [0.9.1-beta.2] - 2026-10-07
lib/Command/Verify.php
+6 -1
View File
@@ -66,7 +66,12 @@ HELP);
if ($result['failed'] !== []) {
$output->writeln('<error>' . count($result['failed']) . ' blob(s) failed verification (corrupted or tampered):</error> ' . implode(', ', $result['failed']));
}
if ($result['missing'] === [] && $result['failed'] === []) {
if ($result['unreadable'] !== []) {
$output->writeln('<error>' . count($result['unreadable']) . ' blob(s) could not be read from the location</error> (storage or network error, '
. 'not a sign of corruption; run the check again): ' . implode(', ', $result['unreadable']));
$output->writeln('Last read error: ' . $result['readError']);
}
if ($result['missing'] === [] && $result['failed'] === [] && $result['unreadable'] === []) {
$output->writeln('<info>OK</info>');
return 0;
}
lib/Repository/Repository.php
+31 -7
View File
@@ -28,6 +28,8 @@ final class Repository {
public const FORMAT = 1;
public const BLOB_SIZE = 4 * 1048576;
private const FLAG_DEFLATE = 1;
/** Reads per blob in a deep verify before it is reported as unreadable (not corrupted). */
private const VERIFY_READ_ATTEMPTS = 3;
private StreamCipher $cipher;
private BlobIndex $index;
@@ -696,8 +698,13 @@ final class Repository {
}
private function loadBlob(string $id): string {
[$pack, $offset, $length, $raw, $flags] = $this->index->get($id);
$encrypted = $this->backend->getRange('packs/' . substr($pack, 0, 2) . '/' . $pack, $offset, $length);
[$pack, $offset, $length] = $this->index->get($id);
return $this->decodeBlob($id, $this->backend->getRange('packs/' . substr($pack, 0, 2) . '/' . $pack, $offset, $length));
}
/** Decrypts and checks a blob read from its pack; throws when it is corrupted or tampered with. */
private function decodeBlob(string $id, string $encrypted): string {
[, , , $raw, $flags] = $this->index->get($id);
$payload = $this->cipher->decryptString($encrypted, 'blob:' . $id);
$data = ($flags & self::FLAG_DEFLATE) ? gzinflate($payload) : $payload;
if ($data === false || strlen($data) !== $raw || $this->keys->blobId($data) !== $id) {
@@ -733,13 +740,13 @@ final class Repository {
*
* @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<string>, failed:list<string>}
* @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>, unreadable:list<string>, readError:string}
*/
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' => []];
$stats = ['filesChecked' => 0, 'blobsChecked' => 0, 'bytesChecked' => 0, 'missing' => [], 'failed' => [], 'unreadable' => [], 'readError' => ''];
$seen = [];
$packSeen = []; // pack id => exists on the backend; avoids one exists() call per blob
foreach ($this->streamEntries($snapshotId) as $entry) {
@@ -778,7 +785,7 @@ final class Repository {
* Same checks as verify(), for a user export: every blob its manifest references must be in
* the freshly re-authenticated index with its pack present; $deep also decrypts each.
*
* @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>}
* @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>, unreadable:list<string>, readError:string}
*/
public function verifyUserExport(string $manifestPath, bool $deep = false, ?callable $heartbeat = null): array {
if (!$this->isRecordedUserExport($manifestPath)) {
@@ -787,7 +794,7 @@ final class Repository {
$heartbeat ??= static function (): void {
};
$this->index->load(true);
$stats = ['filesChecked' => 0, 'blobsChecked' => 0, 'bytesChecked' => 0, 'missing' => [], 'failed' => []];
$stats = ['filesChecked' => 0, 'blobsChecked' => 0, 'bytesChecked' => 0, 'missing' => [], 'failed' => [], 'unreadable' => [], 'readError' => ''];
$seen = [];
$packSeen = [];
$manifest = json_decode($this->getObject($manifestPath), true, 512, JSON_THROW_ON_ERROR);
@@ -824,8 +831,25 @@ final class Repository {
if (!$deep) {
return;
}
// Reading and checking are kept apart: a read that fails (network, storage) says
// nothing about the stored data, so it is retried and reported as unreadable,
// never as corrupted. Only a blob that was read but does not decrypt or match
// its id counts as failed.
[, $offset, $length] = $this->index->get($id);
$encrypted = null;
for ($attempt = 1; $attempt <= self::VERIFY_READ_ATTEMPTS && $encrypted === null; $attempt++) {
try {
$encrypted = $this->backend->getRange('packs/' . substr($pack, 0, 2) . '/' . $pack, $offset, $length);
} catch (\Throwable $e) {
$stats['readError'] = $e->getMessage();
}
}
if ($encrypted === null) {
$stats['unreadable'][] = $id;
return;
}
try {
$stats['bytesChecked'] += strlen($this->loadBlob($id));
$stats['bytesChecked'] += strlen($this->decodeBlob($id, $encrypted));
} catch (\Throwable) {
$stats['failed'][] = $id;
}
lib/Service/VerifyService.php
+2 -2
View File
@@ -18,12 +18,12 @@ final class VerifyService {
) {
}
/** @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>} */
/** @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>, unreadable:list<string>, readError:string} */
public function verify(Target $target, string $snapshotId, bool $deep = false): array {
return $this->run($target, $deep, fn (\OCA\NgBackup\Repository\Repository $repo, callable $heartbeat): array => $repo->verify($snapshotId, $deep, $heartbeat));
}
/** @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>} */
/** @return array{filesChecked:int, blobsChecked:int, bytesChecked:int, missing:list<string>, failed:list<string>, unreadable:list<string>, readError:string} */
public function verifyUserExport(Target $target, string $manifestPath, bool $deep = false): array {
return $this->run($target, $deep, fn (\OCA\NgBackup\Repository\Repository $repo, callable $heartbeat): array => $repo->verifyUserExport($manifestPath, $deep, $heartbeat));
}
tests/Unit/Repository/VerifyTest.php
+67
View File
@@ -4,6 +4,8 @@ declare(strict_types=1);
namespace OCA\NgBackup\Tests\Unit\Repository;
use OCA\NgBackup\Backend\BackendException;
use OCA\NgBackup\Backend\IBackend;
use OCA\NgBackup\Backend\LocalBackend;
use OCA\NgBackup\Crypto\KeyRing;
use OCA\NgBackup\Repository\Repository;
@@ -92,4 +94,69 @@ class VerifyTest extends TestCase {
$this->assertNotSame([], $result['failed']);
}
public function testDeepVerifyRetriesAReadThatFailsOnce(): void {
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'));
$s = $repo->backupDirectory($this->src);
$flaky = new FlakyRangeBackend($this->backend, failures: 1);
$result = Repository::openWithKey($flaky, $this->keys, null, null)->verify($s['snapshot'], deep: true);
$this->assertSame(200000, $result['bytesChecked']);
$this->assertSame([], $result['failed']);
$this->assertSame([], $result['unreadable']);
}
public function testDeepVerifyReportsAPersistentReadErrorAsUnreadableNotCorrupted(): void {
$repo = Repository::initWithKey($this->backend, $this->keys, $this->keys->wrap('pw'));
$s = $repo->backupDirectory($this->src);
$broken = new FlakyRangeBackend($this->backend, failures: PHP_INT_MAX);
$result = Repository::openWithKey($broken, $this->keys, null, null)->verify($s['snapshot'], deep: true);
$this->assertSame([], $result['failed']);
$this->assertNotSame([], $result['unreadable']);
$this->assertSame('connection reset', $result['readError']);
}
}
/** Delegates to a real backend, but getRange() throws for the first $failures calls. */
final class FlakyRangeBackend implements IBackend {
public function __construct(
private IBackend $inner,
private int $failures,
) {
}
public function put(string $path, $stream): void {
$this->inner->put($path, $stream);
}
public function get(string $path) {
return $this->inner->get($path);
}
public function getRange(string $path, int $offset, int $length): string {
if ($this->failures > 0) {
$this->failures--;
throw new BackendException('connection reset');
}
return $this->inner->getRange($path, $offset, $length);
}
public function exists(string $path): bool {
return $this->inner->exists($path);
}
public function move(string $from, string $to): void {
$this->inner->move($from, $to);
}
public function list(string $prefix): array {
return $this->inner->list($prefix);
}
public function delete(string $path): void {
$this->inner->delete($path);
}
}