Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add key cache #5184

Merged
merged 9 commits into from
Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions securedrop/create-dev-data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
for i in range(1, num_sources + 1):
if i == 0:
rmol marked this conversation as resolved.
Show resolved Hide resolved
# 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()
Expand All @@ -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)
Expand Down Expand Up @@ -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))

Expand All @@ -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
Expand Down
56 changes: 38 additions & 18 deletions securedrop/crypto_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ def put(self, item, value):
self.cache.popitem(last=False)

def delete(self, item):
del self.cache[item]
if item in self.cache:
del self.cache[item]


class CryptoException(Exception):
Expand All @@ -99,9 +100,12 @@ class CryptoUtil:
# to set an expiration date.
DEFAULT_KEY_EXPIRATION_DATE = '0'

keycache_limit = 1000
keycache_limit = 10000
keycache = FIFOCache(keycache_limit)

fprcache_limit = 10000
fprcache = FIFOCache(fprcache_limit)

def __init__(self,
scrypt_params,
scrypt_id_pepper,
Expand All @@ -114,7 +118,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
Expand Down Expand Up @@ -243,7 +247,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:
Expand All @@ -254,29 +258,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.fprcache.delete(source_filesystem_id)
self.keycache.delete(self.get_fingerprint(source_filesystem_id))
rmol marked this conversation as resolved.
Show resolved Hide resolved

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.fprcache.get(name)
if fingerprint:
return fingerprint

# cache miss
target_uid = 'Autogenerated Key <{}>'.format(name)
rmol marked this conversation as resolved.
Show resolved Hide resolved
for key in self.gpg.list_keys():
for uid in key['uids']:
if name in uid:
self.keycache.put(name, key['fingerprint'])
return key['fingerprint']
if target_uid in key['uids']:
self.fprcache.put(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.keycache.get(fingerprint)
if key:
return key

key = self.gpg.export_keys(fingerprint)
self.keycache.put(fingerprint, key)
return key

def encrypt(self, plaintext, fingerprints, output=None):
# Verify the output path
if output:
Expand Down
2 changes: 1 addition & 1 deletion securedrop/journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion securedrop/journalist_app/col.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 1 addition & 1 deletion securedrop/journalist_app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
Expand Down
4 changes: 2 additions & 2 deletions securedrop/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand Down
4 changes: 2 additions & 2 deletions securedrop/source_app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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',))
Expand Down
2 changes: 1 addition & 1 deletion securedrop/tests/functional/functional_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
39 changes: 19 additions & 20 deletions securedrop/tests/test_crypto_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def test_encrypt_success(source_app, config, test_source):
with source_app.app_context():
ciphertext = source_app.crypto_util.encrypt(
message,
[source_app.crypto_util.getkey(test_source['filesystem_id']),
[source_app.crypto_util.get_fingerprint(test_source['filesystem_id']),
config.JOURNALIST_KEY],
source_app.storage.path(test_source['filesystem_id'],
'somefile.gpg'))
Expand Down Expand Up @@ -70,7 +70,7 @@ def test_encrypt_without_output(source_app, config, test_source):
with source_app.app_context():
ciphertext = source_app.crypto_util.encrypt(
message,
[source_app.crypto_util.getkey(test_source['filesystem_id']),
[source_app.crypto_util.get_fingerprint(test_source['filesystem_id']),
config.JOURNALIST_KEY])
plaintext = source_app.crypto_util.decrypt(
test_source['codename'],
Expand All @@ -96,7 +96,7 @@ def test_encrypt_binary_stream(source_app, config, test_source):
with io.open(os.path.realpath(__file__)) as fh:
ciphertext = source_app.crypto_util.encrypt(
fh,
[source_app.crypto_util.getkey(test_source['filesystem_id']),
[source_app.crypto_util.get_fingerprint(test_source['filesystem_id']),
config.JOURNALIST_KEY],
source_app.storage.path(test_source['filesystem_id'],
'somefile.gpg'))
Expand All @@ -116,7 +116,7 @@ def test_encrypt_fingerprints_not_a_list_or_tuple(source_app, test_source):
with source_app.app_context():
ciphertext = source_app.crypto_util.encrypt(
message,
source_app.crypto_util.getkey(test_source['filesystem_id']),
source_app.crypto_util.get_fingerprint(test_source['filesystem_id']),
source_app.storage.path(test_source['filesystem_id'],
'somefile.gpg'))
plaintext = source_app.crypto_util.decrypt(test_source['codename'],
Expand All @@ -133,7 +133,7 @@ def test_basic_encrypt_then_decrypt_multiple_recipients(source_app,
with source_app.app_context():
ciphertext = source_app.crypto_util.encrypt(
message,
[source_app.crypto_util.getkey(test_source['filesystem_id']),
[source_app.crypto_util.get_fingerprint(test_source['filesystem_id']),
config.JOURNALIST_KEY],
source_app.storage.path(test_source['filesystem_id'],
'somefile.gpg'))
Expand Down Expand Up @@ -208,7 +208,7 @@ def test_genkeypair(source_app):
db.session.commit()
source_app.crypto_util.genkeypair(source.filesystem_id, codename)

assert source_app.crypto_util.getkey(filesystem_id) is not None
assert source_app.crypto_util.get_fingerprint(filesystem_id) is not None


def parse_gpg_date_string(date_string):
Expand Down Expand Up @@ -241,15 +241,15 @@ def test_reply_keypair_creation_and_expiration_dates(source_app):
db.session.commit()
source_app.crypto_util.genkeypair(source.filesystem_id, codename)

# crypto_util.getkey only returns the fingerprint of the key. We need
# crypto_util.get_fingerprint only returns the fingerprint of the key. We need
# the full output of gpg.list_keys() to check the creation and
# expire dates.
#
# TODO: it might be generally useful to refactor crypto_util.getkey so
# TODO: it might be generally useful to refactor crypto_util.get_fingerprint so
# it always returns the entire key dictionary instead of just the
# fingerprint (which is always easily extracted from the entire key
# dictionary).
new_key_fingerprint = source_app.crypto_util.getkey(filesystem_id)
new_key_fingerprint = source_app.crypto_util.get_fingerprint(filesystem_id)
new_key = [key for key in source_app.crypto_util.gpg.list_keys()
if new_key_fingerprint == key['fingerprint']][0]

Expand All @@ -267,7 +267,7 @@ def test_reply_keypair_creation_and_expiration_dates(source_app):
def test_delete_reply_keypair(source_app, test_source):
fid = test_source['filesystem_id']
source_app.crypto_util.delete_reply_keypair(fid)
assert source_app.crypto_util.getkey(fid) is None
assert source_app.crypto_util.get_fingerprint(fid) is None


def test_delete_reply_keypair_pinentry_status_is_handled(source_app, test_source,
Expand All @@ -285,7 +285,7 @@ def test_delete_reply_keypair_pinentry_status_is_handled(source_app, test_source

captured = capsys.readouterr()
assert "ValueError: Unknown status message: 'PINENTRY_LAUNCHED'" not in captured.err
assert source_app.crypto_util.getkey(fid) is None
assert source_app.crypto_util.get_fingerprint(fid) is None


def test_delete_reply_keypair_no_key(source_app):
Expand All @@ -295,25 +295,24 @@ def test_delete_reply_keypair_no_key(source_app):
source_app.crypto_util.delete_reply_keypair('Reality Winner')


def test_getkey(source_app, test_source):
assert (source_app.crypto_util.getkey(test_source['filesystem_id'])
def test_get_fingerprint(source_app, test_source):
assert (source_app.crypto_util.get_fingerprint(test_source['filesystem_id'])
is not None)

# check that a non-existent key returns None
assert source_app.crypto_util.getkey('x' * 50) is None
assert source_app.crypto_util.get_fingerprint('x' * 50) is None


def test_export_pubkey(source_app, test_source):
def test_get_pubkey(source_app, test_source):
begin_pgp = '-----BEGIN PGP PUBLIC KEY BLOCK----'

# check that a filesystem_id exports the pubkey
exported = source_app.crypto_util.export_pubkey(
test_source['filesystem_id'])
assert exported.startswith(begin_pgp)
pubkey = source_app.crypto_util.get_pubkey(test_source['filesystem_id'])
assert pubkey.startswith(begin_pgp)

# check that a non-existent identifer exports None
exported = source_app.crypto_util.export_pubkey('x' * 50)
assert exported is None
pubkey = source_app.crypto_util.get_pubkey('x' * 50)
assert pubkey is None


@given(
Expand Down
2 changes: 1 addition & 1 deletion securedrop/tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ def _helper_test_reply(journalist_app, source_app, config, test_journo,

# Block up to 15s for the reply keypair, so we can test sending a reply
def assertion():
assert current_app.crypto_util.getkey(filesystem_id) is not None
assert current_app.crypto_util.get_fingerprint(filesystem_id) is not None
utils.asynchronous.wait_for_assertion(assertion, 15)

# Create 2 replies to test deleting on journalist and source interface
Expand Down
4 changes: 2 additions & 2 deletions securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -1691,15 +1691,15 @@ def test_delete_source_deletes_source_key(journalist_app,
utils.db_helper.reply(journo, source, 2)

# Source key exists
source_key = current_app.crypto_util.getkey(
source_key = current_app.crypto_util.get_fingerprint(
test_source['filesystem_id'])
assert source_key is not None

journalist_app_module.utils.delete_collection(
test_source['filesystem_id'])

# Source key no longer exists
source_key = current_app.crypto_util.getkey(
source_key = current_app.crypto_util.get_fingerprint(
test_source['filesystem_id'])
assert source_key is None

Expand Down
2 changes: 1 addition & 1 deletion securedrop/tests/test_journalist_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ def test_authorized_user_can_add_reply(journalist_app, journalist_api_token,

# First we must encrypt the reply, or it will get rejected
# by the server.
source_key = current_app.crypto_util.getkey(
source_key = current_app.crypto_util.get_fingerprint(
test_source['source'].filesystem_id)
reply_content = current_app.crypto_util.gpg.encrypt(
'This is a plaintext reply', source_key).data
Expand Down
2 changes: 1 addition & 1 deletion securedrop/tests/utils/db_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def reply(journalist, source, num_replies):
source.journalist_filename)
current_app.crypto_util.encrypt(
str(os.urandom(1)),
[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))

Expand Down