diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2022-06-08 16:51:07 +0200 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2022-06-14 10:35:33 +0200 |
commit | 429887d57f705e81e305008cfc95162432d7e684 (patch) | |
tree | eb2105c7631ce62567366f2150913efacd47d5ff | |
parent | 3db2119c000a33a207c8188510d26deea7e15286 (diff) |
openpgp: Add Cert::insert_packets2 and Cert::insert_packets_merge.
- Cert::insert_packets2 is a variant of Cert::insert_packets that
returns whether the certificate actually changed. Fixes #528.
- Cert::insert_packets_merge is a variant of Cert::insert_packets2
that allows one to control how duplicate packets are handled.
Fixes #494.
-rw-r--r-- | openpgp/NEWS | 2 | ||||
-rw-r--r-- | openpgp/src/cert.rs | 211 |
2 files changed, 202 insertions, 11 deletions
diff --git a/openpgp/NEWS b/openpgp/NEWS index 8e9eb474..20154a1e 100644 --- a/openpgp/NEWS +++ b/openpgp/NEWS @@ -4,6 +4,8 @@ * Changes in 1.10.0 ** New functionality + - Cert::insert_packets2 + - Cert::insert_packets_merge - Error::UnsupportedCert2 - TryFrom<Packet> for Unknown - types::{Curve, SymmetricAlgorithm, AEADAlgorithm, diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index 005e6a41..562ca652 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -2338,6 +2338,13 @@ impl Cert { /// certificate, it replaces the existing signature. This way, /// the unhashed subpacket area can be updated. /// + /// On success, this function returns the certificate with the + /// packets merged in, and a boolean indicating whether the + /// certificate actually changed. Changed here means that at + /// least one new packet was added, or an existing packet was + /// updated. Alternatively, changed means that the serialized + /// form has changed. + /// /// [Known packets that don't belong in a TPK or TSK]: https://tools.ietf.org/html/rfc4880#section-11 /// [unknown components]: Cert::unknowns() /// @@ -2359,9 +2366,17 @@ impl Cert { /// assert!(cert.is_tsk()); /// /// + /// // Merging in the certificate doesn't change it. + /// let identical_cert = cert.clone(); + /// let (cert, changed) = + /// cert.insert_packets2(identical_cert.into_packets())?; + /// assert!(! changed); + /// + /// /// // Merge in the revocation certificate. /// assert_eq!(cert.primary_key().self_revocations().count(), 0); - /// let cert = cert.insert_packets(rev)?; + /// let (cert, changed) = cert.insert_packets2(rev)?; + /// assert!(changed); /// assert_eq!(cert.primary_key().self_revocations().count(), 1); /// /// @@ -2371,7 +2386,8 @@ impl Cert { /// openpgp::Error::UnsupportedPacketType(tag).into()); /// /// // It shows up as an unknown component. - /// let cert = cert.insert_packets(unknown)?; + /// let (cert, changed) = cert.insert_packets2(unknown)?; + /// assert!(changed); /// assert_eq!(cert.unknowns().count(), 1); /// for p in cert.unknowns() { /// assert_eq!(p.tag(), tag); @@ -2414,7 +2430,8 @@ impl Cert { /// /// // Merge in the public key. Recall: the packets that are /// // being merged into the certificate take precedence. - /// let cert = cert.insert_packets(pk)?; + /// let (cert, changed) = cert.insert_packets2(pk)?; + /// assert!(changed); /// /// // The secret key material is stripped. /// assert!(! cert.primary_key().has_secret()); @@ -2454,7 +2471,8 @@ impl Cert { /// /// // Merge in the signature. Recall: the packets that are /// // being merged into the certificate take precedence. - /// let cert = cert.insert_packets(sig)?; + /// let (cert, changed) = cert.insert_packets2(sig)?; + /// assert!(changed); /// /// // The old binding signature is replaced. /// assert_eq!(cert.userids().nth(0).unwrap().self_signatures().count(), 1); @@ -2464,11 +2482,144 @@ impl Cert { /// .subpackets(SubpacketTag::NotationData).count(), 1); /// # Ok(()) } /// ``` - pub fn insert_packets<I>(self, packets: I) - -> Result<Self> + pub fn insert_packets2<I>(self, packets: I) + -> Result<(Self, bool)> where I: IntoIterator, I::Item: Into<Packet>, { + self.insert_packets_merge(packets, |_old, new| Ok(new)) + } + + /// Adds packets to the certificate with an explicit merge policy. + /// + /// Like [`Cert::insert_packets2`], but also takes a function that + /// will be called on inserts and replacements that can be used to + /// log changes to the certificate, and to influence how packets + /// are merged. The merge function takes two parameters, an + /// optional existing packet, and the packet to be merged in. + /// + /// If a new packet is inserted, there is no packet currently in + /// the certificate. Hence, the first parameter to the merge + /// function is `None`. + /// + /// If an existing packet is updated, there is a packet currently + /// in the certificate that matches the given packet. Hence, the + /// first parameter to the merge function is + /// `Some(existing_packet)`. + /// + /// Both packets given to the merge function are considered equal + /// when considering the normalized form (only comparing public + /// key parameters and ignoring unhashed signature subpackets, see + /// [`Packet::normalized_hash`]). It must return a packet that + /// equals the input packet. In practice that means that the + /// merge function returns either the old packet, the new packet, + /// or a combination of both packets. If the merge function + /// returns a different packet, this function returns + /// `Error::InvalidOperation`. + /// + /// If the merge function returns the existing packet, this + /// function will still consider this as a change to the + /// certificate. In other words, it may return that the + /// certificate has changed even if the serialized representation + /// is not changed. + /// + /// # Examples + /// + /// In the first example, we give an explicit merge function that + /// just returns the new packet. This policy prefers the new + /// packet. This is the policy used by [`Cert::insert_packets2`]. + /// + /// ``` + /// use sequoia_openpgp as openpgp; + /// use openpgp::crypto::Password; + /// use openpgp::cert::prelude::CertBuilder; + /// + /// # fn main() -> openpgp::Result<()> { + /// let p0 = Password::from("old password"); + /// let p1 = Password::from("new password"); + /// + /// // Create a new key. + /// let (cert, rev) = + /// CertBuilder::general_purpose(None, Some("alice@example.org")) + /// .set_password(Some(p0.clone())) + /// .generate()?; + /// assert!(cert.is_tsk()); + /// + /// // Change the password for the primary key. + /// let pk = cert.primary_key().key().clone().parts_into_secret()? + /// .decrypt_secret(&p0)? + /// .encrypt_secret(&p1)?; + /// + /// // Merge it back in, with a policy projecting to the new packet. + /// let (cert, changed) = + /// cert.insert_packets_merge(pk, |_old, new| Ok(new))?; + /// assert!(changed); + /// + /// // Make sure we can still decrypt the primary key using the + /// // new password. + /// assert!(cert.primary_key().key().clone().parts_into_secret()? + /// .decrypt_secret(&p1).is_ok()); + /// # Ok(()) } + /// ``` + /// + /// In the second example, we give an explicit merge function that + /// returns the old packet if given, falling back to the new + /// packet, if not. This policy prefers the existing packets. + /// + /// ``` + /// use sequoia_openpgp as openpgp; + /// use openpgp::crypto::Password; + /// use openpgp::cert::prelude::CertBuilder; + /// + /// # fn main() -> openpgp::Result<()> { + /// let p0 = Password::from("old password"); + /// let p1 = Password::from("new password"); + /// + /// // Create a new key. + /// let (cert, rev) = + /// CertBuilder::general_purpose(None, Some("alice@example.org")) + /// .set_password(Some(p0.clone())) + /// .generate()?; + /// assert!(cert.is_tsk()); + /// + /// // Change the password for the primary key. + /// let pk = cert.primary_key().key().clone().parts_into_secret()? + /// .decrypt_secret(&p0)? + /// .encrypt_secret(&p1)?; + /// + /// // Merge it back in, with a policy preferring to the old packet. + /// let (cert, changed) = + /// cert.insert_packets_merge(pk, |old, new| Ok(old.unwrap_or(new)))?; + /// assert!(changed); // Overestimates changes. + /// + /// // Make sure we can still decrypt the primary key using the + /// // old password. + /// assert!(cert.primary_key().key().clone().parts_into_secret()? + /// .decrypt_secret(&p0).is_ok()); + /// # Ok(()) } + /// ``` + pub fn insert_packets_merge<P, I>(self, packets: P, merge: I) + -> Result<(Self, bool)> + where P: IntoIterator, + P::Item: Into<Packet>, + I: Fn(Option<Packet>, Packet) -> Result<Packet>, + { + self.insert_packets_(&mut packets.into_iter().map(Into::into), + Box::new(merge)) + } + + /// Adds packets to the certificate with an explicit merge policy. + /// + /// This implements all the Cert::insert_packets* functions. Its + /// arguments `packets` and `merge` use dynamic dispatch so that + /// we avoid the cost of monomorphization. + fn insert_packets_<'a>(self, + packets: &mut dyn Iterator<Item = Packet>, + merge: Box<dyn Fn(Option<Packet>, Packet) + -> Result<Packet> + 'a>) + -> Result<(Self, bool)> + { + let mut changed = false; let mut combined = self.into_packets().collect::<Vec<_>>(); // Hashes a packet ignoring the unhashed subpacket area and @@ -2509,7 +2660,6 @@ impl Cert { // Now we merge in the new packets. for p in packets { - let p = p.into(); Cert::valid_packet(&p)?; let hash = hash_packet(&p); @@ -2574,10 +2724,37 @@ impl Cert { match action { Drop => (), - Overwrite(i) => combined[i] = p, + Overwrite(i) => { + // Existing packet. + let existing = + std::mem::replace(&mut combined[i], + Packet::Marker(Default::default())); + let merged = merge(Some(existing), p)?; + let merged_hash = hash_packet(&merged); + if hash != merged_hash { + return Err(Error::InvalidOperation( + format!("merge function changed packet hash \ + (expected: {}, got: {})", + hash, merged_hash)).into()); + } + + combined[i] = merged; + changed = true; + }, Insert => { - // New packet. Add it to combined. - combined.push(p); + // New packet. + let merged = merge(None, p)?; + let merged_hash = hash_packet(&merged); + if hash != merged_hash { + return Err(Error::InvalidOperation( + format!("merge function changed packet hash \ + (expected: {}, got: {})", + hash, merged_hash)).into()); + } + + // Add it to combined. + combined.push(merged); + changed = true; // Because the caller might insert the same packet // multiple times, we need to also add it to @@ -2595,7 +2772,19 @@ impl Cert { } } - Cert::try_from(combined) + Cert::try_from(combined).map(|cert| (cert, changed)) + } + + /// Adds packets to the certificate. + /// + /// Like [`Cert::insert_packets2`], but does not return whether + /// the certificate changed. + pub fn insert_packets<I>(self, packets: I) + -> Result<Self> + where I: IntoIterator, + I::Item: Into<Packet>, + { + self.insert_packets2(packets).map(|(cert, _)| cert) } /// Returns whether at least one of the keys includes secret |