Skip to content

Commit

Permalink
ssh-key: use u64 type for certificate timestamps (#209)
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
tarcieri authored Mar 30, 2024
1 parent bce9963 commit f8bf338
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 41 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions ssh-cipher/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ssh-cipher"
version = "0.2.0"
version = "0.3.0-pre"
description = """
Pure Rust implementation of SSH symmetric encryption including support for the
modern [email protected]/[email protected] and
Expand All @@ -18,8 +18,8 @@ edition = "2021"
rust-version = "1.72"

[dependencies]
cipher = "0.5.0-pre.4"
encoding = { package = "ssh-encoding", version = "0.2", path = "../ssh-encoding" }
cipher = "=0.5.0-pre.4"
encoding = { package = "ssh-encoding", version = "=0.3.0-pre", path = "../ssh-encoding" }

# optional dependencies
aes = { version = "=0.9.0-pre", optional = true, default-features = false }
Expand Down
2 changes: 1 addition & 1 deletion ssh-encoding/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ssh-encoding"
version = "0.2.0"
version = "0.3.0-pre"
description = """
Pure Rust implementation of SSH data type decoders/encoders as described
in RFC4251
Expand Down
6 changes: 3 additions & 3 deletions ssh-key/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ssh-key"
version = "0.6.5"
version = "0.7.0-pre"
description = """
Pure Rust implementation of SSH key file format decoders/encoders as described
in RFC4251/RFC4253 and OpenSSH key formats, as well as "sshsig" signatures and
Expand All @@ -17,8 +17,8 @@ edition = "2021"
rust-version = "1.73"

[dependencies]
cipher = { package = "ssh-cipher", version = "0.2", path = "../ssh-cipher" }
encoding = { package = "ssh-encoding", version = "0.2", features = ["base64", "pem", "sha2"], path = "../ssh-encoding" }
cipher = { package = "ssh-cipher", version = "=0.3.0-pre", path = "../ssh-cipher" }
encoding = { package = "ssh-encoding", version = "=0.3.0-pre", features = ["base64", "pem", "sha2"], path = "../ssh-encoding" }
sha2 = { version = "=0.11.0-pre.3", default-features = false }
signature = { version = "=2.3.0-pre.3", default-features = false }
subtle = { version = "2", default-features = false }
Expand Down
2 changes: 1 addition & 1 deletion ssh-key/src/authorized_keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
44 changes: 26 additions & 18 deletions ssh-key/src/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ mod unix_time;

pub use self::{builder::Builder, cert_type::CertType, field::Field, options_map::OptionsMap};

use self::unix_time::UnixTime;
use crate::{
public::{KeyData, SshFormat},
Algorithm, Error, Fingerprint, HashAlg, Result, Signature,
Expand All @@ -26,7 +25,10 @@ use signature::Verifier;
use serde::{de, ser, Deserialize, Serialize};

#[cfg(feature = "std")]
use std::{fs, path::Path, time::SystemTime};
use {
self::unix_time::UnixTime,
std::{fs, path::Path, time::SystemTime},
};

/// OpenSSH certificate as specified in [PROTOCOL.certkeys].
///
Expand Down Expand Up @@ -137,10 +139,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,
Expand Down Expand Up @@ -281,26 +283,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
Expand Down Expand Up @@ -391,8 +401,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
Expand Down Expand Up @@ -462,8 +470,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)?,
Expand Down
16 changes: 5 additions & 11 deletions ssh-key/src/certificate/builder.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
//! OpenSSH certificate builder.

use super::{unix_time::UnixTime, CertType, Certificate, Field, OptionsMap};
use super::{CertType, Certificate, Field, OptionsMap};
use crate::{public, Result, Signature, SigningKey};
use alloc::{string::String, vec::Vec};

#[cfg(feature = "rand_core")]
use rand_core::CryptoRngCore;

#[cfg(feature = "std")]
use std::time::SystemTime;
use {super::UnixTime, std::time::SystemTime};

#[cfg(doc)]
use crate::PrivateKey;
Expand Down Expand Up @@ -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>,
Expand All @@ -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());
}
Expand Down Expand Up @@ -304,7 +298,7 @@ impl Builder {

#[cfg(debug_assertions)]
cert.validate_at(
cert.valid_after.into(),
cert.valid_after,
&[cert.signature_key.fingerprint(Default::default())],
)?;

Expand Down
2 changes: 1 addition & 1 deletion ssh-key/src/known_hosts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down

0 comments on commit f8bf338

Please sign in to comment.