Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds option to restore from backup file already on server. #5909

Merged
merged 1 commit into from
Jun 9, 2021
Merged

Adds option to restore from backup file already on server. #5909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions admin/securedrop_admin/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -883,25 +883,23 @@ 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),
ansible_cmd_extras = [
"restore_file='{}'".format(restore_file_basename),
]

if args.restore_skip_tor:
ansible_cmd = ansible_cmd_skip_tor
else:
ansible_cmd = ansible_cmd_full_restore
ansible_cmd_extras.append("restore_skip_tor='True'")

if args.restore_manual_transfer:
ansible_cmd_extras.append("restore_manual_transfer='True'")

ansible_cmd.append(' '.join(ansible_cmd_extras))
return subprocess.check_call(ansible_cmd, cwd=args.ansible_path)


Expand Down Expand Up @@ -1161,6 +1159,11 @@ class ArgParseFormatterCombo(argparse.ArgumentDefaultsHelpFormatter,
dest='restore_skip_tor',
help="Preserve the server's current Tor config")

parse_restore.add_argument("--no-transfer", default=False,
action='store_true',
dest='restore_manual_transfer',
help="Restore using a backup file already present on the server")

parse_update = subparsers.add_parser('update', help=update.__doc__)
parse_update.set_defaults(func=update)

Expand Down
1 change: 1 addition & 0 deletions install_files/ansible-base/roles/restore/defaults/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
# backup. add the `--preserve-tor-config` to preserve the server's existing config.

restore_skip_tor: False
restore_manual_transfer: False
29 changes: 29 additions & 0 deletions install_files/ansible-base/roles/restore/tasks/perform_restore.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,40 @@
- "More info: {{ compare_result.stdout }}"
when: not restore_skip_tor

- name: Calculate local backup file checksum
connection: local
become: no
stat:
path: "{{ restore_file }}"
checksum_algorithm: sha256
register: local_backup_file
when: restore_manual_transfer

- name: Calculate remote backup file checksum
stat:
path: "/tmp/{{ restore_file }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about allowing a full path, to allow for /tmp on the server not having enough free space?

Copy link
Contributor Author

@zenmonkeykstop zenmonkeykstop Apr 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not in favour of it - points against:

  • The SD setup instructions as they stand don't result in a separate /tmp partition, so if there's enough space on / this isn't an issue - and the backup job would have failed in the first place as it also uses /tmp
  • this is the current backup location - it would introduce more changes and would require extra work to make sure backup tempfiles are purged from whatever alternate location was specified.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, writing sensitive files to /tmp is not recommended [1], due to the permissions of that partition, and the ability of all system users to write to it. In this case, the servers are effectively single-user systems, so the risks are relatively low, but in the case where an attacker has unprivileged access to the system, they may have access to more information.

[1] some links/references in https://security.openstack.org/guidelines/dg_using-temporary-files-securely.html

checksum_algorithm: sha256
register: remote_backup_file
when: restore_manual_transfer

- name: Verify that local and remote backup file checksums match
assert:
that:
- "remote_backup_file.stat.checksum == local_backup_file.stat.checksum"
fail_msg:
- "Checksum comparison for the local and remote copies of the backup file failed."
- "When using the --no-transfer flag. the same file must be present both locally"
- "in `install_files/ansible-base and on the Application Server in `/tmp`, to allow"
- "for config verification. Please copy the backup file into place on the"
- "Application Server before proceeding, or restore without the --no-transfer flag."
when: restore_manual_transfer

- name: Copy backup to application server
synchronize:
src: "{{ restore_file }}"
dest: /tmp/{{ restore_file }}
partial: yes
when: not restore_manual_transfer

- name: Extract backup
unarchive:
Expand Down