fix Wasi rights system to work with wasi-libc (#6463)

* 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

* preview2: mask file perms. and we really need to enforce them on stream creation

which will mean editing the wit.

* filesystem.wit: make {read, write, append}-via-stream fallible with an error-code

in order to fail appropriately when a file is not open for reading or
writing.

* filesystem.wit: update comments

* preview2 impls: fix error handling for {read,write,append}_via_stream

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

* remove kubkob from labeler

I appreciate his work on this stuff in the past but he hasnt been around
for a few years now

* preview 2 filesystem: allow stat on any opened fd

since file perms is now more accurately the file's access mode, a file
open for writing was having problems here.

in reality i dont think it makes sense to restrict this based on the
perms. if a file is opened, you should be able to stat it.

this fixes the host adapter's path_link test, which was broken by prior
changes.  additionally, this fix lets the path_open_dirfd_not_dir test
pass.

* fix the directory base & inheriting rights

in order to work with wasi-testsuite, it needs to be possible to
path_open(dirfd, ".", ...) with the same rights reported in the
fdstat of that dirfd. When we report the Rights::all() set, both
FD_READ and FD_WRITE are set in the base rights, which results in
unix rejecting an openat2(dirfd, ".", O_RDWR) with EISDIR.

By not having the FD_READ and FD_WRITE rights present in the base
rights, the open syscall defaults to O_RDONLY, which is the only
access mode allowed for opening directories.

* path_open of a directory with read and write succeeds on windows

.github/subscribe-to-label.json

@@ -2,6 +2,5 @@
   "cfallin": ["isle"],
   "fitzgen": ["fuzzing", "isle", "wasmtime:ref-types"],
   "peterhuene": ["wasmtime:api", "wasmtime:c-api"],
-  "kubkon": ["wasi"],
   "saulecabrera": ["winch"]
 }

crates/test-programs/tests/wasi-cap-std-sync.rs

@@ -188,6 +188,10 @@ fn path_open_create_existing() {
     run("path_open_create_existing", true).unwrap()
 }
 #[test_log::test]
+fn path_open_read_write() {
+    run("path_open_read_write", true).unwrap()
+}
+#[test_log::test]
 fn path_open_dirfd_not_dir() {
     run("path_open_dirfd_not_dir", true).unwrap()
 }
@@ -265,3 +269,7 @@ fn symlink_loop() {
 fn unlink_file_trailing_slashes() {
     run("unlink_file_trailing_slashes", true).unwrap()
 }
+#[test_log::test]
+fn path_open_preopen() {
+    run("path_open_preopen", true).unwrap()
+}

crates/test-programs/tests/wasi-preview1-host-in-preview2.rs

@@ -229,6 +229,10 @@ async fn path_open_create_existing() {
     run("path_open_create_existing", true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn path_open_read_write() {
+    run("path_open_read_write", true).await.unwrap()
+}
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn path_open_dirfd_not_dir() {
     run("path_open_dirfd_not_dir", true).await.unwrap()
 }
@@ -313,3 +317,7 @@ async fn symlink_loop() {
 async fn unlink_file_trailing_slashes() {
     run("unlink_file_trailing_slashes", true).await.unwrap()
 }
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn path_open_preopen() {
+    run("path_open_preopen", true).await.unwrap()
+}

crates/test-programs/tests/wasi-preview2-components.rs

@@ -216,6 +216,10 @@ async fn path_open_create_existing() {
     run("path_open_create_existing", true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn path_open_read_write() {
+    run("path_open_read_write", true).await.unwrap()
+}
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn path_open_dirfd_not_dir() {
     run("path_open_dirfd_not_dir", true).await.unwrap()
 }
@@ -300,3 +304,7 @@ async fn symlink_loop() {
 async fn unlink_file_trailing_slashes() {
     run("unlink_file_trailing_slashes", true).await.unwrap()
 }
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn path_open_preopen() {
+    run("path_open_preopen", true).await.unwrap()
+}

crates/test-programs/tests/wasi-tokio.rs

@@ -190,6 +190,10 @@ async fn path_open_create_existing() {
     run("path_open_create_existing", true).await.unwrap()
 }
 #[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn path_open_read_write() {
+    run("path_open_read_write", true).await.unwrap()
+}
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
 async fn path_open_dirfd_not_dir() {
     run("path_open_dirfd_not_dir", true).await.unwrap()
 }
@@ -271,3 +275,7 @@ async fn symlink_loop() {
 async fn unlink_file_trailing_slashes() {
     run("unlink_file_trailing_slashes", true).await.unwrap()
 }
+#[test_log::test(tokio::test(flavor = "multi_thread"))]
+async fn path_open_preopen() {
+    run("path_open_preopen", true).await.unwrap()
+}

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

@@ -0,0 +1,92 @@
+const FIRST_PREOPEN: u32 = 3;
+
+unsafe fn path_open_preopen() {
+    let prestat = wasi::fd_prestat_get(FIRST_PREOPEN).expect("fd 3 is a preopen");
+    assert_eq!(
+        prestat.tag,
+        wasi::PREOPENTYPE_DIR.raw(),
+        "prestat is a directory"
+    );
+    let mut dst = Vec::with_capacity(prestat.u.dir.pr_name_len);
+    wasi::fd_prestat_dir_name(FIRST_PREOPEN, dst.as_mut_ptr(), dst.capacity())
+        .expect("get preopen dir name");
+    dst.set_len(prestat.u.dir.pr_name_len);
+
+    let fdstat = wasi::fd_fdstat_get(FIRST_PREOPEN).expect("get fdstat");
+
+    println!(
+        "preopen dir: {:?} base {:?} inheriting {:?}",
+        String::from_utf8_lossy(&dst),
+        fdstat.fs_rights_base,
+        fdstat.fs_rights_inheriting
+    );
+
+    // Open with same rights it has now:
+    let _ = wasi::path_open(
+        FIRST_PREOPEN,
+        0,
+        ".",
+        0,
+        fdstat.fs_rights_base,
+        fdstat.fs_rights_inheriting,
+        0,
+    )
+    .expect("open with same rights");
+
+    // Open with an empty set of rights:
+    let _ = wasi::path_open(FIRST_PREOPEN, 0, ".", 0, 0, 0, 0).expect("open with empty rights");
+
+    // Open OFLAGS_DIRECTORY with an empty set of rights:
+    let _ = wasi::path_open(FIRST_PREOPEN, 0, ".", wasi::OFLAGS_DIRECTORY, 0, 0, 0)
+        .expect("open with O_DIRECTORY empty rights");
+
+    // Open OFLAGS_DIRECTORY with just the read right:
+    let _ = wasi::path_open(
+        FIRST_PREOPEN,
+        0,
+        ".",
+        wasi::OFLAGS_DIRECTORY,
+        wasi::RIGHTS_FD_READ,
+        0,
+        0,
+    )
+    .expect("open with O_DIRECTORY and read right");
+
+    if !wasi_tests::TESTCONFIG.errno_expect_windows() {
+        // Open OFLAGS_DIRECTORY and read/write rights should fail with isdir:
+        let err = wasi::path_open(
+            FIRST_PREOPEN,
+            0,
+            ".",
+            wasi::OFLAGS_DIRECTORY,
+            wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+            0,
+            0,
+        )
+        .err()
+        .expect("open with O_DIRECTORY and read/write should fail");
+        assert_eq!(
+            err,
+            wasi::ERRNO_ISDIR,
+            "opening directory read/write should fail with ISDIR"
+        );
+    } else {
+        // Open OFLAGS_DIRECTORY and read/write rights will succeed, only on windows:
+        let _ = wasi::path_open(
+            FIRST_PREOPEN,
+            0,
+            ".",
+            wasi::OFLAGS_DIRECTORY,
+            wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+            0,
+            0,
+        )
+        .expect("open with O_DIRECTORY and read/write should succeed on windows");
+    }
+}
+
+fn main() {
+    unsafe {
+        path_open_preopen();
+    }
+}

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

@@ -0,0 +1,140 @@
+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) {
+    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(