From fe6575ef98a65c19a08b62406d855625a4b1463d Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 24 Jan 2023 15:04:40 -0500 Subject: [PATCH] hscrypto: Revise TimePeriod to account for variable offset. Previously, the offset was set to 12 hours unconditionally (like the spec says). But based on a conversation on tor-dev, it seems that the offset should actually be 12 times the voting interval. I'm also opening an MR to change the spec. --- crates/tor-hscrypto/src/pk.rs | 5 +++- crates/tor-hscrypto/src/time.rs | 51 +++++++++++++++++++-------------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/crates/tor-hscrypto/src/pk.rs b/crates/tor-hscrypto/src/pk.rs index 39b893644..1dae12c71 100644 --- a/crates/tor-hscrypto/src/pk.rs +++ b/crates/tor-hscrypto/src/pk.rs @@ -298,7 +298,8 @@ mod test { #[test] fn key_blinding_blackbox() { let mut rng = testing_rng().rng_compat(); - let when = TimePeriod::new(Duration::from_secs(3600), SystemTime::now()).unwrap(); + let offset = Duration::new(12 * 60 * 60, 0); + let when = TimePeriod::new(Duration::from_secs(3600), SystemTime::now(), offset).unwrap(); let keypair = ed25519::Keypair::generate(&mut rng); let id_pub = OnionIdKey::from(keypair.public); let id_sec = OnionIdSecretKey::from(ed25519::ExpandedSecretKey::from(&keypair.secret)); @@ -332,9 +333,11 @@ mod test { )) .unwrap(), ); + let offset = Duration::new(12 * 60 * 60, 0); let time_period = TimePeriod::new( Duration::from_secs(1440), humantime::parse_rfc3339("1970-01-22T01:50:33Z").unwrap(), + offset, ) .unwrap(); assert_eq!(time_period.interval_num, 1234); diff --git a/crates/tor-hscrypto/src/time.rs b/crates/tor-hscrypto/src/time.rs index ff157425d..1726bfc02 100644 --- a/crates/tor-hscrypto/src/time.rs +++ b/crates/tor-hscrypto/src/time.rs @@ -5,35 +5,37 @@ use std::time::{Duration, SystemTime}; /// A period of time, as used in the onion service system. /// /// A `TimePeriod` is defined as a duration (in seconds), and the number of such -/// durations that have elapsed since 12:00 UTC on the Unix epoch. So for -/// example, the interval "86400 seconds, 15", covers `1970-01-16T12:00:00` up -/// to but not including `1970-01-17T12:00:00`. +/// durations that have elapsed since a given offset from the Unix epoch. So +/// for example, the interval "(86400 seconds length, 15 intervals, 12 hours +/// offset)", covers `1970-01-16T12:00:00` up to but not including +/// `1970-01-17T12:00:00`. /// /// These time periods are used to derive a different `BlindedOnionIdKey` during /// each period from each `OnionIdKey`. /// /// # Compatibility Note /// -/// `rend-spec-v3.txt` says that the "12:00UTC" value is a constant, but C Tor -/// treats it as depending on the length of a voting interval. This should give -/// the same results, except when running on certain chutney networks with -/// altered voting intervals. (TODO hs) +/// Although `rend-spec-v3.txt` says that the offset is a constant "12 hours", C +/// Tor doesn't behave that way. Instead, the offset is set to twelve voting +/// intervals. Since this module doesn't (and shouldn't!) have access to the +/// voting interval, we store the offset as part of the TimePeriod. +/// +/// TODO hs: remove or revise this note once the spec is updated; see prop342. #[derive(Copy, Clone, Debug, Eq, PartialEq)] pub struct TimePeriod { /// Index of the time periods that have passed since the unix epoch. pub(crate) interval_num: u64, /// The length of a time period, in seconds. pub(crate) length_in_sec: u32, + /// Our offset from the epoch, in seconds. + pub(crate) offset_in_sec: u32, } -/// The difference between the Unix epoch and our starting time. -const EPOCH_OFFSET: Duration = Duration::from_secs(12 * 60 * 60); - /// Two [`TimePeriod`]s are ordered with respect to one another if they have the -/// same interval length. +/// same interval length and offset. impl PartialOrd for TimePeriod { fn partial_cmp(&self, other: &Self) -> Option { - if self.length_in_sec == other.length_in_sec { + if self.length_in_sec == other.length_in_sec && self.offset_in_sec == other.offset_in_sec { Some(self.interval_num.cmp(&other.interval_num)) } else { None @@ -46,19 +48,24 @@ impl TimePeriod { /// /// The `length` value is rounded down to the nearest second. /// + /// The `epoch_offset` value is the amount of time after the Unix epoch when + /// our epoch begins. It is also rounded down to the nearest second. + /// /// Return None if the Duration is too large or too small, or if `when` /// cannot be represented as a time period. - pub fn new(length: Duration, when: SystemTime) -> Option { + pub fn new(length: Duration, when: SystemTime, epoch_offset: Duration) -> Option { // The algorithm here is specified in rend-spec-v3 section 2.2.1 let length_in_sec = u32::try_from(length.as_secs()).ok()?; + let offset_in_sec = u32::try_from(epoch_offset.as_secs()).ok()?; let interval_num = when - .duration_since(SystemTime::UNIX_EPOCH + EPOCH_OFFSET) + .duration_since(SystemTime::UNIX_EPOCH + epoch_offset) .ok()? .as_secs() / u64::from(length_in_sec); Some(TimePeriod { interval_num, length_in_sec, + offset_in_sec, }) } /// Return the time period after this one. @@ -67,7 +74,7 @@ impl TimePeriod { pub fn next(&self) -> Option { Some(TimePeriod { interval_num: self.interval_num.checked_add(1)?, - length_in_sec: self.length_in_sec, + ..*self }) } /// Return the time period after this one. @@ -76,7 +83,7 @@ impl TimePeriod { pub fn prev(&self) -> Option { Some(TimePeriod { interval_num: self.interval_num.checked_sub(1)?, - length_in_sec: self.length_in_sec, + ..*self }) } /// Return true if this time period contains `when`. @@ -99,10 +106,11 @@ impl TimePeriod { pub fn range(&self) -> Option> { let start_sec = u64::from(self.length_in_sec).checked_mul(self.interval_num)?; let end_sec = start_sec.checked_add(self.length_in_sec.into())?; + let epoch_offset = Duration::new(self.offset_in_sec.into(), 0); let start = - (SystemTime::UNIX_EPOCH + EPOCH_OFFSET).checked_add(Duration::from_secs(start_sec))?; + (SystemTime::UNIX_EPOCH + epoch_offset).checked_add(Duration::from_secs(start_sec))?; let end = - (SystemTime::UNIX_EPOCH + EPOCH_OFFSET).checked_add(Duration::from_secs(end_sec))?; + (SystemTime::UNIX_EPOCH + epoch_offset).checked_add(Duration::from_secs(end_sec))?; Some(start..end) } } @@ -125,14 +133,15 @@ mod test { #[test] fn check_testvec() { // Test case from C tor, taken from rend-spec. + let offset = Duration::new(12 * 60 * 60, 0); let time = parse_rfc3339("2016-04-13T11:00:00Z").unwrap(); let one_day = parse_duration("1day").unwrap(); - let period = TimePeriod::new(one_day, time).unwrap(); + let period = TimePeriod::new(one_day, time, offset).unwrap(); assert_eq!(period.interval_num, 16903); assert!(period.contains(time)); let time = parse_rfc3339("2016-04-13T11:59:59Z").unwrap(); - let period = TimePeriod::new(one_day, time).unwrap(); + let period = TimePeriod::new(one_day, time, offset).unwrap(); assert_eq!(period.interval_num, 16903); // still the same. assert!(period.contains(time)); @@ -140,7 +149,7 @@ mod test { assert_eq!(period.next().unwrap().interval_num, 16904); let time2 = parse_rfc3339("2016-04-13T12:00:00Z").unwrap(); - let period2 = TimePeriod::new(one_day, time2).unwrap(); + let period2 = TimePeriod::new(one_day, time2, offset).unwrap(); assert_eq!(period2.interval_num, 16904); assert!(period < period2); assert!(period2 > period);