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.

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

Explicitly declare onion services as v2 for existing installs #4092

Merged
merged 2 commits into from
Feb 4, 2019

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Jan 31, 2019

Status

Ready for review

Description of Changes

Towards #4031 , this is required to ensure torrc config is compatible with tor 0.3.5.x series.
Modify the torrc file in place to explicitly declare current onion services as v2 onion services. Since securedrop-config is installed after tor hidden services are configured, it will also perform the modification (and be a no-op once #4080 is merged at install-time)

Testing

Clean install

  • Make debs and provision staging environment on this branch
  • Hidden services configurations in /etc/tor/torrc on app and mon server specify version 2 for all hidden services
  • Source, journalist and ssh interfaces are accessible

Upgrade testing

  • Build debs and provision existing SecureDrop 0.11.1 install with these debs
  • Hidden services configurations in /etc/tor/torrc on app and mon server specify version 2 for all hidden services
  • Source, journalist and ssh interfaces are accessible

Deployment

These changes will be deployed to new and existing SecureDrop instances via securedrop-config deb package.

Checklist

If you made changes to the server application code:

  • Linting (make ci-lint) and tests (make -C securedrop test) pass in the development container

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

This will modify the torrc file in place to explicitly declare current onion services as v2 onion services.
Copy link
Contributor

@heartsucker heartsucker left a comment

Choose a reason for hiding this comment

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

This bash function indeed does this right thing, but I'm also on the fence about whether or not we need it. If we're provisioning from ansibleas part of the upgrade story, we don't. However, our current restore from backup logic might create problems since this line would be missing. But, in order to apply migrations after a restore, we run dpkg-reconfigure securedrop-app-code which would trigger this perl bit, so we're safe.

I think at the end of the day it really comes down to us deciding where we want this logic. I would rather it not be in postinst personally but 🤷‍♂️

@redshiftzero
Copy link
Contributor

In my opinion, it's worth adding this for the following scenario that @heartsucker is mentioning above:

  1. Admin makes a backup during the pre-0.12 release time period where we are instructing admins to backup (and the backup contains tor configs).
  2. Admin upgrades to Xenial.
  3. Admin runs securedrop-admin install.
  4. Admin runs restore using an earlier backup file, putting old torrc back on disk breaking the onion services.

But in this case, since we don't dpkg-reconfigure securedrop-config during the restore process, I think this change wouldn't resolve the above scenario. What do y'all think about adding subprocess.check_call(['dpkg-reconfigure', 'securedrop-config']) here?

@conorsch
Copy link
Contributor

conorsch commented Feb 1, 2019

100% agree we want the changes in postinst. At present, the plan for upgrade is:

  1. Admin confirms instance is running at least 0.12.0
  2. Performs manual do-release-upgrade tasks
  3. Runs ./securedrop-admin install to enforce state

We don't also want to have to run ./securedrop-admin install before the do-release-upgrade task, if we can avoid it. These changes satisfy in that regard.

@redshiftzero brings up a great point on the restore script, happy to accommodate there. Still testing the functional changes presented here. I feel strongly there should be corresponding config tests added, will append those as part of review.

@conorsch
Copy link
Contributor

conorsch commented Feb 1, 2019

So far I've validated the staging clean install story configures the services correctly, on both Trusty & Xenial. Logged in interactively to inspect the torrc and confirm the v2 strings were there. Upgrade testing is next; see box metadata in #4093, will use those to base the upgrades on.

Each hidden service declaration provided by the test vars must have
"HiddenServiceVersion 2" immediately after the dir in the torrc.
At a later date we may want to templatize these values, but for now we
only support v2 Onion URLs for SD, so hardcoding is fine.
@conorsch
Copy link
Contributor

conorsch commented Feb 2, 2019

Upgrade scenario from 0.11.1 works smoothly, confirmed resolution of both Source & Journalist Interfaces. Did not test upgrade for Xenial, since that's not terribly relevant (we plan to have folks upgrade to Xenial after release of 0.12.0).

@conorsch conorsch merged commit 97e98fa into develop Feb 4, 2019
@conorsch conorsch deleted the 4031-v2-explicit-v2-onion-services branch February 4, 2019 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants