tetratelabs / wazero

wazero: the zero dependency WebAssembly runtime for Go developers
https://wazero.io
Apache License 2.0
4.97k stars 259 forks source link

Implement proper path resolution #2323

Open ncruces opened 1 month ago

ncruces commented 1 month ago

This is an umbrella issue intended to track the effort to implement the path resolution algorithm described by WASI filesystem.

As described prominently in our documentation, our current WASI filesystem implementation does not properly sandbox file system operations: https://github.com/tetratelabs/wazero/blob/111c51a1bd5acfd1e91afe489d28e52136a3e6d5/fsconfig.go#L64-L88

For emphasis:

The guest will have full access to this directory including escaping it via relative path lookups like "../../". Full access includes operations such as creating or deleting files, limited to any host level access controls.

The reason for this is that it is hard to provide an implementation that is simulateously:

  1. portable to Windows and other non POSIX platforms
  2. offers the POSIX semantics WASI specifies
  3. sandboxes file system access

We have prioritized 1 and 2 over 3.

There's an ongoing effort to improve the situation, in #2254 and #2264 (thanks @yagehu).

This is challenging for various reasons. One is the wazero zero dependency policy that prevents us from using even sys/unix. Another is that this will introduce a performance regression, which can only be partially offset in certain platforms by specialization.

neild commented 1 month ago

FYI, it won't help you right now but https://go.dev/issue/67002 may be of interest. This is a proposal (which I'm hoping to land in Go 1.24, although no promises) to add openat-like functionality to the os package. The proposed API is portable and provides restricted access to a directory tree with defense against symlink escapes:

root, err := os.OpenRoot("/some/base/path")
f, err := root.Open("relative/to/root") // fails if the path escapes the root

It doesn't provide defense against non-symlink escapes (for example, if the directory tree includes a procfs filesystem, this won't defend against opening /proc/fd/*). I suspect that's not a concern for your case.

One possible problem point is "the POSIX semantics WASI specifies". (Incidentally, do you have a reference for where this is specified? I was trying to find something definitive on whether WASI paths support backslash as a separator on Windows, and couldn't find anything.) The proposed APIs in https://go.dev/issue/67002 use the semantics of the local platform. That means that on Windows, paths can use backslashes as separators (minor) and there are some subtle differences in how .. and symlinks interact. (On Unix, "a/../b" will follow any symlink in "a". On Windows, paths are cleaned at the start of a lookup , so "a/../b" is exactly equivalent to just "b", and will work even if "a" doesn't exist at all.)

I'd be very interested to know if there are any gaps in the proposal that would make it unsuitable as a foundation for wazero's file APIs.

ncruces commented 1 month ago

As for WASI being heavily inspired by POSIX just reading the README:

WASI started with launching what is now called Preview 1, an API using the witx IDL, and it is now widely used. Its major influences are POSIX and CloudABI.

As for paths, I don't have a definitive answer to give you. That's the trouble with WASI, TBH. The path resolution document quite explicitly follows POSIX semantics, quite similar to Open Group, or Linux.

But then you go through the repo and see open issues, in which our team has participated, which basically say that my previous strategy (of using path.Clean) is perfectly valid, because Windows does the same (and is case insensitive, and will reject some names like ? or *, and as you point interpret \ the same as /).

So I don't know what to tell you. I decided to do nothing, because what we have now is behaving as documented. I do think WASI is converging into requiring emulating POSIX path resolution as closely as possible; and that if so, openat and friends are the way to go, just because that's what other runtimes will do.

Hence this issue. But our stance is that we need to focus our limited manpower into implementing stable APIs. WASI is not there yet.

Obviously, if the community needs improvements, there are ways to contribute.

yagehu commented 1 month ago

Thanks for the summary @ncruces. Just want to give an update for #2254 and #2264 here. I am still around. I just started my PhD program and am still ramping up. Hence the absence. I will eventually come around to this soon!