Skip to content

Commit

Permalink
Merge pull request #3994 from garrettr/issue/3912-reply-key-expiratio…
Browse files Browse the repository at this point in the history
…n-date-leak

Avoid leaking information in GPG key creation and expiration dates
  • Loading branch information
emkll committed Dec 19, 2018
2 parents 12726bd + 3be51e0 commit ad7e7b2
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
14 changes: 13 additions & 1 deletion securedrop/crypto_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from random import SystemRandom

from base64 import b32encode
from datetime import date
from flask import current_app
from gnupg._util import _is_stream, _make_binary_stream

Expand Down Expand Up @@ -43,6 +44,15 @@ class CryptoUtil:
GPG_KEY_TYPE = "RSA"
DEFAULT_WORDS_IN_RANDOM_ID = 8

# All reply keypairs will be "created" on the same day SecureDrop (then
# Strongbox) was publicly released for the first time.
# https://www.newyorker.com/news/news-desk/strongbox-and-aaron-swartz
DEFAULT_KEY_CREATION_DATE = date(2013, 5, 14)

# '0' is the magic value that tells GPG's batch key generation not
# to set an expiration date.
DEFAULT_KEY_EXPIRATION_DATE = '0'

def __init__(self,
scrypt_params,
scrypt_id_pepper,
Expand Down Expand Up @@ -170,7 +180,9 @@ def genkeypair(self, name, secret):
key_type=self.GPG_KEY_TYPE,
key_length=self.__gpg_key_length,
passphrase=secret,
name_email=name
name_email=name,
creation_date=self.DEFAULT_KEY_CREATION_DATE.isoformat(),
expire_date=self.DEFAULT_KEY_EXPIRATION_DATE
))

def delete_reply_keypair(self, source_filesystem_id):
Expand Down
54 changes: 54 additions & 0 deletions securedrop/tests/test_crypto_util.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# -*- coding: utf-8 -*-
from datetime import datetime
import io
import os
import pytest
Expand Down Expand Up @@ -208,6 +209,59 @@ def test_genkeypair(source_app):
assert source_app.crypto_util.getkey(filesystem_id) is not None


def parse_gpg_date_string(date_string):
"""Parse a date string returned from `gpg --with-colons --list-keys` into a
datetime.
The format of the date strings is complicated; see gnupg doc/DETAILS for a
full explanation.
Key details:
- The creation date of the key is given in UTC.
- the date is usually printed in seconds since epoch, however, we are
migrating to an ISO 8601 format (e.g. "19660205T091500"). A simple
way to detect the new format is to scan for the 'T'.
"""
if 'T' in date_string:
dt = datetime.strptime(date_string, "%Y%m%dT%H%M%S")
else:
dt = datetime.utcfromtimestamp(int(date_string))
return dt


def test_reply_keypair_creation_and_expiration_dates(source_app):
with source_app.app_context():
codename = source_app.crypto_util.genrandomid()
filesystem_id = source_app.crypto_util.hash_codename(codename)
journalist_filename = source_app.crypto_util.display_id()
source = models.Source(filesystem_id, journalist_filename)
db.session.add(source)
db.session.commit()
source_app.crypto_util.genkeypair(source.filesystem_id, codename)

# crypto_util.getkey 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
# 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 = [key for key in source_app.crypto_util.gpg.list_keys()
if new_key_fingerprint == key['fingerprint']][0]

# All keys should share the same creation date to avoid leaking
# information about when sources first created accounts.
creation_date = parse_gpg_date_string(new_key['date'])
assert (creation_date.date() ==
CryptoUtil.DEFAULT_KEY_CREATION_DATE)

# Reply keypairs should not expire
expire_date = new_key['expires']
assert expire_date == ''


def test_delete_reply_keypair(source_app, test_source):
fid = test_source['filesystem_id']
source_app.crypto_util.delete_reply_keypair(fid)
Expand Down

0 comments on commit ad7e7b2

Please sign in to comment.