Secure open function for sudoedit (#1200)

Author: squell

src/system/audit.rs

@@ -1,8 +1,15 @@
+use std::ffi::{CStr, CString};
 use std::fs::{DirBuilder, File, Metadata, OpenOptions};
 use std::io::{self, Error, ErrorKind};
-use std::os::unix::fs::{DirBuilderExt, MetadataExt, PermissionsExt};
-use std::os::unix::prelude::OpenOptionsExt;
-use std::path::Path;
+use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, OwnedFd};
+use std::os::unix::{
+    ffi::OsStrExt,
+    fs::{DirBuilderExt, MetadataExt, PermissionsExt},
+    prelude::OpenOptionsExt,
+};
+use std::path::{Component, Path};
+
+use super::{cerr, User};
 
 // of course we can also write "file & 0o040 != 0", but this makes the intent explicit
 enum Op {
@@ -20,19 +27,23 @@ fn mode(who: Category, what: Op) -> u32 {
     (what as u32) << (3 * who as u32)
 }
 
+/// Open sudo configuration using various security checks
 pub fn secure_open(path: impl AsRef<Path>, check_parent_dir: bool) -> io::Result<File> {
     let mut open_options = OpenOptions::new();
     open_options.read(true);
+
     secure_open_impl(path.as_ref(), &mut open_options, check_parent_dir, false)
 }
 
+/// Open a timestamp cookie file using various security checks
 pub fn secure_open_cookie_file(path: impl AsRef<Path>) -> io::Result<File> {
     let mut open_options = OpenOptions::new();
     open_options
         .read(true)
         .write(true)
         .create(true)
         .mode(mode(Category::Owner, Op::Write) | mode(Category::Owner, Op::Read));
+
     secure_open_impl(path.as_ref(), &mut open_options, true, true)
 }
 
@@ -103,6 +114,93 @@ fn secure_open_impl(
     Ok(file)
 }
 
+#[cfg_attr(not(feature = "sudoedit"), allow(dead_code))]
+fn open_at(parent: BorrowedFd, file_name: &CStr, create: bool) -> io::Result<OwnedFd> {
+    let flags = if create {
+        libc::O_NOFOLLOW | libc::O_RDWR | libc::O_CREAT
+    } else {
+        libc::O_NOFOLLOW | libc::O_RDONLY
+    };
+
+    // the mode for files that are created is hardcoded, as it is in ogsudo
+    let mode = libc::S_IRUSR | libc::S_IWUSR | libc::S_IRGRP | libc::S_IROTH;
+
+    // SAFETY: by design, a correct CStr pointer is passed to openat; only if this call succeeds
+    // is the file descriptor it returns (which is then necessarily valid) passed to from_raw_fd
+    unsafe {
+        let fd = cerr(libc::openat(
+            parent.as_raw_fd(),
+            file_name.as_ptr(),
+            flags,
+            mode,
+        ))?;
+
+        Ok(OwnedFd::from_raw_fd(fd))
+    }
+}
+
+/// This opens a file making sure that
+/// - no directory leading up to the file is editable by the user
+/// - no components are a symbolic link
+#[cfg_attr(not(feature = "sudoedit"), allow(dead_code))]
+fn traversed_secure_open(path: impl AsRef<Path>, user: &User) -> io::Result<File> {
+    let path = path.as_ref();
+
+    let Some(file_name) = path.file_name() else {
+        return Err(io::Error::new(ErrorKind::InvalidInput, "invalid path"));
+    };
+
+    let mut components = path.parent().unwrap_or(Path::new("")).components();
+    if components.next() != Some(Component::RootDir) {
+        return Err(io::Error::new(
+            ErrorKind::InvalidInput,
+            "path must be absolute",
+        ));
+    }
+
+    let user_cannot_write = |file: &File| -> io::Result<()> {
+        let meta = file.metadata()?;
+        let perms = meta.permissions().mode();
+
+        if perms & mode(Category::World, Op::Write) != 0
+            || (perms & mode(Category::Group, Op::Write) != 0) && user.gid.inner() == meta.gid()
+            || (perms & mode(Category::Owner, Op::Write) != 0) && user.uid.inner() == meta.uid()
+        {
+            Err(io::Error::new(
+                ErrorKind::PermissionDenied,
+                "cannot open a file in a path writeable by the user",
+            ))
+        } else {
+            Ok(())
+        }
+    };
+
+    let mut cur = File::open("/")?;
+    user_cannot_write(&cur)?;
+
+    for component in components {
+        let dir: CString = match component {
+            Component::Normal(dir) => CString::new(dir.as_bytes())?,
+            Component::CurDir => cstr!(".").to_owned(),
+            Component::ParentDir => cstr!("..").to_owned(),
+            _ => {
+                return Err(io::Error::new(
+                    ErrorKind::InvalidInput,
+                    "error in provided path",
+                ))
+            }
+        };
+
+        cur = open_at(cur.as_fd(), &dir, false)?.into();
+        user_cannot_write(&cur)?;
+    }
+
+    cur = open_at(cur.as_fd(), &CString::new(file_name.as_bytes())?, true)?.into();
+    user_cannot_write(&cur)?;
+
+    Ok(cur)
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -135,4 +233,50 @@ mod test {
     fn test_secure_open_cookie_file() {
         assert!(secure_open_cookie_file("/etc/hosts").is_err());
     }
+
+    #[test]
+    fn test_traverse_secure_open_negative() {
+        use crate::common::resolve::CurrentUser;
+
+        let root = User::from_name(cstr!("root")).unwrap().unwrap();
+        let user = CurrentUser::resolve().unwrap();
+
+        // not allowed -- invalid
+        assert!(traversed_secure_open("/", &root).is_err());
+        // not allowed since the path is not absolute
+        assert!(traversed_secure_open("./hello.txt", &root).is_err());
+        // not allowed since root can write to "/"
+        assert!(traversed_secure_open("/hello.txt", &root).is_err());
+        // not allowed since "/tmp" is a directory
+        assert!(traversed_secure_open("/tmp", &user).is_err());
+        // not allowed since anybody can write to "/tmp"
+        assert!(traversed_secure_open("/tmp/foo/hello.txt", &user).is_err());
+        // not allowed since "/bin" is a symlink
+        assert!(traversed_secure_open("/bin/hello.txt", &user).is_err());
+    }
+
+    #[test]
+    fn test_traverse_secure_open_positive() {
+        use crate::common::resolve::CurrentUser;
+        use crate::system::{GroupId, UserId};
+
+        let other_user = CurrentUser::fake(User {
+            uid: UserId::new(1042),
+            gid: GroupId::new(1042),
+
+            name: "test".into(),
+            home: "/home/test".into(),
+            shell: "/bin/sh".into(),
+            groups: vec![],
+        });
+
+        // allowed!
+        let path = std::env::current_dir()
+            .unwrap()
+            .join("sudo-rs-test-file.txt");
+        let file = traversed_secure_open(&path, &other_user).unwrap();
+        if file.metadata().is_ok_and(|meta| meta.len() == 0) {
+            std::fs::remove_file(path).unwrap();
+        }
+    }
 }