diff --git a/appinfo/info.xml b/appinfo/info.xml index 9e0c632..8e04c72 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -5,7 +5,7 @@ NG Backup Encrypted, incremental backups of your Nextcloud with restore from the web interface - 0.0.11 + 0.0.12 agpl Dillard Blom NgBackup diff --git a/lib/BackgroundJob/BackupJob.php b/lib/BackgroundJob/BackupJob.php index 95c16cd..8e33b0f 100644 --- a/lib/BackgroundJob/BackupJob.php +++ b/lib/BackgroundJob/BackupJob.php @@ -47,12 +47,26 @@ class BackupJob extends TimedJob { $run = $this->backups->step($run, min($deadline, microtime(true) + $budget)); } } + $this->failStaleRestores(); $this->weeklyPrune(); foreach ($this->targets->list() as $t) { $this->alerts->checkStale($t); } } + /** A restore run that has not moved for 6 hours is considered dead (it would block cleanup forever). */ + private function failStaleRestores(): void { + foreach ($this->runs->findRunning(null, 'restore') as $run) { + if (time() - $run->getUpdatedAt() > 6 * 3600) { + $run->setStatus(\OCA\NgBackup\Db\Run::FAILED); + $run->setError('No progress for 6 hours; marked as failed'); + $run->setFinishedAt(time()); + $this->runs->update($run); + $this->alerts->runFailed($run, '#' . $run->getTargetId()); + } + } + } + /** Once a week (after Sunday's backup), apply the retention policy to non-append-only locations. */ private function weeklyPrune(): void { $lastWeek = $this->appConfig->getValueString(Application::APP_ID, 'last_prune_week', ''); diff --git a/lib/BackgroundJob/RestoreJob.php b/lib/BackgroundJob/RestoreJob.php index 55340ba..4ede668 100644 --- a/lib/BackgroundJob/RestoreJob.php +++ b/lib/BackgroundJob/RestoreJob.php @@ -29,16 +29,22 @@ class RestoreJob extends QueuedJob { } protected function run($argument): void { - $run = $this->runs->find((int)$argument['run']); + $runId = (int)($argument['run'] ?? 0); + try { + $run = $this->runs->find($runId); + } catch (\Throwable $e) { + $this->logger->error('NG Backup restore job: run ' . $runId . ' not found', ['exception' => $e]); + return; + } if ($run->getStatus() !== Run::RUNNING) { return; } - $req = json_decode($run->getState(), true, 512, JSON_THROW_ON_ERROR); - $run->setPhase('restoring'); - $run->setUpdatedAt(time()); - $this->runs->update($run); - @set_time_limit(0); try { + $req = json_decode($run->getState(), true, 512, JSON_THROW_ON_ERROR); + $run->setPhase('restoring'); + $run->setUpdatedAt(time()); + $run = $this->runs->update($run); + @set_time_limit(0); $result = $this->restore->restoreUserFiles($this->targets->get((string)$run->getTargetId()), $req['snapshot'], $req['path'], $req['mode']); $run->setStatus(Run::DONE); $run->setPhase('done'); @@ -47,7 +53,10 @@ class RestoreJob extends QueuedJob { $this->logger->error('NG Backup restore ' . $run->getId() . ' failed: ' . $e->getMessage(), ['exception' => $e]); $run->setStatus(Run::FAILED); $run->setError($e->getMessage()); - $this->alerts->runFailed($run, '#' . $run->getTargetId()); + try { + $this->alerts->runFailed($run, '#' . $run->getTargetId()); + } catch (\Throwable) { + } } $run->setFinishedAt(time()); $run->setUpdatedAt(time()); diff --git a/lib/Command/KeyConfirm.php b/lib/Command/KeyConfirm.php index 31ed327..5488495 100644 --- a/lib/Command/KeyConfirm.php +++ b/lib/Command/KeyConfirm.php @@ -40,6 +40,11 @@ class KeyConfirm extends Command { $output->writeln(''); $output->writeln(wordwrap(self::STATEMENT, 78)); $output->writeln(''); + $code = (string)$this->getHelper('question')->ask($input, $output, new Question('Confirmation code from the recovery kit file ("confirmation_code"): ')); + if (!$this->keys->checkKitCode($code)) { + $output->writeln('That code does not match the current recovery kit (download it with occ backup:key:kit first).'); + return 1; + } $answer = (string)$this->getHelper('question')->ask($input, $output, new Question('Type "' . self::CONFIRMATION . '" to proceed: ')); if (trim($answer) !== self::CONFIRMATION) { $output->writeln('Not confirmed.'); diff --git a/lib/Controller/AdminApiController.php b/lib/Controller/AdminApiController.php index 30c9deb..ee7abc6 100644 --- a/lib/Controller/AdminApiController.php +++ b/lib/Controller/AdminApiController.php @@ -97,11 +97,15 @@ class AdminApiController extends Controller { return new DataDownloadResponse($json, 'ng_backup-recovery-kit-' . $this->keys->fingerprint() . '.json', 'application/json'); } - /** The admin must tick the statement and send the exact confirmation phrase. */ - public function confirmKit(bool $accepted, string $phrase): JSONResponse { + /** The admin must tick the statement, send the exact phrase and the code from the kit file. */ + #[PasswordConfirmationRequired] + public function confirmKit(bool $accepted, string $phrase, string $code = ''): JSONResponse { if (!$accepted || $phrase !== KeyConfirm::CONFIRMATION) { return new JSONResponse(['error' => 'Not confirmed'], Http::STATUS_BAD_REQUEST); } + if (!$this->keys->checkKitCode($code)) { + return new JSONResponse(['error' => 'The confirmation code does not match the current recovery kit'], Http::STATUS_BAD_REQUEST); + } $this->keys->confirmRecoveryKit($this->session->getUser()?->getUID() ?? 'unknown'); return new JSONResponse(['confirmation' => $this->keys->recoveryKitConfirmation()]); } @@ -227,7 +231,7 @@ class AdminApiController extends Controller { #[PasswordConfirmationRequired] public function startRestore(int $targetId, string $snapshotId, string $path, string $mode = RestoreService::MODE_NEW_FOLDER): JSONResponse { if (!in_array($mode, [RestoreService::MODE_NEW_FOLDER, RestoreService::MODE_MERGE, RestoreService::MODE_REPLACE], true) - || !preg_match('#^data/[^/]+/files(/.*)?$#', $path)) { + || !\OCA\NgBackup\Repository\Repository::isSafePath($path) || !preg_match('#^data/[^/]+/files(/.*)?$#', $path)) { return new JSONResponse(['error' => 'Invalid path or mode'], Http::STATUS_BAD_REQUEST); } $run = new Run(); diff --git a/lib/Repository/BlobIndex.php b/lib/Repository/BlobIndex.php index 64d8a14..ccaa4c0 100644 --- a/lib/Repository/BlobIndex.php +++ b/lib/Repository/BlobIndex.php @@ -45,8 +45,9 @@ final class BlobIndex { * and only the index files that are not cached yet are downloaded. Cached packs that no * longer exist on the backend are dropped from the cache. */ - public function load(): void { - $cached = $this->cache?->all($this->repositoryId) ?? []; + public function load(bool $verify = false): void { + $this->blobs = []; + $cached = $verify ? [] : ($this->cache?->all($this->repositoryId) ?? []); $present = []; $this->downloaded = 0; foreach ($this->backend->list('index') as $path) { @@ -65,6 +66,20 @@ final class BlobIndex { $this->blobs[$e['id']] = [$packId, $e['offset'], $e['length'], $e['raw'], $e['flags']]; } } + if ($verify && $this->cache !== null) { + // Refresh the cache from what was just verified. + $this->cache->remove($this->repositoryId, array_keys($this->cache->all($this->repositoryId))); + foreach ($this->packs() as $packId => $_) { + $entries = []; + foreach ($this->blobs as $id => [$pack, $offset, $length, $raw, $flags]) { + if ($pack === $packId) { + $entries[] = ['id' => $id, 'offset' => $offset, 'length' => $length, 'raw' => $raw, 'flags' => $flags]; + } + } + $this->cache->put($this->repositoryId, $packId, $entries); + } + return; + } $stale = array_keys(array_diff_key($cached, $present)); if ($stale !== []) { $this->cache?->remove($this->repositoryId, $stale); diff --git a/lib/Repository/Repository.php b/lib/Repository/Repository.php index 1b28877..0f0a716 100644 --- a/lib/Repository/Repository.php +++ b/lib/Repository/Repository.php @@ -171,6 +171,12 @@ final class Repository { if (!is_dir(dirname($dest))) { mkdir(dirname($dest), 0700, true); } + // Defence in depth: the resolved directory must stay below the target (no symlink escapes). + $base = realpath($target); + $dir = realpath(dirname($dest)); + if ($base === false || $dir === false || ($dir !== $base && !str_starts_with($dir, $base . '/')) || is_link($dest)) { + throw new RepositoryException('Refusing to write outside the restore directory: ' . $entry['p']); + } // Write next to the destination and rename after all blobs verified: at most one file // is ever present twice. $tmp = $dest . '.ngb-restore'; @@ -268,6 +274,19 @@ final class Repository { } } + /** Relative path of non-empty segments, without ".", "..", NUL, backslash or control characters. */ + public static function isSafePath(string $path): bool { + if ($path === '' || str_starts_with($path, '/') || preg_match('/[\x00-\x1f\\\\]/', $path)) { + return false; + } + foreach (explode('/', $path) as $segment) { + if ($segment === '' || $segment === '.' || $segment === '..') { + return false; + } + } + return true; + } + /** Small encrypted objects (manifests). */ public function putObject(string $path, string $data): void { self::putString($this->backend, $path, $this->cipher->encryptString($data, 'object:' . $path)); @@ -352,6 +371,8 @@ final class Repository { * @return array{packs:int, deleted:int, repacked:int, freedBytes:int, keptBlobs:int, unusedBlobs:int} */ public function prune(bool $dryRun = false, float $repackThreshold = 0.5, array $ignoreSnapshots = []): array { + // Deleting is decided on the authenticated index files on the location, never on the local cache. + $this->index->load(true); $used = []; $ignore = array_flip($ignoreSnapshots); // e.g. snapshots a dry run would forget first foreach ($this->listSnapshots() as $sid) { @@ -546,7 +567,12 @@ final class Repository { rewind($plain); fgets($plain); // meta line while (($line = fgets($plain)) !== false) { - yield json_decode($line, true, 512, JSON_THROW_ON_ERROR); + $entry = json_decode($line, true, 512, JSON_THROW_ON_ERROR); + if (!is_array($entry) || !is_string($entry['p'] ?? null) || !self::isSafePath($entry['p']) + || !is_int($entry['s'] ?? null) || !is_array($entry['b'] ?? null)) { + throw new RepositoryException('Invalid entry in snapshot ' . $snapshotId); + } + yield $entry; } fclose($plain); } diff --git a/lib/Service/BackupService.php b/lib/Service/BackupService.php index 0190d70..a118fae 100644 --- a/lib/Service/BackupService.php +++ b/lib/Service/BackupService.php @@ -52,9 +52,22 @@ class BackupService { if ($this->config->getSystemValue('objectstore', null) !== null) { throw new \RuntimeException('Object storage as primary storage is not supported yet'); } - if ($this->locking->isLocked(PruneService::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE)) { - throw new \RuntimeException('A cleanup of ' . $target->getName() . ' is running; try again later'); + // Check and create the run under the location's exclusive lock, so a cleanup cannot start + // in between (it takes the same lock and refuses while a run exists). + $lock = PruneService::lockKey($target); + try { + $this->locking->acquireLock($lock, ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException) { + throw new \RuntimeException('A cleanup or another start for ' . $target->getName() . ' is running; try again later'); } + try { + return $this->createRun($target, $label); + } finally { + $this->locking->releaseLock($lock, ILockingProvider::LOCK_EXCLUSIVE); + } + } + + private function createRun(Target $target, string $label): Run { $running = $this->runs->findRunning($target->getId(), self::KIND_FULL); if ($running !== []) { return $running[0]; @@ -96,6 +109,14 @@ class BackupService { } catch (LockedException) { return $run; } + // Shared lock on the location while this step writes: a cleanup (exclusive) cannot run now. + $targetLock = 'ng_backup/target/' . $run->getTargetId(); + try { + $this->locking->acquireLock($targetLock, ILockingProvider::LOCK_SHARED); + } catch (LockedException) { + $this->locking->releaseLock($lockKey, ILockingProvider::LOCK_EXCLUSIVE); + return $run; + } try { $run = $this->runs->find($run->getId()); // fresh state after acquiring the lock if ($run->getStatus() !== Run::RUNNING || $run->getKind() !== self::KIND_FULL) { @@ -140,6 +161,7 @@ class BackupService { } return $run; } finally { + $this->locking->releaseLock($targetLock, ILockingProvider::LOCK_SHARED); $this->locking->releaseLock($lockKey, ILockingProvider::LOCK_EXCLUSIVE); } } diff --git a/lib/Service/KeyService.php b/lib/Service/KeyService.php index 5695c96..67c4faf 100644 --- a/lib/Service/KeyService.php +++ b/lib/Service/KeyService.php @@ -29,6 +29,7 @@ class KeyService { private const KEY_FINGERPRINT = 'master_key_fingerprint'; private const KEY_DELETE_DELAY = 'delete_delay_days'; private const KEY_KIT_VERSION = 'recovery_kit_version'; + private const KEY_KIT_CODE = 'recovery_kit_code'; public const DEFAULT_DELETE_DELAY = 7; private ?KeyRing $cached = null; @@ -188,6 +189,7 @@ class KeyService { 'app' => 'NG Backup', 'fingerprint' => $this->fingerprint(), 'kit_version' => $this->kitVersion(), + 'confirmation_code' => $this->kitCode(), 'created' => gmdate('c'), 'wrapped_key' => $this->wrappedKey(), 'slots' => $this->slots(), @@ -197,6 +199,36 @@ class KeyService { ]; } + /** + * Short code printed in the current kit file. Typing it back when confirming proves the + * admin actually has the downloaded kit. A new code per kit version; the server only keeps + * its own copy encrypted with the instance secret. + */ + private function kitCode(): string { + $stored = json_decode($this->appConfig->getValueString(Application::APP_ID, self::KEY_KIT_CODE, '{}', true), true) ?: []; + if (($stored['version'] ?? 0) === $this->kitVersion() && isset($stored['code'])) { + return $this->crypto->decrypt($stored['code']); + } + $alphabet = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789'; // no 0/O, 1/I + $code = ''; + for ($i = 0; $i < 8; $i++) { + $code .= $alphabet[random_int(0, strlen($alphabet) - 1)]; + } + $code = substr($code, 0, 4) . '-' . substr($code, 4); + $this->appConfig->setValueString(Application::APP_ID, self::KEY_KIT_CODE, + json_encode(['version' => $this->kitVersion(), 'code' => $this->crypto->encrypt($code)], JSON_THROW_ON_ERROR), true, true); + return $code; + } + + public function checkKitCode(string $code): bool { + $stored = json_decode($this->appConfig->getValueString(Application::APP_ID, self::KEY_KIT_CODE, '{}', true), true) ?: []; + if (($stored['version'] ?? 0) !== $this->kitVersion() || !isset($stored['code'])) { + return false; // no kit downloaded for this version yet + } + $normalise = static fn (string $c) => strtoupper(preg_replace('/[^A-Za-z0-9]/', '', $c) ?? ''); + return hash_equals($normalise($this->crypto->decrypt($stored['code'])), $normalise($code)); + } + public function confirmRecoveryKit(string $uid): void { $this->appConfig->setValueString(Application::APP_ID, self::KEY_CONFIRMED, json_encode(['uid' => $uid, 'time' => time(), 'fingerprint' => $this->fingerprint(), 'kit_version' => $this->kitVersion()], JSON_THROW_ON_ERROR), true); diff --git a/lib/Service/PruneService.php b/lib/Service/PruneService.php index f1a0998..7244e9c 100644 --- a/lib/Service/PruneService.php +++ b/lib/Service/PruneService.php @@ -33,6 +33,31 @@ class PruneService { } public function untrash(Target $target, string $snapshotId): void { + $this->lockExclusive($target); + try { + $this->doUntrash($target, $snapshotId); + } finally { + $this->locking->releaseLock(self::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE); + } + } + + /** + * Exclusive lock on the location, then make sure no run of any kind (backup, restore, export) + * is in progress: their data may not be referenced by a snapshot yet, or is being read. + */ + private function lockExclusive(Target $target): void { + try { + $this->locking->acquireLock(self::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE); + } catch (LockedException) { + throw new \RuntimeException('A backup, restore or cleanup of ' . $target->getName() . ' is in progress; try again later'); + } + if ($this->runs->findRunning($target->getId()) !== []) { + $this->locking->releaseLock(self::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE); + throw new \RuntimeException('A backup or restore of ' . $target->getName() . ' is running; try again later'); + } + } + + private function doUntrash(Target $target, string $snapshotId): void { $repo = $this->targets->repository($target); if (!array_key_exists($snapshotId, $repo->trash())) { throw new \InvalidArgumentException('That snapshot is not in the trash (or was already deleted)'); @@ -84,14 +109,7 @@ class PruneService { if ($target->getAppendOnly()) { throw new \RuntimeException('Location ' . $target->getName() . ' is append-only: NG Backup does not delete anything there'); } - if ($this->runs->findRunning($target->getId(), BackupService::KIND_FULL) !== []) { - throw new \RuntimeException('A backup to ' . $target->getName() . ' is running; try again later'); - } - try { - $this->locking->acquireLock(self::lockKey($target), ILockingProvider::LOCK_EXCLUSIVE); - } catch (LockedException) { - throw new \RuntimeException('Another backup or cleanup of ' . $target->getName() . ' is running'); - } + $this->lockExclusive($target); try { $repo = $this->targets->repository($target); $times = []; diff --git a/lib/Service/RestoreService.php b/lib/Service/RestoreService.php index 9c44f92..bddb514 100644 --- a/lib/Service/RestoreService.php +++ b/lib/Service/RestoreService.php @@ -36,9 +36,25 @@ class RestoreService { public function __construct( private TargetService $targets, private IRootFolder $root, + private \OCP\Lock\ILockingProvider $locking, ) { } + /** Shared lock on the location while restoring: a cleanup (exclusive) must not delete what is being read. */ + private function withSharedLock(Target $target, callable $fn): mixed { + $key = PruneService::lockKey($target); + try { + $this->locking->acquireLock($key, \OCP\Lock\ILockingProvider::LOCK_SHARED); + } catch (\OCP\Lock\LockedException) { + throw new \RuntimeException('A cleanup of ' . $target->getName() . ' is running; try again later'); + } + try { + return $fn(); + } finally { + $this->locking->releaseLock($key, \OCP\Lock\ILockingProvider::LOCK_SHARED); + } + } + /** * Direct children (files and folders) of a path in a snapshot. * @@ -72,7 +88,11 @@ class RestoreService { * @return array{restored:int, trashed:int, target:string} */ public function restoreUserFiles(Target $target, string $snapshotId, string $logicalPath, string $mode = self::MODE_NEW_FOLDER, ?callable $progress = null): array { - if (!preg_match('#^data/([^/]+)/files(?:/(.*))?$#', trim($logicalPath, '/'), $m)) { + return $this->withSharedLock($target, fn () => $this->doRestoreUserFiles($target, $snapshotId, $logicalPath, $mode, $progress)); + } + + private function doRestoreUserFiles(Target $target, string $snapshotId, string $logicalPath, string $mode, ?callable $progress): array { + if (!Repository::isSafePath(trim($logicalPath, '/')) || !preg_match('#^data/([^/]+)/files(?:/(.*))?$#', trim($logicalPath, '/'), $m)) { throw new \InvalidArgumentException('Expected a path like data//files/'); } [$uid, $rel] = [$m[1], trim($m[2] ?? '', '/')]; @@ -127,7 +147,7 @@ class RestoreService { if (!is_dir($directory) && !mkdir($directory, 0750, true) && !is_dir($directory)) { throw new \RuntimeException("Cannot create $directory"); } - return $this->targets->repository($target)->restore($snapshotId, $directory, trim($prefix, '/') === '' ? '' : trim($prefix, '/') . '/'); + return $this->withSharedLock($target, fn () => $this->targets->repository($target)->restore($snapshotId, $directory, trim($prefix, '/') === '' ? '' : trim($prefix, '/') . '/')); } private function writeFile(Repository $repo, Folder $dest, string $relPath, array $entry): void { diff --git a/src/api.js b/src/api.js index 997b6ee..12c4873 100644 --- a/src/api.js +++ b/src/api.js @@ -24,7 +24,7 @@ export default { trash: (targetId) => call('get', `/targets/${targetId}/trash`), untrash: (targetId, snapshotId) => call('post', `/targets/${targetId}/trash/${snapshotId}`), kitUrl: () => url('/key/kit'), - confirmKit: (accepted, phrase) => call('post', '/key/confirm', { accepted, phrase }), + confirmKit: (accepted, phrase, code) => call('post', '/key/confirm', { accepted, phrase, code }), backends: () => call('get', '/backends'), addTarget: (target) => call('post', '/targets', target), removeTarget: (id) => call('delete', `/targets/${id}`), diff --git a/src/components/KeySetup.vue b/src/components/KeySetup.vue index 8be90da..16085f0 100644 --- a/src/components/KeySetup.vue +++ b/src/components/KeySetup.vue @@ -63,8 +63,10 @@ : t('ng_backup', 'Before the first backup: download the recovery kit and store it, together with the passphrase, somewhere safe outside this server.') }} {{ t('ng_backup', 'Download recovery kit') }} + {{ status.statement }} - {{ status.confirmationPhrase }} + {{ status.confirmationPhrase }}

{{ t('ng_backup', 'Recovery kit confirmed by {user} on {date}.', { user: status.key.confirmation.uid, date: new Date(status.key.confirmation.time * 1000).toLocaleString() }) }} @@ -99,6 +101,7 @@ const delay = ref(props.status.key.deleteDelayDays) const slotForm = ref(null) const downloaded = ref(false) const accepted = ref(false) +const code = ref('') const busy = ref(false) const error = ref('') const kitUrl = api.kitUrl() @@ -148,18 +151,10 @@ const removeSlot = (slot) => guarded(async () => { downloaded.value = accepted.value = false }) -async function confirm() { - busy.value = true - error.value = '' - try { - await api.confirmKit(accepted.value, props.status.confirmationPhrase) - emit('changed') - } catch (e) { - error.value = e.message - } finally { - busy.value = false - } -} +const confirm = () => guarded(async () => { + await api.confirmKit(accepted.value, props.status.confirmationPhrase, code.value) + code.value = '' +})