diff --git a/kernel/src/process/credentials/credentials_.rs b/kernel/src/process/credentials/credentials_.rs index f8b51c285..69d360578 100644 --- a/kernel/src/process/credentials/credentials_.rs +++ b/kernel/src/process/credentials/credentials_.rs @@ -76,10 +76,6 @@ impl Credentials_ { } } - fn is_privileged(&self) -> bool { - self.euid.is_root() - } - // ******* Uid methods ******* pub(super) fn ruid(&self) -> Uid { @@ -98,39 +94,25 @@ impl Credentials_ { self.fsuid.load(Ordering::Relaxed) } - pub(super) fn set_uid(&self, uid: Uid) { - if self.is_privileged() { + pub(super) fn set_uid(&self, uid: Uid) -> Result<()> { + if self.effective_capset().contains(CapSet::SETUID) { self.set_resuid_unchecked(Some(uid), Some(uid), Some(uid)); + Ok(()) } else { - // Unprivileged processes can only switch between ruid, euid, suid - if uid != self.ruid.load(Ordering::Relaxed) - && uid != self.euid.load(Ordering::Relaxed) - && uid != self.suid.load(Ordering::Relaxed) - { - // No permission to set to this UID - return; - } - self.set_resuid_unchecked(None, Some(uid), None) + self.set_resuid(None, Some(uid), None) } - - self.set_fsuid_unchecked(uid) } pub(super) fn set_reuid(&self, ruid: Option, euid: Option) -> Result<()> { self.check_uid_perm(ruid.as_ref(), euid.as_ref(), None, false)?; let should_set_suid = ruid.is_some() || euid.is_some_and(|euid| euid != self.ruid()); - - self.set_resuid_unchecked(ruid, euid, None); - - if should_set_suid { - self.suid.store(self.euid(), Ordering::Release); - } - - // FIXME: should we set fsuid here? The linux document for syscall `setfsuid` is contradictory - // with the document of syscall `setreuid`. The `setfsuid` document says the `fsuid` is always - // the same as `euid`, but `setreuid` does not mention the `fsuid` should be set. - self.set_fsuid_unchecked(self.euid()); + let suid = if should_set_suid { + Some(euid.unwrap_or_else(|| self.euid())) + } else { + None + }; + self.set_resuid_unchecked(ruid, euid, suid); Ok(()) } @@ -145,28 +127,24 @@ impl Credentials_ { self.set_resuid_unchecked(ruid, euid, suid); - self.set_fsuid_unchecked(self.euid()); - Ok(()) } - pub(super) fn set_fsuid(&self, fsuid: Option) -> Result { + pub(super) fn set_fsuid(&self, fsuid: Option) -> core::result::Result { let old_fsuid = self.fsuid(); let Some(fsuid) = fsuid else { return Ok(old_fsuid); }; - if self.is_privileged() { - self.fsuid.store(fsuid, Ordering::Release); + if self.effective_capset().contains(CapSet::SETUID) { + self.set_fsuid_unchecked(fsuid); return Ok(old_fsuid); } if fsuid != self.ruid() && fsuid != self.euid() && fsuid != self.suid() { - return_errno_with_message!( - Errno::EPERM, - "fsuid can only be one of old ruid, old euid and old suid." - ) + // The new filesystem UID is not one of the associated UIDs. + return Err(old_fsuid); } self.set_fsuid_unchecked(fsuid); @@ -191,7 +169,7 @@ impl Credentials_ { suid: Option<&Uid>, ruid_may_be_old_suid: bool, ) -> Result<()> { - if self.is_privileged() { + if self.effective_capset().contains(CapSet::SETUID) { return Ok(()); } @@ -257,45 +235,43 @@ impl Credentials_ { old_suid }; + self.set_fsuid_unchecked(new_euid); + // If the `SECBIT_NO_SETUID_FIXUP` bit is set, do not adjust capabilities. // Reference: The "SECBIT_NO_SETUID_FIXUP" section in - // https://man7.org/linux/man-pages/man7/capabilities.7.html + // . if self.securebits().no_setuid_fixup() { return; } // Begin to adjust capabilities. // Reference: The "Effect of user ID changes on capabilities" section in - // https://man7.org/linux/man-pages/man7/capabilities.7.html + // . + let had_root = old_ruid.is_root() || old_euid.is_root() || old_suid.is_root(); let all_nonroot = !new_ruid.is_root() && !new_euid.is_root() && !new_suid.is_root(); + if had_root && all_nonroot && !self.keep_capabilities() { + self.set_permitted_capset(CapSet::empty()); + self.set_inheritable_capset(CapSet::empty()); + // TODO: Clear ambient capabilities when we support it. Note that ambient capabilities + // should be cleared even if `keep_capabilities` is true. + } - if had_root && all_nonroot { - if !self.keep_capabilities() { - self.set_permitted_capset(CapSet::empty()); - self.set_inheritable_capset(CapSet::empty()); - // TODO: Also need to clear ambient capabilities when we support it - } - + if old_euid.is_root() && !new_euid.is_root() { self.set_effective_capset(CapSet::empty()); - } else { - if old_euid.is_root() && !new_euid.is_root() { - self.set_effective_capset(CapSet::empty()); - } - - if !old_euid.is_root() && new_euid.is_root() { - let permitted = self.permitted_capset(); - self.set_effective_capset(permitted); - } + } else if !old_euid.is_root() && new_euid.is_root() { + let permitted = self.permitted_capset(); + self.set_effective_capset(permitted); } } fn set_fsuid_unchecked(&self, fsuid: Uid) { - let old_uid = self.fsuid.swap(fsuid, Ordering::Relaxed); + let old_fsuid = self.fsuid(); + self.fsuid.store(fsuid, Ordering::Relaxed); - if old_uid.is_root() && !fsuid.is_root() { + if old_fsuid.is_root() && !fsuid.is_root() { // Reference: The "Effect of user ID changes on capabilities" section in - // https://man7.org/linux/man-pages/man7/capabilities.7.html + // . let cap_to_remove = CapSet::CHOWN | CapSet::DAC_OVERRIDE | CapSet::FOWNER @@ -327,15 +303,12 @@ impl Credentials_ { self.fsgid.load(Ordering::Relaxed) } - pub(super) fn set_gid(&self, gid: Gid) { - if self.is_privileged() { - self.rgid.store(gid, Ordering::Relaxed); - self.egid.store(gid, Ordering::Relaxed); - self.sgid.store(gid, Ordering::Relaxed); - self.fsgid.store(gid, Ordering::Relaxed); + pub(super) fn set_gid(&self, gid: Gid) -> Result<()> { + if self.effective_capset().contains(CapSet::SETGID) { + self.set_resgid_unchecked(Some(gid), Some(gid), Some(gid)); + Ok(()) } else { - self.egid.store(gid, Ordering::Relaxed); - self.fsgid.store(gid, Ordering::Relaxed); + self.set_resgid(None, Some(gid), None) } } @@ -343,14 +316,12 @@ impl Credentials_ { self.check_gid_perm(rgid.as_ref(), egid.as_ref(), None, false)?; let should_set_sgid = rgid.is_some() || egid.is_some_and(|egid| egid != self.rgid()); - - self.set_resgid_unchecked(rgid, egid, None); - - if should_set_sgid { - self.sgid.store(self.egid(), Ordering::Relaxed); - } - - self.fsgid.store(self.egid(), Ordering::Relaxed); + let sgid = if should_set_sgid { + Some(egid.unwrap_or_else(|| self.egid())) + } else { + None + }; + self.set_resgid_unchecked(rgid, egid, sgid); Ok(()) } @@ -365,41 +336,41 @@ impl Credentials_ { self.set_resgid_unchecked(rgid, egid, sgid); - self.fsgid.store(self.egid(), Ordering::Relaxed); - Ok(()) } - pub(super) fn set_fsgid(&self, fsgid: Option) -> Result { + pub(super) fn set_fsgid(&self, fsgid: Option) -> core::result::Result { let old_fsgid = self.fsgid(); let Some(fsgid) = fsgid else { return Ok(old_fsgid); }; - if self.is_privileged() { - self.fsgid.store(fsgid, Ordering::Relaxed); + if fsgid == old_fsgid { + return Ok(old_fsgid); + } + + if self.effective_capset().contains(CapSet::SETGID) { + self.set_fsgid_unchecked(fsgid); return Ok(old_fsgid); } if fsgid != self.rgid() && fsgid != self.egid() && fsgid != self.sgid() { - return_errno_with_message!( - Errno::EPERM, - "fsuid can only be one of old ruid, old euid and old suid." - ) + // The new filesystem GID is not one of the associated GIDs. + return Err(old_fsgid); } - self.fsgid.store(fsgid, Ordering::Relaxed); + self.set_fsgid_unchecked(fsgid); Ok(old_fsgid) } pub(super) fn set_egid(&self, egid: Gid) { - self.egid.store(egid, Ordering::Relaxed); + self.set_resgid_unchecked(None, Some(egid), None); } pub(super) fn set_sgid(&self, sgid: Gid) { - self.sgid.store(sgid, Ordering::Relaxed); + self.set_resgid_unchecked(None, None, Some(sgid)); } // For `setregid`, rgid can *NOT* be set to old sgid, @@ -411,7 +382,7 @@ impl Credentials_ { sgid: Option<&Gid>, rgid_may_be_old_sgid: bool, ) -> Result<()> { - if self.is_privileged() { + if self.effective_capset().contains(CapSet::SETGID) { return Ok(()); } @@ -463,6 +434,12 @@ impl Credentials_ { if let Some(sgid) = sgid { self.sgid.store(sgid, Ordering::Relaxed); } + + self.set_fsgid_unchecked(self.egid()); + } + + fn set_fsgid_unchecked(&self, fsuid: Gid) { + self.fsgid.store(fsuid, Ordering::Relaxed); } // ******* Supplementary groups methods ******* diff --git a/kernel/src/process/credentials/static_cap.rs b/kernel/src/process/credentials/static_cap.rs index 6a2bee6a1..1b5d59fa6 100644 --- a/kernel/src/process/credentials/static_cap.rs +++ b/kernel/src/process/credentials/static_cap.rs @@ -82,8 +82,8 @@ impl Credentials { /// /// This method requires the `Write` right. #[require(R > Write)] - pub fn set_uid(&self, uid: Uid) { - self.0.set_uid(uid); + pub fn set_uid(&self, uid: Uid) -> Result<()> { + self.0.set_uid(uid) } /// Sets real, effective user ids as `ruid`, `euid` respectively. if `ruid` or `euid` @@ -114,7 +114,7 @@ impl Credentials { /// /// This method requires the `Write` right. #[require(R > Write)] - pub fn set_fsuid(&self, fsuid: Option) -> Result { + pub fn set_fsuid(&self, fsuid: Option) -> core::result::Result { self.0.set_fsuid(fsuid) } @@ -176,8 +176,8 @@ impl Credentials { /// /// This method requires the `Write` right. #[require(R > Write)] - pub fn set_gid(&self, gid: Gid) { - self.0.set_gid(gid); + pub fn set_gid(&self, gid: Gid) -> Result<()> { + self.0.set_gid(gid) } /// Sets real, effective group ids as `rgid`, `egid` respectively. if `rgid` or `egid` @@ -208,7 +208,7 @@ impl Credentials { /// /// This method requires the `Write` right. #[require(R > Write)] - pub fn set_fsgid(&self, fsgid: Option) -> Result { + pub fn set_fsgid(&self, fsgid: Option) -> core::result::Result { self.0.set_fsgid(fsgid) } diff --git a/kernel/src/process/credentials/user.rs b/kernel/src/process/credentials/user.rs index 613952154..809da2495 100644 --- a/kernel/src/process/credentials/user.rs +++ b/kernel/src/process/credentials/user.rs @@ -56,14 +56,8 @@ define_atomic_version_of_integer_like_type!(Uid, { pub(super) struct AtomicUid(AtomicU32); }); -impl AtomicUid { - pub fn is_root(&self) -> bool { - self.load(Ordering::Acquire).is_root() - } -} - impl Clone for AtomicUid { fn clone(&self) -> Self { - Self::new(self.load(Ordering::Acquire)) + Self::new(self.load(Ordering::Relaxed)) } } diff --git a/kernel/src/syscall/setfsgid.rs b/kernel/src/syscall/setfsgid.rs index a49483746..2fd26151a 100644 --- a/kernel/src/syscall/setfsgid.rs +++ b/kernel/src/syscall/setfsgid.rs @@ -4,17 +4,19 @@ use super::SyscallReturn; use crate::{prelude::*, process::Gid}; pub fn sys_setfsgid(gid: i32, ctx: &Context) -> Result { - debug!("gid = {}", gid); - - let fsgid = if gid < 0 { - None + let fsgid = if gid >= 0 { + Some(Gid::new(gid.cast_unsigned())) } else { - Some(Gid::new(gid as u32)) + None }; + debug!("fsgid = {:?}", fsgid); + let old_fsgid = { let credentials = ctx.posix_thread.credentials_mut(); - credentials.set_fsgid(fsgid)? + credentials + .set_fsgid(fsgid) + .unwrap_or_else(|old_fsgid| old_fsgid) }; Ok(SyscallReturn::Return( diff --git a/kernel/src/syscall/setfsuid.rs b/kernel/src/syscall/setfsuid.rs index 22067ad76..bbf1c4edd 100644 --- a/kernel/src/syscall/setfsuid.rs +++ b/kernel/src/syscall/setfsuid.rs @@ -4,17 +4,19 @@ use super::SyscallReturn; use crate::{prelude::*, process::Uid}; pub fn sys_setfsuid(uid: i32, ctx: &Context) -> Result { - debug!("uid = {}", uid); - - let fsuid = if uid < 0 { - None + let fsuid = if uid >= 0 { + Some(Uid::new(uid.cast_unsigned())) } else { - Some(Uid::new(uid as u32)) + None }; + debug!("fsuid = {:?}", fsuid); + let old_fsuid = { let credentials = ctx.posix_thread.credentials_mut(); - credentials.set_fsuid(fsuid)? + credentials + .set_fsuid(fsuid) + .unwrap_or_else(|old_fsuid| old_fsuid) }; Ok(SyscallReturn::Return( diff --git a/kernel/src/syscall/setgid.rs b/kernel/src/syscall/setgid.rs index 03ca2239c..418987591 100644 --- a/kernel/src/syscall/setgid.rs +++ b/kernel/src/syscall/setgid.rs @@ -4,16 +4,15 @@ use super::SyscallReturn; use crate::{prelude::*, process::Gid}; pub fn sys_setgid(gid: i32, ctx: &Context) -> Result { - debug!("gid = {}", gid); - if gid < 0 { - return_errno_with_message!(Errno::EINVAL, "gid cannot be negative"); + return_errno_with_message!(Errno::EINVAL, "GIDs cannot be negative"); } - let gid = Gid::new(gid as u32); + let gid = Gid::new(gid.cast_unsigned()); + debug!("gid = {:?}", gid); let credentials = ctx.posix_thread.credentials_mut(); - credentials.set_gid(gid); + credentials.set_gid(gid)?; Ok(SyscallReturn::Return(0)) } diff --git a/kernel/src/syscall/setregid.rs b/kernel/src/syscall/setregid.rs index 3e2fc1acd..71a605c3e 100644 --- a/kernel/src/syscall/setregid.rs +++ b/kernel/src/syscall/setregid.rs @@ -4,20 +4,20 @@ use super::SyscallReturn; use crate::{prelude::*, process::Gid}; pub fn sys_setregid(rgid: i32, egid: i32, ctx: &Context) -> Result { - debug!("rgid = {}, egid = {}", rgid, egid); - - let rgid = if rgid > 0 { - Some(Gid::new(rgid as u32)) + let rgid = if rgid >= 0 { + Some(Gid::new(rgid.cast_unsigned())) } else { None }; - let egid = if egid > 0 { - Some(Gid::new(egid as u32)) + let egid = if egid >= 0 { + Some(Gid::new(egid.cast_unsigned())) } else { None }; + debug!("rgid = {:?}, egid = {:?}", rgid, egid); + let credentials = ctx.posix_thread.credentials_mut(); credentials.set_regid(rgid, egid)?; diff --git a/kernel/src/syscall/setresgid.rs b/kernel/src/syscall/setresgid.rs index 941acd533..009b9a3d4 100644 --- a/kernel/src/syscall/setresgid.rs +++ b/kernel/src/syscall/setresgid.rs @@ -4,20 +4,20 @@ use super::SyscallReturn; use crate::{prelude::*, process::Gid}; pub fn sys_setresgid(rgid: i32, egid: i32, sgid: i32, ctx: &Context) -> Result { - let rgid = if rgid > 0 { - Some(Gid::new(rgid as u32)) + let rgid = if rgid >= 0 { + Some(Gid::new(rgid.cast_unsigned())) } else { None }; - let egid = if egid > 0 { - Some(Gid::new(egid as u32)) + let egid = if egid >= 0 { + Some(Gid::new(egid.cast_unsigned())) } else { None }; - let sgid = if sgid > 0 { - Some(Gid::new(sgid as u32)) + let sgid = if sgid >= 0 { + Some(Gid::new(sgid.cast_unsigned())) } else { None }; diff --git a/kernel/src/syscall/setresuid.rs b/kernel/src/syscall/setresuid.rs index b2835a692..d42946775 100644 --- a/kernel/src/syscall/setresuid.rs +++ b/kernel/src/syscall/setresuid.rs @@ -4,20 +4,20 @@ use super::SyscallReturn; use crate::{prelude::*, process::Uid}; pub fn sys_setresuid(ruid: i32, euid: i32, suid: i32, ctx: &Context) -> Result { - let ruid = if ruid > 0 { - Some(Uid::new(ruid as u32)) + let ruid = if ruid >= 0 { + Some(Uid::new(ruid.cast_unsigned())) } else { None }; - let euid = if euid > 0 { - Some(Uid::new(euid as u32)) + let euid = if euid >= 0 { + Some(Uid::new(euid.cast_unsigned())) } else { None }; - let suid = if suid > 0 { - Some(Uid::new(suid as u32)) + let suid = if suid >= 0 { + Some(Uid::new(suid.cast_unsigned())) } else { None }; @@ -25,7 +25,6 @@ pub fn sys_setresuid(ruid: i32, euid: i32, suid: i32, ctx: &Context) -> Result Result { - debug!("ruid = {}, euid = {}", ruid, euid); - - let ruid = if ruid > 0 { - Some(Uid::new(ruid as u32)) + let ruid = if ruid >= 0 { + Some(Uid::new(ruid.cast_unsigned())) } else { None }; - let euid = if euid > 0 { - Some(Uid::new(euid as u32)) + let euid = if euid >= 0 { + Some(Uid::new(euid.cast_unsigned())) } else { None }; + debug!("ruid = {:?}, euid = {:?}", ruid, euid); + let credentials = ctx.posix_thread.credentials_mut(); credentials.set_reuid(ruid, euid)?; diff --git a/kernel/src/syscall/setuid.rs b/kernel/src/syscall/setuid.rs index dfc16f6f9..4ffb46d6d 100644 --- a/kernel/src/syscall/setuid.rs +++ b/kernel/src/syscall/setuid.rs @@ -4,16 +4,15 @@ use super::SyscallReturn; use crate::{prelude::*, process::Uid}; pub fn sys_setuid(uid: i32, ctx: &Context) -> Result { - debug!("uid = {}", uid); - if uid < 0 { - return_errno_with_message!(Errno::EINVAL, "uid cannot be negative"); + return_errno_with_message!(Errno::EINVAL, "UIDs cannot be negative"); } - let uid = Uid::new(uid as u32); + let uid = Uid::new(uid.cast_unsigned()); + debug!("uid = {:?}", uid); let credentials = ctx.posix_thread.credentials_mut(); - credentials.set_uid(uid); + credentials.set_uid(uid)?; Ok(SyscallReturn::Return(0)) }