From 3249f91beff7fe41fe20eb54ffddf1a4345c31f5 Mon Sep 17 00:00:00 2001 From: Yuekai Jia <equation618@gmail.com> Date: Wed, 5 Apr 2023 00:56:47 +0800 Subject: [PATCH] fs: add permission checks --- Cargo.lock | 9 ++++ Cargo.toml | 1 + crates/axfs_vfs/src/structs.rs | 12 +++++ crates/capability/Cargo.toml | 11 +++++ crates/capability/src/lib.rs | 71 +++++++++++++++++++++++++++++ modules/axfs/Cargo.toml | 1 + modules/axfs/src/fops.rs | 81 +++++++++++++++++++++++++-------- modules/axfs/src/fs/fatfs.rs | 14 ++++-- modules/axfs/src/root.rs | 7 +++ modules/axfs/tests/test_axfs.rs | 50 +++++++++++++++++++- 10 files changed, 235 insertions(+), 22 deletions(-) create mode 100644 crates/capability/Cargo.toml create mode 100644 crates/capability/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 9b627d7..1c6dd16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -206,6 +206,7 @@ dependencies = [ "axio", "axsync", "axtask", + "capability", "cfg-if", "driver_block", "driver_common", @@ -393,6 +394,14 @@ version = "1.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610" +[[package]] +name = "capability" +version = "0.1.0" +dependencies = [ + "axerrno", + "bitflags 2.0.2", +] + [[package]] name = "cbindgen" version = "0.24.3" diff --git a/Cargo.toml b/Cargo.toml index 1885cc7..253aad0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ members = [ "crates/axfs_devfs", "crates/axfs_vfs", "crates/axio", + "crates/capability", "crates/crate_interface", "crates/driver_block", "crates/driver_common", diff --git a/crates/axfs_vfs/src/structs.rs b/crates/axfs_vfs/src/structs.rs index c2f9af6..923ce6c 100644 --- a/crates/axfs_vfs/src/structs.rs +++ b/crates/axfs_vfs/src/structs.rs @@ -107,6 +107,18 @@ impl VfsNodePerm { } perm } + + pub const fn owner_readable(&self) -> bool { + self.contains(Self::OWNER_READ) + } + + pub const fn owner_writable(&self) -> bool { + self.contains(Self::OWNER_WRITE) + } + + pub const fn owner_executable(&self) -> bool { + self.contains(Self::OWNER_EXEC) + } } impl VfsNodeType { diff --git a/crates/capability/Cargo.toml b/crates/capability/Cargo.toml new file mode 100644 index 0000000..902c490 --- /dev/null +++ b/crates/capability/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "capability" +version = "0.1.0" +edition = "2021" +authors = ["Yuekai Jia <equation618@gmail.com>"] + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +bitflags = "2.0" +axerrno = { path = "../axerrno" } diff --git a/crates/capability/src/lib.rs b/crates/capability/src/lib.rs new file mode 100644 index 0000000..aaf47d0 --- /dev/null +++ b/crates/capability/src/lib.rs @@ -0,0 +1,71 @@ +#![no_std] +#![feature(const_trait_impl)] + +bitflags::bitflags! { + /// Capabilities. + #[derive(Default, Debug, Clone, Copy)] + pub struct Cap: u32 { + const READ = 1 << 0; + const WRITE = 1 << 1; + const EXECUTE = 1 << 2; + } +} + +#[derive(Debug)] +pub struct CapError; + +/// A wrapper that holds a value with a capability. +pub struct WithCap<T> { + inner: T, + cap: Cap, +} + +impl<T> WithCap<T> { + /// Create a new instance with the given capability. + pub fn new(inner: T, cap: Cap) -> Self { + Self { inner, cap } + } + + /// Get the capability. + pub const fn cap(&self) -> Cap { + self.cap + } + + /// Check if the inner data can be accessed with the given capability. + pub const fn can_access(&self, cap: Cap) -> bool { + self.cap.contains(cap) + } + + /// # Safety + /// + /// Caller must ensure not to violate the capability. + pub unsafe fn access_unchecked(&self) -> &T { + &self.inner + } + + /// Access the inner value with the given capability, or return `CapError` + /// if cannot access. + pub const fn access(&self, cap: Cap) -> Result<&T, CapError> { + if self.can_access(cap) { + Ok(&self.inner) + } else { + Err(CapError) + } + } + + /// Access the inner value with the given capability, or return the given + /// `err` if cannot access. + pub fn access_or_err<E>(&self, cap: Cap, err: E) -> Result<&T, E> { + if self.can_access(cap) { + Ok(&self.inner) + } else { + Err(err) + } + } +} + +impl const From<CapError> for axerrno::AxError { + fn from(_: CapError) -> Self { + Self::PermissionDenied + } +} diff --git a/modules/axfs/Cargo.toml b/modules/axfs/Cargo.toml index 186e738..7bede73 100644 --- a/modules/axfs/Cargo.toml +++ b/modules/axfs/Cargo.toml @@ -20,6 +20,7 @@ default = ["use-ramdisk", "devfs", "ramfs", "fatfs"] log = "0.4" cfg-if = "1.0" lazy_init = { path = "../../crates/lazy_init" } +capability = { path = "../../crates/capability" } driver_common = { path = "../../crates/driver_common" } driver_block = { path = "../../crates/driver_block" } axio = { path = "../../crates/axio", features = ["alloc"] } diff --git a/modules/axfs/src/fops.rs b/modules/axfs/src/fops.rs index b91ff89..de1ac6f 100644 --- a/modules/axfs/src/fops.rs +++ b/modules/axfs/src/fops.rs @@ -2,6 +2,7 @@ use axerrno::{ax_err, AxResult}; use axfs_vfs::{VfsError, VfsNodeRef}; +use capability::{Cap, WithCap}; use core::fmt; pub type FileType = axfs_vfs::VfsNodeType; @@ -10,13 +11,13 @@ pub type FileAttr = axfs_vfs::VfsNodeAttr; pub type FilePerm = axfs_vfs::VfsNodePerm; pub struct File { - node: VfsNodeRef, + node: WithCap<VfsNodeRef>, is_append: bool, offset: u64, } pub struct Directory { - node: VfsNodeRef, + node: WithCap<VfsNodeRef>, entry_idx: usize, } @@ -121,45 +122,51 @@ impl File { { return ax_err!(IsADirectory); } + let access_cap = opts.into(); + if !perm_to_cap(attr.perm()).contains(access_cap) { + return ax_err!(PermissionDenied); + } + node.open()?; if opts.truncate { node.truncate(0)?; } - Ok(Self { - node, + node: WithCap::new(node, access_cap), is_append: opts.append, offset: 0, }) } pub fn truncate(&self, size: u64) -> AxResult { - self.node.truncate(size)?; + self.node.access(Cap::WRITE)?.truncate(size)?; Ok(()) } pub fn read(&mut self, buf: &mut [u8]) -> AxResult<usize> { - let read_len = self.node.read_at(self.offset, buf)?; + let node = self.node.access(Cap::READ)?; + let read_len = node.read_at(self.offset, buf)?; self.offset += read_len as u64; Ok(read_len) } pub fn write(&mut self, buf: &[u8]) -> AxResult<usize> { + let node = self.node.access(Cap::WRITE)?; if self.is_append { - self.offset = self.node.get_attr()?.size(); + self.offset = self.get_attr()?.size(); }; - let write_len = self.node.write_at(self.offset, buf)?; + let write_len = node.write_at(self.offset, buf)?; self.offset += write_len as u64; Ok(write_len) } pub fn flush(&self) -> AxResult { - self.node.fsync()?; + self.node.access(Cap::WRITE)?.fsync()?; Ok(()) } pub fn get_attr(&self) -> AxResult<FileAttr> { - self.node.get_attr() + self.node.access(Cap::empty())?.get_attr() } } @@ -174,16 +181,27 @@ impl Directory { } let node = crate::root::lookup(path)?; - if node.get_attr()?.is_dir() { - node.open()?; - Ok(Self { node, entry_idx: 0 }) - } else { - ax_err!(NotADirectory) + let attr = node.get_attr()?; + if !attr.is_dir() { + return ax_err!(NotADirectory); } + let access_cap = opts.into(); + if !perm_to_cap(attr.perm()).contains(access_cap) { + return ax_err!(PermissionDenied); + } + + node.open()?; + Ok(Self { + node: WithCap::new(node, access_cap), + entry_idx: 0, + }) } pub fn read_dir(&mut self, dirents: &mut [DirEntry]) -> AxResult<usize> { - let n = self.node.read_dir(self.entry_idx, dirents)?; + let n = self + .node + .access(Cap::READ)? + .read_dir(self.entry_idx, dirents)?; self.entry_idx += n; Ok(n) } @@ -191,13 +209,13 @@ impl Directory { impl Drop for File { fn drop(&mut self) { - self.node.release().ok(); + unsafe { self.node.access_unchecked().release().ok() }; } } impl Drop for Directory { fn drop(&mut self) { - self.node.release().ok(); + unsafe { self.node.access_unchecked().release().ok() }; } } @@ -225,3 +243,30 @@ impl fmt::Debug for OpenOptions { Ok(()) } } + +impl From<&OpenOptions> for Cap { + fn from(opts: &OpenOptions) -> Cap { + let mut cap = Cap::empty(); + if opts.read { + cap |= Cap::READ; + } + if opts.write | opts.append { + cap |= Cap::WRITE; + } + cap + } +} + +fn perm_to_cap(perm: FilePerm) -> Cap { + let mut cap = Cap::empty(); + if perm.owner_readable() { + cap |= Cap::READ; + } + if perm.owner_writable() { + cap |= Cap::WRITE; + } + if perm.owner_executable() { + cap |= Cap::EXECUTE; + } + cap +} diff --git a/modules/axfs/src/fs/fatfs.rs b/modules/axfs/src/fs/fatfs.rs index e90a529..793126f 100644 --- a/modules/axfs/src/fs/fatfs.rs +++ b/modules/axfs/src/fs/fatfs.rs @@ -1,7 +1,7 @@ use alloc::sync::Arc; use core::cell::UnsafeCell; -use axfs_vfs::{VfsDirEntry, VfsError, VfsResult}; +use axfs_vfs::{VfsDirEntry, VfsError, VfsNodePerm, VfsResult}; use axfs_vfs::{VfsNodeAttr, VfsNodeOps, VfsNodeRef, VfsNodeType, VfsOps}; use axsync::Mutex; use fatfs::{Dir, File, LossyOemCpConverter, NullTimeProvider, Read, Seek, SeekFrom, Write}; @@ -55,7 +55,9 @@ impl VfsNodeOps for FileWrapper<'_> { fn get_attr(&self) -> VfsResult<VfsNodeAttr> { let size = self.0.lock().seek(SeekFrom::End(0)).map_err(as_vfs_err)?; let blocks = (size + BLOCK_SIZE as u64 - 1) / BLOCK_SIZE as u64; - Ok(VfsNodeAttr::new_file(size, blocks)) + // FAT fs doesn't support permissions, we just set everything to 755 + let perm = VfsNodePerm::from_bits_truncate(0o755); + Ok(VfsNodeAttr::new(perm, VfsNodeType::File, size, blocks)) } fn read_at(&self, offset: u64, buf: &mut [u8]) -> VfsResult<usize> { @@ -81,7 +83,13 @@ impl VfsNodeOps for DirWrapper<'static> { axfs_vfs::impl_vfs_dir_default! {} fn get_attr(&self) -> VfsResult<VfsNodeAttr> { - Ok(VfsNodeAttr::new_dir(BLOCK_SIZE as u64, 1)) + // FAT fs doesn't support permissions, we just set everything to 755 + Ok(VfsNodeAttr::new( + VfsNodePerm::from_bits_truncate(0o755), + VfsNodeType::Dir, + BLOCK_SIZE as u64, + 1, + )) } fn lookup(self: Arc<Self>, path: &str) -> VfsResult<Arc<dyn VfsNodeOps>> { diff --git a/modules/axfs/src/root.rs b/modules/axfs/src/root.rs index 3936ef8..64fd71d 100644 --- a/modules/axfs/src/root.rs +++ b/modules/axfs/src/root.rs @@ -173,6 +173,11 @@ pub(crate) fn lookup(path: &str) -> AxResult<VfsNodeRef> { } pub(crate) fn create_file(path: &str) -> AxResult<VfsNodeRef> { + if path.is_empty() { + return ax_err!(NotFound); + } else if path.ends_with('/') { + return ax_err!(NotADirectory); + } let parent = parent_node_of(path); parent.create(path, VfsNodeType::File)?; parent.lookup(path) @@ -187,6 +192,8 @@ pub(crate) fn set_current_dir(path: &str) -> AxResult { let attr = node.get_attr()?; if !attr.is_dir() { ax_err!(NotADirectory) + } else if !attr.perm().owner_executable() { + ax_err!(PermissionDenied) } else { let mut path = if path.starts_with('/') { path.into() diff --git a/modules/axfs/tests/test_axfs.rs b/modules/axfs/tests/test_axfs.rs index 06bee7c..87253ae 100644 --- a/modules/axfs/tests/test_axfs.rs +++ b/modules/axfs/tests/test_axfs.rs @@ -9,6 +9,15 @@ use io::{prelude::*, Error, Result}; const IMG_PATH: &str = "resources/fat16.img"; +macro_rules! assert_err { + ($expr: expr) => { + assert!(($expr).is_err()) + }; + ($expr: expr, $err: ident) => { + assert_eq!(($expr).err(), Some(Error::$err)) + }; +} + fn make_disk() -> std::io::Result<RamDisk> { let path = std::env::current_dir()?.join(IMG_PATH); println!("Loading disk image from {:?} ...", path); @@ -46,7 +55,7 @@ fn test_read_write_file() -> Result<()> { assert_eq!(new_contents2, new_contents + "new line\n"); // open a non-exist file - assert_eq!(File::open("/not/exist/file").err(), Some(Error::NotFound)); + assert_err!(File::open("/not/exist/file"), NotFound); println!("test_read_write_file() OK!"); Ok(()) @@ -63,6 +72,44 @@ fn test_read_dir() -> Result<()> { Ok(()) } +fn test_file_permission() -> Result<()> { + let fname = "./short.txt"; + println!("test permission {:?}:", fname); + + // write a file that open with read-only mode + let mut buf = [0; 256]; + let mut file = File::open(fname)?; + let n = file.read(&mut buf)?; + assert_err!(file.write(&mut buf), PermissionDenied); + drop(file); + + // read a file that open with write-only mode + let mut file = File::create(fname)?; + assert_err!(file.read(&mut buf), PermissionDenied); + assert!(file.write(&buf[..n]).is_ok()); + drop(file); + + // open with empty options + assert_err!(OpenOptions::new().open(fname), InvalidInput); + + // read as a directory + assert_err!(fs::read_dir(fname), NotADirectory); + assert_err!(fs::read("short.txt/"), NotADirectory); + assert_err!(fs::metadata("/short.txt/"), NotADirectory); + + // create as a directory + assert_err!(fs::write("error/", "should not create"), NotADirectory); + assert_err!(fs::metadata("error/"), NotFound); + assert_err!(fs::metadata("error"), NotFound); + + // read/write a directory + assert_err!(fs::read_to_string("/dev"), IsADirectory); + assert_err!(fs::write(".", "test"), IsADirectory); + + println!("test_file_permisson() OK!"); + Ok(()) +} + fn test_devfs() -> Result<()> { const N: usize = 32; let mut buf = [1; N]; @@ -121,5 +168,6 @@ fn test_axfs() { test_read_write_file().expect("test_read_write_file() failed"); test_read_dir().expect("test_read_dir() failed"); + test_file_permission().expect("test_file_permission() failed"); test_devfs().expect("test_devfs() failed"); } -- GitLab