Skip to content

Commit

Permalink
Have JI validate journalist key is valid
Browse files Browse the repository at this point in the history
During the Sequoia migration, we need to export the journalist public
key from the GPG keyring into a file on disk. We also needed to validate
the key was usable by Sequoia (e.g. no SHA-1 binding signatures).

Previously the plan was to validate it during the postinst and error out
if it wasn't valid, but if validation fails for whatever reason, then we
abort, which interrupts the postinst, so, e.g. database upgrades won't
be applied. In retrospect having the validation fail at package install
time is bad, because it requires even more manual work to get an
instance in a working state since you need to manually apply the updates
after doing a key rotation.

Now we validate the journalist key during startup of the Journalist
Interface, printing and logging an error if it doesn't validate and then
exiting. This should bring attention to journalists and therefore the
admin that the instance needs manual attention. We will also include
information about this change in the pre-release and release
announcements.

Fixes #7030.
  • Loading branch information
legoktm committed Oct 26, 2023
1 parent 3a665de commit 7fd01d9
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 35 deletions.
3 changes: 1 addition & 2 deletions securedrop/debian/securedrop-app-code.postinst
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,7 @@ export_journalist_public_key() {
# Export the GPG public key
# shellcheck disable=SC2024
sudo -u www-data gpg2 --homedir=/var/lib/securedrop/keys --export --armor "$fingerprint" > $journalist_pub
# Verify integrity of what we just exported
sudo -u www-data /var/www/securedrop/scripts/validate-pgp-key "$journalist_pub" "$fingerprint"
# We explicitly do not validate the exported key here, that is done during JI startup
fi
fi

Expand Down
24 changes: 23 additions & 1 deletion securedrop/journalist.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import sys

from encryption import EncryptionManager, GpgKeyNotFoundError
from execution import asynchronous
from journalist_app import create_app
from models import Source
from sdconfig import SecureDropConfig

import redwood

config = SecureDropConfig.get_current()
# app is imported by journalist.wsgi
app = create_app(config)
Expand All @@ -21,10 +25,28 @@ def prime_keycache() -> None:
pass


prime_keycache()
def validate_journalist_key() -> None:
"""Verify the journalist PGP key is valid"""
encryption_mgr = EncryptionManager.get_default()
# First check that we can read it
try:
journalist_key = encryption_mgr.get_journalist_public_key()
except Exception as e:
print(f"ERROR: Unable to read journalist public key: {e}", file=sys.stderr)
app.logger.error(f"ERROR: Unable to read journalist public key: {e}")
sys.exit(1)
# And then what we read is valid
try:
redwood.is_valid_public_key(journalist_key)
except redwood.RedwoodError as e:
print(f"ERROR: Journalist public key is not valid: {e}", file=sys.stderr)
app.logger.error(f"ERROR: Journalist public key is not valid: {e}")
sys.exit(1)


if __name__ == "__main__": # pragma: no cover
validate_journalist_key()
prime_keycache()
debug = getattr(config, "env", "prod") != "prod"
# nosemgrep: python.flask.security.audit.app-run-param-config.avoid_app_run_with_bad_host
app.run(debug=debug, host="0.0.0.0", port=8081)
32 changes: 0 additions & 32 deletions securedrop/scripts/validate-pgp-key

This file was deleted.

20 changes: 20 additions & 0 deletions securedrop/tests/test_journalist.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from flaky import flaky
from flask import escape, g, url_for
from flask_babel import gettext, ngettext
from journalist import validate_journalist_key
from journalist_app.sessions import session
from journalist_app.utils import mark_seen
from models import (
Expand Down Expand Up @@ -51,6 +52,8 @@
from tests.utils.instrument import InstrumentedApp
from two_factor import TOTP

from redwood import RedwoodError

from .utils import create_legacy_gpg_key, login_journalist

# Smugly seed the RNG for deterministic testing
Expand Down Expand Up @@ -3772,3 +3775,20 @@ def test_journalist_deletion(journalist_app, app_storage):
assert len(SeenReply.query.filter_by(journalist_id=deleted.id).all()) == 2
# And there are no login attempts
assert JournalistLoginAttempt.query.all() == []


def test_validate_journalist_key(config, capsys):
# The test key passes validation
validate_journalist_key()
# Reading the key file fails
with patch(
"encryption.EncryptionManager.get_journalist_public_key", side_effect=RuntimeError("err")
):
with pytest.raises(SystemExit):
validate_journalist_key()
assert capsys.readouterr().err == "ERROR: Unable to read journalist public key: err\n"
# Key fails validation
with patch("redwood.is_valid_public_key", side_effect=RedwoodError("err")):
with pytest.raises(SystemExit):
validate_journalist_key()
assert capsys.readouterr().err == "ERROR: Journalist public key is not valid: err\n"

0 comments on commit 7fd01d9

Please sign in to comment.