From a7a05138e4ea9a2c7257b166e431c3487d622c89 Mon Sep 17 00:00:00 2001 From: Kunal Mehta Date: Fri, 20 Oct 2023 15:17:49 -0700 Subject: [PATCH 1/5] redwood: Don't armor encrypted files/messages ...except in one test. We didn't have GPG armor ciphertext, so we don't need redwood to either. This will result in a noticable speedup during encryption. Keys will still be armored when being stored in the database. Tests make this a bit more complicated, we want to generate an armored message we can post to the journalist API, so we make it an optional parameter for redwood.encrypt_message() only. We have to adjust other tests now that we can no longer easily look for the "-----BEGIN PGP MESSAGE-----" string. Refs #7022. --- redwood/redwood/__init__.pyi | 4 ++- redwood/src/lib.rs | 45 ++++++++++++------------- securedrop/tests/test_encryption.py | 16 ++++----- securedrop/tests/test_journalist_api.py | 12 +++++-- 4 files changed, 41 insertions(+), 36 deletions(-) diff --git a/redwood/redwood/__init__.pyi b/redwood/redwood/__init__.pyi index 8b74f179c5d..eb6908b6b4e 100644 --- a/redwood/redwood/__init__.pyi +++ b/redwood/redwood/__init__.pyi @@ -5,7 +5,9 @@ from typing import BinaryIO def generate_source_key_pair(passphrase: str, email: str) -> tuple[str, str, str]: ... def is_valid_public_key(input: str) -> str: ... -def encrypt_message(recipients: list[str], plaintext: str, destination: Path) -> None: ... +def encrypt_message( + recipients: list[str], plaintext: str, destination: Path, *, armor: bool = False +) -> None: ... def encrypt_stream(recipients: list[str], plaintext: BinaryIO, destination: Path) -> None: ... def decrypt(ciphertext: bytes, secret_key: str, passphrase: str) -> bytes: ... diff --git a/redwood/src/lib.rs b/redwood/src/lib.rs index 9f38e347370..93ff39d847b 100644 --- a/redwood/src/lib.rs +++ b/redwood/src/lib.rs @@ -119,9 +119,10 @@ pub fn encrypt_message( recipients: Vec, plaintext: String, destination: PathBuf, + armor: Option, ) -> Result<()> { let plaintext = plaintext.as_bytes(); - encrypt(&recipients, plaintext, &destination) + encrypt(&recipients, plaintext, &destination, armor) } /// Encrypt a Python stream (`typing.BinaryIO`) for the specified recipients. @@ -134,7 +135,7 @@ pub fn encrypt_stream( destination: PathBuf, ) -> Result<()> { let stream = stream::Stream { reader: plaintext }; - encrypt(&recipients, stream, &destination) + encrypt(&recipients, stream, &destination, None) } /// Helper function to encrypt readable things. @@ -144,6 +145,7 @@ fn encrypt( recipients: &[String], mut plaintext: impl Read, destination: &Path, + armor: Option, ) -> Result<()> { let mut certs = vec![]; let mut recipient_keys = vec![]; @@ -165,7 +167,11 @@ fn encrypt( .open(destination)?; let mut writer = BufWriter::new(sink); let message = Message::new(&mut writer); - let message = Armorer::new(message).build()?; + let message = if armor.unwrap_or(false) { + Armorer::new(message).build()? + } else { + message + }; let message = Encryptor::for_recipients(message, recipient_keys).build()?; let mut message = LiteralWriter::new(message).build()?; @@ -278,6 +284,7 @@ mod tests { vec![good_key, BAD_KEY.to_string()], SECRET_MESSAGE.to_string(), tmp_dir.path().join("message.asc"), + None, ) .unwrap_err(); assert_eq!(err.to_string(), expected_err_msg); @@ -325,42 +332,31 @@ mod tests { vec![public_key1, public_key2], SECRET_MESSAGE.to_string(), tmp.clone(), + None, ) .unwrap(); - let ciphertext = std::fs::read_to_string(tmp).unwrap(); - // Verify ciphertext looks like an encrypted message - assert!(ciphertext.starts_with("-----BEGIN PGP MESSAGE-----\n")); + let ciphertext = std::fs::read(tmp).unwrap(); // Decrypt as key 1 - let plaintext = decrypt( - ciphertext.clone().into_bytes(), - secret_key1, - PASSPHRASE.to_string(), - ) - .unwrap(); + let plaintext = + decrypt(ciphertext.clone(), secret_key1, PASSPHRASE.to_string()) + .unwrap(); // Verify message is what we put in originally assert_eq!( SECRET_MESSAGE, String::from_utf8(plaintext.to_vec()).unwrap() ); // Decrypt as key 2 - let plaintext = decrypt( - ciphertext.clone().into_bytes(), - secret_key2, - PASSPHRASE.to_string(), - ) - .unwrap(); + let plaintext = + decrypt(ciphertext.clone(), secret_key2, PASSPHRASE.to_string()) + .unwrap(); // Verify message is what we put in originally assert_eq!( SECRET_MESSAGE, String::from_utf8(plaintext.to_vec()).unwrap() ); // Try to decrypt as key 3, expect an error - let err = decrypt( - ciphertext.into_bytes(), - secret_key3, - PASSPHRASE.to_string(), - ) - .unwrap_err(); + let err = decrypt(ciphertext, secret_key3, PASSPHRASE.to_string()) + .unwrap_err(); assert_eq!( err.to_string(), "OpenPGP error: no matching pkesk, wrong secret key provided?" @@ -391,6 +387,7 @@ mod tests { key, // missing or malformed recipient key "Look ma, no key".to_string(), tmp_dir.path().join("message.asc"), + None, ) .unwrap_err(); assert_eq!(err.to_string(), error); diff --git a/securedrop/tests/test_encryption.py b/securedrop/tests/test_encryption.py index a22a9e38694..d423c78fa08 100644 --- a/securedrop/tests/test_encryption.py +++ b/securedrop/tests/test_encryption.py @@ -114,9 +114,9 @@ def test_encrypt_source_message(self, config, tmp_path): message_in=message, encrypted_message_path_out=encrypted_message_path ) - # And the output file contains the encrypted data + # And the output file doesn't contain the message plaintext encrypted_message = encrypted_message_path.read_bytes() - assert encrypted_message.startswith(b"-----BEGIN PGP MESSAGE-----") + assert message.encode() not in encrypted_message # And the journalist is able to decrypt the message decrypted_message = utils.decrypt_as_journalist(encrypted_message).decode() @@ -138,9 +138,9 @@ def test_encrypt_source_file(self, config, tmp_path): encrypted_file_path_out=encrypted_file_path, ) - # And the output file contains the encrypted data + # And the output file doesn't contain the file plaintext encrypted_file = encrypted_file_path.read_bytes() - assert encrypted_file.startswith(b"-----BEGIN PGP MESSAGE-----") + assert file_to_encrypt_path.read_bytes() not in encrypted_file # And the journalist is able to decrypt the file decrypted_file = utils.decrypt_as_journalist(encrypted_file) @@ -173,9 +173,9 @@ def test_encrypt_and_decrypt_journalist_reply( encrypted_reply_path_out=encrypted_reply_path, ) - # And the output file contains the encrypted data + # And the output file doesn't contain the reply plaintext encrypted_reply = encrypted_reply_path.read_bytes() - assert encrypted_reply.startswith(b"-----BEGIN PGP MESSAGE-----") + assert journalist_reply.encode() not in encrypted_reply # And source1 is able to decrypt the reply decrypted_reply = encryption_mgr.decrypt_journalist_reply( @@ -224,9 +224,9 @@ def test_gpg_encrypt_and_decrypt_journalist_reply( encrypted_reply_path_out=encrypted_reply_path, ) - # And the output file contains the encrypted data + # And the output file doesn't contain the reply plaintext encrypted_reply = encrypted_reply_path.read_bytes() - assert encrypted_reply.startswith(b"-----BEGIN PGP MESSAGE-----") + assert journalist_reply.encode() not in encrypted_reply # And source1 is able to decrypt the reply decrypted_reply = encryption_mgr.decrypt_journalist_reply( diff --git a/securedrop/tests/test_journalist_api.py b/securedrop/tests/test_journalist_api.py index 151e0906904..9e09668e6eb 100644 --- a/securedrop/tests/test_journalist_api.py +++ b/securedrop/tests/test_journalist_api.py @@ -777,10 +777,16 @@ def test_authorized_user_can_add_reply( # First we must encrypt the reply, or it will get rejected # by the server. - encryption_mgr = EncryptionManager.get_default() reply_path = tmp_path / "message.gpg" - encryption_mgr.encrypt_journalist_reply( - test_source["source"], "This is a plaintext reply", reply_path + # Use redwood directly, so we can generate an armored message. + redwood.encrypt_message( + recipients=[ + test_source["source"].public_key, + EncryptionManager.get_default().get_journalist_public_key(), + ], + plaintext="This is an encrypted reply", + destination=reply_path, + armor=True, ) reply_content = reply_path.read_text() From a87b40394c1e5ee7996572136210d1ecc5ab88c0 Mon Sep 17 00:00:00 2001 From: Kishan Kumar Rai Date: Thu, 26 Oct 2023 14:52:11 +0530 Subject: [PATCH 2/5] Fixed Broken Link --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3d5727d5adb..1e438bb855c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -5,7 +5,7 @@ See the [introduction for contributors](https://developers.securedrop.org/en/lat If you're just starting, we have labeled some issues as a [*good first issue*](https://github.com/freedomofpress/securedrop/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22). If no one else has indicated they're working on it, please leave a comment saying you're interested, and feel free to get started right away! -Check out our [Development Roadmap](https://github.com/freedomofpress/securedrop/wiki-Roadmap) to see our plans and priorities for upcoming releases. +Check out our [Development Roadmap](https://github.com/freedomofpress/securedrop/wiki/Development-Roadmap) to see our plans and priorities for upcoming releases. For translators, see the [Translator Guide](https://developers.securedrop.org/en/latest/translations.html) to learn how to get started with SecureDrop translations in our [translation portal](https://weblate.securedrop.org/). From 182468bf6d57f85cffbf19c5655d843a82be8614 Mon Sep 17 00:00:00 2001 From: Cory Francis Myers Date: Thu, 19 Oct 2023 18:20:08 -0700 Subject: [PATCH 3/5] build: print current Git ref at the top of the build transcript Closes #6792. --- builder/build-debs.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builder/build-debs.sh b/builder/build-debs.sh index 2d245dd3e19..c91f488f1cc 100755 --- a/builder/build-debs.sh +++ b/builder/build-debs.sh @@ -4,6 +4,8 @@ set -euxo pipefail +git log -1 --oneline --no-color --show-signature + OCI_RUN_ARGUMENTS="--user=root -v $(pwd):/src:Z" # Default to podman if available From e3a534e9069906589a793179873328761254f493 Mon Sep 17 00:00:00 2001 From: Cory Francis Myers Date: Thu, 26 Oct 2023 12:56:03 -0700 Subject: [PATCH 4/5] build(extract-strings): don't version gettext templates We don't use the "Project-Version-Id" header anywhere, so it's not worth updating either manually or via "update_version.sh". --- Makefile | 3 --- .../roles/tails-config/templates/locale/messages.pot | 2 +- securedrop/translations/messages.pot | 2 +- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 98652293995..c5c5b1ee84f 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,6 @@ GCLOUD_VERSION := 222.0.0-1 SDROOT := $(shell git rev-parse --show-toplevel) TAG ?= $(shell git rev-parse HEAD) STABLE_VER := $(shell cat molecule/shared/stable.ver) -VERSION=$(shell python -c "import securedrop.version; print(securedrop.version.__version__)") SDBIN := $(SDROOT)/securedrop/bin DEVSHELL := $(SDBIN)/dev-shell @@ -364,7 +363,6 @@ $(POT): securedrop --charset=utf-8 \ --output=${POT} \ --project="SecureDrop" \ - --version=${VERSION} \ --msgid-bugs-address=securedrop@freedom.press \ --copyright-holder="Freedom of the Press Foundation" \ --add-comments="Translators:" \ @@ -393,7 +391,6 @@ $(DESKTOP_POT): ${DESKTOP_BASE}/*.in -F securedrop/babel.cfg \ --output=${DESKTOP_POT} \ --project=SecureDrop \ - --version=${VERSION} \ --msgid-bugs-address=securedrop@freedom.press \ --copyright-holder="Freedom of the Press Foundation" \ --add-location=never \ diff --git a/install_files/ansible-base/roles/tails-config/templates/locale/messages.pot b/install_files/ansible-base/roles/tails-config/templates/locale/messages.pot index 8aa68fa351c..35d4387ae32 100644 --- a/install_files/ansible-base/roles/tails-config/templates/locale/messages.pot +++ b/install_files/ansible-base/roles/tails-config/templates/locale/messages.pot @@ -6,7 +6,7 @@ #, fuzzy msgid "" msgstr "" -"Project-Id-Version: SecureDrop 2.7.0~rc1\n" +"Project-Id-Version: SecureDrop VERSION\n" "Report-Msgid-Bugs-To: securedrop@freedom.press\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" diff --git a/securedrop/translations/messages.pot b/securedrop/translations/messages.pot index 012a0279d25..7b6a0bdf178 100644 --- a/securedrop/translations/messages.pot +++ b/securedrop/translations/messages.pot @@ -6,7 +6,7 @@ #, fuzzy msgid "" msgstr "" -"Project-Id-Version: SecureDrop 2.7.0~rc1\n" +"Project-Id-Version: SecureDrop VERSION\n" "Report-Msgid-Bugs-To: securedrop@freedom.press\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" From 97af66c143492556313d8048f26583cb920fca73 Mon Sep 17 00:00:00 2001 From: Cory Francis Myers Date: Thu, 26 Oct 2023 18:03:06 -0700 Subject: [PATCH 5/5] fix: Git needs "--no-pager" under non-interactive SSH Otherwise staging-test-with-rebase's SSH from CircleCI stalls on "WARNING: terminal is not fully functional". --- builder/build-debs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/build-debs.sh b/builder/build-debs.sh index c91f488f1cc..99eafaa84d8 100755 --- a/builder/build-debs.sh +++ b/builder/build-debs.sh @@ -4,7 +4,7 @@ set -euxo pipefail -git log -1 --oneline --no-color --show-signature +git --no-pager log -1 --oneline --show-signature --no-color OCI_RUN_ARGUMENTS="--user=root -v $(pwd):/src:Z"