Refactor scrypt hashing and source app session mgmt pt 2/3

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

@@ -219,6 +219,7 @@
   /var/www/securedrop/source_app/forms.py r,
   /var/www/securedrop/source_app/info.py r,
   /var/www/securedrop/source_app/main.py r,
+  /var/www/securedrop/source_app/session_manager.py r,
   /var/www/securedrop/source_app/utils.py r,
   /var/www/securedrop/source_templates/banner_warning_flashed.html r,
   /var/www/securedrop/source_templates/base.html r,

securedrop/crypto_util.py

@@ -7,10 +7,8 @@
 import os
 import io
 import re
-import scrypt
 from random import SystemRandom
 
-from base64 import b32encode
 from datetime import date
 from flask import current_app
 from pretty_bad_protocol._util import _is_stream, _make_binary_stream
@@ -22,7 +20,6 @@
 import rm
 
 from models import Source
-from passphrases import DicewarePassphrase
 
 
 # monkey patch to work with Focal gnupg.
@@ -86,9 +83,6 @@ class CryptoUtil:
     SOURCE_KEY_UID_RE = re.compile(r"(Source|Autogenerated) Key <[-A-Za-z0-9+/=_]+>")
 
     def __init__(self,
-                 scrypt_params: Dict[str, int],
-                 scrypt_id_pepper: str,
-                 scrypt_gpg_pepper: str,
                  securedrop_root: str,
                  nouns_file: str,
                  adjectives_file: str,
@@ -99,13 +93,8 @@ def __init__(self,
             # Optimize crypto to speed up tests (at the expense of security
             # DO NOT use these settings in production)
             self.__gpg_key_length = 1024
-            self.scrypt_params = dict(N=2**1, r=1, p=1)
         else:  # pragma: no cover
             self.__gpg_key_length = 4096
-            self.scrypt_params = scrypt_params
-
-        self.scrypt_id_pepper = scrypt_id_pepper
-        self.scrypt_gpg_pepper = scrypt_gpg_pepper
 
         self.do_runtime_tests()
 
@@ -130,8 +119,6 @@ def __init__(self,
 
     # Make sure these pass before the app can run
     def do_runtime_tests(self) -> None:
-        if self.scrypt_id_pepper == self.scrypt_gpg_pepper:
-            raise AssertionError('scrypt_id_pepper == scrypt_gpg_pepper')
         # crash if we don't have a way to securely remove files
         if not rm.check_secure_delete_capability():
             raise AssertionError("Secure file deletion is not possible.")
@@ -153,18 +140,6 @@ def display_id(self) -> str:
 
         raise ValueError("Could not generate unique journalist designation for new source")
 
-    # TODO(AD): This will be removed in my next PR and replaced by SourceUser
-    def hash_codename(self, codename: DicewarePassphrase, salt: Optional[str] = None) -> str:
-        """Salts and hashes a codename using scrypt.
-
-        :param codename: A source's codename.
-        :param salt: The salt to mix with the codename when hashing.
-        :returns: A base32 encoded string; the salted codename hash.
-        """
-        if salt is None:
-            salt = self.scrypt_id_pepper
-        return b32encode(scrypt.hash(codename, salt, **self.scrypt_params)).decode('utf-8')
-
     def genkeypair(self, source_user: SourceUser) -> gnupg._parsers.GenKey:
         """Generate a GPG key through batch file key generation.
         """
@@ -279,17 +254,6 @@ def encrypt(self, plaintext: str, fingerprints: List[str], output: Optional[str]
         else:
             raise CryptoException(out.stderr)
 
-    def decrypt(self, secret: DicewarePassphrase, ciphertext: bytes) -> str:
-        """
-        >>> crypto = current_app.crypto_util
-        >>> key = crypto.genkeypair('randomid', 'randomid')
-        >>> message = u'Buenos días, mundo hermoso!'
-        >>> ciphertext = crypto.encrypt(message, str(key))
-        >>> crypto.decrypt('randomid', ciphertext) == message.encode('utf-8')
-        True
-        """
-        hashed_codename = self.hash_codename(secret,
-                                             salt=self.scrypt_gpg_pepper)
-        data = self.gpg.decrypt(ciphertext, passphrase=hashed_codename).data
-
+    def decrypt(self, source_user: SourceUser, ciphertext: bytes) -> str:
+        data = self.gpg.decrypt(ciphertext, passphrase=source_user.gpg_secret).data
         return data.decode('utf-8')

securedrop/journalist_app/__init__.py

@@ -79,9 +79,6 @@ def create_app(config: 'SDConfig') -> Flask:
     # TODO: Attaching a CryptoUtil dynamically like this disables all type checking (and
     # breaks code analysis tools) for code that uses current_app.storage; it should be refactored
     app.crypto_util = CryptoUtil(
-        scrypt_params=config.SCRYPT_PARAMS,
-        scrypt_id_pepper=config.SCRYPT_ID_PEPPER,
-        scrypt_gpg_pepper=config.SCRYPT_GPG_PEPPER,
         securedrop_root=config.SECUREDROP_ROOT,
         nouns_file=config.NOUNS,
         adjectives_file=config.ADJECTIVES,

securedrop/requirements/python3/securedrop-app-code-requirements.in

@@ -22,7 +22,6 @@ pyotp
 qrcode
 redis>=3.3.6
 rq>=1.1.0
-scrypt
 setuptools>=56.0.0
 sh
 SQLAlchemy>=1.3.0

securedrop/requirements/python3/securedrop-app-code-requirements.txt

@@ -240,18 +240,6 @@ rq==1.1.0 \
     --hash=sha256:2798d26a7b850e759f23f69695a389d676a9c08f2c14f96f0d34d9648c9d5616 \
     --hash=sha256:4f27c6a690d1bd02b9157e615d8819555b9b359c0c4ec8ff0013d160c31b40bb
     # via -r requirements/python3/securedrop-app-code-requirements.in
-scrypt==0.8.0 \
-    --hash=sha256:3422d11652cd12550540675e9fb54a1de6d60f3cbfedfb067284ef028589e2ee \
-    --hash=sha256:4e3cd639cc83e3f2c2241a01ebef9487c48b1ddf2d78f430d82d8f9ef60c6271 \
-    --hash=sha256:59a03822df232a30246b59a3801292a350d19fa636f11ea86df2214f35dade22 \
-    --hash=sha256:60e8c96a287ab892d9c7e1523d157ccfbdbe66da0c31738c8ed5732c2eea6a23 \
-    --hash=sha256:7e3be5fe70f74a01a83c60da0b010b2fe6c3d3a11b8a1c9532b583c9a440d529 \
-    --hash=sha256:ae2fd88756fb4d98ccc5e2639af55eeb80863b3f9f6e0f539e5ce050964cdd5e \
-    --hash=sha256:c0f90cabb8f6eaec05de5ce9aa9a9b67dc63d644e6b803beb1c43ae9b9452b65 \
-    --hash=sha256:c37a1f8440d7c621d9f23f3c1f2a28848bc50fefbca581fd7a1b01583a083c07 \
-    --hash=sha256:d4a5a4f53450b8ef629bbf1ee4be6105c69936e49b3d8bc621ac2287f0c86020 \
-    --hash=sha256:dc9abe69799ca423b938a06ddc33e5873e493ffcd68dbb9ba48396979b210d39
-    # via -r requirements/python3/securedrop-app-code-requirements.in
 sh==1.12.14 \
     --hash=sha256:ae3258c5249493cebe73cb4e18253a41ed69262484bad36fdb3efcb8ad8870bb \
     --hash=sha256:b52bf5833ed01c7b5c5fb73a7f71b3d98d48e9b9b8764236237bdc7ecae850fc

securedrop/source_app/__init__.py

@@ -4,13 +4,12 @@
 
 import werkzeug
 from flask import (Flask, render_template, escape, flash, Markup, request, g, session,
-                   url_for, redirect)
+                   url_for)
 from flask_babel import gettext
 from flask_assets import Environment
 from flask_wtf.csrf import CSRFProtect, CSRFError
 from jinja2 import evalcontextfilter
 from os import path
-from sqlalchemy.orm.exc import NoResultFound
 from typing import Tuple
 
 import i18n
@@ -19,12 +18,12 @@
 
 from crypto_util import CryptoUtil
 from db import db
-from models import InstanceConfig, Source
+from models import InstanceConfig
 from request_that_secures_file_uploads import RequestThatSecuresFileUploads
 from sdconfig import SDConfig
 from source_app import main, info, api
 from source_app.decorators import ignore_static
-from source_app.utils import logged_in, was_in_generate_flow
+from source_app.utils import clear_session_and_redirect_to_logged_out_page
 from store import Storage
 
 
@@ -77,9 +76,6 @@ def setup_i18n() -> None:
     # TODO: Attaching a CryptoUtil dynamically like this disables all type checking (and
     # breaks code analysis tools) for code that uses current_app.storage; it should be refactored
     app.crypto_util = CryptoUtil(
-        scrypt_params=config.SCRYPT_PARAMS,
-        scrypt_id_pepper=config.SCRYPT_ID_PEPPER,
-        scrypt_gpg_pepper=config.SCRYPT_GPG_PEPPER,
         securedrop_root=config.SECUREDROP_ROOT,
         nouns_file=config.NOUNS,
         adjectives_file=config.ADJECTIVES,
@@ -88,10 +84,7 @@ def setup_i18n() -> None:
 
     @app.errorhandler(CSRFError)
     def handle_csrf_error(e: CSRFError) -> werkzeug.Response:
-        msg = render_template('session_timeout.html')
-        session.clear()
-        flash(Markup(msg), "important")
-        return redirect(url_for('main.index'))
+        return clear_session_and_redirect_to_logged_out_page(session)
 
     assets = Environment(app)
     app.config['assets'] = assets
@@ -139,57 +132,17 @@ def load_instance_config() -> None:
     @app.before_request
     @ignore_static
     def setup_g() -> Optional[werkzeug.Response]:
-        """Store commonly used values in Flask's special g object"""
-
         if 'expires' in session and datetime.utcnow() >= session['expires']:
-            msg = render_template('session_timeout.html')
-
-            # Show expiration message only if the user was
-            # either in the codename generation flow or logged in
-            show_expiration_message = any([
-                session.get('show_expiration_message'),
-                logged_in(),
-                was_in_generate_flow(),
-            ])
-
-            # clear the session after we render the message so it's localized
-            session.clear()
-
-            # Persist this properety across sessions to distinguish users whose sessions expired
-            # from users who never logged in or generated a codename
-            session['show_expiration_message'] = show_expiration_message
-
-            # Redirect to index with flashed message
-            if session['show_expiration_message']:
-                flash(Markup(msg), "important")
-            return redirect(url_for('main.index'))
+            # If the session as has expired, redirect to index with flashed message
+            # if the user was in the middle of the generate flow
+            if 'codenames' in session:
+                return clear_session_and_redirect_to_logged_out_page(session)
 
         session['expires'] = datetime.utcnow() + \
             timedelta(minutes=getattr(config,
                                       'SESSION_EXPIRATION_MINUTES',
                                       120))
 
-        # ignore_static here because `crypto_util.hash_codename` is scrypt
-        # (very time consuming), and we don't need to waste time running if
-        # we're just serving a static resource that won't need to access
-        # these common values.
-        if logged_in():
-            g.codename = session['codename']
-            g.filesystem_id = app.crypto_util.hash_codename(g.codename)
-            try:
-                g.source = Source.query \
-                            .filter(Source.filesystem_id == g.filesystem_id) \
-                            .filter_by(deleted_at=None) \
-                            .one()
-            except NoResultFound as e:
-                app.logger.error(
-                    "Found no Sources when one was expected: %s" %
-                    (e,))
-                del session['logged_in']
-                del session['codename']
-                return redirect(url_for('main.index'))
-            g.loc = app.storage.path(g.filesystem_id)
-
         if app.instance_config.organization_name:
             g.organization_name = app.instance_config.organization_name
         else:

securedrop/source_app/decorators.py

@@ -1,19 +1,28 @@
 from typing import Any
 
-from flask import redirect, url_for, request
+from flask import redirect, url_for, request, session
 from functools import wraps
 
 from typing import Callable
 
-from source_app.utils import logged_in
+from source_app.utils import clear_session_and_redirect_to_logged_out_page
+from source_app.session_manager import SessionManager, UserNotLoggedIn, \
+    UserSessionExpired, UserHasBeenDeleted
 
 
 def login_required(f: Callable) -> Callable:
     @wraps(f)
     def decorated_function(*args: Any, **kwargs: Any) -> Any:
-        if not logged_in():
-            return redirect(url_for('main.login'))
-        return f(*args, **kwargs)
+        try:
+            logged_in_source = SessionManager.get_logged_in_user()
+
+        except (UserSessionExpired, UserHasBeenDeleted):
+            return clear_session_and_redirect_to_logged_out_page(session)
+
+        except UserNotLoggedIn:
+            return redirect(url_for("main.login"))
+
+        return f(*args, **kwargs, logged_in_source=logged_in_source)
     return decorated_function