From 623425e75c6f6fe1b50076d4311a4d0041dc65e0 Mon Sep 17 00:00:00 2001
From: Tony Arcieri <bascule@gmail.com>
Date: Fri, 29 Mar 2024 13:17:52 -0600
Subject: [PATCH] ssh-key: use `u64` type for certificate timestamps

Closes #174

Previously certificates only supported the `i64` range to allow for
infallible conversions to/from `SystemTime`.

Unfortunately OpenSSH defaults to using `u64::MAX` as the `valid_before`
time in order to represent certificate that's valid "forever". The
previous restriction meant that `ssh-key` was incapible of parsing such
certificates.

This commit switches to using a raw `u64` everywhere, and changing
conversions to `SystemTime` to return an `Option<SystemTime>` which is
`None` if the `u64` value overflows an `i64`.
---
 ssh-key/src/authorized_keys.rs     |  2 +-
 ssh-key/src/certificate.rs         | 38 +++++++++++++++++-------------
 ssh-key/src/certificate/builder.rs | 10 ++------
 ssh-key/src/known_hosts.rs         |  2 +-
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/ssh-key/src/authorized_keys.rs b/ssh-key/src/authorized_keys.rs
index a485c7e..cdf0ed8 100644
--- a/ssh-key/src/authorized_keys.rs
+++ b/ssh-key/src/authorized_keys.rs
@@ -36,7 +36,7 @@ const COMMENT_DELIMITER: char = '#';
 ///   the key).
 pub struct AuthorizedKeys<'a> {
     /// Lines of the file being iterated over
-    lines: core::str::Lines<'a>,
+    lines: str::Lines<'a>,
 }
 
 impl<'a> AuthorizedKeys<'a> {
diff --git a/ssh-key/src/certificate.rs b/ssh-key/src/certificate.rs
index c241a88..6003842 100644
--- a/ssh-key/src/certificate.rs
+++ b/ssh-key/src/certificate.rs
@@ -137,10 +137,10 @@ pub struct Certificate {
     valid_principals: Vec<String>,
 
     /// Valid after.
-    valid_after: UnixTime,
+    valid_after: u64,
 
     /// Valid before.
-    valid_before: UnixTime,
+    valid_before: u64,
 
     /// Critical options.
     critical_options: OptionsMap,
@@ -281,26 +281,34 @@ impl Certificate {
         &self.valid_principals
     }
 
-    /// Valid after (Unix time).
+    /// Valid after (Unix time), i.e. certificate issuance time.
     pub fn valid_after(&self) -> u64 {
-        self.valid_after.into()
+        self.valid_after
     }
 
-    /// Valid before (Unix time).
+    /// Valid before (Unix time), i.e. certificate expiration time.
     pub fn valid_before(&self) -> u64 {
-        self.valid_before.into()
+        self.valid_before
     }
 
-    /// Valid after (system time).
+    /// Valid after (system time), i.e. certificate issuance time.
+    ///
+    /// # Returns
+    /// - `Some` if the `u64` value is a valid `SystemTime`
+    /// - `None` if it is not (i.e. overflows `i64`)
     #[cfg(feature = "std")]
-    pub fn valid_after_time(&self) -> SystemTime {
-        self.valid_after.into()
+    pub fn valid_after_time(&self) -> Option<SystemTime> {
+        UnixTime::try_from(self.valid_after).ok().map(Into::into)
     }
 
-    /// Valid before (system time).
+    /// Valid before (system time), i.e. certificate expiration time.
+    ///
+    /// # Returns
+    /// - `Some` if the `u64` value is a valid `SystemTime`
+    /// - `None` if it is not (i.e. overflows `i64`, effectively never-expiring)
     #[cfg(feature = "std")]
-    pub fn valid_before_time(&self) -> SystemTime {
-        self.valid_before.into()
+    pub fn valid_before_time(&self) -> Option<SystemTime> {
+        UnixTime::try_from(self.valid_before).ok().map(Into::into)
     }
 
     /// The critical options section of the certificate specifies zero or more
@@ -391,8 +399,6 @@ impl Certificate {
             return Err(Error::CertificateValidation);
         }
 
-        let unix_timestamp = UnixTime::new(unix_timestamp)?;
-
         // From PROTOCOL.certkeys:
         //
         //  "valid after" and "valid before" specify a validity period for the
@@ -462,8 +468,8 @@ impl Decode for Certificate {
             cert_type: CertType::decode(reader)?,
             key_id: String::decode(reader)?,
             valid_principals: Vec::decode(reader)?,
-            valid_after: UnixTime::decode(reader)?,
-            valid_before: UnixTime::decode(reader)?,
+            valid_after: u64::decode(reader)?,
+            valid_before: u64::decode(reader)?,
             critical_options: OptionsMap::decode(reader)?,
             extensions: OptionsMap::decode(reader)?,
             reserved: Vec::decode(reader)?,
diff --git a/ssh-key/src/certificate/builder.rs b/ssh-key/src/certificate/builder.rs
index 3d00b9c..5b5d87b 100644
--- a/ssh-key/src/certificate/builder.rs
+++ b/ssh-key/src/certificate/builder.rs
@@ -84,8 +84,8 @@ pub struct Builder {
     cert_type: Option<CertType>,
     key_id: Option<String>,
     valid_principals: Option<Vec<String>>,
-    valid_after: UnixTime,
-    valid_before: UnixTime,
+    valid_after: u64,
+    valid_before: u64,
     critical_options: OptionsMap,
     extensions: OptionsMap,
     comment: Option<String>,
@@ -105,12 +105,6 @@ impl Builder {
         valid_after: u64,
         valid_before: u64,
     ) -> Result<Self> {
-        let valid_after =
-            UnixTime::new(valid_after).map_err(|_| Field::ValidAfter.invalid_error())?;
-
-        let valid_before =
-            UnixTime::new(valid_before).map_err(|_| Field::ValidBefore.invalid_error())?;
-
         if valid_before < valid_after {
             return Err(Field::ValidBefore.invalid_error());
         }
diff --git a/ssh-key/src/known_hosts.rs b/ssh-key/src/known_hosts.rs
index e5da0f4..7ceb316 100644
--- a/ssh-key/src/known_hosts.rs
+++ b/ssh-key/src/known_hosts.rs
@@ -46,7 +46,7 @@ const MAGIC_HASH_PREFIX: &str = "|1|";
 ///   the key).
 pub struct KnownHosts<'a> {
     /// Lines of the file being iterated over
-    lines: core::str::Lines<'a>,
+    lines: str::Lines<'a>,
 }
 
 impl<'a> KnownHosts<'a> {