From 8ef8eafbd072d01755d137b60003bee7bfe9cbe9 Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Mon, 10 Feb 2020 16:24:07 +0100 Subject: openpgp: Call 'expiration time' a 'validity period'. - The former is a misnomer inherited from the RFC: It is a duration, not a point in time. 'Validity period' makes that clear, and also emphasizes that the key or signature is valid during that period. - See #429. --- openpgp/src/cert/amalgamation.rs | 4 +-- openpgp/src/cert/builder.rs | 10 +++---- openpgp/src/cert/components.rs | 2 +- openpgp/src/cert/key_amalgamation.rs | 2 +- openpgp/src/cert/mod.rs | 50 +++++++++++++++---------------- openpgp/src/packet/signature/subpacket.rs | 32 ++++++++++---------- 6 files changed, 50 insertions(+), 50 deletions(-) (limited to 'openpgp') diff --git a/openpgp/src/cert/amalgamation.rs b/openpgp/src/cert/amalgamation.rs index a7bb5334..48177d28 100644 --- a/openpgp/src/cert/amalgamation.rs +++ b/openpgp/src/cert/amalgamation.rs @@ -344,8 +344,8 @@ pub trait Amalgamation<'a> { /// 5.2.3.3 of RFC 4880]. /// /// [Section 5.2.3.3 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.3 - fn key_expiration_time(&self) -> Option { - self.map(|s| s.key_expiration_time()) + fn key_validity_period(&self) -> Option { + self.map(|s| s.key_validity_period()) } /// Returns the value of the Revocation Key subpacket, which diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs index cce89fb3..cc33fd1e 100644 --- a/openpgp/src/cert/builder.rs +++ b/openpgp/src/cert/builder.rs @@ -260,7 +260,7 @@ impl CertBuilder { /// Sets the expiration time. /// /// A value of None means never. - pub fn set_expiration_time(mut self, expiration: T) -> Self + pub fn set_validity_period(mut self, expiration: T) -> Self where T: Into> { self.primary.expiration = expiration.into(); @@ -334,7 +334,7 @@ impl CertBuilder { .set_hash_algo(HashAlgorithm::SHA512) .set_features(&Features::sequoia())? .set_key_flags(flags)? - .set_key_expiration_time( + .set_key_validity_period( blueprint.expiration.or(self.primary.expiration))?; if flags.for_transport_encryption() || flags.for_storage_encryption() @@ -400,7 +400,7 @@ impl CertBuilder { .set_features(&Features::sequoia())? .set_key_flags(&self.primary.flags)? .set_signature_creation_time(creation_time)? - .set_key_expiration_time(self.primary.expiration)? + .set_key_validity_period(self.primary.expiration)? .set_issuer_fingerprint(key.fingerprint())? .set_issuer(key.keyid())? .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512])?; @@ -578,12 +578,12 @@ mod tests { } #[test] - fn expiration_times() { + fn validity_periods() { let p = &P::new(); let s = std::time::Duration::new(1, 0); let (cert,_) = CertBuilder::new() - .set_expiration_time(600 * s) + .set_validity_period(600 * s) .add_subkey(KeyFlags::default().set_signing(true), 300 * s) .add_subkey(KeyFlags::default().set_authentication(true), diff --git a/openpgp/src/cert/components.rs b/openpgp/src/cert/components.rs index 585b47fd..d52bfc37 100644 --- a/openpgp/src/cert/components.rs +++ b/openpgp/src/cert/components.rs @@ -293,7 +293,7 @@ impl ComponentBundle { // because a hard revocation is always valid. t!(" revocation not alive ({:?} - {:?}): {}", rev.signature_creation_time().unwrap_or_else(time_zero), - rev.signature_expiration_time() + rev.signature_validity_period() .unwrap_or_else(|| time::Duration::new(0, 0)), err); None diff --git a/openpgp/src/cert/key_amalgamation.rs b/openpgp/src/cert/key_amalgamation.rs index 76486456..95a2dd25 100644 --- a/openpgp/src/cert/key_amalgamation.rs +++ b/openpgp/src/cert/key_amalgamation.rs @@ -439,7 +439,7 @@ impl<'a, P: 'a + key::KeyParts> ValidKeyAmalgamation<'a, P> { { let sig = { let binding = self.binding_signature(); - if binding.key_expiration_time().is_some() { + if binding.key_validity_period().is_some() { Some(binding) } else { self.direct_key_signature() diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs index 7761db1a..9a4ba926 100644 --- a/openpgp/src/cert/mod.rs +++ b/openpgp/src/cert/mod.rs @@ -608,7 +608,7 @@ impl Cert { /// /// This function exists to facilitate testing, which is why it is /// not exported. - fn set_expiration_time_as_of(self, policy: &dyn Policy, + fn set_validity_period_as_of(self, policy: &dyn Policy, primary_signer: &mut dyn Signer, expiration: Option, now: time::SystemTime) @@ -651,7 +651,7 @@ impl Cert { // Generate the signature. sigs.push(signature::Builder::from(template.clone()) - .set_key_expiration_time(expiration)? + .set_key_validity_period(expiration)? .set_signature_creation_time(now)? .sign_hash(primary_signer, hash)?.into()); } @@ -666,12 +666,12 @@ impl Cert { /// /// A policy is needed, because the expiration is updated by adding /// a self-signature to the primary user id. - pub fn set_expiration_time(self, policy: &dyn Policy, + pub fn set_validity_period(self, policy: &dyn Policy, primary_signer: &mut dyn Signer, expiration: Option) -> Result { - self.set_expiration_time_as_of(policy, primary_signer, expiration, + self.set_validity_period_as_of(policy, primary_signer, expiration, time::SystemTime::now()) } @@ -1934,7 +1934,7 @@ mod test { } #[test] - fn set_expiration_time() { + fn set_validity_period() { let p = &P::new(); let (cert, _) = CertBuilder::general_purpose(None, Some("Test")) @@ -1947,7 +1947,7 @@ mod test { + 1 // subkey + 1 // binding signature ); - let cert = check_set_expiration_time(p, cert); + let cert = check_set_validity_period(p, cert); assert_eq!(cert.clone().into_packet_pile().children().count(), 1 // primary key + 1 // direct key signature @@ -1960,31 +1960,31 @@ mod test { ); } #[test] - fn set_expiration_time_uidless() { + fn set_validity_period_uidless() { let p = &P::new(); let (cert, _) = CertBuilder::new() - .set_expiration_time(None) // Just to assert this works. - .set_expiration_time( + .set_validity_period(None) // Just to assert this works. + .set_validity_period( Some(crate::types::Duration::weeks(52).unwrap().into())) .generate().unwrap(); assert_eq!(cert.clone().into_packet_pile().children().count(), 1 // primary key + 1 // direct key signature ); - let cert = check_set_expiration_time(p, cert); + let cert = check_set_validity_period(p, cert); assert_eq!(cert.clone().into_packet_pile().children().count(), 1 // primary key + 1 // direct key signature + 2 // two new direct key signatures ); } - fn check_set_expiration_time(policy: &dyn Policy, cert: Cert) -> Cert { + fn check_set_validity_period(policy: &dyn Policy, cert: Cert) -> Cert { let now = cert.primary_key().creation_time(); let a_sec = time::Duration::new(1, 0); let expiry_orig = cert.primary_key().with_policy(policy, now).unwrap() - .key_expiration_time() + .key_validity_period() .expect("Keys expire by default."); let mut keypair = cert.primary_key().key().clone().mark_parts_secret() @@ -1992,19 +1992,19 @@ mod test { // Clear the expiration. let as_of1 = now + time::Duration::new(10, 0); - let cert = cert.set_expiration_time_as_of( + let cert = cert.set_validity_period_as_of( policy, &mut keypair, None, as_of1).unwrap(); { // If t < as_of1, we should get the original expiry. assert_eq!(cert.primary_key().with_policy(policy, now).unwrap() - .key_expiration_time(), + .key_validity_period(), Some(expiry_orig)); assert_eq!(cert.primary_key().with_policy(policy, as_of1 - a_sec).unwrap() - .key_expiration_time(), + .key_validity_period(), Some(expiry_orig)); // If t >= as_of1, we should get the new expiry. assert_eq!(cert.primary_key().with_policy(policy, as_of1).unwrap() - .key_expiration_time(), + .key_validity_period(), None); } @@ -2015,27 +2015,27 @@ mod test { assert!(expiry_new > time::Duration::new(0, 0)); let as_of2 = as_of1 + time::Duration::new(10, 0); - let cert = cert.set_expiration_time_as_of( + let cert = cert.set_validity_period_as_of( policy, &mut keypair, Some(expiry_new), as_of2).unwrap(); { // If t < as_of1, we should get the original expiry. assert_eq!(cert.primary_key().with_policy(policy, now).unwrap() - .key_expiration_time(), + .key_validity_period(), Some(expiry_orig)); assert_eq!(cert.primary_key().with_policy(policy, as_of1 - a_sec).unwrap() - .key_expiration_time(), + .key_validity_period(), Some(expiry_orig)); // If as_of1 <= t < as_of2, we should get the second // expiry (None). assert_eq!(cert.primary_key().with_policy(policy, as_of1).unwrap() - .key_expiration_time(), + .key_validity_period(), None); assert_eq!(cert.primary_key().with_policy(policy, as_of2 - a_sec).unwrap() - .key_expiration_time(), + .key_validity_period(), None); // If t <= as_of2, we should get the new expiry. assert_eq!(cert.primary_key().with_policy(policy, as_of2).unwrap() - .key_expiration_time(), + .key_validity_period(), Some(expiry_new)); } cert @@ -2303,7 +2303,7 @@ mod test { .set_features(&Features::sequoia()).unwrap() .set_key_flags(&KeyFlags::default()).unwrap() .set_signature_creation_time(t1).unwrap() - .set_key_expiration_time(Some(time::Duration::new(10 * 52 * 7 * 24 * 60 * 60, 0))).unwrap() + .set_key_validity_period(Some(time::Duration::new(10 * 52 * 7 * 24 * 60 * 60, 0))).unwrap() .set_issuer_fingerprint(key.fingerprint()).unwrap() .set_issuer(key.keyid()).unwrap() .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap() @@ -2321,7 +2321,7 @@ mod test { .set_features(&Features::sequoia()).unwrap() .set_key_flags(&KeyFlags::default()).unwrap() .set_signature_creation_time(t3).unwrap() - .set_key_expiration_time(Some(time::Duration::new(10 * 52 * 7 * 24 * 60 * 60, 0))).unwrap() + .set_key_validity_period(Some(time::Duration::new(10 * 52 * 7 * 24 * 60 * 60, 0))).unwrap() .set_issuer_fingerprint(key.fingerprint()).unwrap() .set_issuer(key.keyid()).unwrap() .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap() @@ -2966,7 +2966,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= .set_key_flags(&KeyFlags::default()).unwrap() .set_signature_creation_time(t1).unwrap() // Vary this... - .set_key_expiration_time(Some( + .set_key_validity_period(Some( time::Duration::new((1 + i as u64) * 24 * 60 * 60, 0))) .unwrap() .set_issuer_fingerprint(key.fingerprint()).unwrap() diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs index f671792c..902e9cbd 100644 --- a/openpgp/src/packet/signature/subpacket.rs +++ b/openpgp/src/packet/signature/subpacket.rs @@ -915,7 +915,7 @@ impl SubpacketArea { /// /// Note: if the signature contains multiple instances of this /// subpacket, only the last one is considered. - pub fn signature_expiration_time(&self) -> Option { + pub fn signature_validity_period(&self) -> Option { // 4-octet time field if let Some(sb) = self.subpacket(SubpacketTag::SignatureExpirationTime) { @@ -1045,7 +1045,7 @@ impl SubpacketArea { /// /// Note: if the signature contains multiple instances of this /// subpacket, only the last one is considered. - pub fn key_expiration_time(&self) -> Option { + pub fn key_validity_period(&self) -> Option { // 4-octet time field if let Some(sb) = self.subpacket(SubpacketTag::KeyExpirationTime) { @@ -1663,7 +1663,7 @@ impl SubpacketAreas { (time, tolerance) }; - match (self.signature_creation_time(), self.signature_expiration_time()) + match (self.signature_creation_time(), self.signature_validity_period()) { (None, _) => Err(Error::MalformedPacket("no signature creation time".into()) @@ -1697,7 +1697,7 @@ impl SubpacketAreas { let t = t.into() .unwrap_or_else(|| time::SystemTime::now()); - match self.key_expiration_time() { + match self.key_validity_period() { Some(e) if e.as_secs() > 0 && key.creation_time() + e <= t => Err(Error::Expired(key.creation_time() + e).into()), _ if key.creation_time() > t => @@ -1845,7 +1845,7 @@ impl signature::Builder { /// Sets the value of the Signature Expiration Time subpacket. /// /// If `None` is given, any expiration subpacket is removed. - pub fn set_signature_expiration_time(mut self, + pub fn set_signature_validity_period(mut self, expiration: Option) -> Result { if let Some(e) = expiration { @@ -1915,7 +1915,7 @@ impl signature::Builder { /// seconds after the key's creation. /// /// If `None` is given, any expiration subpacket is removed. - pub fn set_key_expiration_time(mut self, + pub fn set_key_validity_period(mut self, expiration: Option) -> Result { if let Some(e) = expiration { @@ -2227,20 +2227,20 @@ fn accessors() { let minute = time::Duration::new(60, 0); let five_minutes = 5 * minute; let ten_minutes = 10 * minute; - sig = sig.set_signature_expiration_time(Some(five_minutes)).unwrap(); + sig = sig.set_signature_validity_period(Some(five_minutes)).unwrap(); let sig_ = sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap(); - assert_eq!(sig_.signature_expiration_time(), Some(five_minutes)); + assert_eq!(sig_.signature_validity_period(), Some(five_minutes)); assert!(sig_.signature_alive(None, zero_s).is_ok()); assert!(sig_.signature_alive(now, zero_s).is_ok()); assert!(!sig_.signature_alive(now - five_minutes, zero_s).is_ok()); assert!(!sig_.signature_alive(now + ten_minutes, zero_s).is_ok()); - sig = sig.set_signature_expiration_time(None).unwrap(); + sig = sig.set_signature_validity_period(None).unwrap(); let sig_ = sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap(); - assert_eq!(sig_.signature_expiration_time(), None); + assert_eq!(sig_.signature_validity_period(), None); assert!(sig_.signature_alive(None, zero_s).is_ok()); assert!(sig_.signature_alive(now, zero_s).is_ok()); @@ -2276,20 +2276,20 @@ fn accessors() { assert_eq!(sig_.revocable(), Some(false)); key.set_creation_time(now).unwrap(); - sig = sig.set_key_expiration_time(Some(five_minutes)).unwrap(); + sig = sig.set_key_validity_period(Some(five_minutes)).unwrap(); let sig_ = sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap(); - assert_eq!(sig_.key_expiration_time(), Some(five_minutes)); + assert_eq!(sig_.key_validity_period(), Some(five_minutes)); assert!(sig_.key_alive(&key, None).is_ok()); assert!(sig_.key_alive(&key, now).is_ok()); assert!(!sig_.key_alive(&key, now - five_minutes).is_ok()); assert!(!sig_.key_alive(&key, now + ten_minutes).is_ok()); - sig = sig.set_key_expiration_time(None).unwrap(); + sig = sig.set_key_validity_period(None).unwrap(); let sig_ = sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap(); - assert_eq!(sig_.key_expiration_time(), None); + assert_eq!(sig_.key_validity_period(), None); assert!(sig_.key_alive(&key, None).is_ok()); assert!(sig_.key_alive(&key, now).is_ok()); @@ -2567,7 +2567,7 @@ fn subpacket_test_2() { // The signature does not expire. assert!(sig.signature_alive(None, None).is_ok()); - assert_eq!(sig.key_expiration_time(), + assert_eq!(sig.key_validity_period(), Some(Duration::from(63072000).into())); assert_eq!(sig.subpacket(SubpacketTag::KeyExpirationTime), Some(&Subpacket { @@ -2980,7 +2980,7 @@ fn subpacket_test_2() { // } // } - assert_eq!(sig.key_expiration_time(), + assert_eq!(sig.key_validity_period(), Some(Duration::from(63072000).into())); assert_eq!(sig.subpacket(SubpacketTag::KeyExpirationTime), Some(&Subpacket { -- cgit v1.2.3