Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

Include updated release key in base template #12

Merged
merged 4 commits into from
Jun 30, 2020
Merged

Conversation

emkll
Copy link
Contributor

@emkll emkll commented Jun 26, 2020

Status

Ready for Review

Description

Towards freedomofpress/securedrop-workstation#579

Adds securedrop-keyring to the TemplateVM, updates the key used to bootstrap the repo with the updated release key.

Test Plan

In order to test, will require a Fedora-31 dev VM (as per README updates). I have already successfully tested the steps below.

  • modify the build-workstation-template script as below to import my GPG key instead of the release key (see [1]). We will sign a tag w/ release key once the changes are reviewed and merged.
  • run make template
  • Move the template to dom0
  • Template installs successfully in dom0
  • Set VM to HVM and kernel to ''
  • Template boots in grsecurity kernel
  • apt-key finger shows the release key in its own keyring
  • the old key is not present
  • securedrop-keyring package is installed
  • sudo echo hello does not prompt for sudo password

Next steps after initial review:

  • drop 8cce803, mark PR as ready for review
  • Merge this PR
  • Sign tag w/ release key
  • Rebuild template for production

[1] : import/trust dev key for testing purposes:

diff --git a/build-workstation-template b/build-workstation-template
index e280461..6f2fac9 100755
--- a/build-workstation-template
+++ b/build-workstation-template
@@ -36,8 +36,8 @@ make COMPONENTS="builder" get-sources
 # Add signing key to the keyring. The pubkey fingerprint tracked here is the "FPF Authority Key".
 # In order to test PRs, you may want to use a personal key with a temporary signed tag.
 # See comments above regarding feature branches in the git clone operation.
-gpg --homedir ${gpg_homedir} --keyserver pool.sks-keyservers.net --recv-key 22245C81E3BAEB4138B36061310F561200F4AD77 || exit 1;
-echo '22245C81E3BAEB4138B36061310F561200F4AD77:6:' | gpg --homedir ${gpg_homedir} --import-ownertrust;
+gpg --homedir ${gpg_homedir} --keyserver pool.sks-keyservers.net --recv-key AF775782949D263DAABB3387AAFB3575FAC82745 || exit 1;
+echo 'AF775782949D263DAABB3387AAFB3575FAC82745:6:' | gpg --homedir ${gpg_homedir} --import-ownertrust;
 
 # Get all sources
 make get-sources

emkll added 3 commits June 26, 2020 10:14
Fedora-30 is end-of-life
Now expires on 2021-06-30
securedrop-keyring is already a dependency of securedrop-config, but we should still explicitly install it here.
@emkll emkll requested a review from conorsch June 26, 2020 14:19
@conorsch
Copy link
Contributor

@emkll mentioned in chat today that this is ready for review, so marking as such

@conorsch conorsch marked this pull request as ready for review June 26, 2020 22:37
@conorsch
Copy link
Contributor

drop 8cce803, mark PR as ready for review

Of course, the draft state was intentional. Proceeding with review now, I'll amend as described if test plan shows no problems.

@conorsch
Copy link
Contributor

Ran into an error while testing:

Makefile:591: target 'mgmt-salt.grep' given more than once in the same rule
-> Updating sources for builder-debian...
--> Fetching from https://github.com/QubesOS/qubes-builder-debian.git master (options: --depth=1)...
--> Verifying tags...
--> Switching branch from master branch to master
Already on 'master'
Your branch is up to date with 'origin/master'.

-> Updating sources for template-securedrop-workstation...
--> Fetching from https://github.com/freedomofpress/qubes-template-securedrop-workstation release-key-2020 (options: --depth=1)...
--> Verifying tags...
No valid signed tag found!
fatal: No names found, cannot describe anything.
make[1]: *** [Makefile:204: template-securedrop-workstation.get-sources] Error 1
make[1]: Leaving directory '/home/user/qubes-template-securedrop-workstation/qubes-builder'
make: *** [Makefile:2: template] Error 2

That's after I applied the patch to use your personal key. Have definitely encountered this before, my guess is there's another patch i'll have to add locally to skip verifying a signed tag on the remote. Let me know what you think, @emkll, I mostly want to capture these steps in a PR template to make it obvious for future maintainers.

@conorsch
Copy link
Contributor

Ah, was using an outdated tag to attempt the build. Resolved, proceeding with local review...

conorsch
conorsch previously approved these changes Jun 29, 2020
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.

Test plan checks out!

  • Template installs successfully in dom0
  • Set VM to HVM and kernel to ''
  • Template boots in grsecurity kernel
  • apt-key finger shows the release key in its own keyring
  • the old key is not present
  • securedrop-keyring package is installed

Holding off on merge since some commits should be dropped. Please proceed, @emkll, and ping if further review required.

@conorsch
Copy link
Contributor

We may have a regression here. After testing, I went to shut down the template, and was surprised to receive a sudo prompt:

user@securedrop-workstation-buster:~$ sudo poweroff
We trust you have received the usual lecture from the local System
Administrator. It usually boils down to these three things:
    #1) Respect the privacy of others.
    #2) Think before you type.
    #3) With great power comes great responsibility.
[sudo] password for user: 

That doesn't look right—all qubes templates have passwordless sudo by default, so let's investigate what went sideways in the build I just did.

@conorsch conorsch self-requested a review June 29, 2020 17:48
@conorsch conorsch dismissed their stale review June 29, 2020 17:49

Dismissing approval to block merge, to investigate sudo regression

@emkll
Copy link
Contributor Author

emkll commented Jun 29, 2020

Good catch @conorsch ! It seems like the newer builds to not include the qubes-core-agent-passwordless-root package, when compared with the last released templates.

This package is pulled in via the qubes-vm-recommended metapackage. I've noticed previous version of the template contain the qubes-vm-recommended package, but not the latest builds.

I've noticed we are tracking master branch of qubes-builder-debian at build time, which contains a refactor to default packages, which may have introduced either a regression in our config [1]. I see three possible solutions:

  1. builder var to use an existing tag to freeze qubes-builder-debian (which will require more work in the future to keep current with latest changes)
  2. It's possible that we are building minimal templates, or there's a variable we are using in the securedrop-workstation.conf that pulls in the bare minimal packages
  3. The easy fix would be to use the diff below to crudely install the packages from qubes apt repos:
diff --git a/securedrop-workstation/04_install_qubes_post.sh b/securedrop-workstation/04_install_qubes_post.sh
index 76f3ba8..7a9b401 100755
--- a/securedrop-workstation/04_install_qubes_post.sh
+++ b/securedrop-workstation/04_install_qubes_post.sh
@@ -33,7 +33,7 @@ fi
 
 mount --bind /dev "${INSTALLDIR}/dev"
 
-aptInstall apt-transport-https
+aptInstall qubes-vm-recommended apt-transport-https
 
 [ -n "$workstation_repository_suite" ] || workstation_repository_suite="buster"
 [ -n "$workstation_signing_key_fingerprint" ] || workstation_signing_key_fingerprint="22245C81E3BAEB4138B36061310F561200F4AD77"

[1] : QubesOS/qubes-builder-debian@14baf17#diff-96406aed0a83d2b44865692a74531942R2

@conorsch
Copy link
Contributor

Great investigation, thanks for the detailed report, @emkll.

The easy fix would be to use the diff below to crudely install the packages from qubes apt repos:

In terms of unblocking the current template rebuild work, the proposal you make in 3 makes the most sense to keep the wheels turning. I'll amend the PR to evaluate the behavior as described. Going forward, I'd prefer that we pin, as described in 1, and commit to rebuilding at a regular interval. Since we're going to be updating the VM kernels shortly, that's a fine time to revisit the build logic. Let's consider some research time on the next sprint to transfer some knowledge on the builder process, and perhaps get clarity on 2, as well.

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.

Amended to include the additional upstream package @emkll identified. Tacked on a sanity check task for sudo passwords to the test plan, all looks good:

  • Template installs successfully in dom0
  • Set VM to HVM and kernel to ''
  • Template boots in grsecurity kernel
  • apt-key finger shows the release key in its own keyring
  • the old key is not present
  • securedrop-keyring package is installed
  • sudo echo hello does not prompt for sudo password

Approving, back over to you @emkll to drop commits prior to merge.

Due to a regression with sudo password handling, we now explicitly pull
in the `qubes-vm-recommended` package, which pulls in
`qubes-core-agent-passwordless-root` via dependencies. This is a
quick fix, and we should investigate the upstream builder logic more
deeply to resolve fully.
@emkll emkll force-pushed the release-key-2020 branch from db84d7f to 4a2dc76 Compare June 30, 2020 12:56
@emkll
Copy link
Contributor Author

emkll commented Jun 30, 2020

Thanks @conorsch for the fix, dropped the commit tracking the feature branch, will merge, tag, and build.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants