openpgp: Reduce the usage of Cert::primary_key_signature.

5 files changed, 54 insertions, 64 deletions

diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs
index b8451766..42a29942 100644
--- a/openpgp/src/cert/builder.rs
+++ b/openpgp/src/cert/builder.rs
@@ -534,11 +534,9 @@ mod tests {
         assert_eq!(cert1.primary().pk_algo(),
                    PublicKeyAlgorithm::EdDSA);
         assert!(cert1.subkeys().next().is_none());
-        if let Some(sig) = cert1.primary_key_signature(None) {
-            assert!(sig.features().unwrap().supports_mdc());
-        } else {
-            panic!();
-        }
+        assert!(cert1.userids().primary(None).unwrap()
+                .binding_signature().unwrap()
+                .features().unwrap().supports_mdc());
     }
 
     #[test]
@@ -576,14 +574,8 @@ mod tests {
             .primary_key_flags(KeyFlags::default())
             .add_transport_encryption_subkey()
             .generate().unwrap();
-        let sig_pkts = &cert1.primary_key_signature(None).unwrap().hashed_area();
-
-        match sig_pkts.lookup(SubpacketTag::KeyFlags).unwrap().value() {
-            SubpacketValue::KeyFlags(ref ks) => assert!(ks.for_certification()),
-            v => panic!("Unexpected subpacket: {:?}", v),
-        }
-
-        assert_eq!(cert1.subkeys().count(), 1);
+        assert!(cert1.keys().primary(None).unwrap().for_certification());
+        assert_eq!(cert1.keys().subkeys().count(), 1);
     }
 
     #[test]
@@ -666,7 +658,7 @@ mod tests {
         let now = cert.primary().creation_time()
             + 5 * s; // The subkeys may be created a tad later.
         let key = cert.primary();
-        let sig = cert.primary_key_signature(None).unwrap();
+        let sig = &cert.keys().primary_key().self_signatures()[0];
         assert!(sig.key_alive(key, now).is_ok());
         assert!(sig.key_alive(key, now + 590 * s).is_ok());
         assert!(! sig.key_alive(key, now + 610 * s).is_ok());
@@ -697,7 +689,11 @@ mod tests {
             .generate().unwrap();
 
         assert_eq!(cert.primary().creation_time(), UNIX_EPOCH);
-        assert_eq!(cert.primary_key_signature(None).unwrap()
+        assert_eq!(cert.keys().primary(None).unwrap()
+                   .binding_signature()
+                   .signature_creation_time().unwrap(), UNIX_EPOCH);
+        assert_eq!(cert.keys().primary(None).unwrap()
+                   .direct_key_signature().unwrap()
                    .signature_creation_time().unwrap(), UNIX_EPOCH);
         assert_eq!(rev.signature_creation_time().unwrap(), UNIX_EPOCH);
 
diff --git a/openpgp/src/cert/keyiter.rs b/openpgp/src/cert/keyiter.rs
index bd0d648e..3174e6d2 100644
--- a/openpgp/src/cert/keyiter.rs
+++ b/openpgp/src/cert/keyiter.rs
@@ -511,12 +511,10 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> {
                 }
             }
 
-            let binding_signature = ka.binding_signature();
-
             if let Some(flags) = self.flags.as_ref() {
                 if !ka.has_any_key_flag(flags) {
                     t!("Have flags: {:?}, want flags: {:?}... skipping.",
-                       binding_signature.key_flags(), flags);
+                      flags, flags);
                     continue;
                 }
             }
diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs
index 8c1e408f..20de650e 100644
--- a/openpgp/src/cert/mod.rs
+++ b/openpgp/src/cert/mod.rs
@@ -411,7 +411,7 @@ impl Cert {
     /// primary User ID's self signature.  Since these signatures are
     /// associated with the UserID and not the primary key, that
     /// information is not contained in the key binding.  Instead, you
-    /// should use methods like `Cert::primary_key_signature()` to get
+    /// should use methods like `Cert::keys().primary(t)` to get
     /// information about the primary key.
     pub fn primary(&self) -> &key::PublicKey {
         &self.primary.key()
@@ -584,19 +584,24 @@ impl Cert {
         -> Result<Cert>
     {
         let sig = {
-            let (template, userid) = self
-                .primary_key_signature_full(Some(now))
-                .ok_or(Error::MalformedCert("No self-signature".into()))?;
+            let primary = self.keys().primary(now)?;
+            let template = primary.binding_signature();
 
             // Recompute the signature.
             let hash_algo = HashAlgorithm::SHA512;
             let mut hash = hash_algo.context()?;
 
             self.primary().hash(&mut hash);
-            if let Some((userid, _)) = userid {
-                userid.userid().hash(&mut hash);
-            } else {
-                assert_eq!(template.typ(), SignatureType::DirectKey);
+            match template.typ() {
+                SignatureType::DirectKey =>
+                    (), // Nothing to hash.
+                SignatureType::GenericCertification
+                    | SignatureType::PersonaCertification
+                    | SignatureType::CasualCertification
+                    | SignatureType::PositiveCertification =>
+                    self.userids().primary(now).unwrap()
+                    .userid().hash(&mut hash),
+                _ => unreachable!(),
             }
 
             // Generate the signature.
@@ -1784,15 +1789,13 @@ mod test {
     fn merge_with_incomplete_update() {
         let cert = Cert::from_bytes(crate::tests::key("about-to-expire.expired.pgp"))
             .unwrap();
-        assert!(! cert.primary_key_signature(None).unwrap()
-                .key_alive(cert.primary(), None).is_ok());
+        cert.keys().primary(None).unwrap().alive().unwrap_err();
 
         let update =
             Cert::from_bytes(crate::tests::key("about-to-expire.update-no-uid.pgp"))
             .unwrap();
         let cert = cert.merge(update).unwrap();
-        assert!(cert.primary_key_signature(None).unwrap()
-                .key_alive(cert.primary(), None).is_ok());
+        cert.keys().primary(None).unwrap().alive().unwrap();
     }
 
     #[test]
diff --git a/openpgp/src/serialize/cert.rs b/openpgp/src/serialize/cert.rs
index c21b8046..f5fa06e4 100644
--- a/openpgp/src/serialize/cert.rs
+++ b/openpgp/src/serialize/cert.rs
@@ -750,7 +750,8 @@ mod test {
         let uid_binding = uid.bind(
             &mut keypair, &cert,
             signature::Builder::from(
-                cert.primary_key_signature(None).unwrap().clone())
+                cert.keys().primary(None).unwrap()
+                    .direct_key_signature().unwrap().clone())
                 .set_type(SignatureType::PositiveCertification)
                 .set_exportable_certification(false).unwrap()).unwrap();
 
@@ -760,7 +761,8 @@ mod test {
         let ua_binding = ua.bind(
             &mut keypair, &cert,
             signature::Builder::from(
-                cert.primary_key_signature(None).unwrap().clone())
+                cert.keys().primary(None).unwrap()
+                    .direct_key_signature().unwrap().clone())
                 .set_type(SignatureType::PositiveCertification)
                 .set_exportable_certification(false).unwrap()).unwrap();
 
diff --git a/tool/src/commands/inspect.rs b/tool/src/commands/inspect.rs
index 43ba1acc..13d0324c 100644
--- a/tool/src/commands/inspect.rs
+++ b/tool/src/commands/inspect.rs
@@ -4,6 +4,7 @@ use clap;
 
 extern crate sequoia_openpgp as openpgp;
 use crate::openpgp::{Packet, Result};
+use openpgp::packet::key::PublicParts;
 use crate::openpgp::parse::{Parse, PacketParserResult};
 
 use super::dump::Convert;
@@ -133,18 +134,14 @@ fn inspect_cert(output: &mut dyn io::Write, cert: &openpgp::Cert,
     writeln!(output)?;
     writeln!(output, "    Fingerprint: {}", cert.fingerprint())?;
     inspect_revocation(output, "", cert.revoked(None))?;
-    let primary = cert.keys().primary_key();
-    inspect_key(output, "", primary.key(), cert.primary_key_signature(None),
-                primary.certifications(),
-                print_keygrips, print_certifications)?;
+    let primary = cert.keys().primary(None)?;
+    inspect_key(output, "", primary, print_keygrips, print_certifications)?;
     writeln!(output)?;
 
     for ka in cert.keys().policy(None).skip(1) {
         writeln!(output, "         Subkey: {}", ka.key().fingerprint())?;
         inspect_revocation(output, "", ka.revoked())?;
-        inspect_key(output, "", ka.key(), Some(ka.binding_signature()),
-                    ka.binding().certifications(),
-                    print_keygrips, print_certifications)?;
+        inspect_key(output, "", ka, print_keygrips, print_certifications)?;
         writeln!(output)?;
     }
 
@@ -167,21 +164,16 @@ fn inspect_cert(output: &mut dyn io::Write, cert: &openpgp::Cert,
     Ok(())
 }
 
-fn inspect_key<P, R>(output: &mut dyn io::Write,
-                     indent: &str,
-                     key: &openpgp::packet::Key<P, R>,
-                     binding_signature: Option<&openpgp::packet::Signature>,
-                     certs: &[openpgp::packet::Signature],
-                     print_keygrips: bool,
-                     print_certifications: bool)
+fn inspect_key(output: &mut dyn io::Write,
+               indent: &str,
+               ka: openpgp::cert::KeyAmalgamation<PublicParts>,
+               print_keygrips: bool,
+               print_certifications: bool)
         -> Result<()>
-        where P: openpgp::packet::key::KeyParts,
-              R: openpgp::packet::key::KeyRole
 {
-    if let Some(sig) = binding_signature {
-        if let Err(e) = sig.key_alive(key, None) {
-            writeln!(output, "{}                 Invalid: {}", indent, e)?;
-        }
+    let key = ka.key();
+    if let Err(e) = ka.alive() {
+        writeln!(output, "{}                 Invalid: {}", indent, e)?;
     }
 
     if print_keygrips {
@@ -194,20 +186,19 @@ fn inspect_key<P, R>(output: &mut dyn io::Write,
     }
     writeln!(output, "{}  Creation time: {}", indent,
              key.creation_time().convert())?;
-    if let Some(sig) = binding_signature {
-        if let Some(expires) = sig.key_expiration_time() {
-            let expiration_time = key.creation_time() + expires;
-            writeln!(output, "{}Expiration time: {} (creation time + {})",
-                     indent,
-                     expiration_time.convert(),
-                     expires.convert())?;
-        }
+    if let Some(expires) = ka.key_expiration_time() {
+        let expiration_time = key.creation_time() + expires;
+        writeln!(output, "{}Expiration time: {} (creation time + {})",
+                 indent,
+                 expiration_time.convert(),
+                 expires.convert())?;
+    }
 
-        if let Some(flags) = sig.key_flags().and_then(inspect_key_flags) {
-            writeln!(output, "{}       Key flags: {}", indent, flags)?;
-        }
+    if let Some(flags) = sig.key_flags().and_then(inspect_key_flags) {
+        writeln!(output, "{}       Key flags: {}", indent, flags)?;
     }
-    inspect_certifications(output, certs, print_certifications)?;
+    inspect_certifications(output, ka.binding().certifications(),
+                           print_certifications)?;
 
     Ok(())
 }