Create securedrop-app-code package with dh-virtualenv and mod_wsgi #4622

rmol · 2019-07-17T18:39:28Z

Status

Ready for review

Description of Changes

Building on #4555, change the Debian packaging of securedrop-app-code to use dh-virtualenv, allowing us to embed mod_wsgi in the package.

This means we don't have to include wheels and pip install them during postinst. Not requiring python3-pip, which isn't available in the apt repositories in /etc/apt/security.list, means upgrading to Python 3 is going to be smoother.

The securedrop-app-code Debian package now Conflicts/Replaces libapache2-mod-wsgi, so that package will be removed when this new package is installed.

This requires updates to the builder Docker image, so I've made changes under molecule/builder-xenial to support building the Debian packages with a local image.

Fixes #4355 .

Testing

Building Debian packages

Check out this branch.
cd molecule/builder-xenial
make build-container

This will create a Docker image for building the new securedrop-app-code package. You will need to copy the image name from the last line of the output, e.g.:
```
Successfully tagged quay.io/freedomofpress/sd-docker-builder-xenial:2019_07_16
```
Then, to use this image when building the .debs, instead of pulling an image from quay.io based on the content of molecule/builder-xenial/image_hash, set the BUILDER_IMAGE environment variable:

export BUILDER_IMAGE="quay.io/freedomofpress/sd-docker-builder-xenial:2019_07_16"
In the root of your securedrop working copy, run make build-debs. There should be no errors.

Upgrade testing

Create the upgrade test VMs with make upgrade-start. Near the end of the output you will find the source interface onion address, e.g.:

TASK [Spit out tor details] ****************************************************
 ok: [localhost] => {
     "msg": "Source interface available at ybbhys7etvuoig5l.onion"
 }

Visit the source interface in the Tor browser to verify that it's working at version 0.14.0.
Log in to the upgrade app server with molecule login -s upgrade -h app-staging.
Get the journalist interface onion address: sudo cat /var/lib/tor/services/journalist/hostname
Add that address (prefixed with HidServAuth) to the torrc of your local system Tor or Tor Browser, and verify that the journalist interface is also working at 0.14.0.
Back on the upgrade app server, simulate an automatic nightly upgrade: sudo cron-apt -i -s

Check that all the SD packages are at 1.0.0~rc1 with dpkg -l 'securedrop*':

ii  securedrop-app-code                                1.0.0~rc1+xenial
ii  securedrop-config                                  0.1.3+1.0.0~rc1
ii  securedrop-grsec                                   4.4.182
ii  securedrop-keyring                                 0.1.3+1.0.0~rc1
ii  securedrop-ossec-agent                             3.0.0+1.0.0~rc1
un  securedrop-ossec-server                            <none>

Confirm that libapache2-mod-wsgi was removed: dpkg -l 'libapache2-mod-wsgi' should show this:

rc  libapache2-mod-wsgi                                4.3.0-1.1build1                amd64                          Python WSGI adapter module for Apache

Verify that the source and journalist interfaces are still functioning and are now at version 1.0.0~rc1.

Configuration testing

make staging
molecule verify -s libvirt-staging-xenial

Deployment

We intend for this change to be accomplished seamlessly in the automatic nightly updates. The content of the securedrop-app-code package is changing significantly, as instead of bundling wheels to be installed at the system level with pip install, we're moving all of our dependencies into a virtualenv, including mod_wsgi, which has previously been obtained via the system packages.

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 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

kushaldas

We are also missing the proper copyright file, as we are distributing different modules inside of our package, we should create/use proper package copyright file.

We are also missing securedrop-app-code.triggers file.

We should build for the same path in the future versions of dh-virtualenv, https://dh-virtualenv.readthedocs.io/en/0.11/usage.html#building-packages-with-dh-virtualenv is document for the same.

export DH_VIRTUALENV_INSTALL_ROOT=/opt/venvs

MANIFEST.in

kushaldas · 2019-07-18T08:49:15Z

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

+- name: Copy entire SecureDrop repo to build path.
+  synchronize:
+    src: "{{ role_path }}/../../../../"
+    dest: "{{ securedrop_app_code_deb_dir }}"


We should only have the securedrop-1.0.0.tar.gz copied into the right place.

Doesn't dh-virtualenv need the source tree, for setup.py at least?

No, we need a release tarball (made using setup.py) to build debian packages.

I still don't see how the sdist tarball is required, but the latest change uses it to create the directory where the package is built.

We will have to create proper Debian packages following standard guidelines which is reproducible. Having a released source tar ball is the first step there.

OK. It sounds to me like extracting that tarball from this process and publishing it is going to require work that I'd consider beyond the scope of this issue and PR. Could you file a separate issue with more details on the requirements for reproducibility?

Yes, for this PR we will just have to fetch the tarball we are creating into the build directory. Making a comment in next few minutes.

kushaldas · 2019-07-18T08:49:48Z

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

- name: Create new requirements based on build/installed wheels without hashes
-  shell:
-    pip freeze > {{ securedrop_pip_requirements_generated }}
+    pip3 install -r {{ securedrop_pip_requirements }}


We should not install any dependency as part of packaging. This has to be handled by the dh-virtualenv.

This is installing our requirements in the build container, so we can do things like run i18n_tool.py. I don't believe this affects dh-virtualenv, but if I'm wrong, please set me straight.

Yes, we should not run i18_tool.py on the build step, everything should be baked and ready inside of the release tarball.

It has to be run at some point when building this package, or we won't get the .mo files. I've modified the playbook to ensure that the .mo files are added to the sdist tarball, and eventually still make it into the .deb.

emkll

I have built, uploaded and pinned the hashes for a new builder image based on the Dockerfile contained in this branch. Note that I did not create a new container registry repository to host this new image. We could consider creating a sd-builder-xenial-python3 repository if we need to maintain both the python2 and python3 path (with some added logic/complexity in build scripts).

I managed to build the debs locally, though i did get a weird error that did not seem to affect the build, right after the create step:

    TASK [Create builders] *********************************************************
    changed: [localhost] => (item={'name': 'xenial-sd-app', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-generic-ossec-agent', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-generic-ossec-server', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-generic-ossec-agent2', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-generic-ossec-server2', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-grsec', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-config', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-keyring', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-sec-update', 'groups': ['builders']})
    changed: [localhost] => (item={'name': 'xenial-sd-dpkg-verification', 'groups': ['testers']})
    
    PLAY RECAP *********************************************************************
    localhost                  : ok=2    changed=1    unreachable=0    failed=0
    
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable

Also added a couple of comments inline.

emkll · 2019-07-18T14:03:57Z

install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2

@@ -51,6 +51,9 @@
  /etc/apache2/mods-available/rewrite.load r,


Perhaps a nit: From the diff, it seems like there are a lof of changes here. On closer inspection, some entries are simply reordered. Would it be possible to keep the diff as minimal as possible here to more easily understand the changes needed for the new build process?

@emkll I ordered them because I found changing the unsorted rules error-prone, and thought having related rules grouped would make changes (and review 🙂) easier. The diff is still fairly sizable without reordering, unfortunately.

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

emkll · 2019-07-18T14:12:04Z

install_files/securedrop-app-code/debian/control

-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
+Conflicts: libapache2-mod-wsgi
+Replaces: libapache2-mod-wsgi
+Depends: ${python3:Depends},${misc:Depends},apache2,paxctld,apparmor-utils,gnupg2,haveged,secure-delete,sqlite3,${dist:Depends},libapache2-mod-xsendfile,redis-server,supervisor,securedrop-keyring,securedrop-config


it seems here like the 2 extra requirements are ${python3:Depends} and ${misc:Depends}. Do we know what requirements these include? The reason I ask is that the current cron-apt script uses the security list to pull in packages:

securedrop/install_files/ansible-base/roles/common/files/5-security

Line 2 in a1c2e2a

dist-upgrade -y -o APT::Get::Show-Upgraded=true -o Dir::Etc::SourceList=/etc/apt/security.list -o Dpkg::Options::=--force-confdef -o Dpkg::Options::=--force-confold

. If the dependencies are not in that list, the install will fail. I will test the upgrade scenario once I upload the builder image

Those are standard Python3 and if we add anything special based on debian packaging guides.

rmol · 2019-07-18T14:58:26Z

I managed to build the debs locally, though i did get a weird error that did not seem to affect the build, right after the create step:
Exception ignored in: <function WeakValueDictionary.__init__.<locals>.remove at 0x7f19673d0e18>
Traceback (most recent call last):
  File "/home/m/.virtualenvs/securedrop3/lib/python3.5/weakref.py", line 117, in remove
TypeError: 'NoneType' object is not callable

This is a Python 3 bug, still present in the version on Xenial, but it seems to have no effect on Ansible.

conorsch · 2019-07-22T19:37:14Z

devops/gce-nested/gce-runner.sh

@@ -51,10 +51,10 @@ function copy_securedrop_repo() {

 # Main logic
 copy_securedrop_repo
+ssh_gce "sudo apt install python3-dev build-essential"


During standup today, @emkll noted that this line is causing CI to fail. The problem is two-fold:

the dependencies listed here should be baked into the base box used for CI (to save time on CI runs)

the sdci user running these commands lacks sudo rights, so ad-hoc apt install tasks will always fail

Both should be resolved. Will handle as part of review. The changes to the base box won't be reflected in the diff for this PR, but once confirmed working, we can drop the sudo apt install task here prior to merge.

conorsch

Ran through test plan. After creating a new py3-based virtualenv and installing the deps, was able to build the debs as described, and kick off an upgrade scenario. During the verification steps on the upgrade, scenario, however, I encountered a 500 on the Journalist Interface only.

Apache Journalist error log events for 500

[Mon Jul 22 20:02:42.498840 2019] [wsgi:info] [pid 8725:tid 3524817938176] mod_wsgi (pid=8725): Adding '/var/www/securedrop' to path.
[Mon Jul 22 20:02:42.505332 2019] [wsgi:info] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094] mod_wsgi (pid=8725, process='journalist', application='app-staging:8080|'): Loading Python script file '/var/www/journalist.wsgi'.
[Mon Jul 22 20:02:43.366281 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094] ERROR:flask.app:Exception on / [GET]
[Mon Jul 22 20:02:43.366357 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094] Traceback (most recent call last):
[Mon Jul 22 20:02:43.366372 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/app.py", line 2292, in wsgi_app
[Mon Jul 22 20:02:43.366380 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     response = self.full_dispatch_request()
[Mon Jul 22 20:02:43.366386 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/app.py", line 1815, in full_dispatch_request
[Mon Jul 22 20:02:43.366393 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     rv = self.handle_user_exception(e)
[Mon Jul 22 20:02:43.366399 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/app.py", line 1718, in handle_user_exception
[Mon Jul 22 20:02:43.366405 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     reraise(exc_type, exc_value, tb)
[Mon Jul 22 20:02:43.366410 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/_compat.py", line 35, in reraise
[Mon Jul 22 20:02:43.366416 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     raise value
[Mon Jul 22 20:02:43.366422 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/app.py", line 1813, in full_dispatch_request
[Mon Jul 22 20:02:43.366428 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     rv = self.dispatch_request()
[Mon Jul 22 20:02:43.366434 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/app.py", line 1799, in dispatch_request
[Mon Jul 22 20:02:43.366440 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     return self.view_functions[rule.endpoint](**req.view_args)
[Mon Jul 22 20:02:43.366446 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/var/www/securedrop/journalist_app/main.py", line 81, in index
[Mon Jul 22 20:02:43.366454 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     starred=starred)
[Mon Jul 22 20:02:43.366459 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/templating.py", line 135, in render_template
[Mon Jul 22 20:02:43.368266 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     context, ctx.app)
[Mon Jul 22 20:02:43.368351 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask/templating.py", line 117, in _render
[Mon Jul 22 20:02:43.368418 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     rv = template.render(context)
[Mon Jul 22 20:02:43.368480 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/jinja2/environment.py", line 1008, in render
[Mon Jul 22 20:02:43.368551 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     return self.environment.handle_exception(exc_info, True)
[Mon Jul 22 20:02:43.368608 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/jinja2/environment.py", line 780, in handle_exception
[Mon Jul 22 20:02:43.368666 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     reraise(exc_type, exc_value, tb)
[Mon Jul 22 20:02:43.368868 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/jinja2/_compat.py", line 37, in reraise
[Mon Jul 22 20:02:43.368948 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     raise value.with_traceback(tb)
[Mon Jul 22 20:02:43.369029 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/var/www/securedrop/journalist_templates/index.html", line 1, in top-level template code
[Mon Jul 22 20:02:43.369095 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     {% extends "base.html" %}
[Mon Jul 22 20:02:43.369141 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/var/www/securedrop/journalist_templates/base.html", line 49, in top-level template code
[Mon Jul 22 20:02:43.369196 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     {% block body %}{% endblock %}
[Mon Jul 22 20:02:43.369237 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/var/www/securedrop/journalist_templates/index.html", line 8, in block "body"
[Mon Jul 22 20:02:43.369277 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     <input name="csrf_token" type="hidden" value="{{ csrf_token() }}">
[Mon Jul 22 20:02:43.369318 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/flask_wtf/csrf.py", line 47, in generate_csrf
[Mon Jul 22 20:02:43.369358 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     setattr(g, field_name, s.dumps(session[field_name]))
[Mon Jul 22 20:02:43.369398 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/itsdangerous.py", line 565, in dumps
[Mon Jul 22 20:02:43.369439 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     payload = want_bytes(self.dump_payload(obj))
[Mon Jul 22 20:02:43.369479 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/itsdangerous.py", line 847, in dump_payload
[Mon Jul 22 20:02:43.369519 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     json = super(URLSafeSerializerMixin, self).dump_payload(obj)
[Mon Jul 22 20:02:43.369567 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/itsdangerous.py", line 550, in dump_payload
[Mon Jul 22 20:02:43.369623 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     return want_bytes(self.serializer.dumps(obj))
[Mon Jul 22 20:02:43.369680 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages/itsdangerous.py", line 51, in dumps
[Mon Jul 22 20:02:43.369721 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     return json.dumps(obj, separators=(',', ':'))
[Mon Jul 22 20:02:43.369762 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/usr/lib/python3.5/json/__init__.py", line 237, in dumps
[Mon Jul 22 20:02:43.369882 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     **kw).encode(obj)
[Mon Jul 22 20:02:43.369949 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/usr/lib/python3.5/json/encoder.py", line 198, in encode
[Mon Jul 22 20:02:43.370012 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     chunks = self.iterencode(o, _one_shot=True)
[Mon Jul 22 20:02:43.370072 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/usr/lib/python3.5/json/encoder.py", line 256, in iterencode
[Mon Jul 22 20:02:43.370135 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     return _iterencode(o, 0)
[Mon Jul 22 20:02:43.370196 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]   File "/usr/lib/python3.5/json/encoder.py", line 179, in default
[Mon Jul 22 20:02:43.370258 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094]     raise TypeError(repr(o) + " is not JSON serializable")
[Mon Jul 22 20:02:43.370327 2019] [wsgi:error] [pid 8725:tid 3524817938176] [remote 127.0.0.1:38094] TypeError: b'465efea4f735dc4695a27da24d90d42b1ecc896d' is not JSON serializable

Looks to my eye like classic Py2/Py3 string encoding errors. First off, let's try to reproduce that error on the JI, and if so, resolve the problem. Once that's addressed, happy to return for vigorous interactive testing.

Regarding the CI box, I've built and pushed up a new base image. Will bounce CI to trigger a re-run, and monitor results on the staging job while code changes are hammered out.

rmol · 2019-07-22T23:00:26Z

@conorsch I was able to reproduce the error you saw. I believe it's this flask-wtf issue, which I suspect I managed to avoid through sheer luck because I was bouncing Tor Browser so much during testing -- if you clear cookies or restart the browser, the site works again.

They have a fix on master, but haven't cut a release including it.

I'm going to look into adding an error handler that could detect this case. The other options (just noting it in the release notes, or rolling our own requirement with the fix included while maintaining hash checks) seem less desirable.

kushaldas · 2019-07-23T14:49:52Z

I had a chat with @conorsch about a few more required changes into this PR.

git mv the apparmor.d files and also the wsgi files into a top level files/ directory in the source (as shown in my poc branch), and then include them into the source tarball using MANIFEST.in file.
Copy the source tarball out from the container into the build directory, just like we do for the debian packages.

rmol · 2019-07-23T14:53:44Z

Why is the top-level files directory required?

kushaldas · 2019-07-23T14:57:26Z

Why is the top-level files directory required?

Because we need a clean directory structure, otherwise they will inside of the ./install_files/securedrop-app-code/ and ./install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/ directories of the source tree (of the tarball).

conorsch · 2019-07-23T14:58:34Z

Agreed with @kushaldas's points. The POC (#4555) simply copy/pasted the relevant files, rather than using git mv, requiring us to create another branch to implement, which you've done here. While I don't think it's strictly necessary to have a top-level "files/" dir (@kushaldas chime in here if you feel strongly), including the files somewhere within the packaging tree makes sense. When rearranging the apparmor files to be included in the source tarball, make sure:

Ansible doesn't reference them anymore (we'll trust the deb pkg to enforce state)
Ensure that the deb package does not consider the apparmor files as conffiles, which it will by default (due to location within /etc/), unless we override it. Make sure we have tests to confirm this, because a mistake will block adjustments to the apparmor profiles via unattended-upgrades.

Copy the source tarball out from the container into the build directory

To be clear, we don't plan on doing anything with that tarball after it's been fetched back to the host, at least not yet—it's simply a small step toward the story of reproducibility, when we'll likely start signing and uploading the raw tarball as part of the release process. Simply plunking the tarball into build/ alongside the deb, and ensuring it's gitignored, would be sufficient for the context of this PR.

I'm going to look into adding an error handler that could detect this case.

Great research on the bug @rmol, I agree that's the most likely culprit. Eager to see what you come up with as a stopgap. Ping when ready for more interactive testing, happy to take another look at the upgrade story!

kushaldas · 2019-07-23T15:03:18Z

While I don't think it's strictly necessary to have a top-level "files/" dir (@kushaldas chime in here if you feel strongly),

Having a files/ directory enables us to put any such random files there, we actually have it already in ./install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg right now :)

conorsch · 2019-07-23T15:17:34Z

Having a files/ directory enables us to put any such random files there

So, the files/ dir need not be top-level, i.e. at the same level as the git repo root, correct? Makes sense to me to place the files/ dir at, say, install_files/securedrop-app-code/files/; that shouldn't be automatically folded into /files/ without corresponding declaration in the .install file.

rmol · 2019-07-23T15:28:08Z

Could one of you document for posterity the benefits of publishing the separate tarball over just the Debian package, as far as verifiability or reproducibility? How would someone installing SecureDrop use it to gain assurance about the Debian package being applied?

kushaldas · 2019-07-23T15:30:18Z

So, the files/ dir need not be top-level, i.e. at the same level as the git repo root, correct?

When I said top level, I meant git repo root.

conorsch · 2019-07-23T15:37:18Z

When I said top level, I meant git repo root.

Given that this repository is used for building many different Debian packages, that sounds potentially confusing. Do you see a disadvantage to storing the files/ dir adjacent to the rest of the packaging logic for the securedrop-app-code package?

conorsch · 2019-07-23T15:42:41Z

Could one of you document for posterity the benefits of publishing the separate tarball over just the Debian package, as far as verifiability or reproducibility? How would someone installing SecureDrop use it to gain assurance about the Debian package being applied?

In the context of the current ask—i.e. "fetch tarball back to controller"—having the tarball accessible to developers will likely aid in debugging, particularly as we transition to the new build logic. In the future, when we're ready to make a concerted push toward reproducibility for all the SD ecosystem components, we can discuss the best way to provide transparency throughout the build process.

One reason to consider publishing the tarball along with the release object is that we can add a detached signature file, so external parties can verify the sig on that tarball, then use that same tarball to build a debian package, and the debian package should match what's served up by the apt repo. The deb packages themselves are not signed by apt—only the corresponding Release file. Whether it's worthwhile to publish signed tarballs as part of the release process does indeed warrant further debate, @rmol!

kushaldas · 2019-07-23T15:44:23Z

Given that this repository is used for building many different Debian packages, that sounds potentially confusing. Do you see a disadvantage to storing the files/ dir adjacent to the rest of the packaging logic for the securedrop-app-code package?

They have to come inside of the release source code tarball, we can try to do that with current location and then see how it goes.

rmol · 2019-07-23T20:32:57Z

I've added retrieval of the tarball used for building the securedrop-app-code .deb to the playbook. It is currently not very reproducible, as the contents of the package produced by extracting it and running dpkg-buildpackage on it differ from the playbook product.

I did find a solution for the CSRF token TypeError during the upgrade. It was right in the comments of the flask-wtf issue: simply deleting the token from the session in our before_request handler seems to solve the problem neatly.

conorsch · 2019-07-24T14:36:24Z

I did find a solution for the CSRF token TypeError during the upgrade. It was right in the comments of the flask-wtf issue: simply deleting the token from the session in our before_request handler seems to solve the problem neatly.

Confirmed via upgrade testing scenario! After running sudo cron-apt -i -s, a currently-logged-in journalist user is still able to interact with the Journalist Interface, even on 1.0.0~rc1. Great work, @rmol!

500 error as reported has been resolved

kushaldas · 2019-07-24T15:34:02Z

Manually verified the tarball contents and also the debian/ files. @rmol this is great work. I will do the installation and test out upgrade steps in the morning.

Meanwhile, do you know why those app tests are failing?

rmol · 2019-07-24T22:20:51Z

Meanwhile, do you know why those app tests are failing?

The deletion of the CSRF token when it had type bytes was causing an error if there were no csrf_token key in the session. I fixed that.

msheiny · 2019-07-25T16:56:31Z

Re-kicked off tests to scope things working with a newer GCP image

msheiny · 2019-07-25T17:16:01Z

Looks like this is failing until it gets a rebase on #4641

This needs to be moved to a better location for our existing build commands. We will also have to fix the configuration file marking the debian/* files.

Building on freedomofpress#4555, change the Debian packaging of securedrop-app-code to use dh-virtualenv, allowing us to embed mod_wsgi in the package. This means we don't have to include wheels and pip install them during postinst. Not requiring python3-pip, which isn't available in the apt repositories in /etc/apt/security.list, means upgrading to Python 3 is going to be smoother. The securedrop-app-code Debian package now Conflicts/Replaces libapache2-mod-wsgi. It will be removed when this new package is installed. This requires updates to the builder Docker image, so I've made changes under molecule/builder-xenial to support building the Debian packages with a local image. Noting for future removal: this includes a workaround for pallets-eco/flask-wtf#275 -- after upgrading from Python 2 to Python 3, any existing session's csrf_token value will be retrieved as bytes, causing a TypeError. This simple fix, deleting the existing token, was suggested in the issue comments. This code will be safe to remove after Python 2 reaches EOL in 2020, and no supported SecureDrop installations can still have this problem.

The supervisor configuration for our rq worker has been done in the installation playbook. With dh-virtualenv rq is being installed in a different location, so it makes sense for the supervisor configuration to happen when the securedrop-app-code package is installed.

codecov-io · 2019-07-30T18:03:08Z

Codecov Report

Merging #4622 into develop will decrease coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           develop   #4622      +/-   ##
==========================================
- Coverage    82.72%   82.6%   -0.12%     
==========================================
  Files           45      45              
  Lines         3120    3122       +2     
  Branches       337     338       +1     
==========================================
- Hits          2581    2579       -2     
- Misses         453     456       +3     
- Partials        86      87       +1

Impacted Files	Coverage Δ
securedrop/securedrop/source_app/utils.py	`85.96% <0%> (-3.51%)`	⬇️
securedrop/securedrop/journalist_app/__init__.py	`87.87% <0%> (-1.71%)`	⬇️
securedrop/securedrop/i18n_tool.py	`85.23% <0%> (-0.07%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5eaad3b...6c25664. Read the comment docs.

kushaldas

This looks good now, approved. @rmol Thank you for this amazing work 🥇

rmol requested review from conorsch, emkll, heartsucker, kushaldas, msheiny and redshiftzero as code owners July 17, 2019 18:39

rmol force-pushed the dh-virtualenv-mod-wsgi branch 2 times, most recently from cc4470f to ab205b1 Compare July 17, 2019 19:53

kushaldas suggested changes Jul 18, 2019

View reviewed changes

emkll reviewed Jul 18, 2019

View reviewed changes

conorsch reviewed Jul 22, 2019

View reviewed changes

conorsch self-requested a review July 22, 2019 20:20

conorsch previously requested changes Jul 22, 2019

View reviewed changes

rmol force-pushed the dh-virtualenv-mod-wsgi branch from b16ff22 to df25003 Compare July 24, 2019 17:34

conorsch mentioned this pull request Jul 24, 2019

Latest Xenial vagrant boxes fail on staging #4637

Closed

rmol force-pushed the dh-virtualenv-mod-wsgi branch 2 times, most recently from c7837ed to 9bc6f26 Compare July 25, 2019 13:58

rmol force-pushed the dh-virtualenv-mod-wsgi branch from 597bed2 to cb497e4 Compare July 25, 2019 19:42

rmol mentioned this pull request Jul 30, 2019

Bundle mod_wsgi in securedrop-app-code package #4518

Closed

2 tasks

kushaldas and others added 4 commits July 30, 2019 13:13

files required to create a source tarball release of securedrop-app-code

8543105

Adds debian directory for packaing

2714b39

This needs to be moved to a better location for our existing build commands. We will also have to fix the configuration file marking the debian/* files.

rmol force-pushed the dh-virtualenv-mod-wsgi branch from cb497e4 to 6c25664 Compare July 30, 2019 17:15

kushaldas approved these changes Jul 31, 2019

View reviewed changes

kushaldas merged commit ff95b55 into freedomofpress:develop Jul 31, 2019

This was referenced Aug 1, 2019

[RFE] Isolate yourself from the system Python #3407

Closed

Create debian package based on dh-virtualenv #4555

Closed

eloquence mentioned this pull request Aug 15, 2019

Install libapache2-mod-wsgi-py3, which can only be installed if libapache2-mod-wsgi is not there #4292

Closed

brainwane mentioned this pull request Oct 7, 2019

Add SecureDrop python3statement/python3statement.github.io#267

Closed

eloquence mentioned this pull request Jun 11, 2020

Upgrading securedrop-app-code should remove unused Python dependencies #856

Closed

eloquence mentioned this pull request Dec 3, 2020

Remove paths that are no longer in use #5665

Merged

		@@ -51,6 +51,9 @@
		/etc/apache2/mods-available/rewrite.load r,

Create securedrop-app-code package with dh-virtualenv and mod_wsgi #4622

Create securedrop-app-code package with dh-virtualenv and mod_wsgi #4622

Conversation

rmol commented Jul 17, 2019 • edited Loading

Status

Description of Changes

Testing

Building Debian packages

Upgrade testing

Configuration testing

Deployment

Checklist

If you made changes to the server application code:

If you made changes to the system configuration:

If you made non-trivial code changes:

kushaldas left a comment • edited Loading

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

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

emkll 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

rmol commented Jul 18, 2019

Choose a reason for hiding this comment

conorsch left a comment

Choose a reason for hiding this comment

rmol commented Jul 22, 2019

kushaldas commented Jul 23, 2019

rmol commented Jul 23, 2019

kushaldas commented Jul 23, 2019

conorsch commented Jul 23, 2019

kushaldas commented Jul 23, 2019

conorsch commented Jul 23, 2019

rmol commented Jul 23, 2019

kushaldas commented Jul 23, 2019

conorsch commented Jul 23, 2019

conorsch commented Jul 23, 2019

kushaldas commented Jul 23, 2019

rmol commented Jul 23, 2019

conorsch commented Jul 24, 2019

kushaldas commented Jul 24, 2019

rmol commented Jul 24, 2019

msheiny commented Jul 25, 2019

msheiny commented Jul 25, 2019

codecov-io commented Jul 30, 2019

Codecov Report

kushaldas left a comment

Choose a reason for hiding this comment

rmol commented Jul 17, 2019 •

edited

Loading

kushaldas left a comment •

edited

Loading