Add key cache

install_files/ansible-base/roles/app/templates/sites-available/journalist.conf

@@ -2,8 +2,7 @@ ServerName {{ securedrop_app_apache_listening_address }}
 <VirtualHost {{ securedrop_app_apache_listening_address }}:8080>
 
 WSGIDaemonProcess journalist processes=2 threads=30 display-name=%{GROUP} python-path=/var/www/securedrop
-WSGIProcessGroup journalist
-WSGIScriptAlias / /var/www/journalist.wsgi
+WSGIScriptAlias / /var/www/journalist.wsgi process-group=journalist application-group=journalist
 WSGIPassAuthorization On
 
 # Tell the browser not to cache HTML responses in order to minimize the chance

install_files/securedrop-app-code/debian/postinst

@@ -65,18 +65,33 @@ database_migration() {
     fi
 }
 
-# Supports passing authorization headers for the SecureDrop API.
-# Only affects the Journalist Interface. Required for unattended upgrade
-# to v0.9.0.
-function permit_wsgi_authorization() {
+function adjust_wsgi_configuration {
     journalist_conf="/etc/apache2/sites-available/journalist.conf"
     if test -f $journalist_conf; then
-        # First we check whether the line is poresent.
+        # Supports passing authorization headers for the SecureDrop API.
+        # Only affects the Journalist Interface. Required for unattended upgrade
+        # to v0.9.0.
+        #
+        # First we check whether the line is present.
         # Next we find a target line to anchor the insertion.
         # Then we insert the line, along with the target line that was matched.
         if ! grep -qP '^WSGIPassAuthorization' "$journalist_conf"; then
             perl -pi -e 's/^(WSGIScriptAlias .*)/$1\nWSGIPassAuthorization On/' "$journalist_conf"
         fi
+
+        # Add process-group and application-group to WSGIScriptAlias
+        # to ensure the application is initialized at process start
+        # instead of waiting for the first request. (See
+        # https://modwsgi.readthedocs.io/en/latest/configuration-directives/WSGIScriptAlias.html)
+        if grep -qP '^WSGIScriptAlias / /var/www/journalist.wsgi$' "$journalist_conf"; then
+            perl -pi -e 's/^(WSGIScriptAlias .*)/$1 process-group=journalist application-group=journalist/' "$journalist_conf"
+        fi
+
+        # Remove the WSGIProcessGroup directive; it's not needed if
+        # specified in WSGIScriptAlias.
+        if grep -qP '^WSGIProcessGroup journalist' "$journalist_conf"; then
+            perl -pi -e 's/^WSGIProcessGroup journalist.*\n//' "$journalist_conf"
+        fi
     fi
 }
 
@@ -158,7 +173,7 @@ case "$1" in
     aa-enforce /etc/apparmor.d/usr.sbin.apache2
 
     # Munge Apache config while service is stopped.
-    permit_wsgi_authorization
+    adjust_wsgi_configuration
 
     # Restart apache so it loads with the apparmor profiles in enforce mode.
     service apache2 restart

molecule/testinfra/staging/app/apache/test_apache_journalist_interface.py

@@ -92,8 +92,10 @@ def test_apache_headers_journalist_interface(host, header):
       securedrop_test_vars.apache_listening_address),
   "WSGIDaemonProcess journalist processes=2 threads=30 display-name=%{{GROUP}} python-path={}".format(  # noqa
       securedrop_test_vars.securedrop_code),
-  'WSGIProcessGroup journalist',
-  'WSGIScriptAlias / /var/www/journalist.wsgi',
+  (
+      'WSGIScriptAlias / /var/www/journalist.wsgi '
+      'process-group=journalist application-group=journalist'
+  ),
   'WSGIPassAuthorization On',
   'Header set Cache-Control "no-store"',
   "Alias /static {}/static".format(securedrop_test_vars.securedrop_code),

securedrop/create-dev-data.py

@@ -58,12 +58,14 @@ def main(staging=False):
 
         # Add test sources and submissions
         num_sources = int(os.getenv('NUM_SOURCES', 2))
-        for i in range(num_sources):
-            if i == 0:
+        for i in range(1, num_sources + 1):
+            if i == 1:
                 # For the first source, the journalist who replied will be deleted
-                create_source_and_submissions(journalist_who_replied=journalist_tobe_deleted)
+                create_source_and_submissions(
+                    i, num_sources, journalist_who_replied=journalist_tobe_deleted
+                )
                 continue
-            create_source_and_submissions()
+            create_source_and_submissions(i, num_sources)
         # Now let us delete one journalist
         db.session.delete(journalist_tobe_deleted)
         db.session.commit()
@@ -89,8 +91,9 @@ def add_test_user(username, password, otp_secret, is_admin=False,
         db.session.rollback()
 
 
-def create_source_and_submissions(num_submissions=2, num_replies=2,
-                                  journalist_who_replied=None):
+def create_source_and_submissions(
+    source_index, source_count, num_submissions=2, num_replies=2, journalist_who_replied=None
+):
     # Store source in database
     codename = current_app.crypto_util.genrandomid()
     filesystem_id = current_app.crypto_util.hash_codename(codename)
@@ -124,7 +127,7 @@ def create_source_and_submissions(num_submissions=2, num_replies=2,
                                          source.journalist_filename)
         current_app.crypto_util.encrypt(
             next(replies),
-            [current_app.crypto_util.getkey(source.filesystem_id),
+            [current_app.crypto_util.get_fingerprint(source.filesystem_id),
              config.JOURNALIST_KEY],
             current_app.storage.path(source.filesystem_id, fname))
 
@@ -137,9 +140,13 @@ def create_source_and_submissions(num_submissions=2, num_replies=2,
 
     db.session.commit()
 
-    print("Test source (codename: '{}', journalist designation '{}') "
-          "added with {} submissions and {} replies".format(
-              codename, journalist_designation, num_submissions, num_replies))
+    print(
+        "Test source {}/{} (codename: '{}', journalist designation '{}') "
+        "added with {} submissions and {} replies".format(
+            source_index, source_count, codename, journalist_designation,
+            num_submissions, num_replies
+        )
+    )
 
 
 if __name__ == "__main__":  # pragma: no cover

securedrop/crypto_util.py

@@ -1,6 +1,5 @@
 # -*- coding: utf-8 -*-
 
-import collections
 from distutils.version import StrictVersion
 import pretty_bad_protocol as gnupg
 import os
@@ -12,6 +11,7 @@
 from datetime import date
 from flask import current_app
 from pretty_bad_protocol._util import _is_stream, _make_binary_stream
+from redis import Redis
 
 import rm
 
@@ -55,32 +55,6 @@ def monkey_patch_delete_handle_status(self, key, value):
 gnupg._parsers.DeleteResult._handle_status = monkey_patch_delete_handle_status
 
 
-class FIFOCache():
-    """
-    We implemented this simple cache instead of using functools.lru_cache
-    (this uses a different cache replacement policy (FIFO), but either
-    FIFO or LRU works for our key fingerprint cache)
-    due to the inability to remove an item from its cache.
-
-    See: https://bugs.python.org/issue28178
-    """
-    def __init__(self, maxsize: int):
-        self.maxsize = maxsize
-        self.cache = collections.OrderedDict()  # type: collections.OrderedDict
-
-    def get(self, item):
-        if item in self.cache:
-            return self.cache[item]
-
-    def put(self, item, value):
-        self.cache[item] = value
-        if len(self.cache) > self.maxsize:
-            self.cache.popitem(last=False)
-
-    def delete(self, item):
-        del self.cache[item]
-
-
 class CryptoException(Exception):
     pass
 
@@ -99,8 +73,8 @@ class CryptoUtil:
     # to set an expiration date.
     DEFAULT_KEY_EXPIRATION_DATE = '0'
 
-    keycache_limit = 1000
-    keycache = FIFOCache(keycache_limit)
+    REDIS_FINGERPRINT_HASH = "sd/crypto-util/fingerprints"
+    REDIS_KEY_HASH = "sd/crypto-util/keys"
 
     def __init__(self,
                  scrypt_params,
@@ -114,7 +88,7 @@ def __init__(self,
         self.__securedrop_root = securedrop_root
         self.__word_list = word_list
 
-        if os.environ.get('SECUREDROP_ENV') == 'test':
+        if os.environ.get('SECUREDROP_ENV') in ('dev', 'test'):
             # Optimize crypto to speed up tests (at the expense of security
             # DO NOT use these settings in production)
             self.__gpg_key_length = 1024
@@ -148,6 +122,8 @@ def __init__(self,
         with io.open(adjectives_file) as f:
             self.adjectives = f.read().splitlines()
 
+        self.redis = Redis(decode_responses=True)
+
     # Make sure these pass before the app can run
     def do_runtime_tests(self):
         if self.scrypt_id_pepper == self.scrypt_gpg_pepper:
@@ -243,7 +219,7 @@ def genkeypair(self, name, secret):
         return genkey_obj
 
     def delete_reply_keypair(self, source_filesystem_id):
-        key = self.getkey(source_filesystem_id)
+        key = self.get_fingerprint(source_filesystem_id)
         # If this source was never flagged for review, they won't have a reply
         # keypair
         if not key:
@@ -254,29 +230,45 @@ def delete_reply_keypair(self, source_filesystem_id):
         temp_gpg = gnupg.GPG(binary='gpg2', homedir=self.gpg_key_dir)
         # The subkeys keyword argument deletes both secret and public keys.
         temp_gpg.delete_keys(key, secret=True, subkeys=True)
-        self.keycache.delete(source_filesystem_id)
+        self.redis.hdel(self.REDIS_KEY_HASH, self.get_fingerprint(source_filesystem_id))
+        self.redis.hdel(self.REDIS_FINGERPRINT_HASH, source_filesystem_id)
+
+    def get_fingerprint(self, name):
+        """
+        Returns the fingerprint of the GPG key for the given name.
 
-    def getkey(self, name):
-        fingerprint = self.keycache.get(name)
-        if fingerprint:  # cache hit
+        The supplied name is usually a source filesystem ID.
+        """
+        fingerprint = self.redis.hget(self.REDIS_FINGERPRINT_HASH, name)
+        if fingerprint:
             return fingerprint
 
-        # cache miss
         for key in self.gpg.list_keys():
             for uid in key['uids']:
                 if name in uid:
-                    self.keycache.put(name, key['fingerprint'])
+                    self.redis.hset(self.REDIS_FINGERPRINT_HASH, name, key['fingerprint'])
                     return key['fingerprint']
 
         return None
 
-    def export_pubkey(self, name):
-        fingerprint = self.getkey(name)
-        if fingerprint:
-            return self.gpg.export_keys(fingerprint)
-        else:
+    def get_pubkey(self, name):
+        """
+        Returns the GPG public key for the given name.
+
+        The supplied name is usually a source filesystem ID.
+        """
+        fingerprint = self.get_fingerprint(name)
+        if not fingerprint:
             return None
 
+        key = self.redis.hget(self.REDIS_KEY_HASH, fingerprint)
+        if key:
+            return key
+
+        key = self.gpg.export_keys(fingerprint)
+        self.redis.hset(self.REDIS_KEY_HASH, fingerprint, key)
+        return key
+
     def encrypt(self, plaintext, fingerprints, output=None):
         # Verify the output path
         if output:

securedrop/journalist.py

@@ -16,7 +16,7 @@ def prime_keycache():
     """
     with app.app_context():
         for source in Source.query.filter_by(pending=False).all():
-            app.crypto_util.getkey(source.filesystem_id)
+            app.crypto_util.get_pubkey(source.filesystem_id)
 
 
 prime_keycache()

securedrop/journalist_app/col.py

@@ -33,7 +33,7 @@ def remove_star(filesystem_id):
     def col(filesystem_id):
         form = ReplyForm()
         source = get_source(filesystem_id)
-        source.has_key = current_app.crypto_util.getkey(filesystem_id)
+        source.has_key = current_app.crypto_util.get_fingerprint(filesystem_id)
         return render_template("col.html", filesystem_id=filesystem_id,
                                source=source, form=form)
 

securedrop/journalist_app/main.py

@@ -108,7 +108,7 @@ def reply():
                                               g.source.journalist_filename)
         current_app.crypto_util.encrypt(
             form.message.data,
-            [current_app.crypto_util.getkey(g.filesystem_id),
+            [current_app.crypto_util.get_fingerprint(g.filesystem_id),
              config.JOURNALIST_KEY],
             output=current_app.storage.path(g.filesystem_id, filename),
         )

securedrop/models.py

@@ -118,7 +118,7 @@ def collection(self):
 
     @property
     def fingerprint(self):
-        return current_app.crypto_util.getkey(self.filesystem_id)
+        return current_app.crypto_util.get_fingerprint(self.filesystem_id)
 
     @fingerprint.setter
     def fingerprint(self, value):
@@ -131,7 +131,7 @@ def fingerprint(self):
     @property
     def public_key(self):
         # type: () -> str
-        return current_app.crypto_util.export_pubkey(self.filesystem_id)
+        return current_app.crypto_util.get_pubkey(self.filesystem_id)
 
     @public_key.setter
     def public_key(self, value):

securedrop/source_app/main.py

@@ -130,7 +130,7 @@ def lookup():
         # 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.getkey(g.filesystem_id) and \
+        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,
@@ -145,7 +145,7 @@ def lookup():
             replies=replies,
             flagged=g.source.flagged,
             new_user=session.get('new_user', None),
-            haskey=current_app.crypto_util.getkey(
+            haskey=current_app.crypto_util.get_fingerprint(
                 g.filesystem_id))
 
     @view.route('/submit', methods=('POST',))

securedrop/tests/functional/functional_test.py

@@ -307,7 +307,7 @@ def wait_for_source_key(self, source_name):
         filesystem_id = self.source_app.crypto_util.hash_codename(source_name)
 
         def key_available(filesystem_id):
-            assert self.source_app.crypto_util.getkey(filesystem_id)
+            assert self.source_app.crypto_util.get_fingerprint(filesystem_id)
 
         self.wait_for(lambda: key_available(filesystem_id), timeout=60)