redwood: Relax requirements for decrypting messages

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.

We first try validating the key with the StandardPolicy before trying
again with a NullPolicy to avoid downgrade attacks. Since we now have
another type of policy being used, the constant has been renamed to
STANDARD_POLICY to be clearer what kind it is.

Fixes #6991.

redwood/src/decryption.rs

@@ -5,12 +5,10 @@ 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;
 use sequoia_openpgp::KeyID;
 
 pub(crate) struct Helper<'a> {
-    pub(crate) policy: &'a dyn Policy,
     pub(crate) secret: &'a sequoia_openpgp::Cert,
     pub(crate) passphrase: &'a Password,
 }
@@ -46,7 +44,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,

redwood/src/keys.rs

@@ -2,23 +2,9 @@ use crate::{Error, Result};
 use sequoia_openpgp::cert::prelude::ValidErasedKeyAmalgamation;
 use sequoia_openpgp::packet::key::{PublicParts, SecretParts, UnspecifiedRole};
 use sequoia_openpgp::packet::Key;
-use sequoia_openpgp::policy::Policy;
+use sequoia_openpgp::policy::{NullPolicy, 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>(
@@ -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() {
@@ -37,18 +30,35 @@ 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,
+/// Get the secret encryption key from the cert.
+pub(crate) fn secret_key_from_cert(
+    cert: &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();
+    // Get the secret encryption key for decryption. We don't care about
+    // whether it is revoked or expired, or even if its not a weak key
+    // since we're just decrypting.
+    // We first try to find the key using the StandardPolicy and then
+    // fall back to a NullPolicy to prevent downgrade attacks.
+    let key = cert
+        .keys()
+        .secret()
+        .with_policy(crate::STANDARD_POLICY, None)
+        .for_storage_encryption()
+        .next();
+    if let Some(key) = key {
+        return Ok(key.key().clone());
+    }
+
+    // No key found, try again with a null policy
+    let null = NullPolicy::new();
+    let key = cert
+        .keys()
+        .secret()
+        .with_policy(&null, None)
+        .for_storage_encryption()
+        .next();
 
-    // Just return the first encryption key
-    match keys.get(0) {
+    match key {
         Some(key) => Ok(key.key().clone()),
         None => Err(Error::NoSupportedKeys(cert.fingerprint().to_string())),
     }

redwood/src/lib.rs

@@ -24,7 +24,7 @@ mod decryption;
 mod keys;
 mod stream;
 
-const POLICY: &StandardPolicy = &StandardPolicy::new();
+const STANDARD_POLICY: &StandardPolicy = &StandardPolicy::new();
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -103,7 +103,7 @@ pub fn generate_source_key_pair(
 pub fn is_valid_public_key(input: &str) -> Result<String> {
     let cert = Cert::from_str(input)?;
     // We don't need the keys, just need to check there's at least one and no error
-    keys::keys_from_cert(POLICY, &cert)?;
+    keys::keys_from_cert(STANDARD_POLICY, &cert)?;
     // And there is no secret key material
     if cert.keys().secret().next().is_some() {
         return Err(Error::HasSecretKeyMaterial);
@@ -152,7 +152,7 @@ fn encrypt(
     }
 
     for cert in certs.iter() {
-        recipient_keys.extend(keys::keys_from_cert(POLICY, cert)?);
+        recipient_keys.extend(keys::keys_from_cert(STANDARD_POLICY, cert)?);
     }
 
     // In reverse order, we set up a writer that will write an encrypted and
@@ -184,14 +184,13 @@ 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,
     };
 
     // Now, create a decryptor with a helper using the given Certs.
     let mut decryptor = DecryptorBuilder::from_bytes(&ciphertext)?
-        .with_policy(POLICY, None, helper)?;
+        .with_policy(STANDARD_POLICY, None, helper)?;
 
     // Decrypt the data.
     let mut buffer: Vec<u8> = vec![];