Bundle mod_wsgi in securedrop-app-code package #4518

rmol · 2019-06-10T22:35:56Z

Status

Ready for review

Description of Changes

To move to Python 3, we need to remove the dependency on Ubuntu's
libapache2-mod-wsgi package. Since the libapache2-mod-wsgi-py3 package
is in the universe section, this change bundles mod_wsgi in our
securedrop-app-code package.

For now, we have to simply drop libapache2-mod-wsgi as a dependency of
securedrop-app-code. Actually uninstalling it disables the Apache
module, breaking the Apache configuration. So we're leaving it
installed and modifying /etc/apache2/mods-available/wsgi.load to point
to our bundled mod_wsgi. This should be safe even if
libapache2-mod-wsgi is updated, as we're specifying --force-confdef
and --force-confold for cron-apt upgrades.

At some point when we establish more control over the state of /etc/
on SecureDrop servers and can ensure our bundled module stays
configured, we'll be able to remove the libapache2-mod-wsgi package.

While testing these changes in prod VMC, I ran into difficulties with
direct SSH, so I tweaked the Vagrantfile to support that better.

Fixes #4292.

Testing

[ ] Check out this branch.
[ ] Run make build-debs and confirm only the apt up-to-date test fails.

Clean install

[ ] Run make staging and confirm no errors during installation.
[ ] Browse to the Source Interface and confirm that the footer says Powered by SecureDrop 0.14.0~rc1.
[ ] Proceed with functional interactive testing of the Source/Journalist interfaces.

Upgrade behavior

The "upgrade" scenario requires use of libvirt for VM driver.

[ ] Run make upgrade-start to provision local apt VMs.
[ ] Visit the source interface (the onion address is at the end of make upgrade-start output) and confirm that the footer says Powered by SecureDrop 0.13.0 (we haven't upgraded yet).
[ ] Run make upgrade-test-local.
[ ] Browse to the Source Interface and confirm that the footer says Powered by SecureDrop 0.14.0~rc1.
[ ] Proceed with functional interactive testing of the Source/Journalist Interfaces.

Deployment

Switching to Python 3 is kind of a big deal.

Checklist

If you made changes to the system configuration:

Configuration tests pass

If you made non-trivial code changes:

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

conorsch

So far so good, changes are well documented. Pointing out a few areas in need of clarification, otherwise looking forward to testing when ready for review.

conorsch · 2019-06-10T23:47:22Z

build/.gitignore

-*.deb
-trusty/*.deb
-xenial/*.deb
-*.tar.gz


You're right that the .gitignore file is redundant, but it also serves as a placeholder for the build/ pardir—replace with .empty to ensure that build/ exists, otherwise package builds will fail.

That was ... an accident after cleaning build manually. But good point, I'll create .empty.

The .gitignore wasn't completely redundant, so I just restored it.

conorsch · 2019-06-10T23:51:05Z

install_files/securedrop-app-code/debian/postinst

@@ -142,6 +142,9 @@ case "$1" in
    # the ability to send signals to unconfined peers.
    service apache2 stop

+    # Point Apache to our bundled mod_wsgi
+    mod_wsgi-express module-config > /etc/apache2/mods-available/wsgi.load


Add a config test to assert the contents of /etc/apache2/mods-available/wsgi.load, so we have explicitly defined expectations about the module path.

rmol · 2019-06-11T22:54:46Z

@conorsch @kushaldas Minimally invasive version ready for (re)consideration. It just uses Python 3 when building securedrop-app-code; the rest of the changes can be done later and elsewhere.

To move to Python 3, we need to remove the dependency on Ubuntu's libapache2-mod-wsgi package. Since the libapache2-mod-wsgi-py3 package is in the universe section, this change bundles mod_wsgi in our securedrop-app-code package. For now, we have to simply drop libapache2-mod-wsgi as a dependency of securedrop-app-code. Actually uninstalling it disables the Apache module, breaking the Apache configuration. So we're leaving it installed and modifying /etc/apache2/mods-available/wsgi.load to point to our bundled mod_wsgi. This should be safe even if libapache2-mod-wsgi is updated, as we're specifying --force-confdef and --force-confold for cron-apt upgrades. At some point when we establish more control over the state of /etc/ on SecureDrop servers and can ensure our bundled module stays configured, we'll be able to remove the libapache2-mod-wsgi package. While testing these changes in prod VMC, I ran into difficulties with direct SSH, so I tweaked the Vagrantfile to support that better. To run a mix of Python 2 and 3, we need to omit backport requirements when running under 3, so I've added environment markers to enum34 and ipaddress. Also, --generate-hashes for develop and test requirements too.

kushaldas

The changes look good. But, I encountered the following while installing the deb package:

Collecting jsmin==2.2.2 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 17))
Requirement already up-to-date: Magic-file-extensions==0.2 in /usr/lib/python3/dist-packages (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 18))
Collecting Mako==1.0.7 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 19))
Collecting MarkupSafe==1.0 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 20))
Collecting mod-wsgi==4.6.6 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 21))
Collecting passlib==1.7.1 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 22))
Collecting pretty-bad-protocol==3.1.1 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 23))
Collecting psutil==5.4.3 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 24))
Collecting pycparser==2.18 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 25))
Collecting pyotp==2.2.6 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 26))
Collecting python-apt==1.1.0b1+ubuntu0.16.4.4 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 27))
  Could not find a version that satisfies the requirement python-apt==1.1.0b1+ubuntu0.16.4.4 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 27)) (from versions: )
No matching distribution found for python-apt==1.1.0b1+ubuntu0.16.4.4 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements-packaged.txt (line 27))
dpkg: error processing package securedrop-app-code (--install):
 subprocess installed post-installation script returned error exit status 1
Errors were encountered while processing:
 securedrop-app-code

conorsch

A few in-line comments. Also suggesting changes to the test plan:

remove instructions to instantiate a py3 dev env
recommend use of staging vms for clean install eval
recommend use of upgrade scenario for upgrade behavior

We can have multiple reviewers pile on to test the various logic branches, if you agree, @rmol.

conorsch · 2019-06-13T17:20:17Z

Makefile

 		admin/requirements-ansible.in \
 		securedrop/requirements/develop-requirements.in
-	pip-compile --output-file securedrop/requirements/test-requirements.txt \
+	pip-compile --generate-hashes --output-file securedrop/requirements/test-requirements.txt \
 		securedrop/requirements/test-requirements.in
 	pip-compile --generate-hashes --output-file securedrop/requirements/securedrop-app-code-requirements.txt \
 		securedrop/requirements/securedrop-app-code-requirements.in


When generating the reqs files for the Application Server, both test and prod reqs, we must take care to use a Python3 virtualenv, otherwise we'll wind up with Py2 deps from the dev env's pip-compile. Consider wrapping just the app code calls to pip-compile in a container; see example here https://github.com/freedomofpress/securedrop.org/blob/d9388f62220e45f5fdd6d50ec2652680cb48805c/Makefile#L43-L51 . (Likely these changes were previously present in the diff, and dropped after discussion of punting on the dev env migration.)

Done. Slightly differently than your example, as I just used our dev shell with Python 3.

conorsch · 2019-06-13T17:25:14Z

...ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/build_securedrop_app_code_deb.yml

+      - python3-dev
+      - python3-pip
+      - python3-wheel
+    state: present


These py3 dependencies are in addition to the py2 dependencies declared in molecule/builder-xenial/Dockerfile. We should instead modify the dependencies in the Dockerfile for the build, push a new image, and store the hash as part of this PR. Happy to append to this PR if you agree. Storing the dependencies in the container image definition will help keep local builds snappy, leveraging the cached layers.

I've left this in for now, but as soon as we can get the image updated I'll remove this step.

conorsch · 2019-06-13T17:27:26Z

install_files/securedrop-app-code/debian/control

@@ -6,5 +6,5 @@ Homepage: https://securedrop.org

 Package: securedrop-app-code
 Architecture: amd64
-Depends: python-pip,apparmor-utils,gnupg2,haveged,python,secure-delete,sqlite3,${dist:Depends},libapache2-mod-wsgi,libapache2-mod-xsendfile,redis-server,supervisor,securedrop-keyring,securedrop-config,libpython2.7-dev
+Depends: python3-pip,apparmor-utils,gnupg2,haveged,python3,secure-delete,sqlite3,${dist:Depends},libapache2-mod-xsendfile,redis-server,supervisor,securedrop-keyring,securedrop-config,devscripts


In order to avoid upstream updates to libapache2-mod-wsgi breaking our customizations to the /etc/apache2/mods-available/wsgi.load, we should pin the current-latest version of libapache2-mod-wsgi in the dependencies. This is a temporary measure—we still intend to remove that package later.

Pinned. I think I got the version specification correct, as securedrop-app-code is happily installing, but please double check.

Pin libapache2-mod-wsgi so an update can't stop the use of our bundled mod_wsgi. Remove Python 2 backports from securedrop-app-code requirements, and run update-pip-requirements with the securedrop-app-code requirements being updated with Python 3 in a container. When building the securedrop-app-code package: - Stop upgrading pip. - Capture the pip requirements without hashes by transforming the input requirements file instead of running pip freeze, which was pulling in extra dependencies that weren't listed in the Debian control file for securedrop-app-code, so could break its installation. - Remove the parsing of "pip3 wheel" output to determine success; it could mask errors that didn't produce the expected message.

rmol · 2019-06-13T21:28:12Z

@conorsch Test plan updated; please review, and thanks for the help.

rmol · 2019-06-13T21:30:44Z

@kushaldas This latest round of fixes included a change to how we collect the unhashed requirements when building the securedrop-app-code Debian package, to avoid pulling in dependencies that are not declared in its control file. It should have fixed the problem you saw, so if you've got the time to try it again, I'd appreciate it.

conorsch · 2019-06-14T01:33:08Z

@rmol Test plan still appears out of date to me. Suggestions:

Testing

Clean install

Run make build-debs and confirm only the apt up-to-date test fails
Run make staging and confirm no errors during installation
Browse to the Source Interface and confirm no obvious errors
Proceed with functional interactive testing of the Source/Journalist interfaces

Upgrade behavior

The "upgrade" scenario requires use of libvirt for VM driver.

Run make build-debs and confirm only the apt up-to-date test fails
Run make upgrade-start to provision local apt VMs
Log into app machine and run sudo cron-apt -i -s, confirm no errors
Proceed with functional interactive testing of the Source/Journalist Interfaces

Ideally we should modify molecule/upgrade/side_effect.yml to use cron-apt -i -s rather than an apt upgrade, to simulate more accurately the upgrade behavior prod instances will see. That's fine to do in this PR.

The test plan should not include use of a py3 virtualenv, based on visual review of the diff. Right now, I'm seeing CI errors that may be related to CI running on your fork, rather than in the main repo. Building the debs locally, I see test errors that aren't the apt up-to-date test, relating to Flask-Babel from the requirements files.

Although it's not been discussed as in the scope of #4292 to date, even once we get this PR merged, we'll still have the supervisord process running via python2, which isn't good. That's best handled in a separate issue, though—perhaps we can drop supervisor altogether (installing from apt means we're lagging behind the py3 support upstream) and use systemd to keep the service up.

Will take another pass through this with functional testing tomorrow.

rmol · 2019-06-14T20:19:34Z

@conorsch I don't know what happened to my edit. I think I was still previewing, went to do something else, came back, it looked good, and I never hit the update button. 🙄 Anyway, it looked much like yours except I didn't realize make upgrade-test-local wasn't already using cron-apt.

Turns out trying cron-apt was a great suggestion, because we can't upgrade securedrop-app-code with our current cron-apt configuration. Dependencies like python3-pip aren't available in the repos in /etc/apt/security.list, so securedrop-app-code just gets skipped. Historically, pip's been installed as part of securedrop-admin install, so it's always been available.

@emkll had the (reluctant) idea of adding a cron-apt action to explicitly install securedrop-app-code. This works, but it does take two rounds of cron-apt: one to insert the action, and another to actually run it, installing securedrop-app-code and its dependencies with the regular apt cache. And we're left with the non-security action. We could remove this hack in a point release, but the whole thing feels shaky.

We're hoping you're horrified enough to think of something better. I've pushed a couple of commits for review; with any luck we can rebase-erase them. 🙂

redshiftzero · 2019-06-14T22:29:09Z

install_files/securedrop-app-code/debian/control

@@ -6,5 +6,5 @@ Homepage: https://securedrop.org

 Package: securedrop-app-code
 Architecture: amd64
-Depends: python-pip,apparmor-utils,gnupg2,haveged,python,secure-delete,sqlite3,${dist:Depends},libapache2-mod-wsgi,libapache2-mod-xsendfile,redis-server,supervisor,securedrop-keyring,securedrop-config,libpython2.7-dev
+Depends: ${dist:Depends}, apparmor-utils, gnupg2, haveged, libapache2-mod-xsendfile, libapache2-mod-wsgi (= 4.3.0-1.1build1), python3, python3-pip, redis-server, secure-delete, securedrop-config, securedrop-keyring, sqlite3, supervisor


so if we deployed this change using #4355 then we wouldn't need to install python3-pip (problematic as it's from the universe channel) right? we wouldn't need to pip install the wheels because we'd ship a virtualenv cc @kushaldas @rmol @conorsch

I've only read the dh-virtualenv docs but yes, it looks like that would be a better way to get this done.

Once we switch to dh-virtualenv, will this change still require any other package dependency updates that conflict with the current cron-apt update logic (#3376)?

eloquence · 2019-06-17T17:53:09Z

Per discussions on Friday and today, this change won't land in 0.14.0, in significant part due to the problems updating dependencies not in /etc/apt/security.list (#3376) and our desire to avoid universe channel dependencies. Instead, the revised goal is to ship this change using dh-virtualenv (bypassing the need to install pip from the universe channel), and to land it with 0.15.0.

During the current sprint, we'll want to continue to explore how this change could be implemented using dh-virtualenv, pending a team review of the current WIP implementation of dh-virtualenv usage by @kushaldas.

rmol · 2019-07-30T16:42:52Z

Closing in favor of #4622.

rmol requested review from conorsch, emkll, heartsucker, kushaldas, msheiny, redshiftzero and zenmonkeykstop as code owners June 10, 2019 22:35

rmol force-pushed the 4292-pip-mod-wsgi branch from 5c8d630 to 4effc0d Compare June 11, 2019 16:57

conorsch reviewed Jun 11, 2019

View reviewed changes

rmol force-pushed the 4292-pip-mod-wsgi branch 2 times, most recently from 3588407 to 046d00a Compare June 11, 2019 22:47

rmol force-pushed the 4292-pip-mod-wsgi branch from d31bb41 to adf0c18 Compare June 12, 2019 15:12

kushaldas suggested changes Jun 13, 2019

View reviewed changes

conorsch reviewed Jun 13, 2019

View reviewed changes

rmol added 2 commits June 14, 2019 14:39

Use cron-apt in upgrade test. Drop python3-dev dependency.

b27ef8d

Hack to get securedrop-app-code installed via cron-apt

6b73b27

redshiftzero reviewed Jun 14, 2019

View reviewed changes

rmol closed this Jul 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bundle mod_wsgi in securedrop-app-code package #4518

Bundle mod_wsgi in securedrop-app-code package #4518

rmol commented Jun 10, 2019 •

edited

Loading

conorsch left a comment

conorsch Jun 10, 2019

rmol Jun 11, 2019

rmol Jun 11, 2019

conorsch Jun 10, 2019

rmol Jun 11, 2019

rmol commented Jun 11, 2019

kushaldas left a comment

conorsch left a comment

conorsch Jun 13, 2019

rmol Jun 13, 2019

conorsch Jun 13, 2019

rmol Jun 13, 2019

conorsch Jun 13, 2019

rmol Jun 13, 2019

rmol commented Jun 13, 2019

rmol commented Jun 13, 2019

conorsch commented Jun 14, 2019

rmol commented Jun 14, 2019

redshiftzero Jun 14, 2019

rmol Jun 17, 2019

eloquence Jun 17, 2019

eloquence commented Jun 17, 2019

rmol commented Jul 30, 2019

Bundle mod_wsgi in securedrop-app-code package #4518

Bundle mod_wsgi in securedrop-app-code package #4518

Conversation

rmol commented Jun 10, 2019 • edited Loading

Status

Description of Changes

Testing

Clean install

Upgrade behavior

Deployment

Checklist

If you made changes to the system configuration:

If you made non-trivial code changes:

conorsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmol commented Jun 11, 2019

kushaldas left a comment

Choose a reason for hiding this comment

conorsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmol commented Jun 13, 2019

rmol commented Jun 13, 2019

conorsch commented Jun 14, 2019

Testing

Clean install

Upgrade behavior

rmol commented Jun 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eloquence commented Jun 17, 2019

rmol commented Jul 30, 2019

rmol commented Jun 10, 2019 •

edited

Loading