Restoring tarball to Focal shows error for v2

[conorsch]

Description

When restoring a v2-only Xenial backup tarball to a v3-only Focal instance, the restore action fails. It fails even if the admin explicitly requests that the tor config be preserved as-is.

Steps to Reproduce

I used libvirt-based VMs for testing, and performed all admin actions from a virtualized Tails v4.16 VM.

1. Create a v2-only backup tarball from a Xenial host.
2. Perform a clean install of Focal, with v3-only vars.
3. Attempt to restore the backup: ./securedrop-admin --force restore --preserve-tor-config ~/Persistent/backups/xenial-v2-only/sd-backup-2021-02-26--15-57-06.tar.gz

Expected Behavior

Restore action completes, old URLs are restored, and I can proceed with regenerating new v3 URL and finalizing the Xenial -> Focal migration.

Actual Behavior

Restore action fails. Even when I include the --preserve-tor-config flag, it still fails.

Comments

On one hand, the failure is expected, since Focal is v3-only, but in the context of a migration from Xenial, it's likely we're going to have admins migrating to Focal from a recently created backup, so I recommend we defer the fail-closed behavior to a subsequent release. That'd have bearing on WIP docs changes in e..g. freedomofpress/securedrop-docs#133

The above is a policy question, but this ticket is also pointing out some bugs that should be fixed. For one, --preserve-tor-config is not honored, and it should be.

[conorsch]

--preserve-tor-config is not honored

A patch like this should resolve:

--- a/admin/securedrop_admin/__init__.py
+++ b/admin/securedrop_admin/__init__.py
@@ -883,24 +883,15 @@ def restore_securedrop(args: argparse.Namespace) -> int:
     # Would like readable output if there's a problem
     os.environ["ANSIBLE_STDOUT_CALLBACK"] = "debug"
 
-    ansible_cmd_full_restore = [
+    ansible_cmd = [
         'ansible-playbook',
         os.path.join(args.ansible_path, 'securedrop-restore.yml'),
         '-e',
         "restore_file='{}'".format(restore_file_basename),
     ]
 
-    ansible_cmd_skip_tor = [
-        'ansible-playbook',
-        os.path.join(args.ansible_path, 'securedrop-restore.yml'),
-        '-e',
-        "restore_file='{}' restore_skip_tor='True'".format(restore_file_basename),
-    ]
-
     if args.restore_skip_tor:
-        ansible_cmd = ansible_cmd_skip_tor
-    else:
-        ansible_cmd = ansible_cmd_full_restore
+        ansible_cmd += ["-e", "restore_skip_tor='True'"]

In a nutshell, each "extra-arg" passed on the CLI its own -e. That's still not enough, though, since the comparison task that fails has no conditional:

securedrop/install_files/ansible-base/roles/restore/tasks/main.yml

Lines 31 to 35 in 2f73b92

	- name: Check for Tor configuration differences between the backup and server
	connection: local
	become: no
	command: "python {{ role_path }}/files/compare_torrc.py {{ torrc_check_dir.path }}"
	register: compare_result

Whether that task runs should be influenced by the restore_skip_tor var.

[eloquence]

Labeled as release blocker because --preserve-tor-config is release-critical functionality.