From 7c69fe2ec91f4dba1c5dc49d9fd450282a73fcc6 Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Tue, 12 Dec 2023 15:31:57 +0100 Subject: openpgp: Deprecate Cert::into_packets. --- openpgp/NEWS | 2 + openpgp/src/cert.rs | 92 +++++++++++++++++++++++++------------- openpgp/src/cert/builder.rs | 2 +- openpgp/src/cert/parser/mod.rs | 8 ++-- openpgp/src/cert/raw.rs | 2 +- openpgp/src/packet_pile.rs | 12 ++++- openpgp/src/regex/mod.rs | 6 +-- openpgp/tests/for-each-artifact.rs | 2 + 8 files changed, 87 insertions(+), 39 deletions(-) (limited to 'openpgp') diff --git a/openpgp/NEWS b/openpgp/NEWS index 76123c2d..3c0f7f1c 100644 --- a/openpgp/NEWS +++ b/openpgp/NEWS @@ -20,6 +20,8 @@ - Error::ShortKeyID - Cert::into_packets2 - TSK::into_packets +** Deprecated functionality + - Cert::into_packets * Changes in 1.17.0 ** Notable fixes - Sequoia now ignores some formatting errors when reading secret diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index b3dfa833..9ee04ba7 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -1203,7 +1203,7 @@ impl Cert { /// # /// # // Make sure that we keep all keys even if they don't have /// # // any self signatures. - /// # let packets = cert.into_packets() + /// # let packets = cert.into_packets2() /// # .filter(|p| p.tag() != Tag::Signature) /// # .collect::>(); /// # let cert : Cert = packets.try_into()?; @@ -1392,6 +1392,9 @@ impl Cert { /// # Ok(()) /// # } /// ``` + #[deprecated( + since = "1.18.0", + note = "Use Cert::into_packets2 or cert.into_tsk().into_packets()")] pub fn into_packets(self) -> impl Iterator + Send + Sync { fn rewrite(mut p: impl Iterator + Send + Sync) -> impl Iterator + Send + Sync @@ -1503,7 +1506,7 @@ impl Cert { /// /// // We should be able to turn a certificate into a PacketPile /// // and back. - /// assert!(Cert::from_packets(cert.into_packets()).is_ok()); + /// assert!(Cert::from_packets(cert.into_packets2()).is_ok()); /// /// // But a revocation certificate is not a certificate, so this /// // will fail. @@ -2411,7 +2414,7 @@ impl Cert { /// }, /// false).expect("valid"); /// - /// let mut cert_a = cert.clone().into_packets().collect::>(); + /// let mut cert_a = cert.clone().into_packets2().collect::>(); /// match cert_a[1] { /// Packet::Signature(ref mut sig) => { /// let unhashed_area = sig.unhashed_area_mut(); @@ -2423,7 +2426,7 @@ impl Cert { /// }; /// let cert_a = Cert::try_from(cert_a).expect("valid"); /// - /// let mut cert_b = cert.clone().into_packets().collect::>(); + /// let mut cert_b = cert.clone().into_packets2().collect::>(); /// match cert_b[1] { /// Packet::Signature(ref mut sig) => { /// let unhashed_area = sig.unhashed_area_mut(); @@ -2439,7 +2442,7 @@ impl Cert { /// // are merged: /// let merged = cert_a.clone().merge_public(cert_b.clone()) /// .expect("same certificate") - /// .into_packets() + /// .into_packets2() /// .collect::>(); /// match merged[1] { /// Packet::Signature(ref sig) => { @@ -2454,7 +2457,7 @@ impl Cert { /// // packets are merged: /// let merged = cert_b.clone().merge_public(cert_a.clone()) /// .expect("same certificate") - /// .into_packets() + /// .into_packets2() /// .collect::>(); /// match merged[1] { /// Packet::Signature(ref sig) => { @@ -2617,7 +2620,7 @@ impl Cert { /// }, /// false).expect("valid"); /// - /// let mut cert_a = cert.clone().into_packets().collect::>(); + /// let mut cert_a = cert.clone().into_packets2().collect::>(); /// match cert_a[1] { /// Packet::Signature(ref mut sig) => { /// let unhashed_area = sig.unhashed_area_mut(); @@ -2629,7 +2632,7 @@ impl Cert { /// }; /// let cert_a = Cert::try_from(cert_a).expect("valid"); /// - /// let mut cert_b = cert.clone().into_packets().collect::>(); + /// let mut cert_b = cert.clone().into_packets2().collect::>(); /// match cert_b[1] { /// Packet::Signature(ref mut sig) => { /// let unhashed_area = sig.unhashed_area_mut(); @@ -2645,7 +2648,7 @@ impl Cert { /// // are merged: /// let merged = cert_a.clone().merge_public_and_secret(cert_b.clone()) /// .expect("same certificate") - /// .into_packets() + /// .into_packets2() /// .collect::>(); /// match merged[1] { /// Packet::Signature(ref sig) => { @@ -2660,7 +2663,7 @@ impl Cert { /// // packets are merged: /// let merged = cert_b.clone().merge_public_and_secret(cert_a.clone()) /// .expect("same certificate") - /// .into_packets() + /// .into_packets2() /// .collect::>(); /// match merged[1] { /// Packet::Signature(ref sig) => { @@ -2802,7 +2805,7 @@ impl Cert { /// // Merging in the certificate doesn't change it. /// let identical_cert = cert.clone(); /// let (cert, changed) = - /// cert.insert_packets2(identical_cert.into_packets())?; + /// cert.insert_packets2(identical_cert.into_tsk().into_packets())?; /// assert!(! changed); /// /// @@ -3053,7 +3056,8 @@ impl Cert { -> Result<(Self, bool)> { let mut changed = false; - let mut combined = self.into_packets().collect::>(); + let mut combined = + self.as_tsk().into_packets().collect::>(); // Hashes a packet ignoring the unhashed subpacket area and // any secret key material. @@ -3691,7 +3695,21 @@ impl TryFrom for Cert { } impl From for Vec { + /// Converts the `Cert` into a `Vec`. + /// + /// If any packets include secret key material, that secret key + /// material is included in the resulting `Vec`. In + /// contrast, when serializing a `Cert`, or converting a cert to + /// packets with [`Cert::into_packets2`], the secret key material + /// not included. + /// + /// Note: This will change in sequoia-openpgp version 2, which + /// will harmonize the behavior and not include secret key + /// material. + // XXXv2: Drop the note in the doc comment and mentioned it in the + // release notes. fn from(cert: Cert) -> Self { + #[allow(deprecated)] cert.into_packets().collect::>() } } @@ -3720,7 +3738,21 @@ impl IntoIterator for Cert type Item = Packet; type IntoIter = IntoIter; + /// Converts the `Cert` into an iterator over `Packet`s. + /// + /// If any packets include secret key material, that secret key + /// material is included in the resulting iterator. In contrast, + /// when serializing a `Cert`, or converting a cert to packets + /// with [`Cert::into_packets2`], the secret key material not + /// included. + /// + /// Note: This will change in sequoia-openpgp version 2, which + /// will harmonize the behavior and not include secret key + /// material. + // XXXv2: Drop the note in the doc comment and mentioned it in the + // release notes. fn into_iter(self) -> Self::IntoIter { + #[allow(deprecated)] IntoIter(Box::new(self.into_packets())) } } @@ -4989,9 +5021,9 @@ mod test { assert_eq!(rev.len(), 1); assert_eq!(rev[0].tag(), Tag::Signature); - let packets_pre_merge = cert.clone().into_packets().count(); + let packets_pre_merge = cert.clone().into_packets2().count(); let cert = cert.insert_packets(rev).unwrap(); - let packets_post_merge = cert.clone().into_packets().count(); + let packets_post_merge = cert.clone().into_packets2().count(); assert_eq!(packets_post_merge, packets_pre_merge + 1); } @@ -5004,7 +5036,7 @@ mod test { let (cert, _) = CertBuilder::general_purpose(None, Some("Test")) .generate()?; - let packets = cert.clone().into_packets().count(); + let packets = cert.clone().into_packets2().count(); // Merge a signature with different unhashed subpacket areas. // Make sure only the last variant is merged. @@ -5030,14 +5062,14 @@ mod test { let mut sigs = cert2.primary_key().self_signatures(); assert_eq!(sigs.next(), Some(&sig_a)); assert!(sigs.next().is_none()); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert sig_b, make sure it (and it alone) appears. let cert2 = cert.clone().insert_packets(sig_b.clone())?; let mut sigs = cert2.primary_key().self_signatures(); assert_eq!(sigs.next(), Some(&sig_b)); assert!(sigs.next().is_none()); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert sig_a and sig_b. Make sure sig_b (and it alone) // appears. @@ -5046,7 +5078,7 @@ mod test { let mut sigs = cert2.primary_key().self_signatures(); assert_eq!(sigs.next(), Some(&sig_b)); assert!(sigs.next().is_none()); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert sig_b and sig_a. Make sure sig_a (and it alone) // appears. @@ -5055,7 +5087,7 @@ mod test { let mut sigs = cert2.primary_key().self_signatures(); assert_eq!(sigs.next(), Some(&sig_a)); assert!(sigs.next().is_none()); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); Ok(()) } @@ -5064,7 +5096,7 @@ mod test { fn insert_packets_add_userid() -> Result<()> { let (cert, _) = CertBuilder::general_purpose(None, Some("a")) .generate()?; - let packets = cert.clone().into_packets().count(); + let packets = cert.clone().into_packets2().count(); let uid_a = UserID::from("a"); let uid_b = UserID::from("b"); @@ -5074,7 +5106,7 @@ mod test { let mut uids = cert2.userids(); assert_eq!(uids.next().unwrap().userid(), &uid_a); assert!(uids.next().is_none()); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert b, make sure it also appears. let cert2 = cert.clone().insert_packets(uid_b.clone())?; @@ -5085,7 +5117,7 @@ mod test { assert_eq!(uids.next().unwrap(), &uid_a); assert_eq!(uids.next().unwrap(), &uid_b); assert!(uids.next().is_none()); - assert_eq!(cert2.clone().into_packets().count(), packets + 1); + assert_eq!(cert2.clone().into_packets2().count(), packets + 1); Ok(()) } @@ -5095,7 +5127,7 @@ mod test { use crate::crypto::Password; let (cert, _) = CertBuilder::new().generate()?; - let packets = cert.clone().into_packets().count(); + let packets = cert.clone().into_packets2().count(); assert_eq!(cert.keys().count(), 1); let key = cert.keys().secret().next().unwrap().key(); @@ -5109,27 +5141,27 @@ mod test { let cert2 = cert.clone().insert_packets(key_a.clone())?; assert_eq!(cert2.primary_key().key().parts_as_secret().unwrap(), &key_a); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert variant b. let cert2 = cert.clone().insert_packets(key_b.clone())?; assert_eq!(cert2.primary_key().key().parts_as_secret().unwrap(), &key_b); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert variant a then b. We should keep b. let cert2 = cert.clone().insert_packets( vec![ key_a.clone(), key_b.clone() ])?; assert_eq!(cert2.primary_key().key().parts_as_secret().unwrap(), &key_b); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); // Insert variant b then a. We should keep a. let cert2 = cert.clone().insert_packets( vec![ key_b.clone(), key_a.clone() ])?; assert_eq!(cert2.primary_key().key().parts_as_secret().unwrap(), &key_a); - assert_eq!(cert2.clone().into_packets().count(), packets); + assert_eq!(cert2.clone().into_packets2().count(), packets); Ok(()) } @@ -6379,14 +6411,14 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= Ok(()) } - /// Tests that Cert::into_packets() and Cert::serialize(..) agree. + /// Tests that Cert:.into_packets2() and Cert::serialize(..) agree. #[test] - fn test_into_packets() -> Result<()> { + fn test_into_packets2() -> Result<()> { use crate::serialize::SerializeInto; let dkg = Cert::from_bytes(crate::tests::key("dkg.gpg"))?; let mut buf = Vec::new(); - for p in dkg.clone().into_packets() { + for p in dkg.clone().into_packets2() { p.serialize(&mut buf)?; } let dkg = dkg.to_vec()?; diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs index 4b958e2f..0d9f77b1 100644 --- a/openpgp/src/cert/builder.rs +++ b/openpgp/src/cert/builder.rs @@ -1916,7 +1916,7 @@ mod tests { #[test] fn primary_user_things() -> Result<()> { fn count_primary_user_things(c: Cert) -> usize { - c.into_packets().map(|p| match p { + c.into_packets2().map(|p| match p { Packet::Signature(s) if s.primary_userid().unwrap_or(false) => 1, _ => 0, diff --git a/openpgp/src/cert/parser/mod.rs b/openpgp/src/cert/parser/mod.rs index ba39f993..eec5a5d0 100644 --- a/openpgp/src/cert/parser/mod.rs +++ b/openpgp/src/cert/parser/mod.rs @@ -1723,8 +1723,10 @@ mod test { .zip(certs_parsed.into_iter()) .enumerate() { - let ps_orig: Vec = c_orig.into_packets().collect(); - let ps_parsed: Vec = c_parsed.into_packets().collect(); + let ps_orig: Vec = + c_orig.as_tsk().into_packets().collect(); + let ps_parsed: Vec = + c_parsed.as_tsk().into_packets().collect(); assert_eq!(ps_orig.len(), ps_parsed.len(), "number of packets: expected vs. got"); @@ -1815,7 +1817,7 @@ mod test { None, Some("a@example.org")) .generate()?; let cert_1_packets: Vec - = cert_1.into_packets().collect(); + = cert_1.into_packets2().collect(); let (cert_2, _) = CertBuilder::general_purpose( diff --git a/openpgp/src/cert/raw.rs b/openpgp/src/cert/raw.rs index 22f2b5d1..218fa9df 100644 --- a/openpgp/src/cert/raw.rs +++ b/openpgp/src/cert/raw.rs @@ -1516,7 +1516,7 @@ mod test { None, Some("a@example.org")) .generate()?; let cert_1_packets: Vec - = cert_1.into_packets().collect(); + = cert_1.into_packets2().collect(); let (cert_2, _) = CertBuilder::general_purpose( diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs index 1678c523..fcf746c6 100644 --- a/openpgp/src/packet_pile.rs +++ b/openpgp/src/packet_pile.rs @@ -527,8 +527,18 @@ impl From for PacketPile { /// Converts the `Cert` into a `PacketPile`. /// /// If any packets include secret key material, that secret key - /// material is not dropped, as it is when serializing a `Cert`. + /// material is included in the resulting `PacketPile`. In + /// contrast, when serializing a `Cert`, or converting a cert to + /// packets with [`Cert::into_packets2`], the secret key material + /// not included. + /// + /// Note: This will change in sequoia-openpgp version 2, which + /// will harmonize the behavior and not include secret key + /// material. + // XXXv2: Drop the note in the doc comment and mentioned it in the + // release notes. fn from(cert: Cert) -> PacketPile { + #[allow(deprecated)] PacketPile::from(cert.into_packets().collect::>()) } } diff --git a/openpgp/src/regex/mod.rs b/openpgp/src/regex/mod.rs index 2581f425..a3fe379d 100644 --- a/openpgp/src/regex/mod.rs +++ b/openpgp/src/regex/mod.rs @@ -138,7 +138,7 @@ //! alice.primary_key().component(), //! alice_userid)?; //! let alice = alice.insert_packets(alice_certification.clone())?; -//! # assert!(alice.clone().into_packets().any(|p| { +//! # assert!(alice.clone().into_packets2().any(|p| { //! # match p { //! # Packet::Signature(sig) => sig == alice_certification, //! # _ => false, @@ -157,7 +157,7 @@ //! bob.primary_key().component(), //! bob_userid)?; //! let bob = bob.insert_packets(bob_certification.clone())?; -//! # assert!(bob.clone().into_packets().any(|p| { +//! # assert!(bob.clone().into_packets2().any(|p| { //! # match p { //! # Packet::Signature(sig) => sig == bob_certification, //! # _ => false, @@ -181,7 +181,7 @@ //! ca.primary_key().component(), //! ca_userid)?; //! let ca = ca.insert_packets(ca_tsig.clone())?; -//! # assert!(ca.clone().into_packets().any(|p| { +//! # assert!(ca.clone().into_packets2().any(|p| { //! # match p { //! # Packet::Signature(sig) => sig == ca_tsig, //! # _ => false, diff --git a/openpgp/tests/for-each-artifact.rs b/openpgp/tests/for-each-artifact.rs index cb729dc5..7fcf721c 100644 --- a/openpgp/tests/for-each-artifact.rs +++ b/openpgp/tests/for-each-artifact.rs @@ -175,6 +175,7 @@ mod for_each_artifact { // Cert::to_vec only ever returns public keys.) let v = p.to_vec()?; let mut buf = Vec::new(); + #[allow(deprecated)] for p in p.clone().strip_secret_key_material().into_packets() { p.serialize(&mut buf)?; } @@ -188,6 +189,7 @@ mod for_each_artifact { // Cert::as_tsk().to_vec() agree. let v = p.as_tsk().to_vec()?; let mut buf = Vec::new(); + #[allow(deprecated)] for p in p.into_packets() { p.serialize(&mut buf)?; } -- cgit v1.2.3