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

Avoid leaking information in GPG key creation and expiration dates #3994

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
12 changes: 11 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,11 @@ 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)

def __init__(self,
scrypt_params,
scrypt_id_pepper,
Expand Down Expand Up @@ -170,7 +176,11 @@ 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(),
# "0" is the magic value that tells GPG's batch key generation not
# to set an expiration date.
expire_date="0"
))

def delete_reply_keypair(self, source_filesystem_id):
Expand Down
55 changes: 55 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,60 @@ 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):
# TODO: setup copied from test_genkeypair. DRY?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to not be super DRY about this since unit tests also serve as documentaiton.

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