diff --git a/docs/HOWTO.md b/docs/HOWTO.md index 9f15f5e..e6d0097 100644 --- a/docs/HOWTO.md +++ b/docs/HOWTO.md @@ -102,7 +102,9 @@ occ backup:user:restore offsite users// --mode=as-backup use, add `--install-user-migration` to approve that explicitly. The command stops after the installation; run it once more. - `replace` first makes a safety export of the current account, then recreates the user under the - same id. `as-backup` restores into `-bak` and leaves the original untouched. + same id. When the account exists, the command asks first; in scripts, add `--force`. Shares to + others and data of apps without a user_migration migrator are not restored, so prefer + `as-backup` when in doubt: it restores into `-bak` and leaves the original untouched. - A restore onto a server where the user does not exist works. Calendars and address books get an internal name with the prefix `migrated-` (for example `migrated-personal`). That prefix comes from user_migration and cannot be turned off. The user can change the display name of these calendars and @@ -124,6 +126,9 @@ occ backup:list offsite # the snapshots must be lis occ backup:restore:full offsite ``` +The command refuses on an installation that already has more than one user, and asks before it +starts; `--force` skips both (also needed to rerun it after it stopped halfway). + Afterwards, check that users, files and the database are complete, and run a backup of the restored server. The restored installation keeps its own app configuration and secret. diff --git a/lib/Command/KeyImport.php b/lib/Command/KeyImport.php index 4e03669..f982965 100644 --- a/lib/Command/KeyImport.php +++ b/lib/Command/KeyImport.php @@ -76,6 +76,9 @@ HELP); return 1; } $output->writeln('Backup key imported, fingerprint ' . $this->keys->fingerprint() . '.'); + // Read from the kit file, which is not authenticated beyond the wrapped key: show it so a + // wrong or altered value stands out. + $output->writeln('Deletion delay for forgotten snapshots: ' . $this->keys->deleteDelayDays() . ' days (from the kit).'); $output->writeln('Next: connect the backup location with the same details as before (occ backup:target:add), then occ backup:list and occ backup:restore:full.'); return 0; } diff --git a/lib/Command/KeyKit.php b/lib/Command/KeyKit.php index 7cc12c4..5a7638c 100644 --- a/lib/Command/KeyKit.php +++ b/lib/Command/KeyKit.php @@ -39,11 +39,18 @@ final class KeyKit extends Command { $output->write($json, false, OutputInterface::OUTPUT_RAW); return 0; } - if (file_put_contents($file, $json) === false) { - $output->writeln("Cannot write $file"); + // Created owner-only from the start (no window where it is world-readable), never over an + // existing file or symlink someone else may have placed at a predictable path. + $old = umask(0077); + try { + $fh = @fopen($file, 'xb'); + } finally { + umask($old); + } + if ($fh === false || fwrite($fh, $json) !== strlen($json) || !fclose($fh)) { + $output->writeln("Cannot write $file (it must not exist yet)"); return 1; } - @chmod($file, 0600); $output->writeln("Recovery kit written to $file. Copy it off this server, then run occ backup:key:confirm."); return 0; } diff --git a/lib/Command/RestoreFull.php b/lib/Command/RestoreFull.php index 4487b54..524fe63 100644 --- a/lib/Command/RestoreFull.php +++ b/lib/Command/RestoreFull.php @@ -9,7 +9,11 @@ namespace OCA\NgBackup\Command; use OCA\NgBackup\Service\RestoreFullService; use OCA\NgBackup\Service\TargetService; +use OCP\IUserManager; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\QuestionHelper; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Question\ConfirmationQuestion; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -18,6 +22,7 @@ final class RestoreFull extends Command { public function __construct( private TargetService $targets, private RestoreFullService $restoreFull, + private IUserManager $userManager, ) { parent::__construct(); } @@ -27,6 +32,7 @@ final class RestoreFull extends Command { ->setDescription('Disaster recovery: restore the database, data directory and config on a fresh installation') ->addArgument('target', InputArgument::REQUIRED, 'Location name or id') ->addArgument('snapshot', InputArgument::REQUIRED, 'Snapshot id (see backup:list)') + ->addOption('force', null, InputOption::VALUE_NONE, 'Also on an installation that already has more than one user, and without asking') ->setHelp(<<<'HELP' For a FRESH Nextcloud installation only, same major version as the backup, right after occ maintenance:install, occ backup:key:import and occ backup:target:add -- before anyone else @@ -50,12 +56,29 @@ HELP); } protected function execute(InputInterface $input, OutputInterface $output): int { + $force = (bool)$input->getOption('force'); + // A whole-server restore replaces the database and data directory of THIS instance. On + // anything but a fresh install (only the admin from occ maintenance:install) that is + // almost certainly a mistake, so it needs --force. + $users = $this->userManager->countUsersTotal(2); + if (!$force && ($users === false || $users > 1)) { + $output->writeln('This installation already has more than one user: backup:restore:full would replace its whole database and data directory. It is meant for a fresh installation. Use --force if this really is what you want.'); + return 1; + } + if (!$force && $input->isInteractive()) { + $q = new ConfirmationQuestion('This replaces the database, the data directory and config.php of this installation. Continue? [y/N] ', false); + if (!(new QuestionHelper())->ask($input, $output, $q)) { + $output->writeln('Cancelled.'); + return 1; + } + } try { $target = $this->targets->get($input->getArgument('target')); $snapshot = (string)$input->getArgument('snapshot'); $result = $this->restoreFull->restoreFull($target, $snapshot); } catch (\Throwable $e) { $output->writeln('' . $e->getMessage() . ''); + $output->writeln('If this happened after the restore had started, the instance may be partly restored: fix the cause and run the same command again with --force.'); return 1; } $output->writeln(sprintf( @@ -65,6 +88,9 @@ HELP); $result['configKeys'], $result['dataFiles'], )); + if (!$result['dbConsistent']) { + $output->writeln('Warning: this snapshot\'s database dump was made over several steps (web cron), so it is not one consistent point in time. Check the instance carefully; prefer a snapshot made with system cron.'); + } $output->writeln('Next: check trusted_domains and overwritehost/overwriteprotocol match this server, then occ maintenance:repair.'); return 0; } diff --git a/lib/Command/RestoreUser.php b/lib/Command/RestoreUser.php index 6ead952..6fa415e 100644 --- a/lib/Command/RestoreUser.php +++ b/lib/Command/RestoreUser.php @@ -10,7 +10,10 @@ namespace OCA\NgBackup\Command; use OCA\NgBackup\Service\TargetService; use OCA\NgBackup\Service\UserRestoreService; use OCA\NgBackup\UserMigration\UserMigrationInstaller; +use OCP\IUserManager; use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Helper\QuestionHelper; +use Symfony\Component\Console\Question\ConfirmationQuestion; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -21,6 +24,7 @@ final class RestoreUser extends Command { private TargetService $targets, private UserRestoreService $userRestore, private UserMigrationInstaller $installer, + private IUserManager $userManager, ) { parent::__construct(); } @@ -31,6 +35,7 @@ final class RestoreUser extends Command { ->addArgument('target', InputArgument::REQUIRED, 'Location name or id') ->addArgument('manifest', InputArgument::REQUIRED, 'Manifest path printed by backup:user:backup (users//)') ->addOption('mode', 'm', InputOption::VALUE_REQUIRED, 'replace or as-backup', UserRestoreService::MODE_REPLACE) + ->addOption('force', null, InputOption::VALUE_NONE, 'Replace an existing account without asking (required when not interactive)') ->addOption('install-user-migration', null, InputOption::VALUE_NONE, 'Install and enable user_migration if it is missing (non-interactive consent)') ->setHelp(<<<'HELP' Never imports over a live account. Modes: @@ -47,11 +52,26 @@ HELP); } protected function execute(InputInterface $input, OutputInterface $output): int { + $mode = (string)$input->getOption('mode'); + $uid = explode('/', (string)$input->getArgument('manifest'))[1] ?? ''; + if ($mode === UserRestoreService::MODE_REPLACE && $uid !== '' && $this->userManager->userExists($uid) && !$input->getOption('force')) { + // Replace deletes the live account (after a safety export). Shares to others and data of + // apps without a user_migration migrator do not come back, so never do it unasked. + if (!$input->isInteractive()) { + $output->writeln("$uid exists: --mode=replace deletes and recreates it. Add --force, or use --mode=as-backup."); + return 1; + } + $q = new ConfirmationQuestion("$uid exists and will be deleted and recreated from the backup (a safety export is made first; its shares to others and data of apps without a migrator are not restored). Continue? [y/N] ", false); + if (!(new QuestionHelper())->ask($input, $output, $q)) { + $output->writeln('Cancelled.'); + return 1; + } + } try { $output->writeln('Note: on restore, user_migration only imports the app settings on its own allowlist (e.g. calendar view and reminder settings); other app settings of this user are skipped. Reason: NG Backup uses user_migration\'s importer; widening the list would mean building a settings importer of our own.'); $target = $this->targets->get($input->getArgument('target')); $this->installer->ensureEnabled($input, $output); - $user = $this->userRestore->restoreUser($target, (string)$input->getArgument('manifest'), (string)$input->getOption('mode')); + $user = $this->userRestore->restoreUser($target, (string)$input->getArgument('manifest'), $mode); } catch (\Throwable $e) { $output->writeln('' . $e->getMessage() . ''); return 1; diff --git a/lib/Controller/AdminApiController.php b/lib/Controller/AdminApiController.php index 3c42182..3d07195 100644 --- a/lib/Controller/AdminApiController.php +++ b/lib/Controller/AdminApiController.php @@ -168,6 +168,7 @@ final class AdminApiController extends Controller { return new JSONResponse($this->targets->availableBackends()); } + #[PasswordConfirmationRequired] public function addTarget(string $name, string $backend, string $auth, array $options = [], string $path = 'ng_backup', ?float $maxGb = null): JSONResponse { try { $r = $this->targets->add($name, $backend, $auth, $options, $path); diff --git a/lib/Crypto/KeyRing.php b/lib/Crypto/KeyRing.php index 9db9d68..92de548 100644 --- a/lib/Crypto/KeyRing.php +++ b/lib/Crypto/KeyRing.php @@ -96,6 +96,12 @@ final class KeyRing { /** @param array{v:int, salt:string, ops:int, mem:int, nonce:string, key:string} $wrapped */ public static function unwrap(array $wrapped, #[\SensitiveParameter] string $passphrase): self { $b64 = static fn (string $s): string => sodium_base642bin($s, SODIUM_BASE64_VARIANT_ORIGINAL); + // ops/mem come from unauthenticated JSON (kit file, repository config): bound them, so a + // damaged or crafted file cannot make the KDF use unbounded memory or time. + if ($wrapped['ops'] < SODIUM_CRYPTO_PWHASH_OPSLIMIT_INTERACTIVE || $wrapped['ops'] > SODIUM_CRYPTO_PWHASH_OPSLIMIT_SENSITIVE + || $wrapped['mem'] < SODIUM_CRYPTO_PWHASH_MEMLIMIT_INTERACTIVE || $wrapped['mem'] > SODIUM_CRYPTO_PWHASH_MEMLIMIT_SENSITIVE) { + throw new CryptoException('Unsupported key derivation parameters (damaged or foreign key file)'); + } $kek = sodium_crypto_pwhash(SODIUM_CRYPTO_SECRETBOX_KEYBYTES, $passphrase, $b64($wrapped['salt']), $wrapped['ops'], $wrapped['mem'], SODIUM_CRYPTO_PWHASH_ALG_ARGON2ID13); $master = sodium_crypto_secretbox_open($b64($wrapped['key']), $b64($wrapped['nonce']), $kek); diff --git a/lib/Crypto/StreamCipher.php b/lib/Crypto/StreamCipher.php index f0bf081..47b1106 100644 --- a/lib/Crypto/StreamCipher.php +++ b/lib/Crypto/StreamCipher.php @@ -122,6 +122,7 @@ final class StreamCipher { /** fread() may return short reads on network streams; keep reading until $length or EOF. */ private static function readFull($in, int $length): string { $buffer = ''; + $empty = 0; while (strlen($buffer) < $length && !feof($in)) { $chunk = fread($in, $length - strlen($buffer)); if ($chunk === false) { @@ -132,8 +133,15 @@ final class StreamCipher { if (feof($in)) { break; } + // A stalled network stream returns '' without EOF forever: back off, then give up + // (about a minute) instead of spinning at full CPU until the lease expires. + if (++$empty > 600) { + throw new CryptoException('Read stalled'); + } + usleep(100000); continue; } + $empty = 0; $buffer .= $chunk; } return $buffer; diff --git a/lib/Db/DbDumper.php b/lib/Db/DbDumper.php index a43ce98..2202a4c 100644 --- a/lib/Db/DbDumper.php +++ b/lib/Db/DbDumper.php @@ -256,7 +256,11 @@ final class DbDumper { } elseif (is_string($row[$i]) && !mb_check_encoding($row[$i], 'UTF-8')) { // Keep invalid UTF-8 byte-exact instead of substituting characters. $row[$i] = ['$b' => base64_encode($row[$i])]; - } elseif (is_int($row[$i]) || is_float($row[$i]) || is_bool($row[$i])) { + } elseif (is_float($row[$i])) { + // var_export uses serialize_precision (shortest exact form), not the lossy + // 14-digit 'precision' of a (string) cast. + $row[$i] = var_export($row[$i], true); + } elseif (is_int($row[$i]) || is_bool($row[$i])) { // Normalise: drivers differ in returning numbers as int or string. $row[$i] = (string)(is_bool($row[$i]) ? (int)$row[$i] : $row[$i]); } diff --git a/lib/Service/RestoreFullService.php b/lib/Service/RestoreFullService.php index 7269caf..1d429ed 100644 --- a/lib/Service/RestoreFullService.php +++ b/lib/Service/RestoreFullService.php @@ -64,7 +64,7 @@ final class RestoreFullService { ]; } - /** @return array{dbReport:array, dataFiles:int, configKeys:int} */ + /** @return array{dbReport:array, dataFiles:int, configKeys:int, dbConsistent:bool} */ public function restoreFull(Target $target, string $snapshotId): array { $check = $this->checkCompatibility($target, $snapshotId); if (!$check['ok']) { @@ -76,6 +76,7 @@ final class RestoreFullService { $meta = $repo->snapshotMeta($snapshotId); $dbReport = null; + $manifest = []; if (isset($meta['db'])) { $manifest = json_decode($repo->getObject($meta['db']), true, 512, JSON_THROW_ON_ERROR); $prefix = $this->config->getSystemValueString('dbtableprefix', 'oc_'); @@ -124,7 +125,8 @@ final class RestoreFullService { ); } - return ['dbReport' => $dbReport ?? ['skipped' => [], 'restored' => []], 'dataFiles' => $dataFiles, 'configKeys' => count($backupConfig)]; + return ['dbReport' => $dbReport ?? ['skipped' => [], 'restored' => []], 'dataFiles' => $dataFiles, 'configKeys' => count($backupConfig), + 'dbConsistent' => (bool)($manifest['consistent'] ?? true)]; }); } diff --git a/tests/integration/user-roundtrip.sh b/tests/integration/user-roundtrip.sh index 73c2283..175b4b1 100755 --- a/tests/integration/user-roundtrip.sh +++ b/tests/integration/user-roundtrip.sh @@ -48,7 +48,8 @@ ok "as-backup restores into -bak, byte-identical, original account untouche # replace: damage the account, then restore it in place under the same uid. $X sh -c "rm '$DATA/$U/files/Docs/f1.bin' && echo damaged > '$DATA/$U/files/Docs/f1.bin'" occ files:scan "$U" >/dev/null -occ backup:user:restore ci "$MANIFEST" --mode=replace >/dev/null || fail "restore replace" +if occ backup:user:restore ci "$MANIFEST" --mode=replace --no-interaction >/dev/null 2>&1; then fail "replace of an existing account ran without --force"; fi +occ backup:user:restore ci "$MANIFEST" --mode=replace --force >/dev/null || fail "restore replace" occ files:scan "$U" >/dev/null SUM_AFTER=$($X sh -c "cd $DATA/$U/files && find . -type f -print0 | sort -z | xargs -0 md5sum | md5sum") [ "$SUM_BEFORE" = "$SUM_AFTER" ] || fail "files differ after a replace restore"