From 928dd57140a282861c710bf032455f040a746820 Mon Sep 17 00:00:00 2001 From: Joris Kleiber Date: Mon, 28 Jul 2025 22:51:36 +0200 Subject: [PATCH] Replace `system::time::Duration` --- src/defaults/mod.rs | 10 +-- src/defaults/settings_dsl.rs | 6 +- src/pam/converse.rs | 3 +- src/pam/mod.rs | 3 +- src/pam/rpassword.rs | 5 +- src/sudo/mod.rs | 9 ++- src/sudo/pam.rs | 4 +- src/sudo/pipeline.rs | 2 +- src/sudoers/policy.rs | 16 ++--- src/system/time.rs | 128 ++++++++++------------------------- src/system/timestamp.rs | 11 +-- 11 files changed, 67 insertions(+), 130 deletions(-) diff --git a/src/defaults/mod.rs b/src/defaults/mod.rs index 00fba11c6..be5e9e7a7 100644 --- a/src/defaults/mod.rs +++ b/src/defaults/mod.rs @@ -77,7 +77,7 @@ defaults! { "PYTHONINSPECT", "PYTHONUSERBASE", "RUBYLIB", "RUBYOPT", "*=()*"] #ignored } -fn octal_mode(input: &str) -> Option { +fn octal_mode(input: &str) -> Option { ::from_str_radix(input.strip_prefix('0')?, 8) .ok() .map(Into::into) @@ -85,19 +85,19 @@ fn octal_mode(input: &str) -> Option { /// A custom parser to parse seconds as fractional "minutes", the format used by /// passwd_timeout and timestamp_timeout. -fn fractional_minutes(input: &str) -> Option { +fn fractional_minutes(input: &str) -> Option { if let Some((integral, fractional)) = input.split_once('.') { // - 'input' is maximally 18 characters, making fractional.len() at most 17; // 1e17 < 2**63, so the definition of 'shift' will not overflow. // - for the same reason, if both parses in the definition of 'seconds' succeed, // we will have constructed an integer < 1e17. //- 1e17 * 60 = 6e18 < 9e18 < 2**63, so the final line also will not overflow - let shift = 10i64.pow(fractional.len().try_into().ok()?); - let seconds = integral.parse::().ok()? * shift + fractional.parse::().ok()?; + let shift = 10u64.pow(fractional.len().try_into().ok()?); + let seconds = integral.parse::().ok()? * shift + fractional.parse::().ok()?; Some(seconds * 60 / shift) } else { - input.parse::().ok()?.checked_mul(60) + input.parse::().ok()?.checked_mul(60) } } diff --git a/src/defaults/settings_dsl.rs b/src/defaults/settings_dsl.rs index 5491cfada..3ba77a837 100644 --- a/src/defaults/settings_dsl.rs +++ b/src/defaults/settings_dsl.rs @@ -2,7 +2,7 @@ macro_rules! storage_of { ($id:ident, true) => { bool }; ($id:ident, false) => { bool }; ($id:ident, [ $($value: expr),* ]) => { std::collections::HashSet }; - ($id:ident, $(=int $check: expr;)+ $_: expr) => { i64 }; + ($id:ident, $(=int $check: expr;)+ $_: expr) => { u64 }; ($id:ident, $(=enum $k: ident;)+ $_: ident) => { $crate::defaults::enums::$id }; ($id:ident, None) => { Option> }; ($id:ident, $_: expr) => { Box }; @@ -12,7 +12,7 @@ macro_rules! referent_of { ($id:ident, true) => { bool }; ($id:ident, false) => { bool }; ($id:ident, [ $($value: expr),* ]) => { &std::collections::HashSet }; - ($id:ident, $(=int $check: expr;)+ $_: expr) => { i64 }; + ($id:ident, $(=int $check: expr;)+ $_: expr) => { u64 }; ($id:ident, $(=enum $k: ident;)+ $_: ident) => { $crate::defaults::enums::$id }; ($id:ident, None) => { Option<&str> }; ($id:ident, $_: expr) => { &str }; @@ -73,7 +73,7 @@ macro_rules! modifier_of { ($id:ident, =int $first:literal ..= $last: literal $(@ $radix: literal)?; $value: expr) => { #[allow(clippy::from_str_radix_10)] $crate::defaults::SettingKind::Integer(|text| { - i64::from_str_radix(text, 10$(*0 + $radix)?) + u64::from_str_radix(text, 10$(*0 + $radix)?) .ok() .filter(|val| ($first ..= $last).contains(val)) .map(|i| { diff --git a/src/pam/converse.rs b/src/pam/converse.rs index 1eb2c69ab..b12be78b7 100644 --- a/src/pam/converse.rs +++ b/src/pam/converse.rs @@ -1,7 +1,6 @@ -use std::io; +use std::{io, time::Duration}; use crate::cutils::string_from_ptr; -use crate::system::time::Duration; use super::sys::*; diff --git a/src/pam/mod.rs b/src/pam/mod.rs index fb61f5e6c..16c676961 100644 --- a/src/pam/mod.rs +++ b/src/pam/mod.rs @@ -4,10 +4,9 @@ use std::{ os::raw::c_char, os::unix::prelude::OsStrExt, ptr::NonNull, + time::Duration, }; -use crate::system::time::Duration; - use converse::ConverserData; use error::pam_err; pub use error::{PamError, PamErrorType, PamResult}; diff --git a/src/pam/rpassword.rs b/src/pam/rpassword.rs index 16c8234d4..c7469c3a7 100644 --- a/src/pam/rpassword.rs +++ b/src/pam/rpassword.rs @@ -15,13 +15,12 @@ /// use std::io::{self, Error, ErrorKind, Read}; use std::os::fd::{AsFd, AsRawFd, BorrowedFd}; -use std::time::Instant; +use std::time::{Duration, Instant}; use std::{fs, mem}; use libc::{tcsetattr, termios, ECHO, ECHONL, ICANON, TCSANOW, VEOF, VERASE, VKILL}; use crate::cutils::cerr; -use crate::system::time::Duration; use super::securemem::PamBuffer; @@ -187,7 +186,7 @@ struct TimeoutRead<'a> { impl<'a> TimeoutRead<'a> { fn new(fd: BorrowedFd<'a>, timeout: Option) -> TimeoutRead<'a> { TimeoutRead { - timeout_at: timeout.map(|timeout| Instant::now() + timeout.into()), + timeout_at: timeout.map(|timeout| Instant::now() + timeout), fd, } } diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs index 15b2d7124..0d4c46d4f 100644 --- a/src/sudo/mod.rs +++ b/src/sudo/mod.rs @@ -6,12 +6,12 @@ use crate::log::dev_info; use crate::system::interface::UserId; use crate::system::timestamp::RecordScope; use crate::system::User; -use crate::system::{time::Duration, timestamp::SessionRecordFile, Process}; +use crate::system::{timestamp::SessionRecordFile, Process}; #[cfg(test)] pub(crate) use cli::SudoAction; #[cfg(not(test))] use cli::SudoAction; -use std::path::PathBuf; +use std::{path::PathBuf, time::Duration}; mod cli; pub(crate) use cli::{SudoEditOptions, SudoListOptions, SudoRunOptions, SudoValidateOptions}; @@ -91,8 +91,7 @@ fn sudo_process() -> Result<(), Error> { } SudoAction::RemoveTimestamp(_) => { let user = CurrentUser::resolve()?; - let mut record_file = - SessionRecordFile::open_for_user(&user, Duration::seconds(0))?; + let mut record_file = SessionRecordFile::open_for_user(&user, Duration::default())?; record_file.reset()?; Ok(()) } @@ -100,7 +99,7 @@ fn sudo_process() -> Result<(), Error> { if let Some(scope) = RecordScope::for_process(&Process::new()) { let user = CurrentUser::resolve()?; let mut record_file = - SessionRecordFile::open_for_user(&user, Duration::seconds(0))?; + SessionRecordFile::open_for_user(&user, Duration::default())?; record_file.disable(scope, None)?; } Ok(()) diff --git a/src/sudo/pam.rs b/src/sudo/pam.rs index b74592d7f..26ac15bc2 100644 --- a/src/sudo/pam.rs +++ b/src/sudo/pam.rs @@ -1,10 +1,10 @@ -use std::ffi::OsString; +use std::{ffi::OsString, time::Duration}; use crate::common::context::LaunchType; use crate::common::error::Error; use crate::log::{dev_info, user_warn}; use crate::pam::{PamContext, PamError, PamErrorType, PamResult}; -use crate::system::{term::current_tty_name, time::Duration}; +use crate::system::term::current_tty_name; pub(super) struct InitPamArgs<'a> { pub(super) launch: LaunchType, diff --git a/src/sudo/pipeline.rs b/src/sudo/pipeline.rs index 954a87bc5..67cb31a64 100644 --- a/src/sudo/pipeline.rs +++ b/src/sudo/pipeline.rs @@ -1,5 +1,6 @@ use std::ffi::OsStr; use std::process::exit; +use std::time::Duration; use super::cli::{SudoRunOptions, SudoValidateOptions}; use super::diagnostic; @@ -10,7 +11,6 @@ use crate::log::{auth_info, auth_warn}; use crate::pam::PamContext; use crate::sudo::env::environment; use crate::sudo::pam::{attempt_authenticate, init_pam, pre_exec, InitPamArgs}; -use crate::sudo::Duration; use crate::sudoers::{ AuthenticatingUser, Authentication, Authorization, DirChange, Judgement, Restrictions, Sudoers, }; diff --git a/src/sudoers/policy.rs b/src/sudoers/policy.rs index 9d88a4075..85669faec 100644 --- a/src/sudoers/policy.rs +++ b/src/sudoers/policy.rs @@ -6,13 +6,13 @@ use crate::common::{ }; use crate::exec::Umask; use crate::sudoers::ast::{ExecControl, Tag}; -use crate::system::{time::Duration, Hostname, User}; +use crate::system::{Hostname, User}; /// Data types and traits that represent what the "terms and conditions" are after a successful /// permission check. /// /// The trait definitions can be part of some global crate in the future, if we support more /// than just the sudoers file. -use std::collections::HashSet; +use std::{collections::HashSet, time::Duration}; #[must_use] #[cfg_attr(test, derive(Debug, PartialEq))] @@ -38,11 +38,11 @@ impl super::Settings { Authentication { must_authenticate: tag.needs_passwd(), allowed_attempts: self.passwd_tries().try_into().unwrap(), - prior_validity: Duration::seconds(self.timestamp_timeout()), + prior_validity: Duration::from_secs(self.timestamp_timeout()), pwfeedback: self.pwfeedback(), password_timeout: match self.passwd_timeout() { 0 => None, - timeout => Some(Duration::seconds(timeout)), + timeout => Some(Duration::from_secs(timeout)), }, credential: if self.rootpw() { AuthenticatingUser::Root @@ -190,10 +190,10 @@ mod test { Authentication { must_authenticate: true, allowed_attempts: 3, - prior_validity: Duration::minutes(15), + prior_validity: Duration::from_secs(15 * 60), credential: AuthenticatingUser::InvokingUser, pwfeedback: false, - password_timeout: Some(Duration::seconds(300)), + password_timeout: Some(Duration::from_secs(300)), }, ); @@ -207,10 +207,10 @@ mod test { Authentication { must_authenticate: false, allowed_attempts: 3, - prior_validity: Duration::minutes(15), + prior_validity: Duration::from_secs(15 * 60), credential: AuthenticatingUser::InvokingUser, pwfeedback: false, - password_timeout: Some(Duration::seconds(300)), + password_timeout: Some(Duration::from_secs(300)), }, ); assert_eq!(restrictions, restrictions2); diff --git a/src/system/time.rs b/src/system/time.rs index 412b574c0..09b4c068b 100644 --- a/src/system/time.rs +++ b/src/system/time.rs @@ -2,6 +2,7 @@ use std::{ io::{Read, Write}, mem::MaybeUninit, ops::{Add, Sub}, + time::Duration, }; /// A timestamp relative to `CLOCK_BOOTTIME`. @@ -51,85 +52,47 @@ impl SystemTime { i64::from_ne_bytes(nsec_bytes), )) } -} - -impl Sub for SystemTime { - type Output = Duration; - - fn sub(self, rhs: SystemTime) -> Self::Output { - Duration::new(self.secs - rhs.secs, self.nsecs - rhs.nsecs) - } -} - -impl Add for SystemTime { - type Output = SystemTime; - - fn add(self, rhs: Duration) -> Self::Output { - SystemTime::new(self.secs + rhs.secs, self.nsecs + rhs.nsecs) - } -} - -impl Sub for SystemTime { - type Output = SystemTime; - fn sub(self, rhs: Duration) -> Self::Output { - SystemTime::new(self.secs - rhs.secs, self.nsecs - rhs.nsecs) - } -} + #[inline] + pub fn checked_add(self, rhs: Duration) -> Option { + let rhs_secs = rhs.as_nanos().div_euclid(1_000_000_000).try_into().ok()?; + let rhs_nsecs = rhs.as_nanos().rem_euclid(1_000_000_000).try_into().ok()?; -#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)] -pub struct Duration { - secs: i64, - nsecs: i64, -} + let secs = self.secs.checked_add(rhs_secs)?; + let nsecs = self.nsecs.checked_add(rhs_nsecs)?; -impl Duration { - pub fn new(secs: i64, nsecs: i64) -> Duration { - Duration { - secs: secs + nsecs.div_euclid(1_000_000_000), - nsecs: nsecs.rem_euclid(1_000_000_000), - } + Some(SystemTime::new(secs, nsecs)) } - pub fn seconds(secs: i64) -> Duration { - Duration::new(secs, 0) - } + #[inline] + pub fn checked_sub(self, rhs: Duration) -> Option { + let rhs_secs = rhs.as_nanos().div_euclid(1_000_000_000).try_into().ok()?; + let rhs_nsecs = rhs.as_nanos().rem_euclid(1_000_000_000).try_into().ok()?; - #[cfg(test)] - pub fn minutes(minutes: i64) -> Duration { - Duration::seconds(minutes * 60) - } + let secs = self.secs.checked_sub(rhs_secs)?; + let nsecs = self.nsecs.checked_sub(rhs_nsecs)?; - #[cfg(test)] - pub fn milliseconds(ms: i64) -> Duration { - let secs = ms / 1000; - let ms = ms % 1000; - Duration::new(secs, ms * 1_000_000) + Some(SystemTime::new(secs, nsecs)) } } -impl Add for Duration { - type Output = Duration; +impl Add for SystemTime { + type Output = SystemTime; + #[inline] fn add(self, rhs: Duration) -> Self::Output { - Duration::new(self.secs + rhs.secs, self.nsecs + rhs.nsecs) + self.checked_add(rhs) + .expect("overflow when adding duration") } } -impl Sub for Duration { - type Output = Duration; +impl Sub for SystemTime { + type Output = SystemTime; + #[inline] fn sub(self, rhs: Duration) -> Self::Output { - Duration::new(self.secs - rhs.secs, self.nsecs - rhs.nsecs) - } -} - -impl From for std::time::Duration { - fn from(dur: Duration) -> std::time::Duration { - std::time::Duration::new( - dur.secs.try_into().unwrap_or(0), - dur.nsecs.try_into().unwrap_or(0), - ) + self.checked_sub(rhs) + .expect("overflow when subtracting duration") } } @@ -213,13 +176,7 @@ mod tests { use super::*; #[test] - fn test_new_durations_and_times() { - assert_eq!(Duration::new(1, 1_000_000_000), Duration::seconds(2)); - assert_eq!( - Duration::new(-2, 500_000_000), - Duration::seconds(-1) + Duration::milliseconds(-500) - ); - + fn test_new_system_time() { assert_eq!(SystemTime::new(-1, 2_000_000_000), SystemTime::new(1, 0)); assert_eq!( SystemTime::new(2, -500_000_000), @@ -230,37 +187,20 @@ mod tests { #[test] fn test_time_ops() { assert_eq!( - Duration::seconds(2) + Duration::seconds(3), - Duration::seconds(5) - ); - assert_eq!( - Duration::seconds(3) - Duration::seconds(1), - Duration::seconds(2) - ); - assert_eq!( - Duration::seconds(-10) + Duration::seconds(-5), - Duration::seconds(-15) - ); - assert_eq!( - Duration::milliseconds(5555) + Duration::milliseconds(5555), - Duration::seconds(11) + Duration::milliseconds(110) - ); - assert_eq!( - Duration::milliseconds(-5555) + Duration::milliseconds(-1111), - Duration::milliseconds(-6666) - ); - assert_eq!( - Duration::seconds(10) - Duration::seconds(-5), - Duration::seconds(15) + SystemTime::new(0, 0) + Duration::from_secs(3), + SystemTime::new(3, 0) ); - assert_eq!( - SystemTime::new(0, 0) + Duration::seconds(3), + SystemTime::new(0, 500_000_000) + Duration::from_nanos(2_500_000_000), SystemTime::new(3, 0) ); assert_eq!( - SystemTime::new(10, 0) - Duration::seconds(4), + SystemTime::new(10, 0) - Duration::from_secs(4), SystemTime::new(6, 0) ); + assert_eq!( + SystemTime::new(10, 0) - Duration::from_nanos(3_500_000_000), + SystemTime::new(6, 500_000_000) + ); } } diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs index 0cc3a2207..6cc6a644b 100644 --- a/src/system/timestamp.rs +++ b/src/system/timestamp.rs @@ -2,6 +2,7 @@ use std::{ fs::File, io::{self, Cursor, Read, Seek, Write}, path::PathBuf, + time::Duration, }; use crate::{ @@ -13,7 +14,7 @@ use super::{ audit::secure_open_cookie_file, file::FileLock, interface::{DeviceId, ProcessId, UserId}, - time::{Duration, ProcessCreateTime, SystemTime}, + time::{ProcessCreateTime, SystemTime}, Process, WithProcess, }; @@ -650,7 +651,7 @@ mod tests { #[test] fn timestamp_record_written_between_works() { - let some_time = SystemTime::now().unwrap() + Duration::minutes(100); + let some_time = SystemTime::now().unwrap() + Duration::from_secs(100 * 60); let scope = RecordScope::Tty { tty_device: DeviceId::new(12), session_pid: ProcessId::new(1234), @@ -658,7 +659,7 @@ mod tests { }; let sample = SessionRecord::init(scope, UserId::new(1234), true, some_time); - let dur = Duration::seconds(30); + let dur = Duration::from_secs(30); assert!(sample.written_between(some_time, some_time)); assert!(sample.written_between(some_time, some_time + dur)); @@ -686,7 +687,7 @@ mod tests { fn session_record_file_header_checks() { // valid header should remain valid let c = tempfile_with_data(&[0xD0, 0x50, 0x01, 0x00]).unwrap(); - let timeout = Duration::seconds(30); + let timeout = Duration::from_secs(30); assert!(SessionRecordFile::new(TEST_USER_ID, c.try_clone().unwrap(), timeout).is_ok()); let v = data_from_tempfile(c).unwrap(); assert_eq!(&v[..], &[0xD0, 0x50, 0x01, 0x00]); @@ -712,7 +713,7 @@ mod tests { #[test] fn can_create_and_update_valid_file() { - let timeout = Duration::seconds(30); + let timeout = Duration::from_secs(30); let c = tempfile_with_data(&[]).unwrap(); let mut srf = SessionRecordFile::new(TEST_USER_ID, c.try_clone().unwrap(), timeout).unwrap();