diff --git a/install_files/ansible-base/roles/app/handlers/main.yml b/install_files/ansible-base/roles/app/handlers/main.yml index a826cef3eb..5372f01f16 100644 --- a/install_files/ansible-base/roles/app/handlers/main.yml +++ b/install_files/ansible-base/roles/app/handlers/main.yml @@ -11,14 +11,7 @@ name: apache2 state: restarted -## Here, we list apparmor before haveged to ensure the correct AppArmor -## profile is loaded prior to restarting haveged - name: restart apparmor service: name: apparmor state: restarted - -- name: restart haveged - service: - name: haveged - state: restarted diff --git a/install_files/ansible-base/roles/app/tasks/configure_haveged.yml b/install_files/ansible-base/roles/app/tasks/configure_haveged.yml deleted file mode 100644 index 131c316cd4..0000000000 --- a/install_files/ansible-base/roles/app/tasks/configure_haveged.yml +++ /dev/null @@ -1,83 +0,0 @@ ---- -- name: Read haveged defaults file to filter duplicate entries. - command: awk '!a[$0]++' /etc/default/haveged - register: haveged_defaults_no_duplicates_result - # Read-only task, so don't perform change. - # When writing to the file in a subsequent task, - # the "changed" attribute will be honored (if appropriate). - changed_when: false - tags: - - haveged - - hardening - - # Previous Ansible tasks for managing /etc/default/haveged have - # appended redundant lines to the file. To ensure idempotence, - # we'll first remove any duplicate lines, then set the args - # we want via `lineinfile` below. -- name: Remove duplicate entries from haveged defaults file. - copy: - dest: /etc/default/haveged - # Appending newline is necessary for idempotence, - # since the `lineinfile` task below will add it if not found. - content: "{{ haveged_defaults_no_duplicates_result.stdout }}\n" - owner: root - group: root - mode: "0644" - backup: yes - tags: - - haveged - - hardening - -- name: Increase haveged's low entropy watermark to minimize "flag for reply" flow. - lineinfile: - dest: /etc/default/haveged - regexp: '^DAEMON_ARGS' - line: 'DAEMON_ARGS="-w 2400"' - notify: - - restart haveged - tags: - - haveged - - hardening - -# The following three tasks resolve haveged AppArmor issues on Xenial described in -# https://github.com/freedomofpress/securedrop/issues/4098. - -- name: Check whether haveged AppArmor profile exists in expected location for Xenial. - stat: - path: "/etc/apparmor.d/usr.sbin.haveged" - register: haveged_apparmor - # Do not report changed, this task does not modify state. - changed_when: false - tags: - - haveged - - hardening - -# This temporary fix ensures that the pid file haveged uses is whitelisted -# by AppArmor. -# Upstream bug: https://bugs.launchpad.net/ubuntu/+source/haveged/+bug/1708674 -- name: Whitelist haveged pid file in its AppArmor profile. - lineinfile: - dest: /etc/apparmor.d/usr.sbin.haveged - insertafter: " #include " - line: " /run/haveged.pid rw," - when: haveged_apparmor.stat.exists - tags: - - haveged - - hardening - -# This temporary fix resolves a race condition where haveged would start -# prior to AppArmor, in which case haveged would be running unconfined. -# Bug described in: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=824179 -- name: Start apparmor prior to haveged process. - lineinfile: - dest: /lib/systemd/system/haveged.service - regexp: "^After=systemd-random-seed.service" - line: "After=apparmor.service systemd-random-seed.service" - backrefs: yes - when: haveged_apparmor.stat.exists - notify: - - restart apparmor - - restart haveged - tags: - - haveged - - hardening diff --git a/install_files/ansible-base/roles/app/tasks/main.yml b/install_files/ansible-base/roles/app/tasks/main.yml index e2463a8e48..fdebbd7024 100644 --- a/install_files/ansible-base/roles/app/tasks/main.yml +++ b/install_files/ansible-base/roles/app/tasks/main.yml @@ -17,5 +17,3 @@ - include: install_and_harden_apache.yml - include: setup_cron.yml - -- include: configure_haveged.yml 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 b2c480b1ca..1000cb6903 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 @@ -169,6 +169,7 @@ /var/www/securedrop/db.py r, /var/www/securedrop/dictionaries/adjectives.txt r, /var/www/securedrop/dictionaries/nouns.txt r, + /var/www/securedrop/execution.py r, /var/www/securedrop/i18n.py r, /var/www/securedrop/journalist.py r, /var/www/securedrop/journalist_app/ r, @@ -197,7 +198,6 @@ /var/www/securedrop/journalist_templates/delete.html r, /var/www/securedrop/journalist_templates/edit_account.html r, /var/www/securedrop/journalist_templates/error.html r, - /var/www/securedrop/journalist_templates/flag.html r, /var/www/securedrop/journalist_templates/flashed.html r, /var/www/securedrop/journalist_templates/index.html r, /var/www/securedrop/journalist_templates/js-strings.html r, diff --git a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/control.j2 b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/control.j2 index 579b1a62e5..6ecfde32df 100644 --- a/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/control.j2 +++ b/install_files/ansible-base/roles/build-securedrop-app-code-deb-pkg/templates/control.j2 @@ -8,11 +8,11 @@ Standards-Version: 3.9.8 Package: securedrop-app-code Architecture: amd64 -Conflicts: libapache2-mod-wsgi,supervisor -Replaces: libapache2-mod-wsgi,supervisor +Conflicts: haveged, libapache2-mod-wsgi, supervisor +Replaces: haveged, libapache2-mod-wsgi, supervisor {% if securedrop_target_distribution == "focal" %} -Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, haveged, libapache2-mod-xsendfile, libpython3.8, paxctld, python3, python3-distutils, redis-server, securedrop-config, securedrop-keyring, sqlite3 +Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, libapache2-mod-xsendfile, libpython3.8, paxctld, python3, python3-distutils, redis-server, securedrop-config, securedrop-keyring, sqlite3 {% else %} -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 +Depends: ${dist:Depends}, ${misc:Depends}, ${python3:Depends}, apache2, apparmor-utils, coreutils, gnupg2, libapache2-mod-xsendfile, libpython3.5, paxctld, python3 (>= 3.5), python3 (<< 3.6), redis-server, securedrop-config, securedrop-keyring, sqlite3 {% endif %} 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/molecule/builder-focal/Dockerfile b/molecule/builder-focal/Dockerfile index 2f44d0d66a..eb6ef54432 100644 --- a/molecule/builder-focal/Dockerfile +++ b/molecule/builder-focal/Dockerfile @@ -16,7 +16,6 @@ RUN apt-get -y update && apt-get upgrade -y && apt-get install -y \ gdb \ git \ gnupg2 \ - haveged \ inotify-tools \ libffi-dev \ libssl-dev \ diff --git a/molecule/testinfra/app-code/test_haveged.py b/molecule/testinfra/app-code/test_haveged.py deleted file mode 100644 index 3195700240..0000000000 --- a/molecule/testinfra/app-code/test_haveged.py +++ /dev/null @@ -1,40 +0,0 @@ -import testutils - -sdvars = testutils.securedrop_test_vars -testinfra_hosts = [sdvars.app_hostname] - - -def test_haveged_config(host): - """ - Ensure haveged's low entrop watermark is sufficiently high. - """ - f = host.file('/etc/default/haveged') - assert f.is_file - assert f.user == 'root' - assert f.group == 'root' - assert f.mode == 0o644 - assert f.contains('^DAEMON_ARGS="-w 2400"$') - - -def test_haveged_no_duplicate_lines(host): - """ - Regression test to check for duplicate entries. Earlier playbooks - for configuring the SD instances needlessly appended the `DAEMON_ARGS` - line everytime the playbook was run. Fortunately the duplicate lines don't - break the service, but it's still poor form. - """ - c = host.run("uniq --repeated /etc/default/haveged") - assert c.rc == 0 - assert c.stdout == "" - - -def test_haveged_is_running(host): - """ - Ensure haveged service is running, to provide additional entropy. - """ - # sudo is necessary to read /proc when running under grsecurity, - # which the App hosts do. Not technically necessary under development. - with host.sudo(): - s = host.service("haveged") - assert s.is_running - assert s.is_enabled diff --git a/molecule/testinfra/app-code/test_securedrop_app_code.py b/molecule/testinfra/app-code/test_securedrop_app_code.py index d9ec00b5c3..4d21493ccd 100644 --- a/molecule/testinfra/app-code/test_securedrop_app_code.py +++ b/molecule/testinfra/app-code/test_securedrop_app_code.py @@ -22,7 +22,6 @@ def test_apache_default_docroot_is_absent(host): 'apparmor-utils', 'coreutils', 'gnupg2', - 'haveged', 'libapache2-mod-xsendfile', 'libpython{}'.format(python_version), 'paxctld', @@ -41,6 +40,22 @@ def test_securedrop_application_apt_dependencies(host, package): assert host.package(package).is_installed +@pytest.mark.parametrize('package', [ + 'cron-apt', + 'haveged', + 'libapache2-mod-wsgi', + 'ntp', + 'ntpdate', + 'supervisor' +]) +def test_unwanted_packages_absent(host, package): + """ + Ensure packages that conflict with `securedrop-app-code` + or are otherwise unwanted are not present. + """ + assert not host.package(package).is_installed + + @pytest.mark.skip_in_prod def test_securedrop_application_test_locale(host): """ diff --git a/securedrop/alembic/versions/b060f38c0c31_drop_source_flagged.py b/securedrop/alembic/versions/b060f38c0c31_drop_source_flagged.py new file mode 100644 index 0000000000..b591a04590 --- /dev/null +++ b/securedrop/alembic/versions/b060f38c0c31_drop_source_flagged.py @@ -0,0 +1,70 @@ +"""drop Source.flagged + +Revision ID: b060f38c0c31 +Revises: 92fba0be98e9 +Create Date: 2021-05-10 18:15:56.071880 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "b060f38c0c31" +down_revision = "92fba0be98e9" +branch_labels = None +depends_on = None + + +def upgrade(): + with op.batch_alter_table("sources", schema=None) as batch_op: + batch_op.drop_column("flagged") + + +def downgrade(): + # You might be tempted to try Alembic's batch_ops for the + # downgrade too. Don't. SQLite's unnamed check constraints require + # kludges. + + conn = op.get_bind() + conn.execute("PRAGMA legacy_alter_table=ON") + + op.rename_table("sources", "sources_tmp") + + conn.execute( + sa.text( + """ + CREATE TABLE "sources" ( + id INTEGER NOT NULL, + uuid VARCHAR(36) NOT NULL, + filesystem_id VARCHAR(96), + journalist_designation VARCHAR(255) NOT NULL, + last_updated DATETIME, + pending BOOLEAN, + interaction_count INTEGER NOT NULL, + deleted_at DATETIME, + flagged BOOLEAN, + PRIMARY KEY (id), + CHECK (pending IN (0, 1)), + CHECK (flagged IN (0, 1)), + UNIQUE (filesystem_id), + UNIQUE (uuid) + ) + """ + ) + ) + + conn.execute( + """ + INSERT INTO sources ( + id, uuid, filesystem_id, journalist_designation, + last_updated, pending, interaction_count, deleted_at + ) SELECT + id, uuid, filesystem_id, journalist_designation, + last_updated, pending, interaction_count, deleted_at + FROM sources_tmp; + """ + ) + + # Now delete the old table. + op.drop_table("sources_tmp") diff --git a/securedrop/crypto_util.py b/securedrop/crypto_util.py index 19e050a6c7..7a1727e7fa 100644 --- a/securedrop/crypto_util.py +++ b/securedrop/crypto_util.py @@ -219,8 +219,6 @@ def find_source_key(self, fingerprint: str) -> Optional[Dict]: def delete_reply_keypair(self, source_filesystem_id: str) -> None: fingerprint = self.get_fingerprint(source_filesystem_id) - # If this source was never flagged for review, they won't have a reply - # keypair if not fingerprint: return diff --git a/securedrop/dockerfiles/focal/python3/Dockerfile b/securedrop/dockerfiles/focal/python3/Dockerfile index d875c7beec..79ee5238f2 100644 --- a/securedrop/dockerfiles/focal/python3/Dockerfile +++ b/securedrop/dockerfiles/focal/python3/Dockerfile @@ -11,7 +11,7 @@ RUN apt-get update && apt-get install -y paxctl && \ paxctl -cm /usr/bin/mono-sgen && dpkg-reconfigure mono-runtime-sgen && \ apt-get install -y apache2-dev coreutils devscripts vim \ python3-pip python3-all python3-venv virtualenv libpython3.8-dev libssl-dev \ - gnupg2 ruby redis-server git xvfb haveged curl wget \ + gnupg2 ruby redis-server git xvfb curl wget \ gettext paxctl x11vnc enchant libffi-dev sqlite3 gettext sudo \ libasound2 libdbus-glib-1-2 libgtk2.0-0 libfontconfig1 libxrender1 \ libcairo-gobject2 libgtk-3-0 libstartup-notification0 tor diff --git a/securedrop/dockerfiles/xenial/python3/Dockerfile b/securedrop/dockerfiles/xenial/python3/Dockerfile index 397c5a77cf..c1db1b2ee6 100644 --- a/securedrop/dockerfiles/xenial/python3/Dockerfile +++ b/securedrop/dockerfiles/xenial/python3/Dockerfile @@ -11,7 +11,7 @@ RUN apt-get update && apt-get install -y paxctl && \ paxctl -cm /usr/bin/mono-sgen && dpkg-reconfigure mono-runtime-sgen && \ apt-get install -y apache2-dev coreutils devscripts dh-virtualenv vim \ python3-pip python3-all python3-venv virtualenv libpython3.5-dev libssl-dev \ - gnupg2 ruby redis-server git xvfb haveged curl wget \ + gnupg2 ruby redis-server git xvfb curl wget \ gettext paxctl x11vnc enchant libffi-dev sqlite3 gettext sudo \ libasound2 libdbus-glib-1-2 libgtk2.0-0 libfontconfig1 libxrender1 \ libcairo-gobject2 libgtk-3-0 libstartup-notification0 tor diff --git a/securedrop/execution.py b/securedrop/execution.py new file mode 100644 index 0000000000..f1aa12a00f --- /dev/null +++ b/securedrop/execution.py @@ -0,0 +1,11 @@ +from threading import Thread + + +def asynchronous(f): # type: ignore + """ + Wraps a + """ + def wrapper(*args, **kwargs): # type: ignore + thread = Thread(target=f, args=args, kwargs=kwargs) + thread.start() + return wrapper diff --git a/securedrop/journalist.py b/securedrop/journalist.py index 60150c7350..c9f450005c 100644 --- a/securedrop/journalist.py +++ b/securedrop/journalist.py @@ -4,7 +4,7 @@ from journalist_app import create_app from models import Source -from source_app.utils import asynchronous +from execution import asynchronous app = create_app(config) diff --git a/securedrop/journalist_app/api.py b/securedrop/journalist_app/api.py index e7467e91fb..425775fa1f 100644 --- a/securedrop/journalist_app/api.py +++ b/securedrop/journalist_app/api.py @@ -175,11 +175,7 @@ def remove_star(source_uuid: str) -> Tuple[flask.Response, int]: @api.route('/sources//flag', methods=['POST']) @token_required def flag(source_uuid: str) -> Tuple[flask.Response, int]: - source = get_or_404(Source, source_uuid, - column=Source.uuid) - source.flagged = True - db.session.commit() - return jsonify({'message': 'Source flagged for reply'}), 200 + return jsonify({'message': 'Sources no longer need to be flagged for reply'}), 200 @api.route('/sources//submissions', methods=['GET']) @token_required diff --git a/securedrop/journalist_app/main.py b/securedrop/journalist_app/main.py index 320c6828ee..1075d637a8 100644 --- a/securedrop/journalist_app/main.py +++ b/securedrop/journalist_app/main.py @@ -169,13 +169,6 @@ def reply() -> werkzeug.Response: finally: return redirect(url_for('col.col', filesystem_id=g.filesystem_id)) - @view.route('/flag', methods=('POST',)) - def flag() -> str: - g.source.flagged = True - db.session.commit() - return render_template('flag.html', filesystem_id=g.filesystem_id, - codename=g.source.journalist_designation) - @view.route('/bulk', methods=('POST',)) def bulk() -> Union[str, werkzeug.Response]: action = request.form['action'] diff --git a/securedrop/journalist_templates/col.html b/securedrop/journalist_templates/col.html index 7a08c5825d..988f4a1b47 100644 --- a/securedrop/journalist_templates/col.html +++ b/securedrop/journalist_templates/col.html @@ -106,16 +106,9 @@

{{ gettext('Reply') }}

- {% elif source.flagged %} -

{{ gettext("You've flagged this source for reply.") }}

-

{{ gettext("An encryption key will be generated for the source the next time they log in, after which you will be able to reply to the source here.") }}

{% else %} -

{{ gettext("Click below if you would like to write a reply to this source.") }}

-
- - - -
+

{{ gettext("This source has no encryption key.") }}

+

{{ gettext("An encryption key will be generated for the source the next time they log in, after which you will be able to reply to the source here.") }}

{% endif %}
diff --git a/securedrop/journalist_templates/flag.html b/securedrop/journalist_templates/flag.html deleted file mode 100644 index 89e93b0b6d..0000000000 --- a/securedrop/journalist_templates/flag.html +++ /dev/null @@ -1,11 +0,0 @@ -{% extends "base.html" %} -{% block body %} -

- {{ gettext('info icon') }} -{{ gettext('Thanks!') }} -

- -

{{ gettext('SecureDrop will generate a secure encryption key for this source the next time that they log in. Once the key has been generated, a reply box will appear under their collection of documents. You can use this box to write encrypted replies to them.') }}

- -

{{ gettext('Continue to the list of documents for {codename}...').format(codename=codename) }}

-{% endblock %} diff --git a/securedrop/models.py b/securedrop/models.py index cfddd930d5..272e8784bb 100644 --- a/securedrop/models.py +++ b/securedrop/models.py @@ -56,7 +56,6 @@ class Source(db.Model): uuid = Column(String(36), unique=True, nullable=False) filesystem_id = Column(String(96), unique=True) journalist_designation = Column(String(255), nullable=False) - flagged = Column(Boolean, default=False) last_updated = Column(DateTime) star = relationship( "SourceStar", uselist=False, backref="source" @@ -132,7 +131,7 @@ def to_json(self) -> 'Dict[str, Union[str, bool, int, str]]': 'uuid': self.uuid, 'url': url_for('api.single_source', source_uuid=self.uuid), 'journalist_designation': self.journalist_designation, - 'is_flagged': self.flagged, + 'is_flagged': False, 'is_starred': starred, 'last_updated': last_updated, 'interaction_count': self.interaction_count, diff --git a/securedrop/source_app/main.py b/securedrop/source_app/main.py index 057d2eb63d..f426220d61 100644 --- a/securedrop/source_app/main.py +++ b/securedrop/source_app/main.py @@ -19,8 +19,7 @@ from sdconfig import SDConfig from source_app.decorators import login_required from source_app.utils import (logged_in, generate_unique_codename, - async_genkey, normalize_timestamps, - valid_codename, get_entropy_estimate) + normalize_timestamps, valid_codename) from source_app.forms import LoginForm, SubmissionForm @@ -137,24 +136,15 @@ def lookup() -> str: replies.sort(key=operator.attrgetter('date'), reverse=True) # Generate a keypair to encrypt replies from the journalist - # Only do this if the journalist has flagged the source as one - # that they would like to reply to. (Issue #140.) - if not current_app.crypto_util.get_fingerprint(g.filesystem_id) and \ - g.source.flagged: - db_uri = current_app.config['SQLALCHEMY_DATABASE_URI'] - async_genkey(current_app.crypto_util, - db_uri, - g.filesystem_id, - g.codename) + if not current_app.crypto_util.get_fingerprint(g.filesystem_id): + current_app.crypto_util.genkeypair(g.filesystem_id, g.codename) return render_template( 'lookup.html', allow_document_uploads=current_app.instance_config.allow_document_uploads, codename=g.codename, replies=replies, - flagged=g.source.flagged, new_user=session.get('new_user', None), - haskey=current_app.crypto_util.get_fingerprint(g.filesystem_id), form=SubmissionForm(), ) @@ -236,26 +226,11 @@ def submit() -> werkzeug.Response: db.session.add(submission) new_submissions.append(submission) - if g.source.pending: + # If necessary, generate a keypair to encrypt replies from the journalist + if g.source.pending or not current_app.crypto_util.get_fingerprint(g.filesystem_id): + current_app.crypto_util.genkeypair(g.filesystem_id, g.codename) g.source.pending = False - # Generate a keypair now, if there's enough entropy (issue #303) - # (gpg reads 300 bytes from /dev/random) - entropy_avail = get_entropy_estimate() - if entropy_avail >= 2400: - db_uri = current_app.config['SQLALCHEMY_DATABASE_URI'] - - async_genkey(current_app.crypto_util, - db_uri, - g.filesystem_id, - g.codename) - current_app.logger.info("generating key, entropy: {}".format( - entropy_avail)) - else: - current_app.logger.warning( - "skipping key generation. entropy: {}".format( - entropy_avail)) - g.source.last_updated = datetime.utcnow() db.session.commit() diff --git a/securedrop/source_app/utils.py b/securedrop/source_app/utils.py index 9dc9729ac0..df79ea53ef 100644 --- a/securedrop/source_app/utils.py +++ b/securedrop/source_app/utils.py @@ -1,18 +1,12 @@ -import io -import logging import subprocess -from datetime import datetime from flask import session, current_app, abort, g -from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker -from threading import Thread import typing import re -from crypto_util import CryptoUtil, CryptoException +from crypto_util import CryptoException from models import Source from passphrases import PassphraseGenerator, DicewarePassphrase from sdconfig import SDConfig @@ -57,44 +51,6 @@ def generate_unique_codename(config: SDConfig) -> DicewarePassphrase: return passphrase -def get_entropy_estimate() -> int: - with io.open('/proc/sys/kernel/random/entropy_avail') as f: - return int(f.read()) - - -def asynchronous(f): # type: ignore - def wrapper(*args, **kwargs): # type: ignore - thread = Thread(target=f, args=args, kwargs=kwargs) - thread.start() - return wrapper - - -@asynchronous -def async_genkey(crypto_util_: CryptoUtil, - db_uri: str, - filesystem_id: str, - codename: DicewarePassphrase) -> None: - # We pass in the `crypto_util_` so we don't have to reference `current_app` - # here. The app might not have a pushed context during testing which would - # cause this asynchronous function to break. - crypto_util_.genkeypair(filesystem_id, codename) - - # Register key generation as update to the source, so sources will - # filter to the top of the list in the journalist interface if a - # flagged source logs in and has a key generated for them. #789 - session = sessionmaker(bind=create_engine(db_uri))() - try: - source = session.query(Source).filter( - Source.filesystem_id == filesystem_id).one() - source.last_updated = datetime.utcnow() - session.commit() - except Exception as e: - logging.getLogger(__name__).error( - "async_genkey for source (filesystem_id={}): {}" - .format(filesystem_id, e)) - session.close() - - def normalize_timestamps(filesystem_id: str) -> None: """ Update the timestamps on all of the source's submissions. This diff --git a/securedrop/source_templates/lookup.html b/securedrop/source_templates/lookup.html index 7a7bfce05a..9b1c8d359c 100644 --- a/securedrop/source_templates/lookup.html +++ b/securedrop/source_templates/lookup.html @@ -19,16 +19,6 @@
{% include 'flashed.html' %} - - {% if flagged and not haskey %} -
-
- {{ gettext('Sorry we haven\'t responded yet!') }} -

{{ gettext('Our SecureDrop recently experienced a surge of activity. For security reasons, the creation of a two-way communication channel was delayed until you checked in again.') }}

-

{{ gettext('Please rest assured that we were able to download your submission, and check back again later for a reply.') }}

-
-
- {% endif %}
{% if allow_document_uploads %} diff --git a/securedrop/tests/functional/functional_test.py b/securedrop/tests/functional/functional_test.py index cc860c7806..33fe84375b 100644 --- a/securedrop/tests/functional/functional_test.py +++ b/securedrop/tests/functional/functional_test.py @@ -207,87 +207,84 @@ def sd_servers(self): # Patch the two-factor verification to avoid intermittent errors logging.info("Mocking models.Journalist.verify_token") with mock.patch("models.Journalist.verify_token", return_value=True): - logging.info("Mocking source_app.main.get_entropy_estimate") - with mock.patch("source_app.main.get_entropy_estimate", return_value=8192): - - try: - signal.signal(signal.SIGUSR1, lambda _, s: traceback.print_stack(s)) + try: + signal.signal(signal.SIGUSR1, lambda _, s: traceback.print_stack(s)) - source_port = self._unused_port() - journalist_port = self._unused_port() + source_port = self._unused_port() + journalist_port = self._unused_port() - self.source_location = "http://127.0.0.1:%d" % source_port - self.journalist_location = "http://127.0.0.1:%d" % journalist_port + self.source_location = "http://127.0.0.1:%d" % source_port + self.journalist_location = "http://127.0.0.1:%d" % journalist_port - self.source_app = source_app.create_app(config) - self.journalist_app = journalist_app.create_app(config) - self.journalist_app.config["WTF_CSRF_ENABLED"] = True + self.source_app = source_app.create_app(config) + self.journalist_app = journalist_app.create_app(config) + self.journalist_app.config["WTF_CSRF_ENABLED"] = True - self.__context = self.journalist_app.app_context() - self.__context.push() + self.__context = self.journalist_app.app_context() + self.__context.push() - env.create_directories() - db.create_all() - self.gpg = env.init_gpg() + env.create_directories() + db.create_all() + self.gpg = env.init_gpg() - # Add our test user - try: - valid_password = "correct horse battery staple profanity oil chewy" - user = Journalist( - username="journalist", password=valid_password, is_admin=True - ) - user.otp_secret = "JHCOGO7VCER3EJ4L" - db.session.add(user) - db.session.commit() - except IntegrityError: - logging.error("Test user already added") - db.session.rollback() - - # This user is required for our tests cases to login - self.admin_user = { - "name": "journalist", - "password": ("correct horse battery staple" " profanity oil chewy"), - "secret": "JHCOGO7VCER3EJ4L", - } - - self.admin_user["totp"] = pyotp.TOTP(self.admin_user["secret"]) - - def start_journalist_server(app): - app.run(port=journalist_port, debug=True, use_reloader=False, threaded=True) - - self.source_process = Process( - target=lambda: self.start_source_server(source_port) + # Add our test user + try: + valid_password = "correct horse battery staple profanity oil chewy" + user = Journalist( + username="journalist", password=valid_password, is_admin=True ) + user.otp_secret = "JHCOGO7VCER3EJ4L" + db.session.add(user) + db.session.commit() + except IntegrityError: + logging.error("Test user already added") + db.session.rollback() + + # This user is required for our tests cases to login + self.admin_user = { + "name": "journalist", + "password": ("correct horse battery staple" " profanity oil chewy"), + "secret": "JHCOGO7VCER3EJ4L", + } + + self.admin_user["totp"] = pyotp.TOTP(self.admin_user["secret"]) + + def start_journalist_server(app): + app.run(port=journalist_port, debug=True, use_reloader=False, threaded=True) + + self.source_process = Process( + target=lambda: self.start_source_server(source_port) + ) - self.journalist_process = Process( - target=lambda: start_journalist_server(self.journalist_app) - ) + self.journalist_process = Process( + target=lambda: start_journalist_server(self.journalist_app) + ) - self.source_process.start() - self.journalist_process.start() - - for tick in range(30): - try: - requests.get(self.source_location, timeout=1) - requests.get(self.journalist_location, timeout=1) - except Exception: - time.sleep(0.25) - else: - break - yield - finally: - try: - self.source_process.terminate() - except Exception as e: - logging.error("Error stopping source app: %s", e) + self.source_process.start() + self.journalist_process.start() + for tick in range(30): try: - self.journalist_process.terminate() - except Exception as e: - logging.error("Error stopping source app: %s", e) + requests.get(self.source_location, timeout=1) + requests.get(self.journalist_location, timeout=1) + except Exception: + time.sleep(0.25) + else: + break + yield + finally: + try: + self.source_process.terminate() + except Exception as e: + logging.error("Error stopping source app: %s", e) + + try: + self.journalist_process.terminate() + except Exception as e: + logging.error("Error stopping source app: %s", e) - env.teardown() - self.__context.pop() + env.teardown() + self.__context.pop() def wait_for_source_key(self, source_name): filesystem_id = self.source_app.crypto_util.hash_codename(source_name) diff --git a/securedrop/tests/functional/journalist_navigation_steps.py b/securedrop/tests/functional/journalist_navigation_steps.py index 3f3813a7db..f2f3608e00 100644 --- a/securedrop/tests/functional/journalist_navigation_steps.py +++ b/securedrop/tests/functional/journalist_navigation_steps.py @@ -1065,9 +1065,6 @@ def _journalist_delete_one(self): el.location_once_scrolled_into_view ActionChains(self.driver).move_to_element(el).click().perform() - def _journalist_flags_source(self): - self.safe_click_by_id("flag-button") - def _journalist_visits_admin(self): self.driver.get(self.journalist_location + "/admin") diff --git a/securedrop/tests/i18n/securedrop/translations/de_DE/LC_MESSAGES/messages.po b/securedrop/tests/i18n/securedrop/translations/de_DE/LC_MESSAGES/messages.po index 48a4b39cee..0d0154cd8f 100644 --- a/securedrop/tests/i18n/securedrop/translations/de_DE/LC_MESSAGES/messages.po +++ b/securedrop/tests/i18n/securedrop/translations/de_DE/LC_MESSAGES/messages.po @@ -501,10 +501,6 @@ msgstr "" "Sie können der Person, die diese Dokumente eingereicht hat, eine sichere " "Antwort schreiben:" -#: journalist_templates/col.html:86 -msgid "You've flagged this source for reply." -msgstr "Sie haben diese Quelle für eine Antwort markiert." - #: journalist_templates/col.html:87 msgid "" "An encryption key will be generated for the source the next time they log " diff --git a/securedrop/tests/i18n/securedrop/translations/nl/LC_MESSAGES/messages.po b/securedrop/tests/i18n/securedrop/translations/nl/LC_MESSAGES/messages.po index e3da9eddbe..3b701822e5 100644 --- a/securedrop/tests/i18n/securedrop/translations/nl/LC_MESSAGES/messages.po +++ b/securedrop/tests/i18n/securedrop/translations/nl/LC_MESSAGES/messages.po @@ -495,10 +495,6 @@ msgid "" msgstr "" "U kunt beveiligd reageren naar de persoon die deze documenten gestuurd heeft:" -#: journalist_templates/col.html:86 -msgid "You've flagged this source for reply." -msgstr "U heeft deze bron gemarkeerd om te reageren." - #: journalist_templates/col.html:87 msgid "" "An encryption key will be generated for the source the next time they log " diff --git a/securedrop/tests/migrations/migration_b060f38c0c31.py b/securedrop/tests/migrations/migration_b060f38c0c31.py new file mode 100644 index 0000000000..c75f8be001 --- /dev/null +++ b/securedrop/tests/migrations/migration_b060f38c0c31.py @@ -0,0 +1,184 @@ +# -*- coding: utf-8 -*- + +import random +import uuid +from typing import Any, Dict + +import pytest +from sqlalchemy import text +from sqlalchemy.exc import OperationalError + +from db import db +from journalist_app import create_app +from .helpers import random_chars, random_datetime, bool_or_none + +random.seed("ᕕ( ᐛ )ᕗ") + + +def add_submission(source_id): + params = { + "uuid": str(uuid.uuid4()), + "source_id": source_id, + "filename": random_chars(50), + "size": random.randint(0, 1024 * 1024 * 500), + "downloaded": bool_or_none(), + "checksum": random_chars(255, chars="0123456789abcdef") + } + sql = """ + INSERT INTO submissions (uuid, source_id, filename, size, downloaded, checksum) + VALUES (:uuid, :source_id, :filename, :size, :downloaded, :checksum) + """ + db.engine.execute(text(sql), **params) + + +class UpgradeTester: + """ + Verify that the Source.flagged column no longer exists. + """ + + source_count = 10 + original_sources = {} # type: Dict[str, Any] + source_submissions = {} # type: Dict[str, Any] + + def __init__(self, config): + self.config = config + self.app = create_app(config) + + def load_data(self): + with self.app.app_context(): + for i in range(self.source_count): + self.add_source() + + self.original_sources = { + s.uuid: s for s in db.engine.execute(text("SELECT * FROM sources")).fetchall() + } + + for s in self.original_sources.values(): + for i in range(random.randint(0, 3)): + add_submission(s.id) + + self.source_submissions[s.id] = db.engine.execute( + text("SELECT * FROM submissions WHERE source_id = :source_id"), + **{"source_id": s.id} + ).fetchall() + + def add_source(self): + params = { + "uuid": str(uuid.uuid4()), + "filesystem_id": random_chars(96), + "journalist_designation": random_chars(50), + "flagged": bool_or_none(), + "last_updated": random_datetime(nullable=True), + "pending": bool_or_none(), + "interaction_count": random.randint(0, 1000), + } + sql = """ + INSERT INTO sources (uuid, filesystem_id, + journalist_designation, flagged, last_updated, pending, + interaction_count) + VALUES (:uuid, :filesystem_id, :journalist_designation, + :flagged, :last_updated, :pending, :interaction_count) + """ + + db.engine.execute(text(sql), **params) + + def check_upgrade(self): + with self.app.app_context(): + + # check that the flagged column is gone + with pytest.raises(OperationalError, match=".*sources has no column named flagged.*"): + self.add_source() + + # check that the sources are otherwise unchanged + sources = db.engine.execute(text("SELECT * FROM sources")).fetchall() + assert len(sources) == len(self.original_sources) + for source in sources: + assert not hasattr(source, "flagged") + original_source = self.original_sources[source.uuid] + assert source.id == original_source.id + assert source.journalist_designation == original_source.journalist_designation + assert source.last_updated == original_source.last_updated + assert source.pending == original_source.pending + assert source.interaction_count == original_source.interaction_count + + source_submissions = db.engine.execute( + text("SELECT * FROM submissions WHERE source_id = :source_id"), + **{"source_id": source.id} + ).fetchall() + assert source_submissions == self.source_submissions[source.id] + + +class DowngradeTester: + """ + Verify that the Source.flagged column has been recreated properly. + """ + + source_count = 10 + original_sources = {} # type: Dict[str, Any] + source_submissions = {} # type: Dict[str, Any] + + def __init__(self, config): + self.config = config + self.app = create_app(config) + + def add_source(self): + params = { + "uuid": str(uuid.uuid4()), + "filesystem_id": random_chars(96), + "journalist_designation": random_chars(50), + "last_updated": random_datetime(nullable=True), + "pending": bool_or_none(), + "interaction_count": random.randint(0, 1000), + "deleted_at": None, + } + sql = """ + INSERT INTO sources ( + uuid, filesystem_id, journalist_designation, last_updated, pending, + interaction_count + ) VALUES ( + :uuid, :filesystem_id, :journalist_designation, :last_updated, :pending, + :interaction_count + ) + """ + + db.engine.execute(text(sql), **params) + + def load_data(self): + with self.app.app_context(): + for i in range(self.source_count): + self.add_source() + + self.original_sources = { + s.uuid: s for s in db.engine.execute(text("SELECT * FROM sources")).fetchall() + } + + for s in self.original_sources.values(): + for i in range(random.randint(0, 3)): + add_submission(s.id) + + self.source_submissions[s.id] = db.engine.execute( + text("SELECT * FROM submissions WHERE source_id = :source_id"), + **{"source_id": s.id} + ).fetchall() + + def check_downgrade(self): + with self.app.app_context(): + # check that the sources are otherwise unchanged + sources = db.engine.execute(text("SELECT * FROM sources")).fetchall() + assert len(sources) == len(self.original_sources) + for source in sources: + assert hasattr(source, "flagged") + original_source = self.original_sources[source.uuid] + assert source.id == original_source.id + assert source.journalist_designation == original_source.journalist_designation + assert source.last_updated == original_source.last_updated + assert source.pending == original_source.pending + assert source.interaction_count == original_source.interaction_count + assert not hasattr(original_source, "flagged") + assert source.flagged is None + + source_submissions = db.engine.execute( + text("SELECT * FROM submissions WHERE source_id = :source_id"), + **{"source_id": source.id} + ).fetchall() + assert source_submissions == self.source_submissions[source.id] diff --git a/securedrop/tests/pageslayout/test_journalist.py b/securedrop/tests/pageslayout/test_journalist.py index bb4a4f88d7..43cf46455b 100644 --- a/securedrop/tests/pageslayout/test_journalist.py +++ b/securedrop/tests/pageslayout/test_journalist.py @@ -135,19 +135,6 @@ def test_col_has_no_key(self): self._journalist_visits_col() self._screenshot('journalist-col_has_no_key.png') - def test_col_flagged(self): - self._source_visits_source_homepage() - self._source_chooses_to_submit_documents() - self._source_continues_to_submit_page() - self._source_submits_a_file() - self._source_logs_out() - self._journalist_logs_in() - self._source_delete_key() - self._journalist_visits_col() - self._journalist_flags_source() - self._journalist_continues_after_flagging() - self._screenshot('journalist-col_flagged.png') - def test_col(self): self._source_visits_source_homepage() self._source_chooses_to_submit_documents() @@ -274,18 +261,6 @@ def test_admin_interface_index(self): self._admin_visits_admin_interface() self._screenshot('journalist-admin_interface_index.png') - def test_flag(self): - self._source_visits_source_homepage() - self._source_chooses_to_submit_documents() - self._source_continues_to_submit_page() - self._source_submits_a_file() - self._source_logs_out() - self._journalist_logs_in() - self._source_delete_key() - self._journalist_visits_col() - self._journalist_flags_source() - self._screenshot('journalist-flag.png') - def test_index_no_documents(self): self._journalist_logs_in() self._screenshot('journalist-index_no_documents.png') diff --git a/securedrop/tests/pageslayout/test_source.py b/securedrop/tests/pageslayout/test_source.py index 0f66764296..5775d8c7a9 100644 --- a/securedrop/tests/pageslayout/test_source.py +++ b/securedrop/tests/pageslayout/test_source.py @@ -94,21 +94,6 @@ def test_source_checks_for_reply(self): self._source_deletes_a_journalist_reply() self._screenshot('source-deletes_reply.png') - def test_source_flagged(self): - self._source_visits_source_homepage() - self._source_chooses_to_submit_documents() - self._source_continues_to_submit_page() - self._source_submits_a_file() - self._source_logs_out() - self._journalist_logs_in() - self._source_delete_key() - self._journalist_visits_col() - self._journalist_flags_source() - self._source_visits_source_homepage() - self._source_chooses_to_login() - self._source_proceeds_to_login() - self._screenshot('source-flagged.png') - def test_notfound(self): self._source_not_found() self._screenshot('source-notfound.png') diff --git a/securedrop/tests/test_integration.py b/securedrop/tests/test_integration.py index b0065990d0..ab5f993d99 100644 --- a/securedrop/tests/test_integration.py +++ b/securedrop/tests/test_integration.py @@ -10,8 +10,6 @@ from distutils.version import StrictVersion from io import BytesIO -import mock -import pytest from bs4 import BeautifulSoup from flask import current_app, escape, g, session from pyotp import HOTP, TOTP @@ -27,18 +25,6 @@ random.seed('ಠ_ಠ') -@pytest.fixture(autouse=True, scope="module") -def patch_get_entropy_estimate(): - mock_get_entropy_estimate = mock.patch( - "source_app.main.get_entropy_estimate", - return_value=8192 - ).start() - - yield - - mock_get_entropy_estimate.stop() - - def _login_user(app, user_dict): resp = app.post('/login', data={'username': user_dict['username'], @@ -266,7 +252,6 @@ def _helper_test_reply(journalist_app, source_app, config, test_journo, fh=(BytesIO(b''), ''), ), follow_redirects=True) assert resp.status_code == 200 - assert not g.source.flagged app.get('/logout') with journalist_app.test_client() as app: @@ -281,31 +266,7 @@ def _helper_test_reply(journalist_app, source_app, config, test_journo, resp = app.get(col_url) assert resp.status_code == 200 - with source_app.test_client() as app: - resp = app.post('/login', data=dict( - codename=codename), follow_redirects=True) - assert resp.status_code == 200 - assert not g.source.flagged - app.get('/logout') - - with journalist_app.test_client() as app: - _login_user(app, test_journo) - resp = app.post('/flag', data=dict( - filesystem_id=filesystem_id)) - assert resp.status_code == 200 - - with source_app.test_client() as app: - resp = app.post('/login', data=dict( - codename=codename), follow_redirects=True) - assert resp.status_code == 200 - app.get('/lookup') - assert g.source.flagged - app.get('/logout') - - # Block up to 15s for the reply keypair, so we can test sending a reply - def assertion(): - assert current_app.crypto_util.get_fingerprint(filesystem_id) is not None - utils.asynchronous.wait_for_assertion(assertion, 15) + assert current_app.crypto_util.get_fingerprint(filesystem_id) is not None # Create 2 replies to test deleting on journalist and source interface with journalist_app.test_client() as app: @@ -472,7 +433,6 @@ def test_unicode_reply_with_ansi_env(journalist_app, def test_delete_collection(mocker, source_app, journalist_app, test_journo): """Test the "delete collection" button on each collection page""" - async_genkey = mocker.patch('source_app.main.async_genkey') # first, add a source with source_app.test_client() as app: @@ -510,7 +470,6 @@ def test_delete_collection(mocker, source_app, journalist_app, test_journo): "The account and data for the source {} have been deleted.".format(col_name)) in text assert "No documents have been submitted!" in text - assert async_genkey.called # Make sure the collection is deleted from the filesystem def assertion(): @@ -522,7 +481,6 @@ def assertion(): def test_delete_collections(mocker, journalist_app, source_app, test_journo): """Test the "delete selected" checkboxes on the index page that can be used to delete multiple collections""" - async_genkey = mocker.patch('source_app.main.async_genkey') # first, add some sources with source_app.test_client() as app: @@ -552,7 +510,6 @@ def test_delete_collections(mocker, journalist_app, source_app, test_journo): assert resp.status_code == 200 text = resp.data.decode('utf-8') assert "The accounts and all data for {} sources".format(num_sources) in text - assert async_genkey.called # simulate the source_deleter's work journalist_app_module.utils.purge_deleted_sources() diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 03c7267a1d..667aae65f4 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -300,20 +300,6 @@ def test_get_non_existant_source_404s(journalist_app, journalist_api_token): assert response.status_code == 404 -def test_authorized_user_can_flag_a_source(journalist_app, test_source, - journalist_api_token): - with journalist_app.test_client() as app: - uuid = test_source['source'].uuid - source_id = test_source['source'].id - response = app.post(url_for('api.flag', source_uuid=uuid), - headers=get_api_headers(journalist_api_token)) - - assert response.status_code == 200 - - # Verify that the source was flagged. - assert Source.query.get(source_id).flagged - - def test_authorized_user_can_star_a_source(journalist_app, test_source, journalist_api_token): with journalist_app.test_client() as app: diff --git a/securedrop/tests/test_source.py b/securedrop/tests/test_source.py index 04a31ac073..b601a9c083 100644 --- a/securedrop/tests/test_source.py +++ b/securedrop/tests/test_source.py @@ -442,40 +442,6 @@ def test_submit_both(source_app): assert "Thanks! We received your message and document" in text -def test_submit_message_with_low_entropy(source_app): - with patch.object(source_app_main, 'async_genkey') as async_genkey: - with patch.object(source_app_main, 'get_entropy_estimate') \ - as get_entropy_estimate: - get_entropy_estimate.return_value = 300 - - with source_app.test_client() as app: - new_codename(app, session) - _dummy_submission(app) - resp = app.post( - url_for('main.submit'), - data=dict(msg="This is a test.", fh=(StringIO(''), '')), - follow_redirects=True) - assert resp.status_code == 200 - assert not async_genkey.called - - -def test_submit_message_with_enough_entropy(source_app): - with patch.object(source_app_main, 'async_genkey') as async_genkey: - with patch.object(source_app_main, 'get_entropy_estimate') \ - as get_entropy_estimate: - get_entropy_estimate.return_value = 2400 - - with source_app.test_client() as app: - new_codename(app, session) - _dummy_submission(app) - resp = app.post( - url_for('main.submit'), - data=dict(msg="This is a test.", fh=(StringIO(''), '')), - follow_redirects=True) - assert resp.status_code == 200 - assert async_genkey.called - - def test_delete_all_successfully_deletes_replies(source_app): with source_app.app_context(): journalist, _ = utils.db_helper.init_journalist() @@ -732,26 +698,25 @@ def test_failed_normalize_timestamps_logs_warning(source_app): still occur, but a warning should be logged (this will trigger an OSSEC alert).""" - with patch("source_app.main.get_entropy_estimate", return_value=8192): - with patch.object(source_app.logger, 'warning') as logger: - with patch.object(subprocess, 'call', return_value=1): - with source_app.test_client() as app: - new_codename(app, session) - _dummy_submission(app) - resp = app.post( - url_for('main.submit'), - data=dict( - msg="This is a test.", - fh=(StringIO(''), '')), - follow_redirects=True) - assert resp.status_code == 200 - text = resp.data.decode('utf-8') - assert "Thanks! We received your message" in text - - logger.assert_called_once_with( - "Couldn't normalize submission " - "timestamps (touch exited with 1)" - ) + with patch.object(source_app.logger, 'warning') as logger: + with patch.object(subprocess, 'call', return_value=1): + with source_app.test_client() as app: + new_codename(app, session) + _dummy_submission(app) + resp = app.post( + url_for('main.submit'), + data=dict( + msg="This is a test.", + fh=(StringIO(''), '')), + follow_redirects=True) + assert resp.status_code == 200 + text = resp.data.decode('utf-8') + assert "Thanks! We received your message" in text + + logger.assert_called_once_with( + "Couldn't normalize submission " + "timestamps (touch exited with 1)" + ) def test_source_is_deleted_while_logged_in(source_app):