fix restore logic to recreation of onion services #4133
Conversation
heartsucker
requested review from
conorsch,
emkll,
kushaldas and
msheiny
as code owners
heartsucker
force-pushed
the
fix-restore-logic
branch
from
1b94e54
to
7fa0e99
Compare
|
While I was eating lunch I realized on deficiency here is that the ATHS values aren't copied back to the admin workstation, and the torrc isn't updated. This would leave an admin locked out. Part of the restore role should include re-provisioning the workstation. |
There was a problem hiding this comment.
Looks great @heartsucker thanks for the PR.
I've ran through the test as you've described:
- 0.11.1 prod vms, backed up
- built 0.12.0~rc1 prod vms from scratch and restored the backup from the previous step
./securedrop-admin install and ./securedrop-admin tailsconfig and source/journalist interface now work under xenial 🎉
Backup docs could be a tad clearer (https://docs.securedrop.org/en/stable/backup_and_restore.html) : perhaps instructing users to run another ./securedrop-admin install before ./securedrop-admin tailsconfig to copy over the tor onion URLs after the restore.
|
@emkll Does that mean that the issue I raised above is not present? I haven't done a Tails install in a while and this seemed like it might cause issues. |
|
@heartsucker : based on my local testing on this branch if you run |
|
Ok I think that is sufficient for this. I'm giving the thumbs up to that docs change, so you can approve this or we can wait for another homie to come by and do the deed. |
| @@ -35,18 +36,29 @@ def verify_args(): | |||
| def main(): | |||
| verify_args() | |||
|
|
|||
| # Remove the /var/lib/tor/services directories to purge values that may have been | |||
| # generated by running the ansible playbooks | |||
| for d in ['journalist', 'source']: | |||
There was a problem hiding this comment.
@heartsucker @emkll: for posterity can you explain why this change was necessary? Does the stanza below not overwrite the /var/lib/tor/services config files from the backup on the server in the correct locations?
There was a problem hiding this comment.
In retrospect, it does not seem strictly necessary, unless I am missing something: the tar extraction should squash existing values on the server
There was a problem hiding this comment.
I'm pretty sure V3 onion services add additional files, and I can't remember how tar -x works and if those extra files would cause bad behavior in the future.
There was a problem hiding this comment.
Based on the commit history and comments, it's not clear why this stanza is needed... it seems like a reasonable approach here is to determine if it's necessary. If it is, let's document why (at least in the PR). If we actually don't need this stanza, then we can just remove it in the spirit of reducing LOC to maintain.
As noted in a PR review [0] (unfortunately after merge), this stanza is a no-op in the context of a backup restore and should be removed. [0]: #4133 (review)
As noted in a PR review [0] (unfortunately after merge), this stanza is a no-op in the context of a backup restore and should be removed. [0]: #4133 (review)
As noted in a PR review [0] (unfortunately after merge), this stanza is a no-op in the context of a backup restore and should be removed. [0]: #4133 (review) (cherry picked from commit 6a8b339)
As noted in a PR review [0] (unfortunately after merge), this stanza is a no-op in the context of a backup restore and should be removed. [0]: #4133 (review)
Status
Ready for review
Description of Changes
Fixes #3960
Alter the restore logic to ensure that the onion services from the backup are what end up running on prod, and not the new values that may have been generated by Ansible.
Testing
Deployment
This does not change deployment, but it does change restore from a previous deployment. The admin will run the same commands on xenial as on trusty.