diff options
author | Neal H. Walfield <neal@pep.foundation> | 2020-10-16 18:59:01 +0200 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2020-10-18 08:16:39 +0200 |
commit | e2fbf5b9d442f156ee433a3e46c98ec3dc89cc61 (patch) | |
tree | ddc3e436866ea5cf9c8776647fbc9ec68a30250a | |
parent | 3b1856444ef7c618f78f987868ee336207dca62d (diff) |
openpgp: Change Cert::into_packets to not drop any information.
- Change `Cert::into_packets` to return the underlying packets.
That is don't drop secret key material like `Cert::serialize`
does.
-rw-r--r-- | openpgp/src/cert.rs | 45 | ||||
-rw-r--r-- | openpgp/src/cert/bundle.rs | 5 | ||||
-rw-r--r-- | openpgp/src/packet_pile.rs | 26 | ||||
-rw-r--r-- | openpgp/tests/for-each-artifact.rs | 18 |
4 files changed, 80 insertions, 14 deletions
diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index 97bb233e..bcfd79be 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -1264,6 +1264,20 @@ impl Cert { /// Converts the certificate into an iterator over a sequence of /// packets. /// + /// **WARNING**: When serializing a `Cert`, any secret key + /// material is dropped. In order to serialize the secret key + /// material, it is first necessary to convert the `Cert` into a + /// [`TSK`] and serialize that. This behavior makes it harder to + /// accidentally leak secret key material. *This function is + /// different.* If a key contains secret key material, it is + /// exported as a [`SecretKey`] or [`SecretSubkey`], as + /// appropriate. This means that **if you serialize the resulting + /// packets, the secret key material will be serialized too**. + /// + /// [`TSK`]: serialize/struct.TSK.html + /// [`SecretKey`]: enum.Packet.html#variant.SecretKey + /// [`SecretSubkey`]: enum.Packet.html#variant.SecretSubkey + /// /// # Examples /// /// ``` @@ -1280,10 +1294,34 @@ impl Cert { /// # } /// ``` pub fn into_packets(self) -> impl Iterator<Item=Packet> { - self.primary.into_packets() + fn rewrite(mut p: impl Iterator<Item=Packet>) + -> impl Iterator<Item=Packet> + { + let k: Packet = match p.next().unwrap() { + Packet::PublicKey(k) => { + if k.has_secret() { + Packet::SecretKey(k.parts_into_secret().unwrap()) + } else { + Packet::PublicKey(k) + } + } + Packet::PublicSubkey(k) => { + if k.has_secret() { + Packet::SecretSubkey(k.parts_into_secret().unwrap()) + } else { + Packet::PublicSubkey(k) + } + } + _ => unreachable!(), + }; + + std::iter::once(k).chain(p) + } + + rewrite(self.primary.into_packets()) .chain(self.userids.into_iter().flat_map(|b| b.into_packets())) .chain(self.user_attributes.into_iter().flat_map(|b| b.into_packets())) - .chain(self.subkeys.into_iter().flat_map(|b| b.into_packets())) + .chain(self.subkeys.into_iter().flat_map(|b| rewrite(b.into_packets()))) .chain(self.unknowns.into_iter().flat_map(|b| b.into_packets())) .chain(self.bad.into_iter().map(|s| s.into())) } @@ -4701,7 +4739,8 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= .into_children() .filter(|pkt| { match pkt { - &Packet::PublicKey(_) | &Packet::PublicSubkey(_) => true, + &Packet::PublicKey(_) | &Packet::PublicSubkey(_) + | &Packet::SecretKey(_) | &Packet::SecretSubkey(_) => true, &Packet::Signature(ref sig) => { sig.typ() == SignatureType::DirectKey || sig.typ() == SignatureType::SubkeyBinding diff --git a/openpgp/src/cert/bundle.rs b/openpgp/src/cert/bundle.rs index 0e957f7a..6564ad89 100644 --- a/openpgp/src/cert/bundle.rs +++ b/openpgp/src/cert/bundle.rs @@ -600,6 +600,11 @@ impl<C> ComponentBundle<C> { /// /// For a given type of signature, the signatures are ordered by /// their creation time, most recent first. + /// + /// When turning the `Key` in a `KeyBundle` into a `Packet`, this + /// function uses the component's type (`C`) to determine the + /// packet's type; the type is not a function of whether the key + /// has secret key material. pub(crate) fn into_packets<'a>(self) -> impl Iterator<Item=Packet> where Packet: From<C> { diff --git a/openpgp/src/packet_pile.rs b/openpgp/src/packet_pile.rs index a623132f..02758608 100644 --- a/openpgp/src/packet_pile.rs +++ b/openpgp/src/packet_pile.rs @@ -20,17 +20,30 @@ use crate::parse::Cookie; /// An unstructured [packet] sequence. /// -/// To deserialize an OpenPGP packet stream, use either +/// To parse an OpenPGP packet stream into a `PacketPile`, you can use /// [`PacketParser`], [`PacketPileParser`], or /// [`PacketPile::from_file`] (or related routines). /// -/// Normally, you'll want to convert the `PacketPile` to a Cert or a +/// [packet]: https://tools.ietf.org/html/rfc4880#section-4 +/// [`PacketParser`]: parse/struct.PacketParser.html +/// [`PacketPileParser`]: parse/struct.PacketPileParser.html +/// [`PacketPile::from_file`]: struct.PacketPile.html#method.from_file +/// +/// You can also convert a [`Cert`] into a `PacketPile` using +/// `PacketPile::from`. Unlike serializing a `Cert`, this does not +/// drop any secret key material. +/// +/// [`Cert`]: ../struct.Cert.html +/// +/// Normally, you'll want to convert the `PacketPile` to a `Cert` or a /// `Message`. /// /// # Examples /// /// This example shows how to modify packets in PacketPile using [`pathspec`]s. /// +/// [`pathspec`]: struct.PacketPile.html#method.path_ref +/// /// ```rust /// # use sequoia_openpgp as openpgp; /// use std::convert::TryFrom; @@ -93,12 +106,6 @@ use crate::parse::Cookie; /// # Ok(()) /// # } /// ``` -/// -/// [packet]: https://tools.ietf.org/html/rfc4880#section-4 -/// [`PacketParser`]: parse/struct.PacketParser.html -/// [`PacketPileParser`]: parse/struct.PacketPileParser.html -/// [`PacketPile::from_file`]: struct.PacketPile.html#method.from_file -/// [`pathspec`]: struct.PacketPile.html#method.path_ref #[derive(PartialEq, Clone, Default)] pub struct PacketPile { /// At the top level, we have a sequence of packets, which may be @@ -516,6 +523,9 @@ impl PacketPile { impl From<Cert> 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`. fn from(cert: Cert) -> PacketPile { PacketPile::from(cert.into_packets().collect::<Vec<Packet>>()) } diff --git a/openpgp/tests/for-each-artifact.rs b/openpgp/tests/for-each-artifact.rs index 014bf18d..5aa23881 100644 --- a/openpgp/tests/for-each-artifact.rs +++ b/openpgp/tests/for-each-artifact.rs @@ -76,11 +76,23 @@ mod for_each_artifact { assert_eq!(v, w, "Serialize and SerializeInto disagree on {:?}", p); - // Check that Cert::into_packets() and Cert::to_vec() - // agree. + // Check that + // Cert::strip_secret_key_material().into_packets() and + // Cert::to_vec() agree. (Cert::into_packets() returns + // secret keys if secret key material is present; + // Cert::to_vec only ever returns public keys.) let v = p.to_vec()?; let mut buf = Vec::new(); - for p in p.clone().into_packets() { + for p in p.clone().strip_secret_key_material().into_packets() { + p.serialize(&mut buf)?; + } + assert_eq!(buf, v); + + // Check that Cert::into_packets() and + // Cert::as_tsk().to_vec() agree. + let v = p.as_tsk().to_vec()?; + let mut buf = Vec::new(); + for p in p.into_packets() { p.serialize(&mut buf)?; } assert_eq!(buf, v); |