openpgp: Rework certificate lookup in the streaming decryptor.

  - Previously, we called VerificationHelper::get_certs once we saw
    the literal data packet.  The classic OpenPGP rationale for having
    the signer's keyid in the OPS packet is so that consuming
    implementations can avoid hashing the body if they don't have the
    certificate to verify the signature with.

  - However, there is a better opportunity to do that: Just in time
    before doing the actual verification when we have seen all the
    signatures.  This has the advantage that we may know fingerprints
    instead of mere keyids.

  - This is crucial for verifying messages using the Cleartext
    Signature Framework where we do not know the issuers before
    encountering the signatures.

  - Also, deduplicate aliasing key handles, preferring fingerprints.

1 files changed, 57 insertions, 30 deletions

diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index bca36ac9..bb499f04 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -127,7 +127,7 @@ use crate::{
         PKESK,
         SKESK,
     },
-    KeyID,
+    KeyHandle,
     Packet,
     Result,
     packet,
@@ -1708,7 +1708,13 @@ enum Mode {
 /// # Ok(()) }
 pub struct Decryptor<'a, H: VerificationHelper + DecryptionHelper> {
     helper: H,
+
+    /// The issuers collected from OPS and Signature packets.
+    issuers: Vec<KeyHandle>,
+
+    /// The certificates used for signature verification.
     certs: Vec<Cert>,
+
     oppr: Option<PacketParserResult<'a>>,
     identity: Option<Fingerprint>,
     structure: IMessageStructure,
@@ -2303,6 +2309,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
 
         let mut v = Decryptor {
             helper,
+            issuers: Vec::new(),
             certs: Vec::new(),
             oppr: None,
             identity: None,
@@ -2316,10 +2323,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
             policy,
         };
 
-        let mut issuers = Vec::new();
         let mut pkesks: Vec<packet::PKESK> = Vec::new();
         let mut skesks: Vec<packet::SKESK> = Vec::new();
-        let mut saw_content = false;
 
         while let PacketParserResult::Some(mut pp) = ppr {
             v.policy.packet(&pp.packet)?;
@@ -2351,8 +2356,6 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                 Packet::CompressedData(ref p) =>
                     v.structure.new_compression_layer(p.algo()),
                 Packet::SEIP(_) | Packet::AED(_) if v.mode == Mode::Decrypt => {
-                    saw_content = true;
-
                     // Get the symmetric algorithm from the decryption
                     // proxy function.  This is necessary because we
                     // cannot get the algorithm from the SEIP packet.
@@ -2396,12 +2399,10 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                 },
                 Packet::OnePassSig(ref ops) => {
                     v.structure.push_ops(ops);
-                    issuers.push(ops.issuer().clone().into());
+                    v.push_issuer(ops.issuer().clone());
                 },
                 Packet::Literal(_) => {
                     v.structure.insert_missing_signature_group();
-                    // Query keys.
-                    v.certs = v.helper.get_certs(&issuers)?;
                     v.oppr = Some(PacketParserResult::Some(pp));
                     v.finish_maybe()?;
 
@@ -2418,27 +2419,14 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                 Packet::PKESK(pkesk) => pkesks.push(pkesk),
                 Packet::SKESK(skesk) => skesks.push(skesk),
                 Packet::Signature(sig) => {
-                    if ! saw_content {
-                        // The following structure is allowed:
-                        //
-                        //   SIG LITERAL
-                        //
-                        // In this case, we get the issuer from the
-                        // signature itself.
-                        let mut sig_issuers = sig.get_issuers();
-                        if sig_issuers.is_empty() {
-                            // No issuer information.  Push a wildcard
-                            // KeyID to indicate this to the
-                            // VerificationHelper::check.  This is the
-                            // detached-signature equivalent of a
-                            // signed message with a wildcard signer
-                            // KeyID on the OPS packet and no issuer
-                            // information on the signature.
-                            issuers.push(KeyID::wildcard().into());
-                        } else {
-                            issuers.append(&mut sig_issuers);
-                        }
-                    }
+                    // The following structure is allowed:
+                    //
+                    //   SIG LITERAL
+                    //
+                    // In this case, we get the issuer from the
+                    // signature itself.
+                    sig.get_issuers().into_iter()
+                        .for_each(|i| v.push_issuer(i));
                     v.structure.push_bare_signature(sig);
                 }
                 _ => (),
@@ -2447,7 +2435,6 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
         }
 
         if v.mode == Mode::VerifyDetached {
-            v.certs = v.helper.get_certs(&issuers)?;
             return Ok(v);
         }
 
@@ -2506,6 +2493,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
     fn push_sig(&mut self, p: Packet) -> Result<()> {
         match p {
             Packet::Signature(sig) => {
+                sig.get_issuers().into_iter().for_each(|i| self.push_issuer(i));
                 self.structure.push_signature(sig);
             },
             _ => (),
@@ -2513,6 +2501,42 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
         Ok(())
     }
 
+    /// Records the issuer for the later certificate lookup.
+    fn push_issuer<I: Into<KeyHandle>>(&mut self, issuer: I) {
+        let issuer = issuer.into();
+        match issuer {
+            KeyHandle::KeyID(id) if id.is_wildcard() => {
+                // Ignore, they are not useful for lookups.
+                return;
+            },
+
+            KeyHandle::KeyID(_) => {
+                for known in self.issuers.iter() {
+                    if known.aliases(&issuer) {
+                        return;
+                    }
+                }
+
+                // Unknown, record.
+                self.issuers.push(issuer);
+            },
+
+            KeyHandle::Fingerprint(_) => {
+                for known in self.issuers.iter_mut() {
+                    if known.aliases(&issuer) {
+                        // Replace.  We may upgrade a KeyID to a
+                        // Fingerprint.
+                        *known = issuer;
+                        return;
+                    }
+                }
+
+                // Unknown, record.
+                self.issuers.push(issuer);
+            },
+        }
+    }
+
     // If the amount of remaining data does not exceed the reserve,
     // finish processing the OpenPGP packet sequence.
     //
@@ -2570,6 +2594,9 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
         tracer!(TRACE, "Decryptor::verify_signatures", 0);
         t!("called");
 
+        self.certs = self.helper.get_certs(&self.issuers)?;
+        t!("VerificationHelper::get_certs produced {} certs", self.certs.len());
+
         let mut results = MessageStructure::new();
         for layer in self.structure.layers.iter_mut() {
             match layer {