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

Implement proper path resolution #2323

Open
ncruces opened this issue Sep 27, 2024 · 3 comments
Open

Implement proper path resolution #2323

ncruces opened this issue Sep 27, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@ncruces
Copy link
Collaborator

ncruces commented Sep 27, 2024

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:

wazero/fsconfig.go

Lines 64 to 88 in 111c51a

type FSConfig interface {
// WithDirMount assigns a directory at `dir` to any paths beginning at
// `guestPath`.
//
// For example, `dirPath` as / (or c:\ in Windows), makes the entire host
// volume writeable to the path on the guest. The `guestPath` is always a
// POSIX style path, slash (/) delimited, even if run on Windows.
//
// If the same `guestPath` was assigned before, this overrides its value,
// retaining the original precedence. See the documentation of FSConfig for
// more details on `guestPath`.
//
// # Isolation
//
// 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.
//
// # os.DirFS
//
// This configuration optimizes for WASI compatibility which is sometimes
// at odds with the behavior of os.DirFS. Hence, this will not behave
// exactly the same as os.DirFS. See /RATIONALE.md for more.
WithDirMount(dir, guestPath string) FSConfig

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
Copy link

neild commented Sep 27, 2024

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
Copy link
Collaborator Author

ncruces commented Sep 29, 2024

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
Copy link
Contributor

yagehu commented Sep 30, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants