Skip to content

Commit

Permalink
Move passphrase logic from CryptoUtil to PassphrasesGenerator
Browse files Browse the repository at this point in the history
Move passphrase logic from CryptoUtil to PassphrasesGenerator

Move passphrase logic from CryptoUtil to PassphrasesGenerator

Re-add generate test

Ensure passphrases are random

Fix pylint false positive

Fix test

Rename to PassphraseGenerator

Address feedback from review

Fix flake8

Fix test

Allow non-perfect word lists and re-enable french word list

Update test parameters

Bump minimum word count to 6000

Fix test

Tweak word lists requirements
  • Loading branch information
nabla-c0d3 committed Dec 9, 2020
1 parent 5a9e8fc commit 1df827c
Show file tree
Hide file tree
Showing 20 changed files with 294 additions and 221 deletions.
4 changes: 3 additions & 1 deletion securedrop/create-dev-data.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

from flask import current_app

from passphrases import PassphraseGenerator

os.environ["SECUREDROP_ENV"] = "dev" # noqa
import journalist_app

Expand Down Expand Up @@ -104,7 +106,7 @@ def create_source_data(
num_replies: int = 2,
) -> None:
# Store source in database
codename = current_app.crypto_util.genrandomid()
codename = PassphraseGenerator.get_default().generate_passphrase()
filesystem_id = current_app.crypto_util.hash_codename(codename)
journalist_designation = current_app.crypto_util.display_id()
source = Source(filesystem_id, journalist_designation)
Expand Down
53 changes: 7 additions & 46 deletions securedrop/crypto_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import rm

from models import Source
from passphrases import DicewarePassphrase


# monkey patch to work with Focal gnupg.
# https://github.com/isislovecruft/python-gnupg/issues/250
Expand Down Expand Up @@ -66,7 +68,6 @@ class CryptoException(Exception):
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.
Expand All @@ -87,12 +88,10 @@ def __init__(self,
scrypt_id_pepper: str,
scrypt_gpg_pepper: str,
securedrop_root: str,
word_list: str,
nouns_file: str,
adjectives_file: str,
gpg_key_dir: str) -> None:
self.__securedrop_root = securedrop_root
self.__word_list = word_list

if os.environ.get('SECUREDROP_ENV') in ('dev', 'test'):
# Optimize crypto to speed up tests (at the expense of security
Expand All @@ -119,9 +118,6 @@ def __init__(self,
else:
self.gpg = gpg_binary

# map code for a given language to a localized wordlist
self.__language2words = {} # type: Dict[str, List[str]]

with io.open(nouns_file) as f:
self.nouns = f.read().splitlines()

Expand All @@ -138,40 +134,6 @@ def do_runtime_tests(self) -> None:
if not rm.check_secure_delete_capability():
raise AssertionError("Secure file deletion is not possible.")

def get_wordlist(self, locale: str) -> List[str]:
"""" Ensure the wordlist for the desired locale is read and available
in the words global variable. If there is no wordlist for the
desired local, fallback to the default english wordlist.
The localized wordlist are read from wordlists/{locale}.txt but
for backward compatibility purposes the english wordlist is read
from the config.WORD_LIST file.
"""

if locale not in self.__language2words:
if locale != 'en':
path = os.path.join(self.__securedrop_root,
'wordlists',
locale + '.txt')
if os.path.exists(path):
wordlist_path = path
else:
wordlist_path = self.__word_list
else:
wordlist_path = self.__word_list

with io.open(wordlist_path) as f:
content = f.read().splitlines()
self.__language2words[locale] = content

return self.__language2words[locale]

def genrandomid(self,
words_in_random_id: int = DEFAULT_WORDS_IN_RANDOM_ID,
locale: str = 'en') -> str:
return ' '.join(random.choice(self.get_wordlist(locale))
for x in range(words_in_random_id))

def display_id(self) -> str:
"""Generate random journalist_designation until we get an unused one"""

Expand All @@ -189,7 +151,7 @@ def display_id(self) -> str:

raise ValueError("Could not generate unique journalist designation for new source")

def hash_codename(self, codename: str, salt: Optional[str] = None) -> str:
def hash_codename(self, codename: DicewarePassphrase, salt: Optional[str] = None) -> str:
"""Salts and hashes a codename using scrypt.
:param codename: A source's codename.
Expand All @@ -198,10 +160,9 @@ def hash_codename(self, codename: str, salt: Optional[str] = None) -> str:
"""
if salt is None:
salt = self.scrypt_id_pepper
_validate_name_for_diceware(codename)
return b32encode(scrypt.hash(codename, salt, **self.scrypt_params)).decode('utf-8')

def genkeypair(self, name: str, secret: str) -> gnupg._parsers.GenKey:
def genkeypair(self, name: str, secret: DicewarePassphrase) -> gnupg._parsers.GenKey:
"""Generate a GPG key through batch file key generation. A source's
codename is salted with SCRYPT_GPG_PEPPER and hashed with scrypt to
provide the passphrase used to encrypt their private key. Their name
Expand All @@ -222,11 +183,11 @@ def genkeypair(self, name: str, secret: str) -> gnupg._parsers.GenKey:
"""
_validate_name_for_diceware(name)
secret = self.hash_codename(secret, salt=self.scrypt_gpg_pepper)
hashed_secret = self.hash_codename(secret, salt=self.scrypt_gpg_pepper)
genkey_obj = self.gpg.gen_key(self.gpg.gen_key_input(
key_type=self.GPG_KEY_TYPE,
key_length=self.__gpg_key_length,
passphrase=secret,
passphrase=hashed_secret,
name_email=name,
name_real="Source Key",
creation_date=self.DEFAULT_KEY_CREATION_DATE.isoformat(),
Expand Down Expand Up @@ -336,7 +297,7 @@ def encrypt(self, plaintext: str, fingerprints: List[str], output: Optional[str]
else:
raise CryptoException(out.stderr)

def decrypt(self, secret: str, ciphertext: bytes) -> str:
def decrypt(self, secret: DicewarePassphrase, ciphertext: bytes) -> str:
"""
>>> crypto = current_app.crypto_util
>>> key = crypto.genkeypair('randomid', 'randomid')
Expand Down
1 change: 0 additions & 1 deletion securedrop/journalist_app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def create_app(config: 'SDConfig') -> Flask:
scrypt_id_pepper=config.SCRYPT_ID_PEPPER,
scrypt_gpg_pepper=config.SCRYPT_GPG_PEPPER,
securedrop_root=config.SECUREDROP_ROOT,
word_list=config.WORD_LIST,
nouns_file=config.NOUNS,
adjectives_file=config.ADJECTIVES,
gpg_key_dir=config.GPG_KEY_DIR,
Expand Down
8 changes: 6 additions & 2 deletions securedrop/journalist_app/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
flash, session)
from flask_babel import gettext

import i18n
from db import db
from journalist_app.utils import (make_password, set_diceware_password, set_name, validate_user,
from journalist_app.utils import (set_diceware_password, set_name, validate_user,
validate_hotp_secret)
from passphrases import PassphraseGenerator
from sdconfig import SDConfig


Expand All @@ -17,7 +19,9 @@ def make_blueprint(config: SDConfig) -> Blueprint:

@view.route('/account', methods=('GET',))
def edit() -> str:
password = make_password(config)
password = PassphraseGenerator.get_default().generate_passphrase(
preferred_language=i18n.get_language(config)
)
return render_template('edit_account.html',
password=password)

Expand Down
13 changes: 10 additions & 3 deletions securedrop/journalist_app/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,16 @@
from sqlalchemy.exc import IntegrityError
from sqlalchemy.orm.exc import NoResultFound

import i18n
from db import db
from models import (InstanceConfig, Journalist, InvalidUsernameException,
FirstOrLastNameError, PasswordError)
from journalist_app.decorators import admin_required
from journalist_app.utils import (make_password, commit_account_changes, set_diceware_password,
from journalist_app.utils import (commit_account_changes, set_diceware_password,
validate_hotp_secret, revoke_token)
from journalist_app.forms import LogoForm, NewUserForm, SubmissionPreferencesForm
from sdconfig import SDConfig
from passphrases import PassphraseGenerator


def make_blueprint(config: SDConfig) -> Blueprint:
Expand Down Expand Up @@ -124,8 +126,11 @@ def add_user() -> Union[str, werkzeug.Response]:
return redirect(url_for('admin.new_user_two_factor',
uid=new_user.id))

password = PassphraseGenerator.get_default().generate_passphrase(
preferred_language=i18n.get_language(config)
)
return render_template("admin_add_user.html",
password=make_password(config),
password=password,
form=form)

@view.route('/2fa', methods=('GET', 'POST'))
Expand Down Expand Up @@ -221,7 +226,9 @@ def edit_user(user_id: int) -> Union[str, werkzeug.Response]:

commit_account_changes(user)

password = make_password(config)
password = PassphraseGenerator.get_default().generate_passphrase(
preferred_language=i18n.get_language(config)
)
return render_template("edit_account.html", user=user,
password=password)

Expand Down
16 changes: 0 additions & 16 deletions securedrop/journalist_app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
from flask_babel import gettext, ngettext
from sqlalchemy.exc import IntegrityError

import i18n

from db import db
from models import (
BadTokenException,
Expand All @@ -36,8 +34,6 @@
)
from store import add_checksum_for_file

from sdconfig import SDConfig


def logged_in() -> bool:
# When a user is logged in, we push their user ID (database primary key)
Expand Down Expand Up @@ -326,18 +322,6 @@ def col_delete(cols_selected: List[str]) -> werkzeug.Response:
return redirect(url_for('main.index'))


def make_password(config: SDConfig) -> str:
while True:
password = current_app.crypto_util.genrandomid(
7,
i18n.get_language(config))
try:
Journalist.check_password_acceptable(password)
return password
except PasswordError:
continue


def delete_collection(filesystem_id: str) -> None:
# Delete the source's collection of submissions
path = current_app.storage.path(filesystem_id)
Expand Down
16 changes: 3 additions & 13 deletions securedrop/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
from flask.ctx import AppContext
from typing import List

from passphrases import PassphraseGenerator

sys.path.insert(0, "/var/www/securedrop") # noqa: E402

import qrcode
from flask import current_app
from sqlalchemy.orm.exc import NoResultFound

os.environ['SECUREDROP_ENV'] = 'dev' # noqa
Expand All @@ -30,7 +31,6 @@
FirstOrLastNameError,
InvalidUsernameException,
Journalist,
PasswordError,
)
from management import app_context, config
from management.run import run
Expand Down Expand Up @@ -177,24 +177,14 @@ def _get_yubikey_usage() -> bool:
print('Invalid answer. Please type "y" or "n"')


def _make_password() -> str:
while True:
password = current_app.crypto_util.genrandomid(7)
try:
Journalist.check_password_acceptable(password)
return password
except PasswordError:
continue


def _add_user(is_admin: bool = False, context: Optional[AppContext] = None) -> int:
with context or app_context():
username = _get_username()
first_name = _get_first_name()
last_name = _get_last_name()

print("Note: Passwords are now autogenerated.")
password = _make_password()
password = PassphraseGenerator.get_default().generate_passphrase()
print("This user's password is: {}".format(password))

is_hotp = _get_yubikey_usage()
Expand Down
4 changes: 0 additions & 4 deletions securedrop/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,6 @@ class Source(db.Model):
# when deletion of the source was requested
deleted_at = Column(DateTime)

# Don't create or bother checking excessively long codenames to prevent DoS
NUM_WORDS = 7
MAX_CODENAME_LEN = 128

def __init__(self,
filesystem_id: str,
journalist_designation: str) -> None:
Expand Down
Loading

0 comments on commit 1df827c

Please sign in to comment.