openpgp: Add limits to round_down and round_up.

1 files changed, 95 insertions, 31 deletions

diff --git a/openpgp/src/types/timestamp.rs b/openpgp/src/types/timestamp.rs
index 74a0caad..79338a35 100644
--- a/openpgp/src/types/timestamp.rs
+++ b/openpgp/src/types/timestamp.rs
@@ -1,3 +1,4 @@
+use std::cmp;
 use std::convert::{TryFrom, TryInto};
 use std::fmt;
 use std::time::{SystemTime, Duration as SystemDuration, UNIX_EPOCH};
@@ -100,16 +101,22 @@ impl Timestamp {
     /// resolution of 24 days, or roughly a month.  `p` must be lower
     /// than 32.
     ///
-    /// See [`Duration::round_up`](struct.Duration.html#method.round_up).
+    /// The lower limit `floor` represents the earliest time the timestamp will be
+    /// rounded down to.
+    ///
+    /// See also [`Duration::round_up`](struct.Duration.html#method.round_up).
     ///
     /// # Important note
     ///
     /// If we create a signature, it is important that the signature's
     /// creation time does not predate the signing keys creation time,
-    /// or otherwise violate the key's validity constraints.  The
-    /// correct way to use this interface is to round the time down,
-    /// lookup all keys and other objects like userids using this
-    /// time, and on success create the signature:
+    /// or otherwise violate the key's validity constraints.
+    /// This can be achieved by using the `floor` parameter.
+    ///
+    /// To ensure validity, use this function to round the time down,
+    /// using the latest known relevant timestamp as a floor.
+    /// Then, lookup all keys and other objects like userids using this
+    /// timestamp, and on success create the signature:
     ///
     /// ```rust
     /// # use sequoia_openpgp::{*, packet::prelude::*, types::*, cert::*};
@@ -122,25 +129,29 @@ impl Timestamp {
     /// // Let's fix a time.
     /// let now = Timestamp::from(1583436160);
     ///
+    /// let cert_creation_alice = now.checked_sub(Duration::weeks(2)?).unwrap();
+    /// let cert_creation_bob = now.checked_sub(Duration::weeks(1)?).unwrap();
+    ///
     /// // Generate a Cert for Alice.
     /// let (alice, _) = CertBuilder::new()
-    ///     .set_creation_time(now.checked_sub(Duration::weeks(2)?).unwrap())
+    ///     .set_creation_time(cert_creation_alice)
     ///     .set_primary_key_flags(KeyFlags::default().set_certification(true))
     ///     .add_userid("alice@example.org")
     ///     .generate()?;
     ///
     /// // Generate a Cert for Bob.
     /// let (bob, _) = CertBuilder::new()
-    ///     .set_creation_time(now.checked_sub(Duration::weeks(1)?).unwrap())
+    ///     .set_creation_time(cert_creation_bob)
     ///     .set_primary_key_flags(KeyFlags::default().set_certification(true))
     ///     .add_userid("bob@example.org")
     ///     .generate()?;
     ///
     /// let sign_with_p = |p| -> Result<Signature> {
     ///     // Round `now` down, then use `t` for all lookups.
-    ///     let t: std::time::SystemTime = now.round_down(p)?.into();
+    ///     // Use the creation time of Bob's Cert as lower bound for rounding.
+    ///     let t: std::time::SystemTime = now.round_down(p, cert_creation_bob)?.into();
     ///
-    ///     /// First, get the certification key.
+    ///     // First, get the certification key.
     ///     let mut keypair =
     ///         alice.keys().with_policy(policy, t).secret().for_certification()
     ///         .nth(0).ok_or_else(|| anyhow::anyhow!("no valid key at"))?
@@ -159,22 +170,23 @@ impl Timestamp {
     /// };
     ///
     /// assert!(sign_with_p(21).is_ok());
-    /// assert!(sign_with_p(22).is_err()); // Rounded-down t predates key, uid.
+    /// assert!(sign_with_p(22).is_ok());  // Rounded to bob's cert's creation time.
+    /// assert!(sign_with_p(32).is_err()); // Invalid precision
     /// # Ok(()) }
     /// ```
-    ///
-    /// There are two possible policies that can be implemented using
-    /// this mechanism.  If protecting the timestamp is more important
-    /// than the signature, the process must fail.  Otherwise,
-    /// increasing the precision until all constraints are satisfied
-    /// will find a timestamp approximating `now`, assuming that the
-    /// constraints are satisfied at `now`.
-    pub fn round_down<P>(&self, precision: P) -> Result<Timestamp>
-        where P: Into<Option<u8>>
+    pub fn round_down<P, F>(&self, precision: P, floor: F) -> Result<Timestamp>
+        where P: Into<Option<u8>>,
+              F: Into<Option<SystemTime>>
     {
         let p = precision.into().unwrap_or(21) as u32;
         if p < 32 {
-            Ok(Self(self.0 & !((1 << p) - 1)))
+            let rounded = Self(self.0 & !((1 << p) - 1));
+            match floor.into() {
+                Some(floor) => {
+                    Ok(cmp::max(rounded, floor.try_into()?))
+                }
+                None => { Ok(rounded) }
+            }
         } else {
             Err(Error::InvalidArgument(
                 format!("Invalid precision {}", p)).into())
@@ -224,6 +236,12 @@ impl From<Duration> for SystemDuration {
     }
 }
 
+impl From<Duration> for Option<SystemDuration> {
+    fn from(d: Duration) -> Self {
+        Some(d.into())
+    }
+}
+
 impl fmt::Debug for Duration {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(f, "{:?}", SystemDuration::from(*self))
@@ -299,13 +317,22 @@ impl Duration {
     /// `2^p` seconds.  The default is `21`, which results in a
     /// resolution of 24 days, or roughly a month.  `p` must be lower
     /// than 32.
-    pub fn round_up<P>(&self, precision: P) -> Result<Duration>
-        where P: Into<Option<u8>>
+    ///
+    /// The upper limit `ceil` represents the maximum time to round up to.
+    pub fn round_up<P, C>(&self, precision: P, ceil: C) -> Result<Duration>
+        where P: Into<Option<u8>>,
+              C: Into<Option<SystemDuration>>
     {
         let p = precision.into().unwrap_or(21) as u32;
         if p < 32 {
             if let Some(sum) = self.0.checked_add((1 << p) - 1) {
-                Ok(Self(sum & !((1 << p) - 1)))
+                let rounded = Self(sum & !((1 << p) - 1));
+                match ceil.into() {
+                    Some(ceil) => {
+                        Ok(cmp::min(rounded, ceil.try_into()?))
+                    },
+                    None => Ok(rounded)
+                }
             } else {
                 Ok(Self(std::u32::MAX))
             }
@@ -480,21 +507,58 @@ mod tests {
 
     quickcheck! {
         fn timestamp_round_down(t: Timestamp) -> bool {
-            let u = t.round_down(None).unwrap();
+            let u = t.round_down(None, None).unwrap();
             assert!(u <= t);
-            assert_eq!(u32::from(u) & 0b1_1111_1111_1111_1111, 0);
-            assert!(u32::from(t) - u32::from(u) < 2097152);
+            assert_eq!(u32::from(u) & 0b1_1111_1111_1111_1111_1111, 0);
+            assert!(u32::from(t) - u32::from(u) < 2_u32.pow(21));
             true
         }
     }
 
+    #[test]
+    fn timestamp_round_down_floor() -> Result<()> {
+        let t = Timestamp(1585753307);
+        let floor = t.checked_sub(Duration::weeks(1).unwrap()).unwrap();
+
+        let u = t.round_down(21, floor).unwrap();
+        assert!(u < t);
+        assert!(floor < u);
+        assert_eq!(u32::from(u) & 0b1_1111_1111_1111_1111_1111, 0);
+
+        let floor = t.checked_sub(Duration::days(1).unwrap()).unwrap();
+
+        let u = t.round_down(21, floor).unwrap();
+        assert_eq!(u, floor);
+        Ok(())
+    }
+
     quickcheck! {
-        fn duration_round_up(t: Duration) -> bool {
-            let u = t.round_up(None).unwrap();
-            assert!(t <= u);
-            assert_eq!(u32::from(u) & 0b1_1111_1111_1111_1111, 0);
-            assert!(u32::from(u) - u32::from(t) < 2097152);
+        fn duration_round_up(d: Duration) -> bool {
+            let u = d.round_up(None, None).unwrap();
+            assert!(d <= u);
+            assert_eq!(u32::from(u) & 0b1_1111_1111_1111_1111_1111, 0);
+            assert!(u32::from(u) - u32::from(d) < 2_u32.pow(21));
             true
         }
     }
+
+    #[test]
+    fn duration_round_up_ceil() -> Result<()> {
+        let d = Duration(123);
+
+        let ceil = Duration(2_u32.pow(23));
+
+        let u = d.round_up(21, ceil)?;
+        assert!(d < u);
+        assert!(u < ceil);
+        assert_eq!(u32::from(u) & 0b1_1111_1111_1111_1111_1111, 0);
+
+        let ceil = Duration::days(1).unwrap();
+
+        let u = d.round_up(21, ceil)?;
+        assert!(d < u);
+        assert_eq!(u, ceil);
+
+        Ok(())
+    }
 }