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

Switch to 2021 signing key; update tests #700

Merged
merged 2 commits into from Jun 1, 2021
Merged

Conversation

eloquence
Copy link
Member

@eloquence eloquence commented May 21, 2021

Status

Ready for final review

Description of Changes

Fixes #697. Preserves the old key, but only provisions the new one for verification purposes.

Does not remove the legacy key from the RPM database of existing systems; it will simply be let to expire.

Test plan

Estimated testing time: 1-2 hours (significant wall time due to full state run and regular updater run)

  1. make clone this branch into dom0 and sudo dnf reinstall (or make staging if you don't have a previously provisioned environment) the freshly built RPM from rpm-build/RPMS/noarch.
  2. Switch the provisioned environment to a prod state -- ensure that the environment in both /usr/share/securedrop-workstation-dom0-config/config.json and /srv/salt/sd/config.json is set to prod
  3. In dom0, run /opt/securedrop/launcher/sdw-launcher.py --skip-delta 0. This will force an updater run. Because a full migration will run, this is expected to take a long time -- longer still if you have not yet installed fedora-33.
  4. Monitor the console, ~/.securedrop_launcher/logs/launcher.log and ~/.securedrop_launcher/logs/launcher-detail.log for errors. You may see errors "An Exception occurred while executing state.highstate" in launcher-detail.log. This is due to Salt highstate generation doesn’t work on Fedora templates QubesOS/qubes-issues#6580 and expected to be resolved later this week.
    • Observe that no other errors appear in the logs and that the update completes successfully. If you are prompted to reboot, don't do that just yet.
  5. In dom0, assuming your code checkout is in /home/user/securedrop-workstation/ run gpg --with-fingerprint --with-colons ~/securedrop-workstation/sd-workstation/securedrop-release-signing-pubkey-2021.asc.
  6. In dom0, run gpg --with-fingerprint --with-colons /etc/pki/rpm-gpg/RPM-GPG-KEY-securedrop-workstation
  7. In sys-firewall, run gpg --with-fingerprint --with-colons /etc/pki/rpm-gpg/RPM-GPG-KEY-securedrop-workstation
    • Observe that the fingerprints for all three keys are identical, confirming that the new key was successfully provisioned.
  8. If you were prompted to reboot earlier, do so now.
  9. Re-run the updater with /opt/securedrop/launcher/sdw-launcher.py --skip-delta 0.
    • Observe that you do not see a "Migration is required" entry in launcher.log during this run. This shows that we're only running the full migration once.
    • Observe that the update completes successfully. This shows that the new signing key does not interfere with updates.

@eloquence
Copy link
Member Author

eloquence commented May 21, 2021

Tested this by switching my environment to prod (had to manually edit the config.json copy in /srv/salt/sd), installing RPM from this branch, and running the updater. As expected:

  • the update was successful
  • the 2021 prod key was installed (no full migration required, apply_dom0_state took care of it)
  • subsequent runs of the updater completed without error.

@eloquence eloquence added this to In Development in SecureDrop Team Board May 21, 2021
@sssoleileraaa sssoleileraaa self-requested a review May 24, 2021 20:26
@eloquence
Copy link
Member Author

eloquence commented May 24, 2021

I've added a test plan and per discussion with @conorsch and @creviera added a commit to force a full migration run via a flag dropped in postinst, to ensure that the key is provisioned to sys-firewall as well. This is now ready for a first round of testing.

If it looks good so far, we discussed that as a next step, we can add a temporary commit to use the new signing key for yum-test as well, and upload an RPM signed with the new key with a version bump to yum-test. This will allow us to verify that the next release after this week's release (i.e. 0.5.5), can be verified successfully using the new key.

@eloquence eloquence moved this from In Development to Ready for Review in SecureDrop Team Board May 24, 2021
@sssoleileraaa
Copy link
Contributor

Unfortunately I'm stuck on step 1 of the test plan due to an issue with make staging not being able to find qubes-template-fedora-33 (also tried sudo qubes-dom0-update qubes-template-fedora-33 which fails with the error message "Error: Unable to find a match: qubes-template-fedora-33"). I could install the package hosted here: https://yum.qubes-os.org/r4.0/templates-itl/rpm/ as a workaround, but let's discuss tomorrow morning.

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented May 25, 2021

sudo qubes-dom0-update --action=reinstall qubes-template-fedora-33 ended up installing for the first time the fedora-33 package (thanks @eloquence for suggesting this and troubleshooting with me). this is just a temporary workaround - we'll still need to fix the provisioning logic so that works for fresh installs of fedora.

@sssoleileraaa
Copy link
Contributor

  • Observe that no other errors appear in the logs and that the update completes successfully. If you are prompted to reboot, don't do that just yet.

❌ Observe that the fingerprints for all three keys are identical, confirming that the new key was successfully provisioned.

fingerprints were the same between dom0 and sys-firewall when running: gpg --with-fingerprint --with-colons /etc/pki/rpm-gpg/RPM-GPG-KEY-securedrop-workstation (both were the SecureDrop TESTING key) but did not match the fingerprint in dom when running: gpg --with-fingerprint --with-colons ~/securedrop-workstation/sd-workstation/securedrop-release-signing-pubkey-2021.asc (which was the SecureDrop Release Singing Key securedrop-release-key-2021@freedom.press)

  • Re-run the updater and observe that you do not see a "Migration is required" entry in launcher.log during this run. This shows that we're only running the full migration once.
  • Observe that the update completes successfully. This shows that the new signing key does not interfere with updates.

@eloquence
Copy link
Member Author

@creviera Can you confirm that the environment variable in /srv/salt/sd/config.json is set to prod? The provisioning logic will (re-)provision the test key otherwise.

@sssoleileraaa
Copy link
Contributor

it's set to staging but i did update it to prod before running the launcher -- definitely could have been user error where i didn't save or something so lemme run through this test plan again.

and later i will uninstall fedora-33 and switch back to fedora-32 to do a little troubleshooting around the early issue with being unable to find the fedora-33 template

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented May 25, 2021

after running this a second time, i see the correct release key in dom0 but sys-firewall is still showing the test key :/

config.json is set to prod in /srv/salt/sd/config.json

Update

Just spoke to @eloquence about this and learned that the Updater will skip copying the key over to sys-firewall if the migration flag is set (indicating that the migration has already been done). So I will remove this flag by reinstalling fedora-33 and try again.

@sssoleileraaa
Copy link
Contributor

@eloquence now that the updater can access https://deb.qubes-os.org/, it has run successfully, but i'm still unable to successfully complete your test plan because sys-firewall does not get the release key. i've tried twice, double checking that a) /tmp/sdw-migrations/signing-key-update is not present and b) qubes-template-fedora-33 is successfully reinstalled before running the updater. any ideas as to why this is not working? i am tempted to move on to removing the fedora-33 template and switching over to fedora-32 at this point so i can verify that we no longer hit the issue i ran into yesterday.

@conorsch
Copy link
Contributor

The "full state run" logic that kicks in to ensure that sys-firewall is updated as well as dom0 is activated via the postinst flag: 74eef9e Does that flag exist? It'll be created when the securedrop-workstation-dom0-config rpm package is installed. The updater logs should indicate whether it's detected and found, so observe there, but also check directly on the dom0 filesystem.

@sssoleileraaa
Copy link
Contributor

@conorsch /tmp/sdw-migrations/signing-key-update does not exist, which i believe we're calling the flag :/

@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented May 26, 2021

When I switched from using the make staging path to the dnf reinstall path (on step one) I was able to see that the /tmp/sdw-migrations/signing-key-update file was created. @eloquence will confirm whether or not he's able to repro my issue using make staging and in the meantime I'll be runinng through the test plan again -- seems very unlikely that it won't work at this stage.

Also worth noting that I do not use the standard sd-dev name for my workstation development vm, therefore I have to set SECUREDROP_DEV_VM and SECUREDROP_DEV_DIR env vars accordingly. Not sure how well we're testing that everything works with those env vars.

@eloquence
Copy link
Member Author

make staging also reliably drops the /tmp flag for me so we're no closer to solving that particular mystery - we're pretty sure that @creviera's RPM wasn't outdated. Something to keep an eye on if we do additional test runs.

@sssoleileraaa
Copy link
Contributor

Once /usr/share/securedrop-worktaiton-dom0-config/config.json was set to prod this ended up working. The test plan has been updated accordingly because it's easy to forget that we have to update the same config in two different places when switching between staging and prod. I also pushed the temporary commit above so that @conorsch can sign an RPM and push to yum-test.

@kushaldas
Copy link
Contributor

My system managed to find fedora-33 template, but at first attempt it failed because of a faulty mirror.

@sssoleileraaa
Copy link
Contributor

Looks like signing an rpm with the key works great, see freedomofpress/securedrop-yum-test#24 (comment). Next step is to undo the temporary commits and I think we can close freedomofpress/securedrop-yum-test#24 without merging since it was created for testing purposes. @conorsch does that sound good to you?

@eloquence
Copy link
Member Author

eloquence commented May 27, 2021

If we wanted to test a prod-like updater run, we could publish the RC1 RPM to yum-test by merging freedomofpress/securedrop-yum-test#24. Note however that this PR's branch has already been modified to drop your temporary commit and to bump the version, so we would either need to use an older version of the branch, or restore it to the previous state, so that the updater registers the RPM on yum-test as a newer version. I defer to y'all whether the additional confidence in the verification flow via the updater is worth that effort.

@eloquence
Copy link
Member Author

eloquence commented May 27, 2021

I provisioned a staging environment with the prod key and ran the updater, which successfully updated me to the 0.5.4-0 RC1 signed with the prod key. Updater ran without errors.

securedrop-workstation-dom0-config.noarch                                                         0.5.4-0.rc1.1.fc25                                                         qubes-dom0-cached
Qubes OS Repository for Dom0                                                                                                                                   26 MB/s |  26 kB     00:00    
Dependencies resolved.
==============================================================================================================================================================================================
 Package                                                      Arch                             Version                                      Repository                                   Size
==============================================================================================================================================================================================
Upgrading:
 securedrop-workstation-dom0-config                           noarch                           0.5.4-0.rc1.1.fc25                           qubes-dom0-cached                           117 k

Transaction Summary
==============================================================================================================================================================================================
Upgrade  1 Package

Total size: 117 k
Downloading Packages:
Running transaction check
Transaction check succeeded.
Running transaction test
Transaction test succeeded.
Running transaction
  Upgrading   : securedrop-workstation-dom0-config-0.5.4-0.rc1.1.fc25.noarch                                                                                                              1/2 
  Cleanup     : securedrop-workstation-dom0-config-0.5.3-1.fc25.noarch                                                                                                                    2/2 
  Verifying   : securedrop-workstation-dom0-config-0.5.4-0.rc1.1.fc25.noarch                                                                                                              1/2 
  Verifying   : securedrop-workstation-dom0-config-0.5.3-1.fc25.noarch                                                                                                                    2/2 

Upgraded:
  securedrop-workstation-dom0-config.noarch 0.5.4-0.rc1.1.fc25                                                                                                                                

I'll shift gears to a SecureDrop Ansible issue for the rest of today, but tomorrow I can take a look at the dual key logic with @conorsch.

@conorsch conorsch self-requested a review May 28, 2021 22:46
@conorsch
Copy link
Contributor

Spent some more time with these changes today. In short, they work well, and I'm comfortable with proceeding to final release for 0.5.4. Ideally we'd have support for multiple gpg keys, same as we do for apt (vs yum/dnf) in e.g. freedomofpress/securedrop-builder#250, but that deserves its own issue.

I'm going to open a PR to revert the yum-test changes in freedomofpress/securedrop-yum-test#24, and drop the temporary commits here. Then we can proceed with final review.

Since these changes are the last we intend to include for 0.5.4, I'm able to bump the final version in this PR and tag. Also fine to do that in a separate PR post-merge.

conorsch pushed a commit to freedomofpress/securedrop-yum-test that referenced this pull request May 28, 2021
We've performed the manual testing as part of [0], so let's
remove the non-standard signature and overwrite it with
a sig from the standard test key, i.e. 4ED79CC3362D7D12837046024A3BE4A92211B03C.

[0] freedomofpress/securedrop-workstation#700
@eloquence
Copy link
Member Author

Dropped the temporary commit and declaring this ready.

The temporary commit was:

From 1419801f4eefd15d95664fe468d726350971c93a Mon Sep 17 00:00:00 2001
From: Allie Crevier <allie@freedom.press>
Date: Tue, 25 May 2021 18:18:50 -0700
Subject: [PATCH] KEY MIGRATION TEST - DROP ME LATER

---
 dom0/sd-default-config.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dom0/sd-default-config.yml b/dom0/sd-default-config.yml
index 779fbb1..5f7069b 100644
--- a/dom0/sd-default-config.yml
+++ b/dom0/sd-default-config.yml
@@ -8,4 +8,4 @@ prod:
 dev:
   dom0_yum_repo_url: "https://yum-test.securedrop.org/workstation/dom0/f25"
   apt_repo_url: "https://apt-test.freedom.press"
-  signing_key_filename: "apt-test-pubkey.asc"
+  signing_key_filename: "securedrop-release-signing-pubkey-2021.asc"
-- 
2.25.1

@eloquence eloquence marked this pull request as ready for review May 28, 2021 23:53
@sssoleileraaa
Copy link
Contributor

sssoleileraaa commented Jun 1, 2021

once merged i'll open a pr with the version bump and changelog updates so @conorsch can prod-sign the tag and review

Copy link
Contributor

@conorsch conorsch left a comment

Choose a reason for hiding this comment

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

Approving based on copious testing. We'll proceed with version bump and tagging in a subsequent PR, towards #699.

@conorsch conorsch merged commit e180ad0 into main Jun 1, 2021
SecureDrop Team Board automation moved this from Ready for Review to Done Jun 1, 2021
cfm pushed a commit that referenced this pull request Apr 1, 2024
Switch to 2021 signing key; update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Update signing key in dom0
4 participants