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

Repair auto-updates on Tails < 4.19 workstations #6110

Merged

Conversation

rocodes
Copy link
Contributor

@rocodes rocodes commented Sep 27, 2021

Status

Ready for review

Description of Changes

Fixes #6098

Changes proposed in this pull request: Include check for pre-4.19 Tails versions in network hook. Attempt to repair auto-updates on Tails < 4.19 by applying the fix at https://tails.boum.org/news/version_4.18/, with the additional step of verifying the new CA file against existing files on the user's machine prior to installation.

Testing

How should the reviewer test this PR?
Write out any special testing steps here.

On an Admin or Journo Workstation running Tails < 4.19

  • Ensure network connectivity
  • cd ~/Persistent/securedrop and check out this branch
  • Run ./securedrop-admin --force tailsconfig
  • tailsconfig completes successfully
  • After about 30 seconds, the Tails updater GUI appears and prompts you to download the latest version of Tails

On Admin/Journo Workstation running Tails 4.19 or greater (no op)

  • Ensure network connectivity
  • Check out this branch
  • Run ./securedrop-admin --force tailsconfig
  • tailsconfig completes successfully

(note: If you don't have an old Tails workstation and are QAing, these steps aren't recommended but are acceptable)

  • Edit /etc/os-release to change the TAILS_VERSION_ID to a pre-4.19 version
  • touch /usr/local/etc/ssl/certs/tails.boum.org-CA.pem
  • Check out this branch, then run ./securedrop-admin --force tailsconfig
  • tailsconfig completes successfully
  • cat /usr/local/etc/ssl/certs/tails.boum.org-CA.pem and verify that the isrg-root-x1-cross-signed cert was added to the file (https://tails.boum.org/isrg-root-x1-cross-signed.pem)

Deployment

Any special considerations for deployment? Consider both:

  1. Upgrading existing production instances.
  2. New installs.

Checklist

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

If you made non-trivial code changes:

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

Choose one of the following:

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation
  • These changes do not require documentation

@cfm
Copy link
Member

cfm commented Sep 28, 2021

@zenmonkeykstop asked me to take a look at this since I have some bandwidth to spare this week. I've set up a Tails 4.16 admin workstation from which I'll have test results tomorrow.

@cfm cfm self-assigned this Sep 28, 2021
@rocodes rocodes force-pushed the 6098-old-tails-upgrade-fix branch 3 times, most recently from c7bfed8 to b22143c Compare September 28, 2021 14:38
@rocodes rocodes force-pushed the 6098-old-tails-upgrade-fix branch from b22143c to 327e9df Compare September 28, 2021 19:40
@rocodes rocodes marked this pull request as ready for review September 28, 2021 20:00
@rocodes rocodes requested a review from a team as a code owner September 28, 2021 20:00
@rocodes rocodes requested a review from cfm September 28, 2021 20:01
cfm
cfm previously approved these changes Sep 28, 2021
Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

Diff looks good and test plan checks out! Here's a test from (arbitrarily) Tails 4.16:

On an Admin or Journo Workstation running Tails < 4.19

amnesia@amnesia:~$ cat /etc/amnesia/version
4.16 - 20210222
28f284b2539478b0e6348396dc14232e59542728
live-build: 3.0.5+really+is+2.0.12-0.tails5
live-boot: 1:20170112
live-config: 5.20190519
  1. Ensure network connectivity
  2. cd ~/Persistent/securedrop and check out this branch
amnesia@amnesia:~/Persistent/securedrop$ git checkout rocodes/6098-old-tails-upgrade-fix
[...]
HEAD is now at 327e9df4b Include check for pre-4.19 Tails versions in network hook. Attempt to repair auto-updates on those systems
  1. Run ./securedrop-admin --force tailsconfig
  2. tailsconfig completes successfully
  3. After about 30 seconds, the Tails updater GUI appears and prompts you to download the latest version of Tails

...upgrade ensues...

On Admin/Journo Workstation running Tails 4.19 or greater (no op)

amnesia@amnesia:~$ cat /etc/amnesia/version
4.22 - 20210906
2660dc56443bf5f29feddd5c6ae6cacaf4308837
live-build: 3.0.5+really+is+2.0.12-0.tails5
live-boot: 1:20170112
live-config: 5.20190519
  1. Ensure network connectivity
  2. Check out this branch
  3. Run ./securedrop-admin --force tailsconfig
  4. tailsconfig completes successfully

Approving for maintainer's final review.

Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

At @eloquence's urging, given the nature of this change and its deployment, I've done a little extra fuzz-testing to see if I could get this logic to break at all. More than anything else, the case I present below is a way of asking how seriously we want to [d]on't break tailsconfig trying to fix this. :-)

In addition to the intended cases, tailsconfig completes successfully:

  • with /etc/os-release contents pathologically from the future (Tails > 4.22)
  • with /etc/os-release contents pathetically from the past (Tails 3.x)—though it wouldn't actually work, of course!

The failure-mode I was able to induce involves deliberately breaking the curl call and seeing what happens afterward:

--- a/install_files/ansible-base/roles/tails-config/files/securedrop_init.py
+++ b/install_files/ansible-base/roles/tails-config/files/securedrop_init.py
@@ -187,7 +187,7 @@ if tails_current_version:
 
         try:
             subprocess.call(['torsocks', 'curl', '--silent',
-                             'https://tails.boum.org/' + cert_name],
+                             'https://tails.boum.org/x' + cert_name],
                             stdout=pem_file, env=env)
 
             # Verify against /etc/ssl/certs/DST_Root_CA_X3.pem, which cross-signs

The curl call "succeeds", swallowing the HTTP 404; the openssl call returns exit code 2 with the error unable to load certificate; and the Ansible play halts.

@rocodes, I defer to your and the team's deep knowledge of Tails internals and how securedrop-admin both does and ought to interact with them—so I'm raising the questions in this follow-on review as discussion points rather than formally requesting changes. Please use or disregard as you see fit!

@rocodes
Copy link
Contributor Author

rocodes commented Sep 29, 2021

@cfm Thanks for your thoughtful review. I've addressed your comments and also tested these changes with deliberate misconfigurations in all local filepath names, unavailable cert URL on the Tails website, etc., to ensure that tailsconfig runs are not disrupted.

@rocodes rocodes requested a review from cfm September 29, 2021 16:28
@eloquence eloquence added this to the 2.1.0 milestone Sep 29, 2021
Copy link
Member

@cfm cfm left a comment

Choose a reason for hiding this comment

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

Thanks for revising, @rocodes. Looks great as of ef483e0. In particular, in the case in which (e.g.) openssl verify has failed:

  1. tailsconfig completes successfully even without
  2. After about 30 seconds, the Tails updater GUI appears and prompts you to download the latest version of Tails
  3. The network hook does not crash

—as intended for the Ansible play to proceed even if this logic fails for any reason.

I'll flag this case (#6110 (review)) for inclusion in v2.1.0 QA.

Re-approving for final maintainer's review.

@@ -148,3 +149,70 @@
if b'Update needed' in output or os.path.exists(flag_location):
# Start the SecureDrop updater GUI.
subprocess.Popen(['python3', path_gui_updater], env=env)
Copy link
Member

Choose a reason for hiding this comment

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

@zenmonkeykstop observed this morning that the invocation of the SecureDrop updater here, before the new logic that follows checks for a Tails upgrade, means that we will be able to update this logic in a future SecureDrop release in the event that it breaks under a future Tails version. (That's a mouthful, but I hope it's a clear statement of the solution to a risk we've worried about!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, and it's also my hope that we can remove this entire section after a reasonable timeframe (I've suggested Jan 2022 in the comments)

@zenmonkeykstop zenmonkeykstop merged commit 13e0eca into freedomofpress:develop Sep 29, 2021
@zenmonkeykstop
Copy link
Contributor

Thanks @rocodes for getting this in under the wire and @cfm for the timely review!

@cfm cfm mentioned this pull request Oct 8, 2021
26 tasks
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.

Prompt users to fix upgrade issue for Tails 4.14 - 4.18 (or fix it for them?)
4 participants