diff options
author | Neal H. Walfield <neal@pep.foundation> | 2020-08-03 17:22:50 +0200 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2020-08-03 17:35:41 +0200 |
commit | cb6b672a4dbb703cb92661bfa82b7089919a107b (patch) | |
tree | 62aeb4ebca67f058822a484a7fe3668d7dee8792 | |
parent | ad9dac7d5a68f9a6dd2bcae67a405eb9f2756b58 (diff) |
openpgp: Change CertBuilder to use a relative expiration time.
- `CertBuilder::set_expiration_time` takes an absolute time.
- Most callers use a relative time.
- Internally, we need a relative time (that's what the Key
Expiration Time packet takes).
- Converting the absolute time to a relative time is error prone:
should it be relative to the creation time when called or when
`CertBuilder` is finalized?
- KISS: Change it to just take a relative time.
- To better reflect the new semantics, also change the name to
`CertBuilder::set_validity_period`.
-rw-r--r-- | openpgp/src/cert/amalgamation/key.rs | 5 | ||||
-rw-r--r-- | openpgp/src/cert/builder.rs | 46 | ||||
-rw-r--r-- | openpgp/src/cert/mod.rs | 14 | ||||
-rw-r--r-- | openpgp/src/types/timestamp.rs | 4 | ||||
-rw-r--r-- | tool/src/commands/key.rs | 22 |
5 files changed, 46 insertions, 45 deletions
diff --git a/openpgp/src/cert/amalgamation/key.rs b/openpgp/src/cert/amalgamation/key.rs index 4c2fc1ca..78f34f5d 100644 --- a/openpgp/src/cert/amalgamation/key.rs +++ b/openpgp/src/cert/amalgamation/key.rs @@ -1793,12 +1793,11 @@ impl<'a, P, R, R2> ValidKeyAmalgamation<'a, P, R, R2> /// let now: time::SystemTime = now.try_into()?; /// /// let a_week = time::Duration::from_secs(7 * 24 * 60 * 60); - /// let a_week_later = now + a_week; /// /// let (cert, _) = /// CertBuilder::general_purpose(None, Some("alice@example.org")) /// .set_creation_time(now) - /// .set_expiration_time(a_week_later) + /// .set_validity_period(a_week) /// .generate()?; /// /// assert_eq!(cert.primary_key().with_policy(p, None)?.key_validity_period(), @@ -1849,7 +1848,7 @@ impl<'a, P, R, R2> ValidKeyAmalgamation<'a, P, R, R2> /// let (cert, _) = /// CertBuilder::general_purpose(None, Some("alice@example.org")) /// .set_creation_time(now) - /// .set_expiration_time(a_week_later) + /// .set_validity_period(a_week) /// .generate()?; /// /// assert_eq!(cert.primary_key().with_policy(p, None)?.key_expiration_time(), diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs index ebe42d69..0750143a 100644 --- a/openpgp/src/cert/builder.rs +++ b/openpgp/src/cert/builder.rs @@ -125,7 +125,7 @@ impl CipherSuite { #[derive(Clone, Debug)] pub struct KeyBlueprint { flags: KeyFlags, - expiration: Option<time::SystemTime>, + validity: Option<time::Duration>, // If not None, uses the specified ciphersuite. Otherwise, uses // CertBuilder::ciphersuite. ciphersuite: Option<CipherSuite>, @@ -212,7 +212,7 @@ impl CertBuilder { ciphersuite: CipherSuite::default(), primary: KeyBlueprint{ flags: KeyFlags::default().set_certification(), - expiration: None, + validity: None, ciphersuite: None, }, subkeys: vec![], @@ -257,9 +257,7 @@ impl CertBuilder { flags: KeyFlags::default() .set_certification() .set_signing(), - expiration: Some( - time::SystemTime::now() - + time::Duration::new(3 * 52 * 7 * 24 * 60 * 60, 0)), + validity: Some(time::Duration::new(3 * 52 * 7 * 24 * 60 * 60, 0)), ciphersuite: None, }, subkeys: vec![ @@ -267,7 +265,7 @@ impl CertBuilder { flags: KeyFlags::default() .set_transport_encryption() .set_storage_encryption(), - expiration: None, + validity: None, ciphersuite: None, } ], @@ -741,11 +739,11 @@ impl CertBuilder { /// // expire in a year. /// let (cert,_) = CertBuilder::new() /// .set_creation_time(now) - /// .set_expiration_time(now + 2 * y) + /// .set_validity_period(2 * y) /// .add_subkey(KeyFlags::empty() /// .set_storage_encryption() /// .set_transport_encryption(), - /// now + y, + /// y, /// None) /// .generate()?; /// @@ -760,14 +758,14 @@ impl CertBuilder { /// .set_transport_encryption())); /// # Ok(()) } /// ``` - pub fn add_subkey<T, C>(mut self, flags: KeyFlags, expiration: T, cs: C) + pub fn add_subkey<T, C>(mut self, flags: KeyFlags, validity: T, cs: C) -> Self - where T: Into<Option<time::SystemTime>>, + where T: Into<Option<time::Duration>>, C: Into<Option<CipherSuite>>, { self.subkeys.push(KeyBlueprint { flags, - expiration: expiration.into(), + validity: validity.into(), ciphersuite: cs.into(), }); self @@ -838,9 +836,13 @@ impl CertBuilder { self } - /// Sets the certificate's expiration time. + /// Sets the certificate's validity period. /// - /// A value of None means never. + /// The determines how long the certificate is valid. That is, + /// after the validity period, the certificate is considered to be + /// expired. + /// + /// A value of None means that the certificate never expires. // /// # Examples /// @@ -860,7 +862,7 @@ impl CertBuilder { /// // Make the certificate expire in 10 minutes. /// let (cert,_) = CertBuilder::new() /// .set_creation_time(now) - /// .set_expiration_time(now + 600 * s) + /// .set_validity_period(600 * s) /// .generate()?; /// /// assert!(cert.with_policy(p, now)?.primary_key().alive().is_ok()); @@ -868,10 +870,10 @@ impl CertBuilder { /// assert!(cert.with_policy(p, now + 600 * s)?.primary_key().alive().is_err()); /// # Ok(()) } /// ``` - pub fn set_expiration_time<T>(mut self, expiration: T) -> Self - where T: Into<Option<time::SystemTime>> + pub fn set_validity_period<T>(mut self, validity: T) -> Self + where T: Into<Option<time::Duration>> { - self.primary.expiration = expiration.into(); + self.primary.validity = validity.into(); self } @@ -1018,9 +1020,7 @@ impl CertBuilder { .set_hash_algo(HashAlgorithm::SHA512) .set_features(&Features::sequoia())? .set_key_flags(flags)? - .set_key_expiration_time( - &subkey, - blueprint.expiration.or(self.primary.expiration))?; + .set_key_validity_period(blueprint.validity.or(self.primary.validity))?; if flags.for_certification() || flags.for_signing() { // We need to create a primary key binding signature. @@ -1070,7 +1070,7 @@ impl CertBuilder { .set_features(&Features::sequoia())? .set_key_flags(&self.primary.flags)? .set_signature_creation_time(creation_time)? - .set_key_expiration_time(&key, self.primary.expiration)? + .set_key_validity_period(self.primary.validity)? .set_preferred_hash_algorithms(vec![ HashAlgorithm::SHA512 ])? @@ -1264,9 +1264,9 @@ mod tests { let (cert,_) = CertBuilder::new() .set_creation_time(now) - .set_expiration_time(now + 600 * s) + .set_validity_period(600 * s) .add_subkey(KeyFlags::default().set_signing(), - now + 300 * s, None) + 300 * s, None) .add_subkey(KeyFlags::default().set_authentication(), None, None) .generate().unwrap(); diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs index efdc0268..f932292a 100644 --- a/openpgp/src/cert/mod.rs +++ b/openpgp/src/cert/mod.rs @@ -2749,13 +2749,15 @@ impl<'a> ValidCert<'a> { /// /// let creation_time = time::SystemTime::now(); /// let before_creation = creation_time - a_second; - /// let expiration_time = creation_time + 60 * a_second; + /// let validity_period = 60 * a_second; + /// let expiration_time = creation_time + validity_period; /// let before_expiration_time = expiration_time - a_second; /// let after_expiration_time = expiration_time + a_second; /// /// let (cert, _) = CertBuilder::new() /// .add_userid("Alice") - /// .set_expiration_time(expiration_time) + /// .set_creation_time(creation_time) + /// .set_validity_period(validity_period) /// .generate()?; /// /// // There is no binding signature before the certificate was created. @@ -3732,14 +3734,12 @@ mod test { #[test] fn set_validity_period_uidless() { - use crate::types::{Duration, Timestamp}; + use crate::types::Duration; let p = &P::new(); let (cert, _) = CertBuilder::new() - .set_expiration_time(None) // Just to assert this works. - .set_expiration_time( - Some(Timestamp::now().checked_add( - Duration::weeks(52).unwrap()).unwrap().into())) + .set_validity_period(None) // Just to assert this works. + .set_validity_period(Some(Duration::weeks(52).unwrap().try_into().unwrap())) .generate().unwrap(); assert_eq!(cert.clone().into_packet_pile().children().count(), 1 // primary key diff --git a/openpgp/src/types/timestamp.rs b/openpgp/src/types/timestamp.rs index 3a9a0e4c..59b903d1 100644 --- a/openpgp/src/types/timestamp.rs +++ b/openpgp/src/types/timestamp.rs @@ -267,11 +267,11 @@ impl Arbitrary for Timestamp { /// let p = &StandardPolicy::new(); /// /// let now = Timestamp::now(); -/// let then = now.checked_add(Duration::days(365)?).unwrap(); +/// let validity_period = Duration::days(365)?; /// /// let (cert,_) = CertBuilder::new() /// .set_creation_time(now) -/// .set_expiration_time(then) +/// .set_validity_period(validity_period) /// .generate()?; /// /// let vc = cert.with_policy(p, now)?; diff --git a/tool/src/commands/key.rs b/tool/src/commands/key.rs index abd95433..dc78866f 100644 --- a/tool/src/commands/key.rs +++ b/tool/src/commands/key.rs @@ -33,22 +33,24 @@ pub fn generate(m: &ArgMatches, force: bool) -> Result<()> { // Expiration. match (m.value_of("expires"), m.value_of("expires-in")) { (None, None) => // Default expiration. - builder = builder.set_expiration_time( - Some(SystemTime::now() - + Duration::new(3 * SECONDS_IN_YEAR, 0))), + builder = builder.set_validity_period( + Some(Duration::new(3 * SECONDS_IN_YEAR, 0))), (Some(t), None) if t == "never" => - builder = builder.set_expiration_time(None), + builder = builder.set_validity_period(None), (Some(t), None) => { - let t = - crate::parse_iso8601(t, chrono::NaiveTime::from_hms(0, 0, 0))?; - builder = builder.set_expiration_time(Some(t.into())); + let now = builder.creation_time() + .unwrap_or_else(std::time::SystemTime::now); + let expiration = SystemTime::from( + crate::parse_iso8601(t, chrono::NaiveTime::from_hms(0, 0, 0))?); + let validity = expiration.duration_since(now)?; + builder = builder.set_creation_time(now) + .set_validity_period(validity); }, (None, Some(d)) if d == "never" => - builder = builder.set_expiration_time(None), + builder = builder.set_validity_period(None), (None, Some(d)) => { let d = parse_duration(d)?; - builder = builder.set_expiration_time( - Some(SystemTime::now() + d)); + builder = builder.set_validity_period(Some(d)); }, (Some(_), Some(_)) => unreachable!("conflicting args"), } |