diff --git a/MANIFEST.in b/MANIFEST.in index 48691e6892..9d19a1c7a2 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,5 +1,6 @@ graft debian graft etc +graft lib graft var prune var/www/securedrop/tests include requirements.txt diff --git a/docs/backup_and_restore.rst b/docs/backup_and_restore.rst index d1c931971b..1e6f74f310 100644 --- a/docs/backup_and_restore.rst +++ b/docs/backup_and_restore.rst @@ -41,7 +41,7 @@ currently on the *Application Server*: log in over SSH and run queued deletion jobs by logging in to the *Application Server* over SSH and running:: - sudo tail -f /var/log/securedrop_worker/rqworker.err + sudo journalctl -u securedrop_rqworker If you find you cannot perform a backup or restore due to this constraint, @@ -85,8 +85,8 @@ to debug your connectivity before proceeding further. Make sure: * Ansible is installed * the *Admin Workstation* is connected to the Internet * Tor starts successfully -* The appropriate onion service configuration files are present in - ``~/Persistent/securedrop/install_files/ansible-base`` and the +* The appropriate onion service configuration files are present in + ``~/Persistent/securedrop/install_files/ansible-base`` and the ``./securedrop-admin tailsconfig`` command completes successfully If Ansible is not installed, or ``./securedrop-admin tailsconfig`` fails, see diff --git a/install_files/ansible-base/roles/app/handlers/main.yml b/install_files/ansible-base/roles/app/handlers/main.yml index b54e86fe0b..d795106e22 100644 --- a/install_files/ansible-base/roles/app/handlers/main.yml +++ b/install_files/ansible-base/roles/app/handlers/main.yml @@ -14,16 +14,6 @@ name: apache2 state: restarted -- name: reload supervisor - supervisorctl: - name: securedrop_worker - state: present - -- name: reload supervisor - supervisorctl: - name: securedrop_rqrequeue - state: present - ## Here, we list apparmor before haveged to ensure the correct AppArmor ## profile is loaded prior to restarting haveged - name: restart apparmor diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml index af7bf662b8..6202d43f53 100644 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/defaults/main.yml @@ -16,10 +16,14 @@ securedrop_venv_site_packages: "{{ securedrop_venv }}/lib/python3.5/site-package # SecureDrop code installation directory securedrop_code: /var/www/securedrop -# Configuration files for SecureDrop programs run under supervisord -supervisor_programs: - - securedrop_rqrequeue.conf - - securedrop_worker.conf +# Location of the application storage on disk, including database. +securedrop_data: /var/lib/securedrop + +# Configuration files for SecureDrop systemd services +systemd_services: + - securedrop_rqrequeue.service + - securedrop_rqworker.service + - securedrop_shredder.service # SecureDrop rq worker log directory securedrop_worker_log_dir: /var/log/securedrop_worker diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2 b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2 index e8fbf1d383..f5f34c9d6f 100644 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2 +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/files/usr.sbin.apache2 @@ -102,7 +102,6 @@ /usr/lib{,32,64}/** mr, /usr/share/file/magic r, /usr/share/file/magic.mgc r, - /usr/share/pyshared/supervisor-*-nspkg.pth r, /opt/venvs/securedrop-app-code/**/__pycache__/ rw, /opt/venvs/securedrop-app-code/**/__pycache__/* rw, /opt/venvs/securedrop-app-code/bin/python3 r, @@ -132,6 +131,8 @@ /var/lib/securedrop/keys/secring.gpg.tmp rw, /var/lib/securedrop/keys/trustdb.gpg rw, /var/lib/securedrop/keys/trustdb.gpg.lock rwl, + /var/lib/securedrop/shredder/** rw, + /var/lib/securedrop/shredder/*/ w, /var/lib/securedrop/store/** rw, /var/lib/securedrop/store/*/ w, /var/lib/securedrop/tmp/** rw, diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/main.yml b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/main.yml index abdb6c7b75..b639d34c7e 100644 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/main.yml +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/tasks/main.yml @@ -57,19 +57,19 @@ src: "changelog-{{ securedrop_package_dist }}" dest: "{{ securedrop_app_code_prep_dir }}/debian/changelog" -- name: Create supervisor conf.d directory in prep directory +- name: Create lib/systemd/services directory in prep directory file: state: directory - dest: "{{ securedrop_app_code_prep_dir }}/etc/supervisor/conf.d" - tags: supervisor + dest: "{{ securedrop_app_code_prep_dir }}/lib/systemd/system" + tags: systemd -- name: Copy supervisor program configurations to prep path +- name: Copy systemd service configurations to prep path template: src: "{{ item }}" - dest: "{{ securedrop_app_code_prep_dir }}/etc/supervisor/conf.d/{{ item }}" + dest: "{{ securedrop_app_code_prep_dir }}/lib/systemd/system/{{ item }}" mode: 0644 - with_items: "{{ supervisor_programs }}" - tags: supervisor + with_items: "{{ systemd_services }}" + tags: systemd - name: Create sdist in prep dir command: python3 setup.py sdist diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqrequeue.conf b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqrequeue.conf deleted file mode 100644 index f668bb0998..0000000000 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqrequeue.conf +++ /dev/null @@ -1,10 +0,0 @@ -[program:securedrop_rqrequeue] -command={{ securedrop_venv_bin }}/python {{ securedrop_code }}/scripts/rqrequeue --interval 60 -directory={{ securedrop_code }} -environment=PYTHONPATH="{{ securedrop_code }}:{{ securedrop_venv_site_packages }}" -autostart=true -autorestart=true -startretries=3 -stderr_logfile={{ securedrop_worker_log_dir }}/rqrequeue.err -stdout_logfile={{ securedrop_worker_log_dir }}/rqrequeue.out -user={{ securedrop_user }} diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqrequeue.service b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqrequeue.service new file mode 100644 index 0000000000..b04b208f41 --- /dev/null +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqrequeue.service @@ -0,0 +1,21 @@ +[Unit] +Description=SecureDrop rqrequeue process +After=redis-server.service +Wants=redis-server.service + +[Service] +Environment=PYTHONPATH="{{ securedrop_code }}:{{ securedrop_venv_site_packages }}" +ExecStart={{ securedrop_venv_bin }}/python {{ securedrop_code }}/scripts/rqrequeue --interval 60 +PrivateDevices=yes +PrivateTmp=yes +ProtectSystem=full +ReadOnlyDirectories=/ +ReadWriteDirectories={{ securedrop_data }} +Restart=always +RestartSec=10s +UMask=077 +User=www-data +WorkingDirectory={{ securedrop_code }} + +[Install] +WantedBy=multi-user.target diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqworker.service b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqworker.service new file mode 100644 index 0000000000..9b43f72415 --- /dev/null +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_rqworker.service @@ -0,0 +1,21 @@ +[Unit] +Description=SecureDrop rq worker +After=redis-server.service +Wants=redis-server.service + +[Service] +Environment=PYTHONPATH="{{ securedrop_code }}:{{ securedrop_venv_site_packages }}" +ExecStart={{ securedrop_venv_bin }}/rqworker +PrivateDevices=yes +PrivateTmp=yes +ProtectSystem=full +ReadOnlyDirectories=/ +ReadWriteDirectories={{ securedrop_data }} +Restart=always +RestartSec=10s +UMask=077 +User=www-data +WorkingDirectory={{ securedrop_code }} + +[Install] +WantedBy=multi-user.target diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_shredder.service b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_shredder.service new file mode 100644 index 0000000000..53b219368c --- /dev/null +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_shredder.service @@ -0,0 +1,19 @@ +[Unit] +Description=SecureDrop shredder + +[Service] +Environment=PYTHONPATH="{{ securedrop_code }}:{{ securedrop_venv_site_packages }}" +ExecStart={{ securedrop_venv_bin }}/python {{ securedrop_code }}/scripts/shredder --interval 60 +PrivateDevices=yes +PrivateTmp=yes +ProtectSystem=full +ReadOnlyDirectories=/ +ReadWriteDirectories={{ securedrop_data }} +Restart=always +RestartSec=10s +UMask=077 +User=www-data +WorkingDirectory={{ securedrop_code }} + +[Install] +WantedBy=multi-user.target diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_worker.conf b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_worker.conf deleted file mode 100644 index c7a997b47e..0000000000 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/securedrop_worker.conf +++ /dev/null @@ -1,10 +0,0 @@ -[program:securedrop_worker] -command={{ securedrop_venv_bin }}/rqworker -directory={{ securedrop_code }} -environment=PYTHONPATH="{{ securedrop_code }}:{{ securedrop_venv_site_packages }}" -autostart=true -autorestart=true -startretries=3 -stderr_logfile={{ securedrop_worker_log_dir }}/rqworker.err -stdout_logfile={{ securedrop_worker_log_dir }}/rqworker.out -user={{ securedrop_user }} diff --git a/install_files/securedrop-app-code/debian/control b/install_files/securedrop-app-code/debian/control index 02cc2329b6..07d1ea6274 100644 --- a/install_files/securedrop-app-code/debian/control +++ b/install_files/securedrop-app-code/debian/control @@ -3,13 +3,13 @@ Section: web Priority: optional Maintainer: SecureDrop Team Homepage: https://securedrop.org -Build-Depends: debhelper (>= 9), dh-python, python3-all, python3-setuptools, dh-virtualenv +Build-Depends: debhelper (>= 9), dh-python, python3-all, python3-setuptools, dh-systemd, dh-virtualenv Standards-Version: 3.9.8 X-Python3-Version: >= 3.5 Package: securedrop-app-code Architecture: amd64 -Conflicts: libapache2-mod-wsgi -Replaces: libapache2-mod-wsgi -Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), redis-server, securedrop-config, securedrop-keyring, sqlite3, supervisor -Description: Packages the SecureDrop application code pip dependencies and apparmor profiles. This package will put the apparmor profiles in enforce mode. This package does use pip to install the pip wheelhouse +Conflicts: libapache2-mod-wsgi,supervisor +Replaces: libapache2-mod-wsgi,supervisor +Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), redis-server, securedrop-config, securedrop-keyring, sqlite3 +Description: SecureDrop application code, dependencies, Apache configuration, systemd services, and AppArmor profiles. This package will put the AppArmor profiles in enforce mode. diff --git a/install_files/securedrop-app-code/debian/postinst b/install_files/securedrop-app-code/debian/postinst index 1c05c603ed..3d156a8487 100644 --- a/install_files/securedrop-app-code/debian/postinst +++ b/install_files/securedrop-app-code/debian/postinst @@ -95,13 +95,6 @@ set_paxctld_config() { fi } -mkdir_rqworker_logs() { - worker_logs_dir="/var/log/securedrop_worker" - mkdir -p "$worker_logs_dir" - chown root:root "$worker_logs_dir" - chmod 0700 "$worker_logs_dir" -} - case "$1" in configure) @@ -109,13 +102,11 @@ case "$1" in set_paxctld_config # Ensure SecureDrop's necessary directories are created - for dir in /var/lib/securedrop/{,tmp,store,keys,/keys/private-keys-v1.d,/keys/openpgp-revocs.d,backups} /var/www/securedrop; do + for dir in /var/lib/securedrop/{,tmp,shredder,store,keys,/keys/private-keys-v1.d,/keys/openpgp-revocs.d,backups} /var/www/securedrop; do mkdir -p "$dir" chmod 0700 "$dir" done - mkdir_rqworker_logs - # Ensure required gpg-agent.conf is in place, see #4013. if [ -e "/var/lib/securedrop/keys/gpg-agent.conf" ]; then @@ -186,16 +177,8 @@ case "$1" in # in versions prior to 0.5.1 a custom logo was installed with u-w chmod u+w /var/www/securedrop/static/i/logo.png - # This removes the MAC "hmac-sha1" from sshd_config. - # Ansible was updated, so future instances will not have this line present. - # This if-block may be removed from this script on 2019-01-01. - if grep -qE 'MACs\s.*hmac-sha1' /etc/ssh/sshd_config; then - sed -i 's/^\s*MACs\s.*/MACs hmac-sha2-256,hmac-sha2-512/' /etc/ssh/sshd_config; - fi - - service supervisor restart - database_migration + ;; abort-upgrade|abort-remove|abort-deconfigure) diff --git a/install_files/securedrop-app-code/debian/rules b/install_files/securedrop-app-code/debian/rules index e0eb9bfd6a..a591e56fd1 100755 --- a/install_files/securedrop-app-code/debian/rules +++ b/install_files/securedrop-app-code/debian/rules @@ -5,7 +5,7 @@ DEB_DH_INSTALL_ARGS=-X .git SECUREDROP_BUILD_PLATFORM=$(shell lsb_release -sc) %: - dh $@ --with python-virtualenv + dh $@ --with python-virtualenv --with systemd override_dh_gencontrol: dh_gencontrol -- $(SUBSTVARS) @@ -32,3 +32,20 @@ override_dh_virtualenv: --extra-pip-arg "--ignore-installed" \ --extra-pip-arg "--no-binary=:all:" \ --extra-pip-arg "--no-cache-dir" + +# +# Have to override the automatic service handling since we have more +# than one. +# +override_dh_installinit: + dh_installinit --noscripts + +override_dh_systemd_enable: + dh_systemd_enable --name=securedrop_rqrequeue + dh_systemd_enable --name=securedrop_rqworker + dh_systemd_enable --name=securedrop_shredder + +override_dh_systemd_start: + dh_systemd_start --name=securedrop_rqrequeue + dh_systemd_start --name=securedrop_rqworker + dh_systemd_start --name=securedrop_shredder diff --git a/install_files/securedrop-app-code/debian/securedrop-app-code.install b/install_files/securedrop-app-code/debian/securedrop-app-code.install index 9494853fab..9e1f866b20 100644 --- a/install_files/securedrop-app-code/debian/securedrop-app-code.install +++ b/install_files/securedrop-app-code/debian/securedrop-app-code.install @@ -1,5 +1,6 @@ var/www /var/ etc/apparmor.d/usr.sbin.apache2 /etc/apparmor.d etc/apparmor.d/usr.sbin.tor /etc/apparmor.d -etc/supervisor/conf.d/securedrop_rqrequeue.conf /etc/supervisor/conf.d -etc/supervisor/conf.d/securedrop_worker.conf /etc/supervisor/conf.d \ No newline at end of file +lib/systemd/system/securedrop_rqrequeue.service /lib/systemd/system +lib/systemd/system/securedrop_rqworker.service /lib/systemd/system +lib/systemd/system/securedrop_shredder.service /lib/systemd/system diff --git a/molecule/builder-xenial/Dockerfile b/molecule/builder-xenial/Dockerfile index 825d0794c5..13256c8066 100644 --- a/molecule/builder-xenial/Dockerfile +++ b/molecule/builder-xenial/Dockerfile @@ -12,6 +12,7 @@ RUN apt-get -y update && apt-get upgrade -y && apt-get install -y \ debhelper \ devscripts \ dh-python \ + dh-systemd \ dh-virtualenv \ gdb \ git \ diff --git a/molecule/builder-xenial/image_hash b/molecule/builder-xenial/image_hash index f34c084ce9..5f301403e7 100644 --- a/molecule/builder-xenial/image_hash +++ b/molecule/builder-xenial/image_hash @@ -1,2 +1,2 @@ -# sha256 digest quay.io/freedomofpress/sd-docker-builder-xenial:2019_10_21 -0288d35d316047302e6e15887eb34fb5440415054835cf0c0f25f5cc8ab80279 +# sha256 digest quay.io/freedomofpress/sd-docker-builder-xenial:2019_10_24 +981d2190f643964aee7d1ea8e4fe6a99fd3ab8ebab7179886cd8d558b20d044d diff --git a/molecule/testinfra/staging/app-code/test_redis_worker.py b/molecule/testinfra/staging/app-code/test_redis_worker.py deleted file mode 100644 index bba6808271..0000000000 --- a/molecule/testinfra/staging/app-code/test_redis_worker.py +++ /dev/null @@ -1,51 +0,0 @@ -import pytest -import re - - -testinfra_hosts = ["app-staging"] -securedrop_test_vars = pytest.securedrop_test_vars - - -@pytest.mark.parametrize( - "config_line", - [ - "[program:securedrop_worker]", - "command=/opt/venvs/securedrop-app-code/bin/rqworker", - "directory={}".format(securedrop_test_vars.securedrop_code), - ( - 'environment=PYTHONPATH="/var/www/securedrop:' - '/opt/venvs/securedrop-app-code/lib/python3.5/site-packages"' - ), - "autostart=true", - "autorestart=true", - "startretries=3", - "stderr_logfile=/var/log/securedrop_worker/rqworker.err", - "stdout_logfile=/var/log/securedrop_worker/rqworker.out", - "user={}".format(securedrop_test_vars.securedrop_user), - ], -) -def test_redis_worker_configuration(host, config_line): - """ - Ensure SecureDrop Redis worker config for supervisor service - management is configured correctly. - """ - f = host.file("/etc/supervisor/conf.d/securedrop_worker.conf") - # Config lines may have special characters such as [] which will - # throw off the regex matching, so let's escape those chars. - regex = re.escape(config_line) - assert f.contains("^{}$".format(regex)) - - -def test_redis_worker_config_file(host): - """ - Ensure SecureDrop Redis worker config for supervisor service - management has proper ownership and mode. - - Using separate test so that the parametrization doesn't rerun - the file mode checks, which would be useless. - """ - f = host.file("/etc/supervisor/conf.d/securedrop_worker.conf") - assert f.is_file - assert f.mode == 0o644 - assert f.user == "root" - assert f.group == "root" diff --git a/molecule/testinfra/staging/app-code/test_rqrequeue_conf.py b/molecule/testinfra/staging/app-code/test_rqrequeue_conf.py deleted file mode 100644 index 84ebc70ed1..0000000000 --- a/molecule/testinfra/staging/app-code/test_rqrequeue_conf.py +++ /dev/null @@ -1,52 +0,0 @@ -import pytest -import re - - -testinfra_hosts = ["app-staging"] -securedrop_test_vars = pytest.securedrop_test_vars - - -@pytest.mark.parametrize( - "config_line", - [ - "[program:securedrop_rqrequeue]", - ( - "command=/opt/venvs/securedrop-app-code/bin/python " - "/var/www/securedrop/scripts/rqrequeue --interval 60" - ), - "directory={}".format(securedrop_test_vars.securedrop_code), - ( - 'environment=PYTHONPATH="/var/www/securedrop:' - '/opt/venvs/securedrop-app-code/lib/python3.5/site-packages"' - ), - "autostart=true", - "autorestart=true", - "startretries=3", - "stderr_logfile=/var/log/securedrop_worker/rqrequeue.err", - "stdout_logfile=/var/log/securedrop_worker/rqrequeue.out", - "user={}".format(securedrop_test_vars.securedrop_user), - ], -) -def test_rqrequeue_configuration(host, config_line): - """ - Ensure Supervisor config for rqrequeue is correct. - """ - f = host.file("/etc/supervisor/conf.d/securedrop_rqrequeue.conf") - # Config lines may have special characters such as [] which will - # throw off the regex matching, so let's escape those chars. - regex = re.escape(config_line) - assert f.contains("^{}$".format(regex)) - - -def test_rqrequeue_config_file(host): - """ - Check ownership and mode of Supervisor config for rqrequeue. - - Using separate test so that the parametrization doesn't rerun - the file mode checks, which would be useless. - """ - f = host.file("/etc/supervisor/conf.d/securedrop_rqrequeue.conf") - assert f.is_file - assert f.mode == 0o644 - assert f.user == "root" - assert f.group == "root" diff --git a/molecule/testinfra/staging/app-code/test_securedrop_app_code.py b/molecule/testinfra/staging/app-code/test_securedrop_app_code.py index a02ddc3a67..3a84e6c251 100644 --- a/molecule/testinfra/staging/app-code/test_securedrop_app_code.py +++ b/molecule/testinfra/staging/app-code/test_securedrop_app_code.py @@ -28,7 +28,6 @@ def test_apache_default_docroot_is_absent(host): 'securedrop-config', 'securedrop-keyring', 'sqlite3', - 'supervisor', ]) def test_securedrop_application_apt_dependencies(host, package): """ diff --git a/molecule/testinfra/staging/app-code/test_securedrop_rqrequeue.py b/molecule/testinfra/staging/app-code/test_securedrop_rqrequeue.py new file mode 100644 index 0000000000..6fff8a68b7 --- /dev/null +++ b/molecule/testinfra/staging/app-code/test_securedrop_rqrequeue.py @@ -0,0 +1,50 @@ +import pytest + + +testinfra_hosts = ["app-staging"] + + +def test_securedrop_rqrequeue_service(host): + """ + Verify configuration of securedrop_rqrequeue systemd service. + """ + securedrop_test_vars = pytest.securedrop_test_vars + service_file = "/lib/systemd/system/securedrop_rqrequeue.service" + expected_content = "\n".join([ + "[Unit]", + "Description=SecureDrop rqrequeue process", + "After=redis-server.service", + "Wants=redis-server.service", + "", + "[Service]", + 'Environment=PYTHONPATH="{}:{}"'.format( + securedrop_test_vars.securedrop_code, securedrop_test_vars.securedrop_venv_site_packages + ), + "ExecStart={}/python /var/www/securedrop/scripts/rqrequeue --interval 60".format( + securedrop_test_vars.securedrop_venv_bin + ), + "PrivateDevices=yes", + "PrivateTmp=yes", + "ProtectSystem=full", + "ReadOnlyDirectories=/", + "ReadWriteDirectories={}".format(securedrop_test_vars.securedrop_data), + "Restart=always", + "RestartSec=10s", + "UMask=077", + "User={}".format(securedrop_test_vars.securedrop_user), + "WorkingDirectory={}".format(securedrop_test_vars.securedrop_code), + "", + "[Install]", + "WantedBy=multi-user.target\n", + ]) + + f = host.file(service_file) + assert f.is_file + assert f.mode == 0o644 + assert f.user == "root" + assert f.group == "root" + assert f.content_string == expected_content + + s = host.service("securedrop_rqrequeue") + assert s.is_enabled + assert s.is_running diff --git a/molecule/testinfra/staging/app-code/test_securedrop_rqworker.py b/molecule/testinfra/staging/app-code/test_securedrop_rqworker.py new file mode 100644 index 0000000000..cd850c4275 --- /dev/null +++ b/molecule/testinfra/staging/app-code/test_securedrop_rqworker.py @@ -0,0 +1,49 @@ +import pytest + + +testinfra_hosts = ["app-staging"] + + +def test_securedrop_rqworker_service(host): + """ + Verify configuration of securedrop_rqworker systemd service. + """ + securedrop_test_vars = pytest.securedrop_test_vars + service_file = "/lib/systemd/system/securedrop_rqworker.service" + + expected_content = "\n".join([ + "[Unit]", + "Description=SecureDrop rq worker", + "After=redis-server.service", + "Wants=redis-server.service", + "", + "[Service]", + 'Environment=PYTHONPATH="{}:{}"'.format( + securedrop_test_vars.securedrop_code, securedrop_test_vars.securedrop_venv_site_packages + ), + "ExecStart={}/rqworker".format(securedrop_test_vars.securedrop_venv_bin), + "PrivateDevices=yes", + "PrivateTmp=yes", + "ProtectSystem=full", + "ReadOnlyDirectories=/", + "ReadWriteDirectories={}".format(securedrop_test_vars.securedrop_data), + "Restart=always", + "RestartSec=10s", + "UMask=077", + "User={}".format(securedrop_test_vars.securedrop_user), + "WorkingDirectory={}".format(securedrop_test_vars.securedrop_code), + "", + "[Install]", + "WantedBy=multi-user.target\n", + ]) + + f = host.file(service_file) + assert f.is_file + assert f.mode == 0o644 + assert f.user == "root" + assert f.group == "root" + assert f.content_string == expected_content + + s = host.service("securedrop_rqworker") + assert s.is_enabled + assert s.is_running diff --git a/molecule/testinfra/staging/app-code/test_securedrop_shredder_configuration.py b/molecule/testinfra/staging/app-code/test_securedrop_shredder_configuration.py new file mode 100644 index 0000000000..b6cb1a906c --- /dev/null +++ b/molecule/testinfra/staging/app-code/test_securedrop_shredder_configuration.py @@ -0,0 +1,48 @@ +import pytest + + +testinfra_hosts = ["app-staging"] + + +def test_securedrop_shredder_service(host): + """ + Verify configuration of securedrop_shredder systemd service. + """ + securedrop_test_vars = pytest.securedrop_test_vars + service_file = "/lib/systemd/system/securedrop_shredder.service" + expected_content = "\n".join([ + "[Unit]", + "Description=SecureDrop shredder", + "", + "[Service]", + 'Environment=PYTHONPATH="{}:{}"'.format( + securedrop_test_vars.securedrop_code, securedrop_test_vars.securedrop_venv_site_packages + ), + "ExecStart={}/python /var/www/securedrop/scripts/shredder --interval 60".format( + securedrop_test_vars.securedrop_venv_bin + ), + "PrivateDevices=yes", + "PrivateTmp=yes", + "ProtectSystem=full", + "ReadOnlyDirectories=/", + "ReadWriteDirectories={}".format(securedrop_test_vars.securedrop_data), + "Restart=always", + "RestartSec=10s", + "UMask=077", + "User={}".format(securedrop_test_vars.securedrop_user), + "WorkingDirectory={}".format(securedrop_test_vars.securedrop_code), + "", + "[Install]", + "WantedBy=multi-user.target\n", + ]) + + f = host.file(service_file) + assert f.is_file + assert f.mode == 0o644 + assert f.user == "root" + assert f.group == "root" + assert f.content_string == expected_content + + s = host.service("securedrop_shredder") + assert s.is_enabled + assert s.is_running diff --git a/molecule/testinfra/staging/app/test_appenv.py b/molecule/testinfra/staging/app/test_appenv.py index 4115b69743..38bc253c21 100644 --- a/molecule/testinfra/staging/app/test_appenv.py +++ b/molecule/testinfra/staging/app/test_appenv.py @@ -48,6 +48,11 @@ def test_app_code_pkg(host): assert host.package("securedrop-app-code").is_installed +def test_supervisor_not_installed(host): + """ ensure supervisor package is not installed """ + assert host.package("supervisor").is_installed is False + + def test_gpg_key_in_keyring(host): """ ensure test gpg key is present in app keyring """ with host.sudo(sdvars.securedrop_user): @@ -71,13 +76,3 @@ def test_securedrop_tmp_clean_cron(host): cronlist = host.run("crontab -l").stdout cronjob = "@daily {}/manage.py clean-tmp".format(sdvars.securedrop_code) assert cronjob in cronlist - - -def test_app_workerlog_dir(host): - """ ensure directory for worker logs is present """ - f = host.file('/var/log/securedrop_worker') - with host.sudo(): - assert f.is_directory - assert f.user == "root" - assert f.group == "root" - assert f.mode == 0o700 diff --git a/molecule/testinfra/staging/vars/app-staging.yml b/molecule/testinfra/staging/vars/app-staging.yml index 37968f5735..b3c2ce1e50 100644 --- a/molecule/testinfra/staging/vars/app-staging.yml +++ b/molecule/testinfra/staging/vars/app-staging.yml @@ -11,6 +11,9 @@ wanted_apache_headers: - "Header set Content-Security-Policy: \"default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'; font-src 'self';\"" - 'Header unset Etag' +securedrop_venv: /opt/venvs/securedrop-app-code +securedrop_venv_bin: "{{ securedrop_venv }}/bin" +securedrop_venv_site_packages: "{{ securedrop_venv }}/lib/python3.5/site-packages" securedrop_code: /var/www/securedrop securedrop_data: /var/lib/securedrop securedrop_user: www-data diff --git a/molecule/testinfra/staging/vars/staging.yml b/molecule/testinfra/staging/vars/staging.yml index 188a84ad03..3b7181e68f 100644 --- a/molecule/testinfra/staging/vars/staging.yml +++ b/molecule/testinfra/staging/vars/staging.yml @@ -11,6 +11,9 @@ wanted_apache_headers: - "Header set Content-Security-Policy: \"default-src 'none'; script-src 'self'; style-src 'self'; img-src 'self'; font-src 'self';\"" - 'Header unset Etag' +securedrop_venv: /opt/venvs/securedrop-app-code +securedrop_venv_bin: "/opt/venvs/securedrop-app-code/bin" +securedrop_venv_site_packages: "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages" securedrop_code: /var/www/securedrop securedrop_data: /var/lib/securedrop securedrop_user: www-data diff --git a/molecule/upgrade/playbook.yml b/molecule/upgrade/playbook.yml index cffb68f176..9a9d99d9d7 100644 --- a/molecule/upgrade/playbook.yml +++ b/molecule/upgrade/playbook.yml @@ -50,7 +50,6 @@ state: directory path: "/var/log/{{ item }}" with_items: - - securedrop_worker - cron-apt # Reuse existing "reboot-and-wait" logic from site config - import_tasks: ../../install_files/ansible-base/tasks/reboot.yml @@ -75,13 +74,6 @@ state: started enabled: yes tags: always - - - name: Ensure supervisor is running - service: - name: supervisor - state: started - delegate_to: app-staging - tags: always become: yes - import_playbook: apt.yml diff --git a/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py b/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py index 303b45c5f9..56739163b3 100644 --- a/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py +++ b/securedrop/alembic/versions/3da3fcab826a_delete_orphaned_submissions.py @@ -10,7 +10,6 @@ import os from alembic import op import sqlalchemy as sa -from rm import secure_delete # raise the errors if we're not in production raise_errors = os.environ.get("SECUREDROP_ENV", "prod") != "prod" @@ -19,7 +18,6 @@ from journalist_app import create_app from sdconfig import config from store import NoFileFoundException, TooManyFilesException - from worker import create_queue except ImportError: # This is a fresh install, and config.py has not been created yet. if raise_errors: @@ -63,8 +61,8 @@ def upgrade(): """).bindparams(id=submission.id) ) - file_path = app.storage.path_without_filesystem_id(submission.filename) - create_queue().enqueue(secure_delete, file_path) + path = app.storage.path_without_filesystem_id(submission.filename) + app.storage.move_to_shredder(path) except NoFileFoundException: # The file must have been deleted by the admin, remove the row conn.execute( @@ -85,8 +83,8 @@ def upgrade(): """).bindparams(id=reply.id) ) - file_path = app.storage.path_without_filesystem_id(reply.filename) - create_queue().enqueue(secure_delete, file_path) + path = app.storage.path_without_filesystem_id(reply.filename) + app.storage.move_to_shredder(path) except NoFileFoundException: # The file must have been deleted by the admin, remove the row conn.execute( diff --git a/securedrop/bin/dev-deps b/securedrop/bin/dev-deps index 44785fe575..079990f6d8 100755 --- a/securedrop/bin/dev-deps +++ b/securedrop/bin/dev-deps @@ -22,12 +22,6 @@ function run_redis() { setsid redis-server >& /tmp/redis.out || cat /tmp/redis.out } -function run_supervisor() { - mkdir -p /tmp/supervisor/log /tmp/supervisor/run && printf "[unix_http_server]\nfile=/tmp/supervisor/run/supervisor.sock\nchmod=0700\n\n[supervisord]logfile=/tmp/supervisor/log/supervisord.log\npidfile=/tmp/supervisor/run/supervisord.pid\nchildlogdir=/tmp/supervisor/log\n[rpcinterface:supervisor]\nsupervisor.rpcinterface_factory = supervisor.rpcinterface:make_main_rpcinterface\n\n[supervisorctl]\nserverurl=unix:///tmp/supervisor/run/supervisor.sock\n\n[program:securedrop_worker]\ncommand=/opt/venvs/securedrop-app-code/bin/rqworker\ndirectory=$(pwd)\nautostart=true\nautorestart=true\nstartretries=3\nstderr_logfile=/tmp/supervisor/log/securedrop_worker.err\nstdout_logfile=/tmp/supervisor/log/securedrop_worker.out\nuser=%s\n" "$USER_NAME" > /tmp/supervisor/supervisor.conf - - setsid supervisord -c /tmp/supervisor/supervisor.conf >& /tmp/supervisor/log/supervisor.out || cat /tmp/supervisor/log/supervisor.out -} - function setup_vncauth { x11vnc -storepasswd freedom /tmp/vncpasswd } diff --git a/securedrop/bin/new-migration b/securedrop/bin/new-migration index 3dbe35db03..269050f518 100755 --- a/securedrop/bin/new-migration +++ b/securedrop/bin/new-migration @@ -3,7 +3,7 @@ set -e set -u sudo mkdir -p /var/lib/securedrop/ -sudo chmod 0755 /var/lib/securedrop/ +sudo chmod 0700 /var/lib/securedrop/ sudo chown "$(id -u):$(id -g)" /var/lib/securedrop rm -f /var/lib/securedrop/db.sqlite sqlite3 /var/lib/securedrop/db.sqlite .databases > /dev/null diff --git a/securedrop/bin/run b/securedrop/bin/run index 2e58ebddeb..d4a367304d 100755 --- a/securedrop/bin/run +++ b/securedrop/bin/run @@ -9,10 +9,13 @@ cd "${REPOROOT}/securedrop" source "${BASH_SOURCE%/*}/dev-deps" run_redis & -run_supervisor & urandom run_sass --watch & maybe_create_config_py reset_demo +# run the batch processing services normally managed by systemd +/opt/venvs/securedrop-app-code/bin/rqworker & +PYTHONPATH="${REPOROOT}/securedrop" /opt/venvs/securedrop-app-code/bin/python "${REPOROOT}/securedrop/scripts/rqrequeue" --interval 60 & + ./manage.py run diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index 5ec0545f5e..cf6d76585a 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -211,16 +211,13 @@ def download_reply(source_uuid, reply_uuid): @token_required def single_submission(source_uuid, submission_uuid): if request.method == 'GET': - source = get_or_404(Source, source_uuid, column=Source.uuid) - submission = get_or_404(Submission, submission_uuid, - column=Submission.uuid) + get_or_404(Source, source_uuid, column=Source.uuid) + submission = get_or_404(Submission, submission_uuid, column=Submission.uuid) return jsonify(submission.to_json()), 200 elif request.method == 'DELETE': - submission = get_or_404(Submission, submission_uuid, - column=Submission.uuid) - source = get_or_404(Source, source_uuid, column=Source.uuid) - utils.delete_file(source.filesystem_id, submission.filename, - submission) + get_or_404(Source, source_uuid, column=Source.uuid) + submission = get_or_404(Submission, submission_uuid, column=Submission.uuid) + utils.delete_file_object(submission) return jsonify({'message': 'Submission deleted'}), 200 @api.route('/sources//replies', methods=['GET', 'POST']) @@ -290,13 +287,12 @@ def all_source_replies(source_uuid): methods=['GET', 'DELETE']) @token_required def single_reply(source_uuid, reply_uuid): - source = get_or_404(Source, source_uuid, column=Source.uuid) + get_or_404(Source, source_uuid, column=Source.uuid) reply = get_or_404(Reply, reply_uuid, column=Reply.uuid) if request.method == 'GET': return jsonify(reply.to_json()), 200 elif request.method == 'DELETE': - utils.delete_file(source.filesystem_id, reply.filename, - reply) + utils.delete_file_object(reply) return jsonify({'message': 'Reply deleted'}), 200 @api.route('/submissions', methods=['GET']) diff --git a/securedrop/journalist_app/utils.py b/securedrop/journalist_app/utils.py index 80f87895f7..67e06f34f4 100644 --- a/securedrop/journalist_app/utils.py +++ b/securedrop/journalist_app/utils.py @@ -13,9 +13,7 @@ from models import (get_one_or_else, Source, Journalist, InvalidUsernameException, WrongPasswordException, FirstOrLastNameError, LoginThrottledException, BadTokenException, SourceStar, PasswordError, Submission, RevokedToken) -from rm import secure_delete from store import add_checksum_for_file -from worker import create_queue import typing # https://www.python.org/dev/peps/pep-0484/#runtime-or-type-checking @@ -171,16 +169,16 @@ def download(zip_basename, submissions): as_attachment=True) -def delete_file(filesystem_id, filename, file_object): - file_path = current_app.storage.path(filesystem_id, filename) - create_queue().enqueue(secure_delete, file_path) +def delete_file_object(file_object): + path = current_app.storage.path(file_object.source.filesystem_id, file_object.filename) + current_app.storage.move_to_shredder(path) db.session.delete(file_object) db.session.commit() def bulk_delete(filesystem_id, items_selected): for item in items_selected: - delete_file(filesystem_id, item.filename, item) + delete_file_object(item) flash(ngettext("Submission deleted.", "{num} submissions deleted.".format( @@ -260,7 +258,8 @@ def make_password(config): def delete_collection(filesystem_id): # Delete the source's collection of submissions - job = create_queue().enqueue(secure_delete, current_app.storage.path(filesystem_id)) + path = current_app.storage.path(filesystem_id) + current_app.storage.move_to_shredder(path) # Delete the source's reply keypair current_app.crypto_util.delete_reply_keypair(filesystem_id) @@ -269,7 +268,6 @@ def delete_collection(filesystem_id): source = get_source(filesystem_id) db.session.delete(source) db.session.commit() - return job def set_name(user, first_name, last_name): diff --git a/securedrop/requirements/python3/develop-requirements.in b/securedrop/requirements/python3/develop-requirements.in index 683e0f1bbf..c264a69e28 100644 --- a/securedrop/requirements/python3/develop-requirements.in +++ b/securedrop/requirements/python3/develop-requirements.in @@ -24,7 +24,6 @@ setuptools sphinx sphinx-autobuild sphinx_rtd_theme -supervisor testinfra urllib3>=1.25.3 yamllint diff --git a/securedrop/requirements/python3/develop-requirements.txt b/securedrop/requirements/python3/develop-requirements.txt index dad174a3ea..4085afcb61 100644 --- a/securedrop/requirements/python3/develop-requirements.txt +++ b/securedrop/requirements/python3/develop-requirements.txt @@ -2,7 +2,7 @@ # This file is autogenerated by pip-compile # To update, run: # -# pip-compile --generate-hashes --allow-unsafe --output-file=requirements/python3/develop-requirements.txt ../admin/requirements-ansible.in ../admin/requirements.in requirements/python3/develop-requirements.in +# pip-compile --allow-unsafe --generate-hashes --output-file=requirements/python3/develop-requirements.txt ../admin/requirements-ansible.in ../admin/requirements.in requirements/python3/develop-requirements.in # alabaster==0.7.10 \ --hash=sha256:2eef172f44e8d301d25aff8068fddd65f767a3f04b5f15b0f4922f113aa1c732 \ @@ -317,10 +317,6 @@ mccabe==0.6.1 \ --hash=sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42 \ --hash=sha256:dd8d182285a0fe56bace7f45b5e7d1a6ebcbf524e8f3bd87eb0f125271b8831f \ # via flake8, pylint -meld3==2.0.0 \ - --hash=sha256:1efda676264490db2e30bfb81b27a918cc6d9c2de6d609491aa43410b9537eb9 \ - --hash=sha256:a78cdcb6c0342b7ac82d900b584bd918ef72f86b60cdbf04754f840c070eb20c \ - # via supervisor molecule==2.22 \ --hash=sha256:12fa4231ed69c6e7f50432588eaace36cea917a8c73c1751269ce55df32ced24 \ --hash=sha256:d9d7621167041ae2a8eb19f1f8dc23c071cdab2cd3ca80655e2c8796b4c00e09 @@ -570,9 +566,6 @@ stevedore==1.28.0 \ --hash=sha256:e3d96b2c4e882ec0c1ff95eaebf7b575a779fd0ccb4c741b9832bed410d58b3d \ --hash=sha256:f1c7518e7b160336040fee272174f1f7b29a46febb3632502a8f2055f973d60b \ # via bandit -supervisor==4.0.4 \ - --hash=sha256:43e87c7b572a94acdb586aaebb06844dae1aa02856b984c5a738032abd753fb7 \ - --hash=sha256:9644990d21a1ba03b1a7ac5e9a0c0c62e12822e258f9e98f4a0b128461b3f10a tabulate==0.8.3 \ --hash=sha256:8af07a39377cee1103a5c8b3330a421c2d99b9141e9cc5ddd2e3263fea416943 \ # via molecule diff --git a/securedrop/rm.py b/securedrop/rm.py index 9bf7121545..99d651331b 100644 --- a/securedrop/rm.py +++ b/securedrop/rm.py @@ -52,7 +52,7 @@ def shred(path, delete=True): def secure_delete(path): - # type: (str) -> str + # type: (str) -> None """ Securely deletes the file at ``path``. @@ -86,9 +86,6 @@ def secure_delete(path): for d in reversed(sorted(directories_to_remove)): os.rmdir(d) - # We need to return a non-`None` value so the rq worker writes this back to Redis - return "success" - def check_secure_delete_capability(): # type: () -> bool diff --git a/securedrop/scripts/shredder b/securedrop/scripts/shredder new file mode 100755 index 0000000000..f4188d8cab --- /dev/null +++ b/securedrop/scripts/shredder @@ -0,0 +1,57 @@ +#!/opt/venvs/securedrop-app-code/bin/python + +# +# Clears the SecureDrop shredder directory, securely deleting its contents. +# + +import argparse +import logging +import sys +import time + +sys.path.insert(0, ".") # noqa: E402 +sys.path.insert(0, "/opt/venvs/securedrop-app-code/lib/python3.5/site-packages") # noqa: E402 +sys.path.insert(0, "/var/www/securedrop") # noqa: E402 + +import journalist_app +from sdconfig import config + + +def parse_args(): + parser = argparse.ArgumentParser( + prog=__file__, description="Utility for clearing deleted content in the SecureDrop store." + ) + parser.add_argument( + "-i", + "--interval", + type=int, + help="Keep running every 'interval' seconds.", + ) + + return parser.parse_args() + + +def clear_shredder(): + try: + app.storage.clear_shredder() + except Exception as e: + logging.info("Error clearing shredder: {}".format(e)) + + +def main(): + args = parse_args() + logging.basicConfig(format="%(asctime)s %(levelname)s %(message)s", level=logging.INFO) + if args.interval: + logging.info("Running every {} seconds.".format(args.interval)) + while 1: + clear_shredder() + time.sleep(args.interval) + else: + logging.info("Running once.") + clear_shredder() + + +if __name__ == "__main__": + app = journalist_app.create_app(config) + with app.app_context(): + main() diff --git a/securedrop/store.py b/securedrop/store.py index 4ed7b2b5e7..cb252f8a56 100644 --- a/securedrop/store.py +++ b/securedrop/store.py @@ -14,6 +14,7 @@ from secure_tempfile import SecureTemporaryFile +import rm from worker import create_queue @@ -32,8 +33,8 @@ VALIDATE_FILENAME = re.compile( - r"^(?P\d+)\-[a-z0-9-_]*" - r"(?Pmsg|doc\.(gz|zip)|reply)\.gpg$").match + r"^(?P\d+)\-[a-z0-9-_]*(?Pmsg|doc\.(gz|zip)|reply)\.gpg$" +).match class PathException(Exception): @@ -83,45 +84,66 @@ def __init__(self, storage_path, temp_dir, gpg_key): self.__gpg_key = gpg_key - def verify(self, p): - # type: (str) -> bool - """Assert that the path is absolute, normalized, inside - `self.__storage_path`, and matches the filename format. + # where files and directories are sent to be securely deleted + self.__shredder_path = os.path.abspath(os.path.join(self.__storage_path, "../shredder")) + if not os.path.exists(self.__shredder_path): + os.makedirs(self.__shredder_path, mode=0o700) + + @property + def storage_path(self): + return self.__storage_path + + @property + def shredder_path(self): + return self.__shredder_path + + def shredder_contains(self, path: str) -> bool: + """ + Returns True if the fully-resolved path lies within the shredder. + """ + common_path = os.path.commonpath((os.path.realpath(path), self.__shredder_path)) + return common_path == self.__shredder_path + + def store_contains(self, path: str) -> bool: """ + Returns True if the fully-resolved path lies within the store. + """ + common_path = os.path.commonpath((os.path.realpath(path), self.__storage_path)) + return common_path == self.__storage_path + + def verify(self, p: str) -> bool: + """ + Verify that a given path is valid for the store. + """ + + if self.store_contains(p): + # verifying a hypothetical path + if not os.path.exists(p): + return True - # os.path.abspath makes the path absolute and normalizes - # '/foo/../bar' to '/bar', etc. We have to check that the path is - # normalized before checking that it starts with the - # `self.__storage_path` or else a malicious actor could append a - # bunch of '../../..' to access files outside of the store. - if not p == os.path.abspath(p): - raise PathException("The path is not absolute and/or normalized") - - # Check that the path p is in self.__storage_path - if os.path.relpath(p, self.__storage_path).startswith('..'): - raise PathException("Invalid directory %s" % (p, )) - - if os.path.isfile(p): - filename = os.path.basename(p) - ext = os.path.splitext(filename)[-1] - if filename == '_FLAG': + # extant paths must be directories or correctly-named plain files + if os.path.isdir(p): return True - if ext != '.gpg': - # if there's an extension, verify it's a GPG - raise PathException("Invalid file extension %s" % (ext, )) - if not VALIDATE_FILENAME(filename): - raise PathException("Invalid filename %s" % (filename, )) - return False + if os.path.isfile(p) and VALIDATE_FILENAME(os.path.basename(p)): + return True - def path(self, *s): - # type: (*str) -> str - """Get the normalized, absolute file path, within - `self.__storage_path`. + raise PathException("Path not valid in store: {}".format(p)) + + def path(self, filesystem_id: str, filename: str = '') -> str: + """ + Returns the path resolved within `self.__storage_path`. + + Raises PathException if `verify` doesn't like the path. """ - joined = os.path.join(os.path.abspath(self.__storage_path), *s) - absolute = os.path.abspath(joined) - self.verify(absolute) + joined = os.path.join(os.path.realpath(self.__storage_path), filesystem_id, filename) + absolute = os.path.realpath(joined) + if not self.verify(absolute): + raise PathException( + """Could not resolve ("{}", "{}") to a path within the store.""".format( + filesystem_id, filename + ) + ) return absolute def path_without_filesystem_id(self, filename): @@ -132,7 +154,7 @@ def path_without_filesystem_id(self, filename): """ joined_paths = [] - for rootdir, _, files in os.walk(os.path.abspath(self.__storage_path)): + for rootdir, _, files in os.walk(os.path.realpath(self.__storage_path)): for file_ in files: if file_ in filename: joined_paths.append(os.path.join(rootdir, file_)) @@ -144,7 +166,10 @@ def path_without_filesystem_id(self, filename): else: absolute = joined_paths[0] - self.verify(absolute) + if not self.verify(absolute): + raise PathException( + """Could not resolve "{}" to a path within the store.""".format(filename) + ) return absolute def get_bulk_archive(self, selected_submissions, zip_directory=''): @@ -164,9 +189,7 @@ def get_bulk_archive(self, selected_submissions, zip_directory=''): submissions = [s for s in selected_submissions if s.source.journalist_designation == source] for submission in submissions: - filename = self.path(submission.source.filesystem_id, - submission.filename) - self.verify(filename) + filename = self.path(submission.source.filesystem_id, submission.filename) document_number = submission.filename.split('-')[0] if zip_directory == submission.source.journalist_filename: fname = zip_directory @@ -180,6 +203,79 @@ def get_bulk_archive(self, selected_submissions, zip_directory=''): )) return zip_file + def move_to_shredder(self, path: str): + """ + Moves content from the store to the shredder for secure deletion. + + Python's os.renames (and the underlying rename(2) calls) will + silently overwrite content, which could bypass secure + deletion, so we create a temporary directory under the + shredder directory and move the specified content there. + + This function is intended to be atomic and quick, for use in + deletions via the UI and API. The actual secure deletion is + performed by an asynchronous process that monitors the + shredder directory. + """ + if not self.verify(path): + raise ValueError( + """Path is not within the store: "{}" """.format(path) + ) + + if not os.path.exists(path): + raise ValueError( + """Path does not exist: "{}" """.format(path) + ) + + relpath = os.path.relpath(path, start=self.storage_path) + dest = os.path.join(tempfile.mkdtemp(dir=self.__shredder_path), relpath) + current_app.logger.info("Moving {} to shredder: {}".format(path, dest)) + os.renames(path, dest) + + def clear_shredder(self): + current_app.logger.info("Clearing shredder") + directories = [] + targets = [] + for directory, subdirs, files in os.walk(self.shredder_path): + for subdir in subdirs: + real_subdir = os.path.realpath(os.path.join(directory, subdir)) + if self.shredder_contains(real_subdir): + directories.append(real_subdir) + for f in files: + abs_file = os.path.abspath(os.path.join(directory, f)) + if os.path.islink(abs_file): + # Somehow, a symbolic link was created in the + # store. This shouldn't happen in normal + # operations. Just remove the link; don't try to + # shred its target. Note that we only have special + # handling for symlinks. Hard links -- which + # again, shouldn't occur in the store -- will + # result in the file data being shredded once for + # each link. + current_app.logger.info( + "Deleting link {} to {}".format( + abs_file, os.readlink(abs_file) + ) + ) + os.unlink(abs_file) + continue + if self.shredder_contains(abs_file): + targets.append(abs_file) + + target_count = len(targets) + current_app.logger.info("Files to delete: {}".format(target_count)) + for i, t in enumerate(targets, 1): + current_app.logger.info("Securely deleting file {}/{}: {}".format(i, target_count, t)) + rm.secure_delete(t) + current_app.logger.info("Securely deleted file {}/{}: {}".format(i, target_count, t)) + + directories_to_remove = set(directories) + dir_count = len(directories_to_remove) + for i, d in enumerate(reversed(sorted(directories_to_remove)), 1): + current_app.logger.debug("Removing directory {}/{}: {}".format(i, dir_count, d)) + os.rmdir(d) + current_app.logger.debug("Removed directory {}/{}: {}".format(i, dir_count, d)) + def save_file_submission(self, filesystem_id, count, journalist_filename, filename, stream): # type: (str, int, str, str, BufferedIOBase) -> str diff --git a/securedrop/tests/test_journalist.py b/securedrop/tests/test_journalist.py index 3e429ddc3e..d095990b62 100644 --- a/securedrop/tests/test_journalist.py +++ b/securedrop/tests/test_journalist.py @@ -1689,19 +1689,15 @@ def test_delete_source_deletes_docs_on_disk(journalist_app, utils.db_helper.submit(source, 2) utils.db_helper.reply(journo, source, 2) - # Encrypted documents exists - dir_source_docs = os.path.join(config.STORE_DIR, - test_source['filesystem_id']) + dir_source_docs = os.path.join(config.STORE_DIR, test_source['filesystem_id']) assert os.path.exists(dir_source_docs) - job = journalist_app_module.utils.delete_collection( - test_source['filesystem_id']) + journalist_app_module.utils.delete_collection(test_source['filesystem_id']) - # Wait up to 5s to wait for Redis worker secure deletion to complete - utils.asynchronous.wait_for_redis_worker(job) + def assertion(): + assert not os.path.exists(dir_source_docs) - # Encrypted documents no longer exist - assert not os.path.exists(dir_source_docs) + utils.asynchronous.wait_for_assertion(assertion) def test_login_with_invalid_password_doesnt_call_argon2(mocker, test_journo): diff --git a/securedrop/tests/test_store.py b/securedrop/tests/test_store.py index 4b6d8e877e..d981c39a2a 100644 --- a/securedrop/tests/test_store.py +++ b/securedrop/tests/test_store.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- +import logging import os import io import pytest import re -import store +import stat import zipfile os.environ['SECUREDROP_ENV'] = 'test' # noqa @@ -12,6 +13,7 @@ from db import db from journalist_app import create_app from models import Submission, Reply +import store from store import Storage, queued_add_checksum_for_file, async_add_checksum_for_file @@ -102,16 +104,29 @@ def test_verify_path_not_absolute(journalist_app, config): def test_verify_in_store_dir(journalist_app, config): with pytest.raises(store.PathException) as e: - journalist_app.storage.verify(config.STORE_DIR + "_backup") - - assert 'Invalid directory' in str(e) + path = config.STORE_DIR + "_backup" + journalist_app.storage.verify(path) + assert e.message == "Path not valid in store: {}".format(path) def test_verify_store_path_not_absolute(journalist_app): with pytest.raises(store.PathException) as e: journalist_app.storage.verify('..') + assert e.message == "Path not valid in store: .." + - assert 'The path is not absolute and/or normalized' in str(e) +def test_verify_rejects_symlinks(journalist_app): + """ + Test that verify rejects paths involving links outside the store. + """ + try: + link = os.path.join(journalist_app.storage.storage_path, "foo") + os.symlink("/foo", link) + with pytest.raises(store.PathException) as e: + journalist_app.storage.verify(link) + assert e.message == "Path not valid in store: {}".format(link) + finally: + os.unlink(link) def test_verify_store_dir_not_absolute(): @@ -130,9 +145,15 @@ def test_verify_store_temp_dir_not_absolute(): assert re.compile('temp_dir.*is not absolute').match(msg) -def test_verify_flagged_file_in_sourcedir_returns_true(journalist_app, config): +def test_verify_regular_submission_in_sourcedir_returns_true(journalist_app, config): + """ + Tests that verify is happy with a regular submission file. + + Verify should return True for a regular file that matches the + naming scheme of submissions. + """ source_directory, file_path = create_file_in_source_dir( - config, 'example-filesystem-id', '_FLAG' + config, 'example-filesystem-id', '1-regular-doc.gz.gpg' ) assert journalist_app.storage.verify(file_path) @@ -148,7 +169,7 @@ def test_verify_invalid_file_extension_in_sourcedir_raises_exception( with pytest.raises(store.PathException) as e: journalist_app.storage.verify(file_path) - assert 'Invalid file extension .txt' in str(e) + assert 'Path not valid in store: {}'.format(file_path) in str(e) def test_verify_invalid_filename_in_sourcedir_raises_exception( @@ -160,8 +181,7 @@ def test_verify_invalid_filename_in_sourcedir_raises_exception( with pytest.raises(store.PathException) as e: journalist_app.storage.verify(file_path) - - assert 'Invalid filename NOTVALID.gpg' in str(e) + assert e.message == 'Path not valid in store: {}'.format(file_path) def test_get_zip(journalist_app, test_source, config): @@ -292,3 +312,74 @@ def test_async_add_checksum_for_file(config, db_model): # requery to get a new object db_obj = db_model.query.filter_by(id=db_obj_id).one() assert db_obj.checksum == 'sha256:' + expected_hash + + +def test_path_configuration_is_immutable(journalist_app): + """ + Check that the store's paths cannot be changed. + + They're exposed via properties that are supposed to be + read-only. It is of course possible to change them via the mangled + attribute names, but we want to confirm that accidental changes + are prevented. + """ + with pytest.raises(AttributeError): + journalist_app.storage.storage_path = "/foo" + + original_storage_path = journalist_app.storage.storage_path[:] + journalist_app.storage.__storage_path = "/foo" + assert journalist_app.storage.storage_path == original_storage_path + + with pytest.raises(AttributeError): + journalist_app.storage.shredder_path = "/foo" + + original_shredder_path = journalist_app.storage.shredder_path[:] + journalist_app.storage.__shredder_path = "/foo" + assert journalist_app.storage.shredder_path == original_shredder_path + + +def test_shredder_configuration(journalist_app): + """ + Ensure we're creating the shredder directory correctly. + + We want to ensure that it's a sibling of the store directory, with + mode 0700. + """ + store_path = journalist_app.storage.storage_path + shredder_path = journalist_app.storage.shredder_path + assert os.path.dirname(shredder_path) == os.path.dirname(store_path) + s = os.stat(shredder_path) + assert stat.S_ISDIR(s.st_mode) is True + assert stat.S_IMODE(s.st_mode) == 0o700 + + +def test_shredder_deletes_symlinks(journalist_app, caplog): + """ + Confirm that `store.clear_shredder` removes any symlinks in the shredder. + """ + caplog.set_level(logging.DEBUG) + + link_target = "/foo" + link = os.path.abspath(os.path.join(journalist_app.storage.shredder_path, "foo")) + os.symlink(link_target, link) + journalist_app.storage.clear_shredder() + assert "Deleting link {} to {}".format(link, link_target) in caplog.text + assert not os.path.exists(link) + + +def test_shredder_shreds(journalist_app, caplog): + """ + Confirm that `store.clear_shredder` removes files. + """ + caplog.set_level(logging.DEBUG) + + testdir = os.path.abspath(os.path.join(journalist_app.storage.shredder_path, "testdir")) + os.makedirs(testdir) + testfile = os.path.join(testdir, "testfile") + with open(testfile, "w") as f: + f.write("testdata\n") + + journalist_app.storage.clear_shredder() + assert "Securely deleted file 1/1: {}".format(testfile) in caplog.text + assert not os.path.isfile(testfile) + assert not os.path.isdir(testdir)