From 9d4b91098d37549347fdfff9513b629fcd3d973f Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Mon, 27 Apr 2020 10:26:07 +0200 Subject: openpgp: Convert `Cert::from_packet_pile` into `TryFrom` - Drop `Cert::from_packet_pile`. - Fixes #462. --- autocrypt/src/lib.rs | 3 +- examples/guide-exploring-openpgp.rs | 3 +- guide/src/chapter_03.md | 4 +- openpgp-ffi/src/cert.rs | 2 +- openpgp/src/cert/builder.rs | 6 +- openpgp/src/cert/mod.rs | 132 +++++++++++++++++++----------------- openpgp/src/packet_pile.rs | 5 +- sqv/tests/revoked-key.rs | 7 +- tool/src/commands/inspect.rs | 5 +- 9 files changed, 90 insertions(+), 77 deletions(-) diff --git a/autocrypt/src/lib.rs b/autocrypt/src/lib.rs index 479aeb8d..f467ef29 100644 --- a/autocrypt/src/lib.rs +++ b/autocrypt/src/lib.rs @@ -13,6 +13,7 @@ use base64; +use std::convert::TryFrom; use std::io; use std::io::prelude::*; use std::io::BufReader; @@ -146,7 +147,7 @@ impl AutocryptHeader { } } - let cleaned_cert = Cert::from_packet_pile(acc.into())?; + let cleaned_cert = Cert::try_from(acc)?; Ok(AutocryptHeader { header_type: AutocryptHeaderType::Sender, diff --git a/examples/guide-exploring-openpgp.rs b/examples/guide-exploring-openpgp.rs index 298fcd6c..82296cf4 100644 --- a/examples/guide-exploring-openpgp.rs +++ b/examples/guide-exploring-openpgp.rs @@ -1,6 +1,7 @@ //! https://sequoia-pgp.org/guide/exploring-openpgp/ extern crate sequoia_openpgp as openpgp; +use std::convert::TryFrom; use crate::openpgp::parse::Parse; use crate::openpgp::policy::StandardPolicy as P; @@ -51,7 +52,7 @@ fn main() { println!(); // Parse into Cert. - let cert = openpgp::Cert::from_packet_pile(pile).unwrap(); + let cert = openpgp::Cert::try_from(pile).unwrap(); println!("Fingerprint: {}", cert.fingerprint()); // List userids. diff --git a/guide/src/chapter_03.md b/guide/src/chapter_03.md index 19a42147..4d48717c 100644 --- a/guide/src/chapter_03.md +++ b/guide/src/chapter_03.md @@ -95,12 +95,12 @@ fn main() { [`PacketPile`]s are unstructured sequences of OpenPGP packets. Packet piles can be inspected, manipulated, validated using a formal grammar and thereby turned into [`Message`]s or [`Cert`]s using -[`Message::try_from`] or [`Cert::from_packet_pile`], or just +[`Message::try_from`] or [`Cert::try_from`], or just turned into a vector of [`Packet`]s: [`PacketPile`]: ../../sequoia_openpgp/struct.PacketPile.html [`Packet`]: ../../sequoia_openpgp/enum.Packet.html -[`Cert::from_packet_pile`]: ../../sequoia_openpgp/cert/struct.Cert.html#method.from_packet_pile +[`Cert::try_from`]: ../../sequoia_openpgp/cert/struct.Cert.html#method.try_from [`Message::try_from`]: ../../sequoia_openpgp/struct.Message.html#method.try_from ```rust diff --git a/openpgp-ffi/src/cert.rs b/openpgp-ffi/src/cert.rs index 16350cd1..4a7c2238 100644 --- a/openpgp-ffi/src/cert.rs +++ b/openpgp-ffi/src/cert.rs @@ -68,7 +68,7 @@ pub struct Cert(openpgp::Cert); fn pgp_cert_from_packet_pile(errp: Option<&mut *mut crate::error::Error>, m: *mut PacketPile) -> Maybe { - openpgp::Cert::from_packet_pile(m.move_from_raw()).move_into_raw(errp) + openpgp::Cert::try_from(m.move_from_raw()).move_into_raw(errp) } /// Returns the first Cert found in the packet parser. diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs index 7619e6d4..4ea6dd27 100644 --- a/openpgp/src/cert/builder.rs +++ b/openpgp/src/cert/builder.rs @@ -293,6 +293,7 @@ impl CertBuilder { pub fn generate(mut self) -> Result<(Cert, Signature)> { use crate::{PacketPile, Packet}; use crate::types::ReasonForRevocation; + use std::convert::TryFrom; let creation_time = self.creation_time.unwrap_or_else(std::time::SystemTime::now); @@ -326,7 +327,7 @@ impl CertBuilder { let sig = sig.set_revocation_key(vec![])?; let mut cert = - Cert::from_packet_pile(PacketPile::from(packets))?; + Cert::try_from(PacketPile::from(packets))?; // Sign UserIDs. for uid in self.userids.into_iter() { @@ -581,13 +582,14 @@ mod tests { #[test] fn builder_roundtrip() { use crate::PacketPile; + use std::convert::TryFrom; let (cert,_) = CertBuilder::new() .set_cipher_suite(CipherSuite::Cv25519) .add_signing_subkey() .generate().unwrap(); let pile = cert.clone().into_packet_pile().into_children().collect::>(); - let exp = Cert::from_packet_pile(PacketPile::from(pile)) + let exp = Cert::try_from(PacketPile::from(pile)) .unwrap(); assert_eq!(cert, exp); diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs index 4ace6639..b873ef23 100644 --- a/openpgp/src/cert/mod.rs +++ b/openpgp/src/cert/mod.rs @@ -574,6 +574,7 @@ use super::*; /// # extern crate sequoia_openpgp as openpgp; /// # use openpgp::Result; /// # use openpgp::parse::{Parse, PacketParserResult, PacketParser}; +/// use std::convert::TryFrom; /// use openpgp::cert::prelude::*; /// /// # fn main() { f().unwrap(); } @@ -631,7 +632,7 @@ use super::*; /// for s in cert.bad_signatures() { acc.push(s.clone().into()) } /// /// // Finally, parse into Cert. -/// Cert::from_packet_pile(acc.into()) +/// Cert::try_from(acc) /// } /// /// let (cert, _) = @@ -1313,48 +1314,6 @@ impl Cert { self.into() } - /// Returns the first certificate found in the `PacketPile`. - /// - /// If the [`PacketPile`] does not start with a certificate - /// (specifically, if it does not start with a primary key - /// packet), then this fails. - /// - /// If the `PacketPile` contains multiple keys (i.e., it is a key - /// ring) and you want all of the certificates, you should use - /// [`CertParser`] instead of this function. - /// - /// [`PacketPile`]: ../struct.PacketPile.html - /// [`CertParser`]: struct.CertParser.html - /// - /// # Examples - /// - /// ``` - /// use sequoia_openpgp as openpgp; - /// use openpgp::cert::prelude::*; - /// use openpgp::packet::prelude::*; - /// use openpgp::PacketPile; - /// - /// # fn main() -> openpgp::Result<()> { - /// let (cert, rev) = - /// CertBuilder::general_purpose(None, Some("alice@example.org")) - /// .generate()?; - /// - /// // We should be able to turn a certificate into a PacketPile - /// // and back. - /// let pp : PacketPile = cert.into(); - /// assert!(Cert::from_packet_pile(pp).is_ok()); - /// - /// // But a revocation certificate is not a certificate, so this - /// // will fail. - /// let pp : PacketPile = Packet::from(rev).into(); - /// assert!(Cert::from_packet_pile(pp).is_err()); - /// # Ok(()) - /// # } - /// ``` - pub fn from_packet_pile(p: PacketPile) -> Result { - Self::from_packets(p.into_children()) - } - fn canonicalize(mut self) -> Self { tracer!(TRACE, "canonicalize", 0); @@ -2127,7 +2086,7 @@ impl Cert { } } - Cert::from_packet_pile(PacketPile::from(combined)) + Cert::try_from(PacketPile::from(combined)) } /// Returns whether at least one of the keys includes secret @@ -2274,6 +2233,53 @@ impl TryFrom for Cert { } } +impl TryFrom for Cert { + type Error = anyhow::Error; + + /// Returns the first certificate found in the `PacketPile`. + /// + /// If the [`PacketPile`] does not start with a certificate + /// (specifically, if it does not start with a primary key + /// packet), then this fails. + /// + /// If the `PacketPile` contains multiple keys (i.e., it is a key + /// ring) and you want all of the certificates, you should use + /// [`CertParser`] instead of this function. + /// + /// [`PacketPile`]: ../struct.PacketPile.html + /// [`CertParser`]: struct.CertParser.html + /// + /// # Examples + /// + /// ``` + /// use sequoia_openpgp as openpgp; + /// use openpgp::cert::prelude::*; + /// use openpgp::packet::prelude::*; + /// use openpgp::PacketPile; + /// use std::convert::TryFrom; + /// + /// # fn main() -> openpgp::Result<()> { + /// let (cert, rev) = + /// CertBuilder::general_purpose(None, Some("alice@example.org")) + /// .generate()?; + /// + /// // We should be able to turn a certificate into a PacketPile + /// // and back. + /// let pp : PacketPile = cert.into(); + /// assert!(Cert::try_from(pp).is_ok()); + /// + /// // But a revocation certificate is not a certificate, so this + /// // will fail. + /// let pp : PacketPile = Packet::from(rev).into(); + /// assert!(Cert::try_from(pp).is_err()); + /// # Ok(()) + /// # } + /// ``` + fn try_from(p: PacketPile) -> Result { + Self::from_packets(p.into_children()) + } +} + impl From for Vec { fn from(cert: Cert) -> Self { cert.into_packets().collect::>() @@ -2458,7 +2464,7 @@ mod test { fn parse_cert(data: &[u8], as_message: bool) -> Result { if as_message { let pile = PacketPile::from_bytes(data).unwrap(); - Cert::from_packet_pile(pile) + Cert::try_from(pile) } else { Cert::from_bytes(data) } @@ -2938,30 +2944,30 @@ mod test { #[test] fn packet_pile_roundtrip() { - // Make sure Cert::from_packet_pile(Cert::to_packet_pile(cert)) + // Make sure Cert::try_from(Cert::to_packet_pile(cert)) // does a clean round trip. let cert = Cert::from_bytes(crate::tests::key("already-revoked.pgp")).unwrap(); let cert2 - = Cert::from_packet_pile(cert.clone().into_packet_pile()).unwrap(); + = Cert::try_from(cert.clone().into_packet_pile()).unwrap(); assert_eq!(cert, cert2); let cert = Cert::from_bytes( crate::tests::key("already-revoked-direct-revocation.pgp")).unwrap(); let cert2 - = Cert::from_packet_pile(cert.clone().into_packet_pile()).unwrap(); + = Cert::try_from(cert.clone().into_packet_pile()).unwrap(); assert_eq!(cert, cert2); let cert = Cert::from_bytes( crate::tests::key("already-revoked-userid-revocation.pgp")).unwrap(); let cert2 - = Cert::from_packet_pile(cert.clone().into_packet_pile()).unwrap(); + = Cert::try_from(cert.clone().into_packet_pile()).unwrap(); assert_eq!(cert, cert2); let cert = Cert::from_bytes( crate::tests::key("already-revoked-subkey-revocation.pgp")).unwrap(); let cert2 - = Cert::from_packet_pile(cert.clone().into_packet_pile()).unwrap(); + = Cert::try_from(cert.clone().into_packet_pile()).unwrap(); assert_eq!(cert, cert2); } @@ -3434,7 +3440,7 @@ mod test { (bind1, rev1, bind2, rev2) }; let pk : key::PublicKey = key.into(); - let cert = Cert::from_packet_pile(PacketPile::from(vec![ + let cert = Cert::try_from(PacketPile::from(vec![ pk.into(), bind1.into(), bind2.into(), @@ -3804,7 +3810,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= }) .collect::>(); eprintln!("parse back"); - let cert = Cert::from_packet_pile(PacketPile::from(pile)).unwrap(); + let cert = Cert::try_from(PacketPile::from(pile)).unwrap(); assert_eq!(cert.subkeys().len(), 2); } @@ -4067,7 +4073,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= key.set_creation_time(t1).unwrap(); let mut pair = key.clone().into_keypair().unwrap(); let pk : key::PublicKey = key.clone().into(); - let mut cert = Cert::from_packet_pile(PacketPile::from(vec![ + let mut cert = Cert::try_from(PacketPile::from(vec![ pk.into(), ])).unwrap(); let uid: UserID = "foo@example.org".into(); @@ -4235,7 +4241,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= let primary: Key<_, key::PrimaryRole> = key::Key4::generate_ecc(true, Curve::Ed25519)?.into(); let mut primary_pair = primary.clone().into_keypair()?; - let cert = Cert::from_packet_pile(vec![primary.into()].into())?; + let cert = Cert::try_from(vec![primary.into()])?; // We now add components without binding signatures. They // should be kept, be enumerable, but ignored if a policy is @@ -4327,16 +4333,16 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= let primary_pub = primary_sec.clone().take_secret().0; let cert_p = - Cert::from_packet_pile(vec![primary_pub.clone().into()].into())?; + Cert::try_from(vec![primary_pub.clone().into()])?; let cert_s = - Cert::from_packet_pile(vec![primary_sec.clone().into()].into())?; + Cert::try_from(vec![primary_sec.clone().into()])?; let cert = cert_p.merge(cert_s)?; assert!(cert.primary_key().has_secret()); let cert_p = - Cert::from_packet_pile(vec![primary_pub.clone().into()].into())?; + Cert::try_from(vec![primary_pub.clone().into()])?; let cert_s = - Cert::from_packet_pile(vec![primary_sec.clone().into()].into())?; + Cert::try_from(vec![primary_sec.clone().into()])?; let cert = cert_s.merge(cert_p)?; assert!(cert.primary_key().has_secret()); Ok(()) @@ -4348,7 +4354,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= let primary: Key<_, key::PrimaryRole> = key::Key4::generate_ecc(true, Curve::Ed25519)?.into(); let mut primary_pair = primary.clone().into_keypair()?; - let cert = Cert::from_packet_pile(vec![primary.clone().into()].into())?; + let cert = Cert::try_from(vec![primary.clone().into()])?; let subkey_sec: Key<_, key::SubordinateRole> = key::Key4::generate_ecc(false, Curve::Cv25519)?.into(); @@ -4358,23 +4364,23 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= .set_transport_encryption(true))?; let binding = subkey_sec.bind(&mut primary_pair, &cert, builder)?; - let cert = Cert::from_packet_pile(vec![ + let cert = Cert::try_from(vec![ primary.clone().into(), subkey_pub.clone().into(), binding.clone().into(), subkey_sec.clone().into(), binding.clone().into(), - ].into())?; + ])?; assert_eq!(cert.keys().subkeys().count(), 1); assert_eq!(cert.keys().unencrypted_secret().subkeys().count(), 1); - let cert = Cert::from_packet_pile(vec![ + let cert = Cert::try_from(vec![ primary.clone().into(), subkey_sec.clone().into(), binding.clone().into(), subkey_pub.clone().into(), binding.clone().into(), - ].into())?; + ])?; assert_eq!(cert.keys().subkeys().count(), 1); assert_eq!(cert.keys().unencrypted_secret().subkeys().count(), 1); Ok(()) diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs index c8782b76..ff9f981f 100644 --- a/openpgp/src/packet_pile.rs +++ b/openpgp/src/packet_pile.rs @@ -33,6 +33,7 @@ use crate::parse::Cookie; /// /// ```rust /// # extern crate sequoia_openpgp as openpgp; +/// use std::convert::TryFrom; /// use openpgp::{Packet, PacketPile}; /// use openpgp::packet::signature::Signature4; /// use openpgp::packet::Signature; @@ -57,7 +58,7 @@ use crate::parse::Cookie; /// // Certificate is considered revoked because it is accompanied with its /// // revocation signature /// let pp: PacketPile = PacketPile::from_bytes(&buffer)?; -/// let cert = Cert::from_packet_pile(pp)?; +/// let cert = Cert::try_from(pp)?; /// if let Revoked(_) = cert.revocation_status(policy, None) { /// // cert is considered revoked /// } @@ -81,7 +82,7 @@ use crate::parse::Cookie; /// }).into(); /// } /// -/// let cert = Cert::from_packet_pile(pp)?; +/// let cert = Cert::try_from(pp)?; /// if let CouldBe(_) = cert.revocation_status(policy, None) { /// // revocation signature is broken and the key is not definitely revoked /// } diff --git a/sqv/tests/revoked-key.rs b/sqv/tests/revoked-key.rs index 7befc140..79d6913e 100644 --- a/sqv/tests/revoked-key.rs +++ b/sqv/tests/revoked-key.rs @@ -250,6 +250,7 @@ mod integration { #[allow(dead_code)] fn create_key() { use std::fs::File; + use std::convert::TryFrom; use sequoia_openpgp::{ Cert, Packet, @@ -352,7 +353,7 @@ fn create_key() { .unwrap(); let sk_bind2 = b.sign_subkey_binding(&mut signer, &key, &subkey).unwrap(); - let cert = Cert::from_packet_pile(PacketPile::from(vec![ + let cert = Cert::try_from(PacketPile::from(vec![ key.clone().into(), direct1.clone().into(), direct2.clone().into(), @@ -385,7 +386,7 @@ fn create_key() { } let rev = b.sign_direct_key(&mut signer).unwrap(); - let cert = Cert::from_packet_pile(PacketPile::from(vec![ + let cert = Cert::try_from(PacketPile::from(vec![ key.clone().into(), direct1.clone().into(), rev.clone().into(), @@ -412,7 +413,7 @@ fn create_key() { } let rev = b.sign_subkey_binding(&mut signer, &key, &subkey).unwrap(); - let cert = Cert::from_packet_pile(PacketPile::from(vec![ + let cert = Cert::try_from(PacketPile::from(vec![ key.clone().into(), direct1.clone().into(), direct2.clone().into(), diff --git a/tool/src/commands/inspect.rs b/tool/src/commands/inspect.rs index e76b680b..b6f96662 100644 --- a/tool/src/commands/inspect.rs +++ b/tool/src/commands/inspect.rs @@ -1,3 +1,4 @@ +use std::convert::TryFrom; use std::io::{self, Read}; use clap; @@ -43,7 +44,7 @@ pub fn inspect(m: &clap::ArgMatches, policy: &dyn Policy, output: &mut dyn io::W } let pp = openpgp::PacketPile::from( ::std::mem::replace(&mut packets, Vec::new())); - let cert = openpgp::Cert::from_packet_pile(pp)?; + let cert = openpgp::Cert::try_from(pp)?; inspect_cert(policy, output, &cert, print_keygrips, print_certifications)?; } @@ -102,7 +103,7 @@ pub fn inspect(m: &clap::ArgMatches, policy: &dyn Policy, output: &mut dyn io::W } else if is_cert.is_ok() || is_keyring.is_ok() { let pp = openpgp::PacketPile::from(packets); - let cert = openpgp::Cert::from_packet_pile(pp)?; + let cert = openpgp::Cert::try_from(pp)?; inspect_cert(policy, output, &cert, print_keygrips, print_certifications)?; } else if packets.is_empty() && ! sigs.is_empty() { -- cgit v1.2.3