diff --git a/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md index a0554a9fb..0f8867728 100644 --- a/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md +++ b/.github/PULL_REQUEST_TEMPLATE/pull_request_template.md @@ -2,7 +2,7 @@ A clear and concise description of the changes done by your pull request. **Pull Request Checklist** -- [] I have read and accepted the [code of conduct](https://github.com/memorysafety/sudo-rs/blob/master/CODE_OF_CONDUCT.md) for this project. +- [] I have read and accepted the [code of conduct](https://github.com/trifectatechfoundation/sudo-rs/blob/master/CODE_OF_CONDUCT.md) for this project. - [] I have tested, formatted and ran clippy over my changes. - [] I have commented and documented my changes. -- [] This pull request will fix issue https://github.com/memorysafety/sudo-rs/issues/<#issue> where a proper discussion about a solution has taken place. +- [] This pull request will fix issue https://github.com/trifectatechfoundation/sudo-rs/issues/<#issue> where a proper discussion about a solution has taken place. diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 44562c4eb..3db4ee482 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -397,11 +397,7 @@ jobs: shared-key: "stable" - name: Run clippy - uses: actions-rs/clippy-check@v1 - with: - name: clippy-result - token: ${{ secrets.GITHUB_TOKEN }} - args: --no-deps -- --deny warnings + run: cargo clippy --no-deps --all-targets --all-features -- --deny warnings docs: needs: clippy diff --git a/CHANGELOG.md b/CHANGELOG.md index e8f02c1c9..ecc58e6b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [0.2.3] - 2024-07-11 + +### Changed +- Portability: sudo-rs now is compatible with s390x-unknown-linux-gnu +- Removed unneeded code & fix hints given by newer Rust version + +### Fixed +- `visudo` would not properly truncate a `sudoers` file +- high CPU load when child process did not terminate after closure of a terminal + ## [0.2.2] - 2024-02-02 ### Changed @@ -111,9 +121,10 @@ - Use canonicalized paths for the executed binaries - Simplified CLI help to only display supported actions -[0.2.2]: https://github.com/memorysafety/sudo-rs/compare/v0.2.1...v0.2.2 -[0.2.1]: https://github.com/memorysafety/sudo-rs/compare/v0.2.0...v0.2.1 -[0.2.0]: https://github.com/memorysafety/sudo-rs/compare/v0.2.0-dev.20230711...v0.2.0 -[0.2.0-dev.20230711]: https://github.com/memorysafety/sudo-rs/compare/v0.2.0-dev.20230703...v0.2.0-dev.20230711 -[0.2.0-dev.20230703]: https://github.com/memorysafety/sudo-rs/compare/v0.2.0-dev.20230627...v0.2.0-dev.20230703 -[0.2.0-dev.20230627]: https://github.com/memorysafety/sudo-rs/compare/v0.1.0-dev.20230620...v0.2.0-dev.20230627 +[0.2.3]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.2.2...v0.2.3 +[0.2.2]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.2.1...v0.2.2 +[0.2.1]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.2.0...v0.2.1 +[0.2.0]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.2.0-dev.20230711...v0.2.0 +[0.2.0-dev.20230711]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.2.0-dev.20230703...v0.2.0-dev.20230711 +[0.2.0-dev.20230703]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.2.0-dev.20230627...v0.2.0-dev.20230703 +[0.2.0-dev.20230627]: https://github.com/trifectatechfoundation/sudo-rs/compare/v0.1.0-dev.20230620...v0.2.0-dev.20230627 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97f334656..1c7fccab1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -51,7 +51,7 @@ If you find a security problem that can be used to used to compromise a system, do follow our [security policy] and report a vulnerability instead of using the issue tracker. -[security policy]: https://github.com/memorysafety/sudo-rs/security/policy +[security policy]: https://github.com/trifectatechfoundation/sudo-rs/security/policy ## Working on a bigger issue diff --git a/COPYRIGHT b/COPYRIGHT index 3dc0ac8a8..f9839973d 100644 --- a/COPYRIGHT +++ b/COPYRIGHT @@ -1,4 +1,4 @@ -Copyright (c) 2022-2023 Internet Security Research Group +Copyright (c) 2022-2024 Trifecta Tech Foundation and contributors Except as otherwise noted (below and/or in individual files), sudo-rs is licensed under the Apache License, Version 2.0 or diff --git a/Cargo.lock b/Cargo.lock index 7155b9f28..76c4ac033 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -16,15 +16,15 @@ checksum = "d2fabcfbdc87f4758337ca535fb41a6d701b65693ce38287d856d1674551ec9b" [[package]] name = "libc" -version = "0.2.153" +version = "0.2.155" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" +checksum = "97b3888a4aecf77e811145cadf6eef5901f4782c53886191b2f693f24761847c" [[package]] name = "log" -version = "0.4.20" +version = "0.4.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b5e6163cb8c49088c2c36f57875e58ccd8c87c7427f7fbd50ea6710b2f3f2e8f" +checksum = "a7a70ba024b9dc04c27ea2f0c0548feb474ec5c54bba33a7f72f873a39d07b24" [[package]] name = "pretty_assertions" @@ -38,7 +38,7 @@ dependencies = [ [[package]] name = "sudo-rs" -version = "0.2.2" +version = "0.2.3" dependencies = [ "glob", "libc", diff --git a/Cargo.toml b/Cargo.toml index 0111f4274..377a321c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,11 +1,11 @@ [package] name = "sudo-rs" description = "A memory safe implementation of sudo and su." -version = "0.2.2" +version = "0.2.3" license = "Apache-2.0 OR MIT" edition = "2021" -repository = "https://github.com/memorysafety/sudo-rs" -homepage = "https://github.com/memorysafety/sudo-rs" +repository = "https://github.com/trifectatechfoundation/sudo-rs" +homepage = "https://github.com/trifectatechfoundation/sudo-rs" publish = true categories = ["command-line-interface"] diff --git a/LICENSE-MIT b/LICENSE-MIT index c9857bc25..0e50be23a 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,5 +1,5 @@ - -Copyright (c) 2022-2023 Internet Security Research Group +Copyright (c) 2022-2024 Trifecta Tech Foundation +and contributors Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated diff --git a/README.md b/README.md index d6bf5409e..e55e35bb7 100644 --- a/README.md +++ b/README.md @@ -7,8 +7,8 @@ A safety oriented and memory safe implementation of sudo and su written in Rust. Sudo-rs is being developed further; features you might expect from original sudo may still be unimplemented or not planned. If there is an important one you need, please request it using the issue tracker. If you encounter any usability bugs, -also please report them on the [issue tracker](https://github.com/memorysafety/sudo-rs/issues). -Suspected vulnerabilities can be reported on our [security page](https://github.com/memorysafety/sudo-rs/security). +also please report them on the [issue tracker](https://github.com/trifectatechfoundation/sudo-rs/issues). +Suspected vulnerabilities can be reported on our [security page](https://github.com/trifectatechfoundation/sudo-rs/security). An [audit of sudo-rs version 0.2.0](docs/audit/audit-report-sudo-rs.pdf) has been performed in August 2023. The findings from that audit are addressed in the current version. @@ -20,6 +20,16 @@ or newer is necessary to run sudo-rs. The recommended way to start using `sudo-rs` is via the package manager of your Linux distribution. +### Arch Linux + +Arch Linux can be installed via AUR [sudo-rs](https://aur.archlinux.org/packages/sudo-rs) or [sudo-rs-git](https://aur.archlinux.org/packages/sudo-rs-git). + +Note: [AUR usage help](https://wiki.archlinux.org/title/AUR_helpers) + +```sh +yay -Syu sudo-rs +``` + ### Debian/Ubuntu If you are running Debian 13 (trixie) or later, or Ubuntu 24.04 (Noble Numbat) or later, you can use: ```sh @@ -30,7 +40,11 @@ via the usual commands `sudo` and `su` instead, prepend `/usr/lib/cargo/bin` to ### Fedora -For Fedora, we expect sudo-rs to be available via its package manager soon. +If you are running Fedora 38 or later, you can use: +```sh +sudo dnf install sudo-rs +``` +This will offer the functionality using the commands `su-rs` and `sudo-rs`. ### Installing our pre-compiled x86-64 binaries @@ -150,5 +164,6 @@ extract parts of our work in usable crates for other people. ## Sponsors -The development of sudo-rs is an initiative of the [Prossimo project by ISRG](https://www.memorysafety.org/). +The initial development of sudo-rs was started and funded by the [Internet Security Research Group](https://www.abetterinternet.org/) as part of the [Prossimo project](https://www.memorysafety.org/). + An independent security audit of sudo-rs was made possible by the [NLNet Foundation](https://nlnet.nl/). diff --git a/SECURITY.md b/SECURITY.md index 39485b90a..17fb19cc3 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,7 +1,7 @@ # Security policy **Do not report security vulnerabilities through public GitHub issues.** -Instead, you can report them using [our security page](https://github.com/memorysafety/sudo-rs/security). Alternatively, you can also send them +Instead, you can report them using [our security page](https://github.com/trifectatechfoundation/sudo-rs/security). Alternatively, you can also send them by email to security+sudo@tweedegolf.com. You can encrypt your email using GnuPG if you want. Use the GPG key with fingerprint [C2E4 CAC4 B122 25DE 1C3B B1C9 289D 0820 03D0 1E95](https://keys.openpgp.org/search?q=C2E4CAC4B12225DE1C3BB1C9289D082003D01E95). @@ -24,4 +24,4 @@ We prefer to receive reports in English. If necessary, we also understand Spanis Like original sudo, we adhere to the principle of [Coordinated Vulnerability Disclosure](https://vuls.cert.org/confluence/display/CVD/Executive+Summary). # Security Advisories -Security advisories will be published [on GitHub](https://github.com/memorysafety/sudo-rs/security/advisories) and possibly through other channels. +Security advisories will be published [on GitHub](https://github.com/trifectatechfoundation/sudo-rs/security/advisories) and possibly through other channels. diff --git a/docs/man/su.1.md b/docs/man/su.1.md index 7623f84cf..86b6567ab 100644 --- a/docs/man/su.1.md +++ b/docs/man/su.1.md @@ -1,5 +1,5 @@ # NAME diff --git a/docs/man/sudo.8.md b/docs/man/sudo.8.md index 2d7e40c3a..218688f71 100644 --- a/docs/man/sudo.8.md +++ b/docs/man/sudo.8.md @@ -1,5 +1,5 @@ # NAME diff --git a/docs/man/visudo.8.md b/docs/man/visudo.8.md index 9fea0c46f..f2de127a1 100644 --- a/docs/man/visudo.8.md +++ b/docs/man/visudo.8.md @@ -1,5 +1,5 @@ # NAME diff --git a/src/common/error.rs b/src/common/error.rs index f30d19a15..711402814 100644 --- a/src/common/error.rs +++ b/src/common/error.rs @@ -13,6 +13,7 @@ pub enum Error { other_user: Option, }, SelfCheck, + KernelCheck, CommandNotFound(PathBuf), InvalidCommand(PathBuf), ChDirNotAllowed { @@ -56,6 +57,7 @@ impl fmt::Display for Error { Error::SelfCheck => { f.write_str("sudo must be owned by uid 0 and have the setuid bit set") } + Error::KernelCheck => f.write_str("sudo needs a Kernel >= 5.9"), Error::CommandNotFound(p) => write!(f, "'{}': command not found", p.display()), Error::InvalidCommand(p) => write!(f, "'{}': invalid command", p.display()), Error::UserNotFound(u) => write!(f, "user '{u}' not found"), diff --git a/src/defaults/settings_dsl.rs b/src/defaults/settings_dsl.rs index 561eaa687..bc5d6fcb2 100644 --- a/src/defaults/settings_dsl.rs +++ b/src/defaults/settings_dsl.rs @@ -1,5 +1,6 @@ macro_rules! add_from { ($ctor:ident, $type:ty) => { + #[allow(non_local_definitions)] impl From<$type> for $crate::defaults::SudoDefault { fn from(value: $type) -> Self { $crate::defaults::SudoDefault::$ctor(value.into()) @@ -8,6 +9,7 @@ macro_rules! add_from { }; ($ctor:ident, $type:ty, negatable$(, $vetting_function:expr)?) => { + #[allow(non_local_definitions)] impl From<$type> for $crate::defaults::SudoDefault { fn from(value: $type) -> Self { $crate::defaults::SudoDefault::$ctor(OptTuple { @@ -17,6 +19,7 @@ macro_rules! add_from { } } + #[allow(non_local_definitions)] impl From<($type, $type)> for $crate::defaults::SudoDefault { fn from((value, neg): ($type, $type)) -> Self { $crate::defaults::SudoDefault::$ctor(OptTuple { diff --git a/src/exec/use_pty/parent.rs b/src/exec/use_pty/parent.rs index 31b70aae1..c5ae13be9 100644 --- a/src/exec/use_pty/parent.rs +++ b/src/exec/use_pty/parent.rs @@ -700,7 +700,13 @@ impl Process for ParentClosure { match event { ParentEvent::Signal => self.on_signal(registry), ParentEvent::Tty(poll_event) => { - self.tty_pipe.on_left_event(poll_event, registry).ok(); + // Check if tty which existed is now gone. + if self.tty_pipe.left().tcgetsid().is_err() { + dev_warn!("tty gone (closed/detached), ignoring future events"); + self.tty_pipe.ignore_events(registry); + } else { + self.tty_pipe.on_left_event(poll_event, registry).ok(); + } } ParentEvent::Pty(poll_event) => { self.tty_pipe.on_right_event(poll_event, registry).ok(); diff --git a/src/exec/use_pty/pipe/ring_buffer.rs b/src/exec/use_pty/pipe/ring_buffer.rs index f87555ecf..c66ed68b4 100644 --- a/src/exec/use_pty/pipe/ring_buffer.rs +++ b/src/exec/use_pty/pipe/ring_buffer.rs @@ -25,6 +25,8 @@ impl RingBuffer { self.len == self.storage.len() } + // rustc 1.77.1 clippy gives false diagnostics, https://github.com/rust-lang/rust-clippy/issues/12519 + #[allow(clippy::unused_io_amount)] pub(super) fn insert(&mut self, read: &mut R) -> io::Result { let inserted_len = if self.is_empty() { // Case 1.1. The buffer is empty, meaning that there are two unfilled slices in @@ -61,6 +63,8 @@ impl RingBuffer { self.len == 0 } + // rustc 1.77.1 clippy gives false diagnostics, https://github.com/rust-lang/rust-clippy/issues/12519 + #[allow(clippy::unused_io_amount)] pub(super) fn remove(&mut self, write: &mut W) -> io::Result { let removed_len = if self.is_full() { // Case 2.1. The buffer is full, meaning that there are two filled slices in `storage`: diff --git a/src/pam/error.rs b/src/pam/error.rs index e850cba05..ba468cbe5 100644 --- a/src/pam/error.rs +++ b/src/pam/error.rs @@ -6,23 +6,17 @@ use super::sys::*; pub type PamResult = Result; +// TODO: add missing doc-comments #[derive(PartialEq, Eq, Debug)] pub enum PamErrorType { /// There was no error running the PAM command Success, - /// OpenError, - /// SymbolError, - /// ServiceError, - /// SystemError, - /// BufferError, - /// ConversationError, - /// PermissionDenied, /// The maximum number of authentication attempts was reached and no more /// attempts should be made. @@ -47,27 +41,17 @@ pub enum PamErrorType { CredentialsError, /// The user account is expired and can no longer be used. AccountExpired, - /// AuthTokenExpired, - /// SessionError, - /// AuthTokenError, - /// AuthTokenRecoveryError, - /// AuthTokenLockBusy, - /// AuthTokenDisableAging, - /// NoModuleData, - /// Ignore, /// The application should exit immediately. Abort, - /// TryAgain, - /// ModuleUnknown, /// The application tried to set/delete an undefined or inaccessible item. BadItem, // Extension in OpenPAM and LinuxPAM diff --git a/src/pam/sys.rs b/src/pam/sys.rs index d222b0467..2d8029d02 100644 --- a/src/pam/sys.rs +++ b/src/pam/sys.rs @@ -240,409 +240,6 @@ extern "C" { extern "C" { pub fn pam_chauthtok(pamh: *mut pam_handle_t, flags: libc::c_int) -> libc::c_int; } -pub type __uid_t = libc::c_uint; -pub type __gid_t = libc::c_uint; -pub type gid_t = __gid_t; -pub type uid_t = __uid_t; -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct passwd { - pub pw_name: *mut libc::c_char, - pub pw_passwd: *mut libc::c_char, - pub pw_uid: __uid_t, - pub pw_gid: __gid_t, - pub pw_gecos: *mut libc::c_char, - pub pw_dir: *mut libc::c_char, - pub pw_shell: *mut libc::c_char, -} -#[test] -fn bindgen_test_layout_passwd() { - const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); - let ptr = UNINIT.as_ptr(); - assert_eq!( - ::std::mem::align_of::(), - ::std::mem::align_of::<*mut libc::c_char>(), - concat!("Alignment of ", stringify!(passwd)) - ); - let mut offset: usize = 0; - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_name) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_name) - ) - ); - offset = - aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_passwd) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_passwd) - ) - ); - offset = aligned_offset::<__uid_t>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_uid) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_uid) - ) - ); - offset = aligned_offset::<__gid_t>(offset + ::std::mem::size_of::<__uid_t>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_gid) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_gid) - ) - ); - offset = aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<__gid_t>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_gecos) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_gecos) - ) - ); - offset = - aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_dir) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_dir) - ) - ); - offset = - aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).pw_shell) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(passwd), - "::", - stringify!(pw_shell) - ) - ); - offset = - aligned_offset::<*mut libc::c_void>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - ::std::mem::size_of::(), - offset, - concat!("Size of: ", stringify!(passwd)) - ); -} -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct group { - pub gr_name: *mut libc::c_char, - pub gr_passwd: *mut libc::c_char, - pub gr_gid: __gid_t, - pub gr_mem: *mut *mut libc::c_char, -} -#[test] -fn bindgen_test_layout_group() { - const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); - let ptr = UNINIT.as_ptr(); - assert_eq!( - ::std::mem::align_of::(), - ::std::mem::align_of::<*mut libc::c_char>(), - concat!("Alignment of ", stringify!(group)) - ); - let mut offset: usize = 0; - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).gr_name) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(group), - "::", - stringify!(gr_name) - ) - ); - offset = - aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).gr_passwd) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(group), - "::", - stringify!(gr_passwd) - ) - ); - offset = aligned_offset::<__gid_t>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).gr_gid) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(group), - "::", - stringify!(gr_gid) - ) - ); - offset = aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<__gid_t>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).gr_mem) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(group), - "::", - stringify!(gr_mem) - ) - ); - offset = - aligned_offset::<*mut libc::c_void>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - ::std::mem::size_of::(), - offset, - concat!("Size of: ", stringify!(group)) - ); -} -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct spwd { - pub sp_namp: *mut libc::c_char, - pub sp_pwdp: *mut libc::c_char, - pub sp_lstchg: libc::c_long, - pub sp_min: libc::c_long, - pub sp_max: libc::c_long, - pub sp_warn: libc::c_long, - pub sp_inact: libc::c_long, - pub sp_expire: libc::c_long, - pub sp_flag: libc::c_ulong, -} -#[test] -fn bindgen_test_layout_spwd() { - const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); - let ptr = UNINIT.as_ptr(); - assert_eq!( - ::std::mem::align_of::(), - ::std::mem::align_of::<*mut libc::c_char>(), - concat!("Alignment of ", stringify!(spwd)) - ); - let mut offset: usize = 0; - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_namp) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_namp) - ) - ); - offset = - aligned_offset::<*mut libc::c_char>(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_pwdp) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_pwdp) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::<*mut libc::c_char>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_lstchg) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_lstchg) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_min) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_min) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_max) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_max) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_warn) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_warn) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_inact) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_inact) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_expire) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_expire) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).sp_flag) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(spwd), - "::", - stringify!(sp_flag) - ) - ); - offset = aligned_offset::<*mut libc::c_void>(offset + ::std::mem::size_of::()); - assert_eq!( - ::std::mem::size_of::(), - offset, - concat!("Size of: ", stringify!(spwd)) - ); -} -#[repr(C)] -#[derive(Debug, Copy, Clone)] -pub struct pam_modutil_privs { - pub grplist: *mut gid_t, - pub number_of_groups: libc::c_int, - pub allocated: libc::c_int, - pub old_gid: gid_t, - pub old_uid: uid_t, - pub is_dropped: libc::c_int, -} -#[test] -fn bindgen_test_layout_pam_modutil_privs() { - const UNINIT: ::std::mem::MaybeUninit = ::std::mem::MaybeUninit::uninit(); - let ptr = UNINIT.as_ptr(); - assert_eq!( - ::std::mem::align_of::(), - ::std::mem::align_of::<*mut gid_t>(), - concat!("Alignment of ", stringify!(pam_modutil_privs)) - ); - let mut offset: usize = 0; - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).grplist) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(pam_modutil_privs), - "::", - stringify!(grplist) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::<*mut gid_t>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).number_of_groups) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(pam_modutil_privs), - "::", - stringify!(number_of_groups) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).allocated) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(pam_modutil_privs), - "::", - stringify!(allocated) - ) - ); - offset = aligned_offset::<__gid_t>(offset + ::std::mem::size_of::()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).old_gid) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(pam_modutil_privs), - "::", - stringify!(old_gid) - ) - ); - offset = aligned_offset::<__uid_t>(offset + ::std::mem::size_of::<__gid_t>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).old_uid) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(pam_modutil_privs), - "::", - stringify!(old_uid) - ) - ); - offset = aligned_offset::(offset + ::std::mem::size_of::<__uid_t>()); - assert_eq!( - unsafe { ::std::ptr::addr_of!((*ptr).is_dropped) as usize - ptr as usize }, - offset, - concat!( - "Offset of field: ", - stringify!(pam_modutil_privs), - "::", - stringify!(is_dropped) - ) - ); - offset = aligned_offset::<*mut libc::c_void>(offset + ::std::mem::size_of::()); - assert_eq!( - ::std::mem::size_of::(), - offset, - concat!("Size of: ", stringify!(pam_modutil_privs)) - ); -} #[cfg(test)] fn aligned_offset(offset: usize) -> usize { diff --git a/src/su/cli.rs b/src/su/cli.rs index fed7238bf..9587d5d54 100644 --- a/src/su/cli.rs +++ b/src/su/cli.rs @@ -22,6 +22,7 @@ impl SuAction { } #[cfg(test)] + #[allow(clippy::result_large_err)] pub fn try_into_run(self) -> Result { if let Self::Run(v) = self { Ok(v) diff --git a/src/sudo/cli/mod.rs b/src/sudo/cli/mod.rs index 33081d34f..cdb6c6d32 100644 --- a/src/sudo/cli/mod.rs +++ b/src/sudo/cli/mod.rs @@ -10,6 +10,8 @@ pub mod help; #[cfg(test)] mod tests; +// remove dead_code when sudoedit has been implemented +#[allow(dead_code)] pub enum SudoAction { Edit(SudoEditOptions), Help(SudoHelpOptions), @@ -80,6 +82,7 @@ impl SudoAction { } #[cfg(test)] + #[allow(clippy::result_large_err)] pub fn try_into_run(self) -> Result { if let Self::Run(v) = self { Ok(v) @@ -204,6 +207,7 @@ impl TryFrom for SudoValidateOptions { } // sudo -e [-ABkNnS] [-r role] [-t type] [-C num] [-D directory] [-g group] [-h host] [-p prompt] [-R directory] [-T timeout] [-u user] file ... +#[allow(dead_code)] pub struct SudoEditOptions { // -k pub reset_timestamp: bool, diff --git a/src/sudo/env/environment.rs b/src/sudo/env/environment.rs index 6b9da2c01..d762bfe9e 100644 --- a/src/sudo/env/environment.rs +++ b/src/sudo/env/environment.rs @@ -133,6 +133,9 @@ fn is_safe_tz(value: &[u8]) -> bool { }; if check_value.starts_with(&[b'/']) { + // clippy 1.79 wants to us to optimise this check away; but we don't know what this will always + // be possible; and the compiler is clever enough to do that for us anyway if it can be. + #[allow(clippy::const_is_empty)] if !PATH_ZONEINFO.is_empty() { if !check_value.starts_with(PATH_ZONEINFO.as_bytes()) || check_value.get(PATH_ZONEINFO.len()) != Some(&b'/') diff --git a/src/sudo/env/tests.rs b/src/sudo/env/tests.rs index a0028be02..15fc0f68d 100644 --- a/src/sudo/env/tests.rs +++ b/src/sudo/env/tests.rs @@ -96,8 +96,6 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { let current_group = Group { gid: 1000, name: "test".to_string(), - passwd: String::new(), - members: Vec::new(), }; let root_user = User { @@ -114,8 +112,6 @@ fn create_test_context(sudo_options: &SudoRunOptions) -> Context { let root_group = Group { gid: 0, name: "root".to_string(), - passwd: String::new(), - members: Vec::new(), }; Context { diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs index ded2dd0d4..bf6bc405c 100644 --- a/src/sudo/mod.rs +++ b/src/sudo/mod.rs @@ -3,6 +3,7 @@ use crate::common::resolve::CurrentUser; use crate::common::{Context, Error}; use crate::log::dev_info; +use crate::system::kernel::kernel_check; use crate::system::timestamp::RecordScope; use crate::system::User; use crate::system::{time::Duration, timestamp::SessionRecordFile, Process}; @@ -16,7 +17,7 @@ use pipeline::{Pipeline, PolicyPlugin}; use std::path::Path; mod cli; -mod diagnostic; +pub mod diagnostic; mod env; mod pam; mod pipeline; @@ -84,6 +85,7 @@ fn sudo_process() -> Result<(), Error> { dev_info!("development logs are enabled"); self_check()?; + kernel_check(5, 9)?; let pipeline = Pipeline { policy: SudoersPolicy::default(), diff --git a/src/sudoers/basic_parser.rs b/src/sudoers/basic_parser.rs index 46899b317..8b4364833 100644 --- a/src/sudoers/basic_parser.rs +++ b/src/sudoers/basic_parser.rs @@ -73,7 +73,6 @@ pub fn maybe(status: Parsed) -> Parsed> { pub use super::char_stream::CharStream; /// All implementations of the Parse trait must satisfy this contract: -/// /// If the `parse` method of this trait returns None, the iterator is not advanced; otherwise it is /// advanced beyond the accepted part of the input. i.e. if some input is consumed the method /// *MUST* be producing a `Some` value. @@ -274,6 +273,7 @@ impl Parse for Option { } /// Parsing method for lists of items separated by a given character; this adheres to the contract of the [Parse] trait. +#[allow(clippy::multiple_bound_locations)] pub(super) fn parse_list( sep_by: char, max: usize, diff --git a/src/sudoers/mod.rs b/src/sudoers/mod.rs index 15f7e38e0..6f37d2379 100644 --- a/src/sudoers/mod.rs +++ b/src/sudoers/mod.rs @@ -456,7 +456,7 @@ fn match_group_alias(group: &impl UnixGroup) -> impl Fn(&UserSpecifier) -> bool move |spec| match spec { UserSpecifier::User(ident) => match_group(group)(ident), /* the parser does not allow this, but can happen due to Runas_Alias, - * see https://github.com/memorysafety/sudo-rs/issues/13 */ + * see https://github.com/trifectatechfoundation/sudo-rs/issues/13 */ _ => { auth_warn!("warning: ignoring %group syntax in runas_alias for checking sudo -g"); false @@ -590,160 +590,159 @@ fn analyze( } } - impl Sudoers { - fn include( - &mut self, - parent: &Path, - path: &Path, - diagnostics: &mut Vec, - count: &mut u8, - ) { - if *count >= INCLUDE_LIMIT { - diagnostics.push(Error { - source: Some(parent.to_owned()), - location: None, - message: format!("include file limit reached opening '{}'", path.display()), - }) - // FIXME: this will cause an error in `visudo` if we open a non-privileged sudoers file - // that includes another non-privileged sudoer files. - } else { - match open_subsudoers(path) { - Ok(subsudoer) => { - *count += 1; - self.process(path, subsudoer, diagnostics, count) - } - Err(e) => { - let message = if e.kind() == io::ErrorKind::NotFound { - // improve the error message in this case - format!("cannot open sudoers file '{}'", path.display()) - } else { - e.to_string() - }; + fn include( + cfg: &mut Sudoers, + parent: &Path, + path: &Path, + diagnostics: &mut Vec, + count: &mut u8, + ) { + if *count >= INCLUDE_LIMIT { + diagnostics.push(Error { + source: Some(parent.to_owned()), + location: None, + message: format!("include file limit reached opening '{}'", path.display()), + }) + // FIXME: this will cause an error in `visudo` if we open a non-privileged sudoers file + // that includes another non-privileged sudoer files. + } else { + match open_subsudoers(path) { + Ok(subsudoer) => { + *count += 1; + process(cfg, path, subsudoer, diagnostics, count) + } + Err(e) => { + let message = if e.kind() == io::ErrorKind::NotFound { + // improve the error message in this case + format!("cannot open sudoers file '{}'", path.display()) + } else { + e.to_string() + }; - diagnostics.push(Error { - source: Some(parent.to_owned()), - location: None, - message, - }) - } + diagnostics.push(Error { + source: Some(parent.to_owned()), + location: None, + message, + }) } } } + } - fn process( - &mut self, - cur_path: &Path, - sudoers: impl IntoIterator>, - diagnostics: &mut Vec, - safety_count: &mut u8, - ) { - for item in sudoers { - match item { - Ok(line) => match line { - Sudo::LineComment => {} - - Sudo::Spec(permission) => self.rules.push(permission), - - Sudo::Decl(UserAlias(mut def)) => self.aliases.user.1.append(&mut def), - Sudo::Decl(HostAlias(mut def)) => self.aliases.host.1.append(&mut def), - Sudo::Decl(CmndAlias(mut def)) => self.aliases.cmnd.1.append(&mut def), - Sudo::Decl(RunasAlias(mut def)) => self.aliases.runas.1.append(&mut def), - - Sudo::Decl(Defaults(params)) => { - for (name, value) in params { - self.set_default(name, value) - } + fn process( + cfg: &mut Sudoers, + cur_path: &Path, + sudoers: impl IntoIterator>, + diagnostics: &mut Vec, + safety_count: &mut u8, + ) { + for item in sudoers { + match item { + Ok(line) => match line { + Sudo::LineComment => {} + + Sudo::Spec(permission) => cfg.rules.push(permission), + + Sudo::Decl(UserAlias(mut def)) => cfg.aliases.user.1.append(&mut def), + Sudo::Decl(HostAlias(mut def)) => cfg.aliases.host.1.append(&mut def), + Sudo::Decl(CmndAlias(mut def)) => cfg.aliases.cmnd.1.append(&mut def), + Sudo::Decl(RunasAlias(mut def)) => cfg.aliases.runas.1.append(&mut def), + + Sudo::Decl(Defaults(params)) => { + for (name, value) in params { + set_default(cfg, name, value) } + } - Sudo::Include(path) => self.include( - cur_path, - &resolve_relative(cur_path, path), - diagnostics, - safety_count, - ), - - Sudo::IncludeDir(path) => { - if path.contains("%h") { - diagnostics.push(Error{ + Sudo::Include(path) => include( + cfg, + cur_path, + &resolve_relative(cur_path, path), + diagnostics, + safety_count, + ), + + Sudo::IncludeDir(path) => { + if path.contains("%h") { + diagnostics.push(Error{ source: Some(cur_path.to_owned()), location: None, message: format!("cannot open sudoers file {path}: percent escape %h in includedir is unsupported")}); - continue; - } - - let path = resolve_relative(cur_path, path); - let Ok(files) = std::fs::read_dir(&path) else { - diagnostics.push(Error { - source: Some(cur_path.to_owned()), - location: None, - message: format!("cannot open sudoers file {}", path.display()), - }); - continue; - }; - let mut safe_files = files - .filter_map(|direntry| { - let path = direntry.ok()?.path(); - let text = path.file_name()?.to_str()?; - if text.ends_with('~') || text.contains('.') { - None - } else { - Some(path) - } - }) - .collect::>(); - safe_files.sort(); - for file in safe_files { - self.include(cur_path, file.as_ref(), diagnostics, safety_count) - } + continue; } - }, - Err(basic_parser::Status::Fatal(pos, message)) => diagnostics.push(Error { - source: Some(cur_path.to_owned()), - location: Some(pos), - message, - }), - Err(_) => panic!("internal parser error"), - } + let path = resolve_relative(cur_path, path); + let Ok(files) = std::fs::read_dir(&path) else { + diagnostics.push(Error { + source: Some(cur_path.to_owned()), + location: None, + message: format!("cannot open sudoers file {}", path.display()), + }); + continue; + }; + let mut safe_files = files + .filter_map(|direntry| { + let path = direntry.ok()?.path(); + let text = path.file_name()?.to_str()?; + if text.ends_with('~') || text.contains('.') { + None + } else { + Some(path) + } + }) + .collect::>(); + safe_files.sort(); + for file in safe_files { + include(cfg, cur_path, file.as_ref(), diagnostics, safety_count) + } + } + }, + + Err(basic_parser::Status::Fatal(pos, message)) => diagnostics.push(Error { + source: Some(cur_path.to_owned()), + location: Some(pos), + message, + }), + Err(_) => panic!("internal parser error"), } } + } - fn set_default(&mut self, name: String, value: ConfigValue) { - match value { - Flag(value) => { - if value { - self.settings.flags.insert(name); - } else { - self.settings.flags.remove(&name); - } + fn set_default(cfg: &mut Sudoers, name: String, value: ConfigValue) { + match value { + Flag(value) => { + if value { + cfg.settings.flags.insert(name); + } else { + cfg.settings.flags.remove(&name); } - List(mode, values) => { - let slot: &mut _ = self.settings.list.entry(name).or_default(); - match mode { - Mode::Set => *slot = values.into_iter().collect(), - Mode::Add => slot.extend(values), - Mode::Del => { - for key in values { - slot.remove(&key); - } + } + List(mode, values) => { + let slot: &mut _ = cfg.settings.list.entry(name).or_default(); + match mode { + Mode::Set => *slot = values.into_iter().collect(), + Mode::Add => slot.extend(values), + Mode::Del => { + for key in values { + slot.remove(&key); } } } - Text(value) => { - self.settings.str_value.insert(name, value); - } - Enum(value) => { - self.settings.enum_value.insert(name, value); - } - Num(value) => { - self.settings.int_value.insert(name, value); - } + } + Text(value) => { + cfg.settings.str_value.insert(name, value); + } + Enum(value) => { + cfg.settings.enum_value.insert(name, value); + } + Num(value) => { + cfg.settings.int_value.insert(name, value); } } } let mut diagnostics = vec![]; - result.process(path, sudoers, &mut diagnostics, &mut 0); + process(&mut result, path, sudoers, &mut diagnostics, &mut 0); let alias = &mut result.aliases; alias.user.0 = sanitize_alias_table(&alias.user.1, &mut diagnostics); diff --git a/src/system/kernel.rs b/src/system/kernel.rs new file mode 100644 index 000000000..767a78cae --- /dev/null +++ b/src/system/kernel.rs @@ -0,0 +1,37 @@ +use std::ffi::CStr; + +use std::mem::zeroed; + +use crate::common::Error; + +pub fn kernel_check(major: u32, minor: u32) -> Result<(), Error> { + let mut utsname: libc::utsname = unsafe { zeroed() }; + + if unsafe { libc::uname(&mut utsname) } != 0 { + // Could not get the kernel version. Try to run anyway + return Ok(()); + } + + let release = unsafe { CStr::from_ptr(utsname.release.as_ptr()) } + .to_string_lossy() + .into_owned(); + + let version_parts: Vec<&str> = release.split('.').collect(); + + if version_parts.len() < 2 { + // Could not get the kernel version. Try to run anyway + return Ok(()); + } + + // Parse the major and minor version numbers + if let (Ok(major_version), Ok(minor_version)) = ( + version_parts[0].parse::(), + version_parts[1].parse::(), + ) { + if major_version > major || (major_version == major && minor_version >= minor) { + return Ok(()); + } + } + + Err(Error::KernelCheck) +} diff --git a/src/system/mod.rs b/src/system/mod.rs index f90588ce4..9dda04053 100644 --- a/src/system/mod.rs +++ b/src/system/mod.rs @@ -30,6 +30,8 @@ mod audit; // generalized traits for when we want to hide implementations pub mod interface; +pub mod kernel; + pub mod file; pub mod time; @@ -420,8 +422,6 @@ impl User { pub struct Group { pub gid: GroupId, pub name: String, - pub passwd: String, - pub members: Vec, } impl Group { @@ -430,24 +430,9 @@ impl Group { /// In particular the grp.gr_mem pointer is assumed to be non-null, and pointing to a /// null-terminated list; the pointed-to strings are expected to be null-terminated. unsafe fn from_libc(grp: &libc::group) -> Group { - // find out how many members we have - let mut mem_count = 0; - while !(*grp.gr_mem.offset(mem_count)).is_null() { - mem_count += 1; - } - - // convert the members to a slice and then put them into a vec of strings - let mut members = Vec::with_capacity(mem_count as usize); - let mem_slice = std::slice::from_raw_parts(grp.gr_mem, mem_count as usize); - for mem in mem_slice { - members.push(string_from_ptr(*mem)); - } - Group { gid: grp.gr_gid, name: string_from_ptr(grp.gr_name), - passwd: string_from_ptr(grp.gr_passwd), - members, } } @@ -514,9 +499,7 @@ impl WithProcess { pub struct Process { pub pid: ProcessId, pub parent_pid: Option, - pub group_id: ProcessId, pub session_id: ProcessId, - pub name: PathBuf, } impl Default for Process { @@ -530,16 +513,10 @@ impl Process { Process { pid: Self::process_id(), parent_pid: Self::parent_id(), - group_id: Self::group_id(), session_id: Self::session_id(), - name: Self::process_name().unwrap_or_else(|| PathBuf::from("sudo")), } } - pub fn process_name() -> Option { - std::env::args().next().map(PathBuf::from) - } - /// Return the process identifier for the current process pub fn process_id() -> ProcessId { // NOTE libstd casts the `i32` that `libc::getpid` returns into `u32` @@ -559,11 +536,6 @@ impl Process { } } - /// Return the process group id for the current process - pub fn group_id() -> ProcessId { - unsafe { libc::getpgid(0) } - } - /// Get the session id for the current process pub fn session_id() -> ProcessId { unsafe { libc::getsid(0) } @@ -672,6 +644,13 @@ pub fn escape_os_str_lossy(s: &std::ffi::OsStr) -> String { s.to_string_lossy().escape_default().collect() } +pub fn make_zeroed_sigaction() -> libc::sigaction { + // SAFETY: since sigaction is a C struct, all-zeroes is a valid representation + // We cannot use a "literal struct" initialization method since the exact representation + // of libc::sigaction is not fixed, see e.g. https://github.com/trifectatechfoundation/sudo-rs/issues/829 + unsafe { std::mem::zeroed() } +} + #[cfg(test)] mod tests { use std::{ @@ -747,9 +726,7 @@ mod tests { }, Group { name: name.to_string(), - passwd: passwd.to_string(), gid, - members: mem.iter().map(|s| s.to_string()).collect(), } ) } @@ -894,10 +871,3 @@ mod tests { assert_eq!(status.exit_status(), Some(0)); } } - -pub fn make_zeroed_sigaction() -> libc::sigaction { - // SAFETY: since sigaction is a C struct, all-zeroes is a valid representation - // We cannot use a "literal struct" initialization method since the exact representation - // of libc::sigaction is not fixed, see e.g. https://github.com/memorysafety/sudo-rs/issues/829 - unsafe { std::mem::zeroed() } -} diff --git a/src/system/term/mod.rs b/src/system/term/mod.rs index f23cc8b85..57b537507 100644 --- a/src/system/term/mod.rs +++ b/src/system/term/mod.rs @@ -145,6 +145,7 @@ pub(crate) trait Terminal: sealed::Sealed { fn make_controlling_terminal(&self) -> io::Result<()>; fn ttyname(&self) -> io::Result; fn is_terminal(&self) -> bool; + fn tcgetsid(&self) -> io::Result; } impl Terminal for F { @@ -179,6 +180,10 @@ impl Terminal for F { fn is_terminal(&self) -> bool { safe_isatty(self.as_raw_fd()) } + + fn tcgetsid(&self) -> io::Result { + cerr(unsafe { libc::tcgetsid(self.as_raw_fd()) }) + } } /// Try to get the path of the current TTY diff --git a/src/system/timestamp.rs b/src/system/timestamp.rs index c5d5542de..2e6666baf 100644 --- a/src/system/timestamp.rs +++ b/src/system/timestamp.rs @@ -17,20 +17,6 @@ use super::{ Process, WithProcess, }; -/// Truncates or extends the underlying data -pub trait SetLength { - /// After this is called, the underlying data will either be truncated - /// up to new_len bytes, or it will have been extended by zero bytes up to - /// new_len. - fn set_len(&mut self, new_len: usize) -> io::Result<()>; -} - -impl SetLength for File { - fn set_len(&mut self, new_len: usize) -> io::Result<()> { - File::set_len(self, new_len as u64) - } -} - type BoolStorage = u8; const SIZE_OF_TS: i64 = std::mem::size_of::() as i64; @@ -329,6 +315,7 @@ pub enum TouchResult { NotFound, } +#[cfg_attr(not(test), allow(dead_code))] pub enum CreateResult { /// The record was found and it was refreshed Updated { @@ -584,26 +571,6 @@ mod tests { const TEST_USER_ID: UserId = 1000; - impl SetLength for Cursor> { - fn set_len(&mut self, new_len: usize) -> io::Result<()> { - self.get_mut().truncate(new_len); - while self.get_mut().len() < new_len { - self.get_mut().push(0); - } - Ok(()) - } - } - - impl SetLength for Cursor<&mut Vec> { - fn set_len(&mut self, new_len: usize) -> io::Result<()> { - self.get_mut().truncate(new_len); - while self.get_mut().len() < new_len { - self.get_mut().push(0); - } - Ok(()) - } - } - #[test] fn can_encode_and_decode() { let tty_sample = SessionRecord::new( @@ -770,9 +737,10 @@ mod tests { std::thread::sleep(std::time::Duration::from_millis(1)); let res = srf.create(tty_scope, auth_user).unwrap(); - let CreateResult::Updated { .. } = res else { + let CreateResult::Updated { old_time, new_time } = res else { panic!("Expected record to be updated"); }; + assert_ne!(old_time, new_time); // reset the file assert!(srf.reset().is_ok()); diff --git a/src/visudo/mod.rs b/src/visudo/mod.rs index ebe5169c6..4aec4abbd 100644 --- a/src/visudo/mod.rs +++ b/src/visudo/mod.rs @@ -11,6 +11,7 @@ use std::{ }; use crate::{ + sudo::diagnostic, sudoers::Sudoers, system::{ can_execute, @@ -106,9 +107,14 @@ fn check(file_arg: Option<&str>, perms: bool, owner: bool) -> io::Result<()> { return Ok(()); } - let mut stderr = io::stderr(); - for crate::sudoers::Error { message, .. } in errors { - writeln!(stderr, "syntax error: {message}")?; + for crate::sudoers::Error { + message, + source, + location, + } in errors + { + let path = source.as_deref().unwrap_or(sudoers_path); + diagnostic::diagnostic!("syntax error: {message}", path @ location); } Err(io::Error::new(io::ErrorKind::Other, "invalid sudoers file")) @@ -172,6 +178,7 @@ fn run(file_arg: Option<&str>, perms: bool, owner: bool) -> io::Result<()> { .read(true) .write(true) .create(true) + .truncate(true) .open(&tmp_path)?; tmp_file.set_permissions(Permissions::from_mode(0o700))?; @@ -282,6 +289,8 @@ fn edit_sudoers_file( writeln!(stderr, "visudo: {} unchanged", tmp_path.display())?; } else { sudoers_file.write_all(&tmp_contents)?; + let new_size = sudoers_file.stream_position()?; + sudoers_file.set_len(new_size)?; } lock.unlock()?; diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_user.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_user.rs index 61ede4b37..11383c77d 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_user.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/flag_user.rs @@ -108,7 +108,7 @@ fn user_can_become_another_user() -> Result<()> { Ok(()) } -// regression test for memorysafety/sudo-rs#81 +// regression test for trifectatechfoundation/sudo-rs#81 #[test] fn invoking_user_groups_are_lost_when_becoming_another_user() -> Result<()> { let invoking_user = USERNAME; diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs b/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs index a6a3b2c09..b502c32f3 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs @@ -22,7 +22,7 @@ fn ps1_env_var_is_set_when_sudo_ps1_is_set() -> Result<()> { let sudo_env = helpers::parse_env_output(&stdout)?; assert_eq!(Some(ps1), sudo_env.get("PS1").copied()); - assert!(sudo_env.get("SUDO_PS1").is_none()); + assert!(!sudo_env.contains_key("SUDO_PS1")); Ok(()) } @@ -43,8 +43,8 @@ fn ps1_env_var_is_not_set_when_sudo_ps1_is_set_and_flag_login_is_used() -> Resul .stdout()?; let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get("PS1").is_none()); - assert!(sudo_env.get("SUDO_PS1").is_none()); + assert!(!sudo_env.contains_key("PS1")); + assert!(!sudo_env.contains_key("SUDO_PS1")); Ok(()) } @@ -68,7 +68,7 @@ fn can_start_with_parentheses() -> Result<()> { let sudo_env = helpers::parse_env_output(&stdout)?; assert_eq!(Some(ps1), sudo_env.get("PS1").copied()); - assert!(sudo_env.get("SUDO_PS1").is_none()); + assert!(!sudo_env.contains_key("SUDO_PS1")); Ok(()) } diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudoers/env.rs b/test-framework/sudo-compliance-tests/src/sudo/sudoers/env.rs index 1a83be706..847945f08 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/sudoers/env.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/sudoers/env.rs @@ -220,7 +220,7 @@ fn equal_overrides(env_list: EnvList) -> Result<()> { .stdout()?; let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get(env_name2).is_none()); + assert!(!sudo_env.contains_key(env_name2)); assert_eq!(Some(env_val1), sudo_env.get(env_name1).copied()); Ok(()) @@ -412,7 +412,7 @@ fn if_value_starts_with_parentheses_variable_is_removed(env_list: EnvList) -> Re .stdout()?; let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get(env_name).is_none()); + assert!(!sudo_env.contains_key(env_name)); Ok(()) } @@ -525,7 +525,7 @@ fn minus_equal_removes(env_list: EnvList) -> Result<()> { let sudo_env = helpers::parse_env_output(&stdout)?; assert_eq!(Some(env_val1), sudo_env.get(env_name1).copied()); - assert!(sudo_env.get(env_name2).is_none()); + assert!(!sudo_env.contains_key(env_name2)); Ok(()) } @@ -550,7 +550,7 @@ fn minus_equal_an_element_not_in_the_list_is_not_an_error(env_list: EnvList) -> let stdout = output.stdout()?; let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get(env_name).is_none()); + assert!(!sudo_env.contains_key(env_name)); Ok(()) } @@ -576,8 +576,8 @@ fn bang_clears_the_whole_list(env_list: EnvList) -> Result<()> { let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get(env_name1).is_none()); - assert!(sudo_env.get(env_name1).is_none()); + assert!(!sudo_env.contains_key(env_name1)); + assert!(!sudo_env.contains_key(env_name1)); Ok(()) } @@ -604,7 +604,7 @@ fn can_append_after_bang(env_list: EnvList) -> Result<()> { let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get(env_name1).is_none()); + assert!(!sudo_env.contains_key(env_name1)); assert_eq!(Some(env_val2), sudo_env.get(env_name2).copied()); Ok(()) @@ -632,7 +632,7 @@ fn can_override_after_bang(env_list: EnvList) -> Result<()> { let sudo_env = helpers::parse_env_output(&stdout)?; - assert!(sudo_env.get(env_name1).is_none()); + assert!(!sudo_env.contains_key(env_name1)); assert_eq!(Some(env_val2), sudo_env.get(env_name2).copied()); Ok(()) diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudoers/env/keep.rs b/test-framework/sudo-compliance-tests/src/sudo/sudoers/env/keep.rs index d494efc78..dcfa89372 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/sudoers/env/keep.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/sudoers/env/keep.rs @@ -158,7 +158,7 @@ fn equal_can_disable_preservation_of_vars_display_path_but_not_term() -> Result< let sudo_env = helpers::parse_env_output(&stdout)?; // can be disabled - assert!(sudo_env.get("DISPLAY").is_none()); + assert!(!sudo_env.contains_key("DISPLAY")); assert_eq!(Some(SUDO_ENV_DEFAULT_PATH), sudo_env.get("PATH").copied()); // cannot be disabled @@ -190,7 +190,7 @@ fn minus_equal_can_disable_preservation_of_vars_display_path_but_not_term() -> R let sudo_env = helpers::parse_env_output(&stdout)?; // can be disabled - assert!(sudo_env.get("DISPLAY").is_none()); + assert!(!sudo_env.contains_key("DISPLAY")); assert_eq!(Some(SUDO_ENV_DEFAULT_PATH), sudo_env.get("PATH").copied()); // cannot be disabled @@ -218,7 +218,7 @@ fn bang_can_disable_preservation_of_vars_display_path_but_not_term() -> Result<( let sudo_env = helpers::parse_env_output(&stdout)?; // can be disabled - assert!(sudo_env.get("DISPLAY").is_none()); + assert!(!sudo_env.contains_key("DISPLAY")); assert_eq!(Some(SUDO_ENV_DEFAULT_PATH), sudo_env.get("PATH").copied()); // cannot be disabled diff --git a/util/Dockerfile-release b/util/Dockerfile-release index b507f5be5..020bdfc02 100644 --- a/util/Dockerfile-release +++ b/util/Dockerfile-release @@ -1,2 +1,2 @@ -FROM rust:1-slim-bullseye +FROM rust:1.79-slim-bullseye RUN apt-get update -y && apt-get install -y clang libclang-dev libpam0g-dev diff --git a/util/build-release.sh b/util/build-release.sh index 849893563..30ba01130 100755 --- a/util/build-release.sh +++ b/util/build-release.sh @@ -1,6 +1,5 @@ #!/usr/bin/env bash -DATE="2023-09-21" SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd) PROJECT_DIR=$(dirname "$SCRIPT_DIR") SUDO_RS_VERSION="$(cargo metadata --format-version 1 --manifest-path "$PROJECT_DIR/Cargo.toml" | jq '.packages[] | select(.name=="sudo-rs") | .version' -r)" @@ -9,6 +8,9 @@ TARGET_DIR_BASE="$PROJECT_DIR/target/pkg" set -eo pipefail +# Fetch the date from the changelog +DATE=$(grep -m1 '^##' "$PROJECT_DIR"/CHANGELOG.md | grep -o '[0-9]\{4\}-[0-9]\{2\}-[0-9]\{2\}') + # Build binaries docker build --pull --tag "$BUILDER_IMAGE_TAG" --file "$SCRIPT_DIR/Dockerfile-release" "$SCRIPT_DIR" docker run --rm --user "$(id -u):$(id -g)" -v "$PROJECT_DIR:/build" -w "/build" "$BUILDER_IMAGE_TAG" cargo clean diff --git a/util/update-version.sh b/util/update-version.sh index a8b79f298..bbce9d1f6 100755 --- a/util/update-version.sh +++ b/util/update-version.sh @@ -1,16 +1,32 @@ #!/usr/bin/env bash -if [ "$#" -lt 1 ]; then - echo "Missing new version" - exit 1 -fi - SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd) PROJECT_DIR=$(dirname "$SCRIPT_DIR") NEW_VERSION="$1" -echo "Updating version in Cargo.toml" -sed -i 's/^version\s*=\s*".*"/version = "'"$NEW_VERSION"'"/' "$PROJECT_DIR/Cargo.toml" +# Fetch current version +CURRENT_VERSION=$(sed -n '/version\s*=\s*"\([^"]*\)"/{s//\1/p;q}' "$PROJECT_DIR/Cargo.toml") + +# Fetch new version from changelog +NEW_VERSION=$(grep -m1 '^##' "$PROJECT_DIR"/CHANGELOG.md | grep -o "\[[0-9]\+.[0-9]\+.[0-9]\+\]" | tr -d '[]') + +if [ -z "$NEW_VERSION" ]; then + echo "Could not fetch version from CHANGELOG.md; you probably made a mistake." + exit 1 +fi + +if [ "$CURRENT_VERSION" \> "$NEW_VERSION" ]; then + echo "New version number must be higher than current version: $CURRENT_VERSION" + echo "Create a new changelog entry before running this script!" + exit 1 +fi + +if [ "$CURRENT_VERSION" == "$NEW_VERSION" ]; then + echo "Cargo.toml was already at $NEW_VERSION" +else + echo "Updating version in Cargo.toml to $NEW_VERSION" + sed -i 's/^version\s*=\s*".*"/version = "'"$NEW_VERSION"'"/' "$PROJECT_DIR/Cargo.toml" +fi echo "Updating version in man pages" sed -i 's/^title: SU(1) sudo-rs .*/title: SU(1) sudo-rs '"$NEW_VERSION"' | sudo-rs/' "$PROJECT_DIR"/docs/man/su.1.md @@ -19,5 +35,3 @@ sed -i 's/^title: VISUDO(8) sudo-rs .*/title: VISUDO(8) sudo-rs '"$NEW_VERSION"' echo "Rebuilding project" (cd $PROJECT_DIR && cargo build --release) - -echo "Version changes complete, you must still fill in the changelog entries"