[9.0.0] fix Wasi rights system to work with wasi-libc (#6462)

* wasi-common: need FileEntry to track the mode with which a file was opened

* add a test for read, write access modes in path_open

* a directory needs to report full set of rights in fdstat

implementations such as wasi-libc use these as a mask for any rights
they request for a new fd when invoking path_open

* test dirfd rights as well

* wasi-common: normalize wrong access mode error on windows to badf

crates/test-programs/wasi-tests/src/bin/path_open_read_write.rs

@@ -0,0 +1,158 @@
+use std::{env, process};
+use wasi_tests::{assert_errno, create_file, open_scratch_directory};
+
+unsafe fn test_path_open_read_write(dir_fd: wasi::Fd) {
+    let stat = wasi::fd_fdstat_get(dir_fd).expect("get dirfd stat");
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ,
+        "dirfd has base read right"
+    );
+    assert!(
+        stat.fs_rights_inheriting & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ,
+        "dirfd has inheriting read right"
+    );
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE,
+        "dirfd has base write right"
+    );
+    assert!(
+        stat.fs_rights_inheriting & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE,
+        "dirfd has inheriting write right"
+    );
+
+    create_file(dir_fd, "file");
+
+    let f_readonly = wasi::path_open(dir_fd, 0, "file", 0, wasi::RIGHTS_FD_READ, 0, 0)
+        .expect("open file readonly");
+
+    let stat = wasi::fd_fdstat_get(f_readonly).expect("get fdstat readonly");
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ,
+        "readonly has read right"
+    );
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == 0,
+        "readonly does not have write right"
+    );
+
+    let buffer = &mut [0u8; 100];
+    let iovec = wasi::Iovec {
+        buf: buffer.as_mut_ptr(),
+        buf_len: buffer.len(),
+    };
+    let nread = wasi::fd_read(f_readonly, &[iovec]).expect("reading readonly file");
+    assert_eq!(nread, 0, "readonly file is empty");
+
+    let write_buffer = &[1u8; 50];
+    let ciovec = wasi::Ciovec {
+        buf: write_buffer.as_ptr(),
+        buf_len: write_buffer.len(),
+    };
+    assert_errno!(
+        wasi::fd_write(f_readonly, &[ciovec])
+            .err()
+            .expect("read of writeonly fails"),
+        wasi::ERRNO_BADF
+    );
+
+    wasi::fd_close(f_readonly).expect("close readonly");
+    drop(f_readonly);
+
+    // =============== WRITE ONLY ==================
+    let f_writeonly = wasi::path_open(dir_fd, 0, "file", 0, wasi::RIGHTS_FD_WRITE, 0, 0)
+        .expect("open file writeonly");
+
+    let stat = wasi::fd_fdstat_get(f_writeonly).expect("get fdstat writeonly");
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_READ == 0,
+        "writeonly does not have read right"
+    );
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE,
+        "writeonly has write right"
+    );
+
+    assert_errno!(
+        wasi::fd_read(f_writeonly, &[iovec])
+            .err()
+            .expect("read of writeonly fails"),
+        wasi::ERRNO_BADF
+    );
+    let bytes_written = wasi::fd_write(f_writeonly, &[ciovec]).expect("write to writeonly");
+    assert_eq!(bytes_written, write_buffer.len());
+
+    wasi::fd_close(f_writeonly).expect("close writeonly");
+    drop(f_writeonly);
+
+    // ============== READ WRITE =======================
+
+    let f_readwrite = wasi::path_open(
+        dir_fd,
+        0,
+        "file",
+        0,
+        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        0,
+        0,
+    )
+    .expect("open file readwrite");
+    let stat = wasi::fd_fdstat_get(f_readwrite).expect("get fdstat readwrite");
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_READ == wasi::RIGHTS_FD_READ,
+        "readwrite has read right"
+    );
+    assert!(
+        stat.fs_rights_base & wasi::RIGHTS_FD_WRITE == wasi::RIGHTS_FD_WRITE,
+        "readwrite has write right"
+    );
+
+    let nread = wasi::fd_read(f_readwrite, &[iovec]).expect("reading readwrite file");
+    assert_eq!(
+        nread,
+        write_buffer.len(),
+        "readwrite file contains contents from writeonly open"
+    );
+
+    let write_buffer_2 = &[2u8; 25];
+    let ciovec = wasi::Ciovec {
+        buf: write_buffer_2.as_ptr(),
+        buf_len: write_buffer_2.len(),
+    };
+    let bytes_written = wasi::fd_write(f_readwrite, &[ciovec]).expect("write to readwrite");
+    assert_eq!(bytes_written, write_buffer_2.len());
+
+    let filestat = wasi::fd_filestat_get(f_readwrite).expect("get filestat readwrite");
+    assert_eq!(
+        filestat.size as usize,
+        write_buffer.len() + write_buffer_2.len(),
+        "total written is both write buffers"
+    );
+
+    wasi::fd_close(f_readwrite).expect("close readwrite");
+    drop(f_readwrite);
+
+    wasi::path_unlink_file(dir_fd, "file").expect("removing a file");
+}
+
+fn main() {
+    let mut args = env::args();
+    let prog = args.next().unwrap();
+    let arg = if let Some(arg) = args.next() {
+        arg
+    } else {
+        eprintln!("usage: {} <scratch directory>", prog);
+        process::exit(1);
+    };
+
+    // Open scratch directory
+    let dir_fd = match open_scratch_directory(&arg) {
+        Ok(dir_fd) => dir_fd,
+        Err(err) => {
+            eprintln!("{}", err);
+            process::exit(1)
+        }
+    };
+
+    // Run the tests.
+    unsafe { test_path_open_read_write(dir_fd) }
+}

crates/wasi-common/cap-std-sync/src/lib.rs

@@ -49,7 +49,7 @@ pub use sched::sched_ctx;
 use crate::net::Socket;
 use cap_rand::{Rng, RngCore, SeedableRng};
 use std::path::Path;
-use wasi_common::{table::Table, Error, WasiCtx, WasiFile};
+use wasi_common::{file::FileAccessMode, table::Table, Error, WasiCtx, WasiFile};
 
 pub struct WasiCtxBuilder(WasiCtx);
 
@@ -126,7 +126,8 @@ impl WasiCtxBuilder {
     pub fn preopened_socket(self, fd: u32, socket: impl Into<Socket>) -> Result<Self, Error> {
         let socket: Socket = socket.into();
         let file: Box<dyn WasiFile> = socket.into();
-        self.0.insert_file(fd, file);
+        self.0
+            .insert_file(fd, file, FileAccessMode::READ | FileAccessMode::WRITE);
         Ok(self)
     }
     pub fn build(self) -> WasiCtx {

crates/wasi-common/src/ctx.rs

@@ -1,6 +1,6 @@
 use crate::clocks::WasiClocks;
 use crate::dir::{DirEntry, WasiDir};
-use crate::file::{FileEntry, WasiFile};
+use crate::file::{FileAccessMode, FileEntry, WasiFile};
 use crate::sched::WasiSched;
 use crate::string_array::StringArray;
 use crate::table::Table;
@@ -51,12 +51,18 @@ impl WasiCtx {
         s
     }
 
-    pub fn insert_file(&self, fd: u32, file: Box<dyn WasiFile>) {
-        self.table().insert_at(fd, Arc::new(FileEntry::new(file)));
+    pub fn insert_file(&self, fd: u32, file: Box<dyn WasiFile>, access_mode: FileAccessMode) {
+        self.table()
+            .insert_at(fd, Arc::new(FileEntry::new(file, access_mode)));
     }
 
-    pub fn push_file(&self, file: Box<dyn WasiFile>) -> Result<u32, Error> {
-        self.table().push(Arc::new(FileEntry::new(file)))
+    pub fn push_file(
+        &self,
+        file: Box<dyn WasiFile>,
+        access_mode: FileAccessMode,
+    ) -> Result<u32, Error> {
+        self.table()
+            .push(Arc::new(FileEntry::new(file, access_mode)))
     }
 
     pub fn insert_dir(&self, fd: u32, dir: Box<dyn WasiDir>, path: PathBuf) {
@@ -92,15 +98,15 @@ impl WasiCtx {
     }
 
     pub fn set_stdin(&self, f: Box<dyn WasiFile>) {
-        self.insert_file(0, f);
+        self.insert_file(0, f, FileAccessMode::READ);
     }
 
     pub fn set_stdout(&self, f: Box<dyn WasiFile>) {
-        self.insert_file(1, f);
+        self.insert_file(1, f, FileAccessMode::WRITE);
     }
 
     pub fn set_stderr(&self, f: Box<dyn WasiFile>) {
-        self.insert_file(2, f);
+        self.insert_file(2, f, FileAccessMode::WRITE);
     }
 
     pub fn push_preopened_dir(

crates/wasi-common/src/file.rs

@@ -220,17 +220,26 @@ impl TableFileExt for crate::table::Table {
 
 pub(crate) struct FileEntry {
     pub file: Box<dyn WasiFile>,
+    pub access_mode: FileAccessMode,
+}
+
+bitflags! {
+    pub struct FileAccessMode : u32 {
+        const READ = 0b1;
+        const WRITE= 0b10;
+    }
 }
 
 impl FileEntry {
-    pub fn new(file: Box<dyn WasiFile>) -> Self {
-        FileEntry { file }
+    pub fn new(file: Box<dyn WasiFile>, access_mode: FileAccessMode) -> Self {
+        FileEntry { file, access_mode }
     }
 
     pub async fn get_fdstat(&self) -> Result<FdStat, Error> {
         Ok(FdStat {
             filetype: self.file.get_filetype().await?,
             flags: self.file.get_fdflags().await?,
+            access_mode: self.access_mode,
         })
     }
 }
@@ -239,6 +248,7 @@ impl FileEntry {
 pub struct FdStat {
     pub filetype: FileType,
     pub flags: FdFlags,
+    pub access_mode: FileAccessMode,
 }
 
 #[derive(Debug, Clone)]

crates/wasi-common/src/snapshots/preview_1.rs

@@ -1,8 +1,8 @@
 use crate::{
     dir::{DirEntry, OpenResult, ReaddirCursor, ReaddirEntity, TableDirExt},
     file::{
-        Advice, FdFlags, FdStat, FileEntry, FileType, Filestat, OFlags, RiFlags, RoFlags, SdFlags,
-        SiFlags, TableFileExt, WasiFile,
+        Advice, FdFlags, FdStat, FileAccessMode, FileEntry, FileType, Filestat, OFlags, RiFlags,
+        RoFlags, SdFlags, SiFlags, TableFileExt, WasiFile,
     },
     sched::{
         subscription::{RwEventFlags, SubscriptionResult},
@@ -177,8 +177,8 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
             let _dir_entry: Arc<DirEntry> = table.get(fd)?;
             let dir_fdstat = types::Fdstat {
                 fs_filetype: types::Filetype::Directory,
-                fs_rights_base: types::Rights::empty(),
-                fs_rights_inheriting: types::Rights::empty(),
+                fs_rights_base: types::Rights::all(),
+                fs_rights_inheriting: types::Rights::all(),
                 fs_flags: types::Fdflags::empty(),
             };
             Ok(dir_fdstat)
@@ -293,6 +293,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         iovs: &types::IovecArray<'a>,
     ) -> Result<types::Size, Error> {
         let f = self.table().get_file(u32::from(fd))?;
+        // Access mode check normalizes error returned (windows would prefer ACCES here)
+        if !f.access_mode.contains(FileAccessMode::READ) {
+            Err(types::Errno::Badf)?
+        }
         let f = &f.file;
 
         let iovs: Vec<wiggle::GuestPtr<[u8]>> = iovs
@@ -364,6 +368,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         offset: types::Filesize,
     ) -> Result<types::Size, Error> {
         let f = self.table().get_file(u32::from(fd))?;
+        // Access mode check normalizes error returned (windows would prefer ACCES here)
+        if !f.access_mode.contains(FileAccessMode::READ) {
+            Err(types::Errno::Badf)?
+        }
         let f = &f.file;
 
         let iovs: Vec<wiggle::GuestPtr<[u8]>> = iovs
@@ -436,6 +444,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         ciovs: &types::CiovecArray<'a>,
     ) -> Result<types::Size, Error> {
         let f = self.table().get_file(u32::from(fd))?;
+        // Access mode check normalizes error returned (windows would prefer ACCES here)
+        if !f.access_mode.contains(FileAccessMode::WRITE) {
+            Err(types::Errno::Badf)?
+        }
         let f = &f.file;
 
         let guest_slices: Vec<wiggle::GuestCow<u8>> = ciovs
@@ -463,6 +475,10 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         offset: types::Filesize,
     ) -> Result<types::Size, Error> {
         let f = self.table().get_file(u32::from(fd))?;
+        // Access mode check normalizes error returned (windows would prefer ACCES here)
+        if !f.access_mode.contains(FileAccessMode::WRITE) {
+            Err(types::Errno::Badf)?
+        }
         let f = &f.file;
 
         let guest_slices: Vec<wiggle::GuestCow<u8>> = ciovs
@@ -728,14 +744,24 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
 
         let read = fs_rights_base.contains(types::Rights::FD_READ);
         let write = fs_rights_base.contains(types::Rights::FD_WRITE);
+        let access_mode = if read {
+            FileAccessMode::READ
+        } else {
+            FileAccessMode::empty()
+        } | if write {
+            FileAccessMode::WRITE
+        } else {
+            FileAccessMode::empty()
+        };
+
         let file = dir_entry
             .dir
             .open_file(symlink_follow, path.deref(), oflags, read, write, fdflags)
             .await?;
         drop(dir_entry);
 
         let fd = match file {
-            OpenResult::File(file) => table.push(Arc::new(FileEntry::new(file)))?,
+            OpenResult::File(file) => table.push(Arc::new(FileEntry::new(file, access_mode)))?,
             OpenResult::Dir(child_dir) => table.push(Arc::new(DirEntry::new(None, child_dir)))?,
         };
         Ok(types::Fd::from(fd))
@@ -1087,7 +1113,7 @@ impl wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         let table = self.table();
         let f = table.get_file(u32::from(fd))?;
         let file = f.file.sock_accept(FdFlags::from(flags)).await?;
-        let fd = table.push(Arc::new(FileEntry::new(file)))?;
+        let fd = table.push(Arc::new(FileEntry::new(file, FileAccessMode::all())))?;
         Ok(types::Fd::from(fd))
     }
 
@@ -1214,9 +1240,16 @@ impl From<types::Advice> for Advice {
 
 impl From<&FdStat> for types::Fdstat {
     fn from(fdstat: &FdStat) -> types::Fdstat {
+        let mut fs_rights_base = types::Rights::empty();
+        if fdstat.access_mode.contains(FileAccessMode::READ) {
+            fs_rights_base |= types::Rights::FD_READ;
+        }
+        if fdstat.access_mode.contains(FileAccessMode::WRITE) {
+            fs_rights_base |= types::Rights::FD_WRITE;
+        }
         types::Fdstat {
             fs_filetype: types::Filetype::from(&fdstat.filetype),
-            fs_rights_base: types::Rights::empty(),
+            fs_rights_base,
             fs_rights_inheriting: types::Rights::empty(),
             fs_flags: types::Fdflags::from(fdstat.flags),
         }