Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implicit "read" access when no rights specified in path_open on Unix #617

Open
kubkon opened this issue Nov 21, 2019 · 6 comments
Open

Implicit "read" access when no rights specified in path_open on Unix #617

kubkon opened this issue Nov 21, 2019 · 6 comments
Labels
wasi:api Issues pertaining to the WASI API, not necessarily specific to Wasmtime. wasi:impl Issues pertaining to WASI implementation in Wasmtime

Comments

@kubkon
Copy link
Member

kubkon commented Nov 21, 2019

When discussing base and inheriting rights in path_open in #570, we've discovered that, on Unix, our implementation of path_open will implicitly open the derived descriptor with "read" access even when both rights_base and rights_inheriting are set to 0. @marmistrz and I have since been wondering whether this is the intended behaviour or not (for reference, here's the link to the offending bit: sys/unix/hostcalls_impl/fs.rs#L102). Note further that, if you specify __WASI_RIGHT_FD_WRITE, we don't implicitly inherit the read right. All in all then, the possible states for rights_base currently are:

  • 0 implies "read" access
  • __WASI_RIGHT_FD_READ implies "read" access
  • __WASI_RIGHT_FD_WRITE implies "write" access
  • __WASI_RIGHT_FD_READ | __WASI_RIGHT_FD_WRITE implies "read-write" access

This could have a potential unexpected behaviour in implementations wrapping the WASI syscalls such as std::fs::OpenOptions which assumes that __WASI_RIGHT_FD_READ is acquired only if the client has indeed specified that they want the path opened for reading which with the current behaviour will not always be the case. Here's the link to the relevant implementation bit in Rust: sys/wasi/fs.rs#L332.

@kubkon kubkon added wasi:api Issues pertaining to the WASI API, not necessarily specific to Wasmtime. wasi:impl Issues pertaining to WASI implementation in Wasmtime labels Nov 21, 2019
@kubkon
Copy link
Member Author

kubkon commented Nov 21, 2019

@marmistrz
Copy link
Contributor

I also created a test exposing this behavior: #618. We get read access even if we literally drop the right to call fd_read.

@sunfishcode
Copy link
Member

The reason for opening with O_RDONLY when __WASI_RIGHT_FD_READ isn't set is because classic Unix doesn't have a way to open a file without either read or write access. But we may still do some operations on it, like fd_filestat_get, which require us to have some kind of open file descriptor.

fd_read should still disallow attempts to read if __WASI_RIGHT_FD_READ isn't set. Is that not happening?

Linux has O_PATH which is a way to open a file without read or write access, and in theory we could use that for these kinds of cases. That said, not all platforms have that, so it'd be good to figure out how to solve this without that too.

@kubkon
Copy link
Member Author

kubkon commented Nov 21, 2019

@sunfishcode it seems that our Unix impl still allows fd_read with __WASI_RIGHT_FD_READ dropped (see @marmistrz’s test case in #618). Incidentally, we can observe the correct behaviour in our Windows impl, so I guess this is a bug on Unix :-)

@marmistrz
Copy link
Contributor

The problem is that the derived fd will implicitly receive RIGHT_FD_READ, as shown in the new version of the testcase. The reason is that FdEntry::from will call determine_type_and_access_rights on the newly created descriptor, find out it's opened as O_RDONLY and conclude that the new file should have the right RIGHT_FD_READ.

@alexcrichton
Copy link
Member

I don't personally have much to add here myself unfortunately, I'd be happy with whatever outcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi:api Issues pertaining to the WASI API, not necessarily specific to Wasmtime. wasi:impl Issues pertaining to WASI implementation in Wasmtime
Projects
None yet
Development

No branches or pull requests

4 participants