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

Update dev container browser software and validation #5472

Merged
merged 3 commits into from
Sep 2, 2020

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Aug 31, 2020

Status

Ready for review

Description of Changes

Mozilla's keyserver went down, and they might just leave it that way. Their release signing key is available on another one, but that doesn't seem to be the result of any official policy I can find. The closest thing to guidance comes from a blog post instructing us to obtain the key from the archive server along with the software. This changes the Dockerfile to do that.

Recent versions of geckodriver are being signed with the release key, so we'll now use it to verify that package too.

We were also using an expired version of the Tor release signing key, kept in our repo, so I'm following their instructions for obtaining the key instead.

Finally, this updates all three packages.

Finally finally, this includes a workaround for the recent setuptools breakage.

Fixes #5470.
Fixes #5471.

Testing

  • Check out the 5470-keystone-kops branch.
  • Start the dev server with make dev. Confirm that the development container is built and started properly.

Deployment

Dev/test only.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make 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

Mozilla's keyserver went down, and they might just leave it that
way. Their release signing key is available on another one, but that
doesn't seem to be the result of any official policy I can find. The
closest thing to guidance comes from a blog post instructing us to
obtain the key from the archive server along with the
software[1]. This changes the Dockerfile to do that.

Recent versions of geckodriver are being signed with the release key,
so we'll now use it to verify that package too.

We were also using an expired version of the Tor release signing key,
kept in our repo, so I'm following their instructions[2] for obtaining
the key instead.

Finally, this updates all three packages.

[1] https://blog.mozilla.org/security/2019/06/13/updated-firefox-gpg-key/
[2] https://support.torproject.org/tbb/how-to-verify-signature/
@rmol rmol requested review from conorsch and emkll as code owners August 31, 2020 22:08
emkll
emkll previously approved these changes Aug 31, 2020
Copy link
Contributor

@emkll emkll 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 the fixes @rmol , good to merge when CI passes

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

We have to find a way to use the older (46.0.0) setuptools here :(

@kushaldas
Copy link
Contributor

I am going to push a separate PR just to test one idea, if that is good, then we can bring in the changes here.

Install dh-virtualenv from Debian Sid, switch to its alternative build
system, and tell it to use our pinned version of setuptools.
@rmol rmol dismissed kushaldas’s stale review September 1, 2020 19:30

I've pushed a change that does pin setuptools for dh-virtualenv, using a newer version of it with the alternative build system.

@sssoleileraaa
Copy link
Contributor

Looks like all checks have passed so this is ready for another review. Hoping to get this merged today, so wondering if someone is already reviewing?

@conorsch
Copy link
Contributor

conorsch commented Sep 1, 2020

@creviera Thanks for pinging. I'll take a pass through the test plan today, but given the complexity of the package build changes, this may not land in develop. So I recommend you branch locally if you're blocked at the moment!

@rmol Since this PR also addresses #5471, I'm going to extend the test plan with some package comparisons.

@rmol
Copy link
Contributor Author

rmol commented Sep 1, 2020

@conorsch Sounds good. Thanks.

@sssoleileraaa
Copy link
Contributor

@creviera Thanks for pinging. I'll take a pass through the test plan today, but given the complexity of the package build changes, this may not land in develop. So I recommend you branch locally if you're blocked at the moment!

ah i thought it just updated the keyserver string - that's why our PRs are failing in the SDK repo. there's an SDK PR that allows us to merge a couple client PRs. at this point everything is tested so don't need to branch from this - just want to be able to merge BUT no rush. there plennnnty of other things to work on.

@sssoleileraaa
Copy link
Contributor

ah right! it also includes the setuptools in xenial workaround

@conorsch
Copy link
Contributor

conorsch commented Sep 2, 2020

There are significant changes in the resulting deb packages for Xenial. Some are appear superficial:

For all bundled Python dependencies, the directory path for metadata uses the "dist" format rather than the "<python_version>-egg" format, e.g.

  • /opt/​venvs/​securedrop-​app-​code/​lib/​python3.​5/​site-​packages/​mod_wsgi-​4.​6.​7-​py3.​5.​egg-​info/​
    becomes
  • /opt/​venvs/​securedrop-​app-​code/​lib/​python3.​5/​site-​packages/​mod_wsgi-​4.​6.​7.​dist-​info/​

Fortunately there's only package metadata in those directories, not source code for the libraries. The AppArmor profile
also liberally grants access to /opt/venvs/securedrop-app-code/lib/python3.5/, so no need to adjust grants there.

More concerning, the postinst for securedrop-app-code is several hundred lines longer, due to injection of systemd helper code. Perhaps the override_dh_systemd_* targets are no longer honored?

The virtualenv path declared in /opt/venvs/securedrop-app-code/bin/activate has changed from

  • VIRTUAL_ENV="/​opt/​venvs/​securedrop-​app-​code" to
  • VIRTUAL_ENV="/​tmp/​securedrop-​app-​code_1.​6.​0~rc1+xenial_amd64/​build/​opt/​venvs/​securedrop-​app-​code"

The latter path is an emphemeral build dir, and does not exist on the App Server, nor is it bundled in the deb package. Fortunately our WSGI settings hardcode /opt/venvs/securedrop-app-code as the PythonHome, so the app still works well in a staging environment. The fact that build-time artifacts have slipped into the prod package suggests we may want further refinements to the build logic given the upgrade to dh-virtualenv.

That said, the app does work, based on some interactive spot-checking I tried in a local staging env, and CI is passing again, so I'm going to proceed with merge here.

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.

See comment listing some potentials areas for follow-up. Build is passing, though, so let's get it into develop. Thanks for plowing through multiple fixes to unbreak builds, @rmol!

@conorsch conorsch merged commit 2777a9f into develop Sep 2, 2020
@rmol
Copy link
Contributor Author

rmol commented Sep 2, 2020

More concerning, the postinst for securedrop-app-code is several hundred lines longer, due to injection of systemd helper code. Perhaps the override_dh_systemd_* targets are no longer honored?

What were you comparing it to? I grabbed the 1.5.0 package from apt.freedom.press and compared to the local deb, and the systemd content was the same.

The virtualenv path declared in /opt/venvs/securedrop-app-code/bin/activate has changed

I'm looking into this. dh-virtualenv's other path correction logic (shebangs in other scripts) seems to be working, just not the activate adjustment. Still obviously a showstopper.

@conorsch
Copy link
Contributor

conorsch commented Sep 2, 2020

What were you comparing it to? I grabbed the 1.5.0 package from apt.freedom.press and compared to the local deb, and the systemd content was the same.

Good point! Looks like I used 1.5.0~rc1 debs, but dated 2020-08-10, meaning they were out of date—left over from reviewing Focal build logic changes. Comparing with the 1.5.0 prod debs, the diff is much smaller, exactly what we want. Thanks for checking, @rmol !

@rmol
Copy link
Contributor Author

rmol commented Sep 3, 2020

Just a note for future us: the alternative build system lacks the path correction of the virtualenv activation scripts. It's only in the dh-virtualenv Python script, and would need to be replicated in the debhelper Perl module, or our own code: hacks in rules or the like.

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.

make build-debs is failing on develop make dev fails when retrieving Mozilla's GPG key
6 participants