Modified CLI update function to use keys.openpgp.org for release key

[zenmonkeykstop]

Status

Ready for review

Description of Changes

Towards #4128.

Updates the securedrop-admin tool to use only hkps://keys.openpgp.org when retrieving the release key for tag signature verification.

Testing

On an Admin Workstation (virtual or USB stick):

• Verify that the update process works when a release key is already in the GPG keychain, using the following commands in a terminal:

cd ~/Persistent/securedrop
gpg --recv-key "22245C81E3BAEB4138B36061310F561200F4AD77"   # use Tails' default keyserver
git remote add zenmonkeykstop https://github.com/zenmonkeykstop/securedrop.git
git checkout openpgp-admin-cli-update
./securedrop-admin update
git status    # should now be on 0.13.1 tag

• Next, verify the process works with no pre-existing key:

gpg --delete-keys "22245C81E3BAEB4138B36061310F561200F4AD77"
git checkout openpgp-admin-cli-update
./securedrop-admin update
git status    # should now be on 0.13.1 tag

Deployment

This will be deployed via the GUI updater, CLI update as above, or directly by checking out the release tag containing the change. Nothing special required.

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

[kushaldas]

It is still failing for me. Here is the output from the manual command.

$ gpg --batch --no-tty --recv-key --keyserver hkps://keys.opengpg.org 22245C81E3BAEB4138B36061310F561200F4AD77
gpg: keyserver receive failed: Server indicated a failure

[kushaldas]

It is still failing for me. Here is the output from the manual command.

$ gpg --batch --no-tty --recv-key --keyserver hkps://keys.opengpg.org 22245C81E3BAEB4138B36061310F561200F4AD77
gpg: keyserver receive failed: Server indicated a failure

It seeems there was typo in my manual command, but, the ./securedrop-admin update also failed a few times. I will update with the error message next time.

[emkll]

It might make sense to add a test case for the updated key (with UID) the introduced in #4578, a test similar to https://github.com/freedomofpress/securedrop/blob/develop/admin/tests/test_securedrop-admin.py#L193 with different output, containing the new style key :

gpg: Signature made Thu 20 Jul 2017 08:12:25 PM EDT
gpg:                using RSA key 22245C81E3BAEB4138B36061310F561200F4AD77
gpg: Good signature from "SecureDrop Release Signing Key" [unknown]
gpg:                 aka "SecureDrop Release Signing Key <[email protected]>" [unknown]

In cases where the key is directly imported or the key is refreshed, the (good) signature output should be as above. The existing case should cover non refreshed key.

[kushaldas]

INFO: Update needed
INFO: Verifying signature on latest update...
gpg: keyserver receive failed: Connection closed in DNS
ERROR (run with -v for more): Command '['timeout', '45', 'gpg', '--batch', '--no-tty', '--recv-key', '--keyserver', 'hkps://keys.openpgp.org', '22245C81E3BAEB4138B36061310F561200F4AD77']' returned non-zero exit status 2

The error message.

[redshiftzero]

@emkll makes a good point, parametrizing test_update_signature_verifies to cover:

• the git output already in that test
• the git output @emkll flags above
• the following git output:

gpg: Signature made Thu 20 Jul 2017 08:12:25 PM EDT
gpg:                using RSA key 22245C81E3BAEB4138B36061310F561200F4AD77
gpg: Good signature from "SecureDrop Release Signing Key <[email protected]>" 

would be good to add prior to merge

[emkll]

To me, these two new lines introduced appear complex, especially for such a critical part of the verification code. Do you think it makes sense to clarify the logic here (either through comments or making the logic more explicit)?

[zenmonkeykstop]

I reorganized things to make it a little clearer and added comments.

[redshiftzero]

tested in Tails, this looks good to me

Dismissing as discussed as this will need to be merged today. I wasn't able to reproduce your issue but if you keep seeing it on latest Tails please file a followup and tag "Release QA"

[redshiftzero]

restamping