Skip to content

Commit

Permalink
redwood: Relax requirements for decrypting messages
Browse files Browse the repository at this point in the history
Even if a source key is no longer valid per policy, we still want them
to be able to decrypt a previously valid message for them. We can also
drop the revocation/expiry filters, which were mostly theoretical in the
SecureDrop context anyways.

Fixes #6991.
  • Loading branch information
legoktm committed Oct 12, 2023
1 parent 0856005 commit 10a6822
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 28 deletions.
4 changes: 1 addition & 3 deletions redwood/src/decryption.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ use crate::keys::secret_key_from_cert;
use anyhow::anyhow;
use sequoia_openpgp::crypto::{Password, SessionKey};
use sequoia_openpgp::parse::stream::*;
use sequoia_openpgp::policy::Policy;
use sequoia_openpgp::types::SymmetricAlgorithm;

pub(crate) struct Helper<'a> {
pub(crate) policy: &'a dyn Policy,
pub(crate) secret: &'a sequoia_openpgp::Cert,
pub(crate) passphrase: &'a Password,
}
Expand Down Expand Up @@ -45,7 +43,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
where
D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool,
{
let key = secret_key_from_cert(self.policy, self.secret)?;
let key = secret_key_from_cert(self.secret)?;

for pkesk in pkesks {
// Note: this check won't work for messages encrypted with --throw-keyids,
Expand Down
39 changes: 15 additions & 24 deletions redwood/src/keys.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,10 @@
use crate::{Error, Result};
use sequoia_openpgp::cert::prelude::ValidErasedKeyAmalgamation;
use sequoia_openpgp::packet::key::{PublicParts, SecretParts, UnspecifiedRole};
use sequoia_openpgp::packet::key::{PublicParts, SecretParts, SubordinateRole};
use sequoia_openpgp::packet::Key;
use sequoia_openpgp::policy::Policy;
use sequoia_openpgp::Cert;

/// We want to use the same iterators on public and secret keys but it's not
/// really possible to do it in a function because of type differences so we
/// use a macro instead.
macro_rules! filter_keys {
( $keys:expr, $policy: ident ) => {
$keys
.with_policy($policy, None)
.supported()
.alive()
.revoked(false)
.for_storage_encryption()
};
}

/// Get public encryption keys from the specified cert, returning an error if
/// no valid keys are found.
pub(crate) fn keys_from_cert<'a>(
Expand All @@ -27,7 +13,14 @@ pub(crate) fn keys_from_cert<'a>(
) -> Result<Vec<ValidErasedKeyAmalgamation<'a, PublicParts>>> {
// Pull the encryption keys that are compatible with by the standard policy
// (e.g. not SHA-1) supported by Sequoia, and not revoked.
let keys: Vec<_> = filter_keys!(cert.keys(), policy).collect();
let keys: Vec<_> = cert
.keys()
.with_policy(policy, None)
.supported()
.alive()
.revoked(false)
.for_storage_encryption()
.collect();

// Each certificate must have at least one supported encryption key
if keys.is_empty() {
Expand All @@ -38,14 +31,12 @@ pub(crate) fn keys_from_cert<'a>(
}

/// Get the secret encryption key, which is the first and only subkey, from the cert.
pub(crate) fn secret_key_from_cert<'a>(
policy: &'a dyn Policy,
cert: &'a Cert,
) -> Result<Key<SecretParts, UnspecifiedRole>> {
// Pull the encryption keys that are compatible with by the standard policy
// (e.g. not SHA-1) supported by Sequoia, and not revoked.
// These filter options should be kept in sync with `Helper::decrypt()`.
let keys: Vec<_> = filter_keys!(cert.keys().secret(), policy).collect();
pub(crate) fn secret_key_from_cert(
cert: &Cert,
) -> Result<Key<SecretParts, SubordinateRole>> {
// We explicitly do not check whether keys are revoked/expired (not going to happen for sources)
// and even if a key doesn't meet the current policy, we should still decrypt the message to them.
let keys: Vec<_> = cert.keys().subkeys().secret().collect();

// Just return the first encryption key
match keys.get(0) {
Expand Down
1 change: 0 additions & 1 deletion redwood/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ pub fn decrypt(
let recipient = Cert::from_str(&secret_key)?;
let passphrase: Password = passphrase.into();
let helper = decryption::Helper {
policy: POLICY,
secret: &recipient,
passphrase: &passphrase,
};
Expand Down

0 comments on commit 10a6822

Please sign in to comment.