Support Absolute Symlinks In WASI

[TheLostLambda]

Motivation

Hello again! Wasmer has become a core part of a plugin system I maintain for Zellij, but one of my plugins (a file-browser) chokes on absolute symlinks!

thread 'wasm' panicked at 'not implemented: Absolute symlinks are not yet supported': /home/tll/.cargo/registry/src/github.com-1ecc6299db9ec823/wasmer-wasi-1.0.2/src/state/mod.rs:812

Proposed solution

I'd love to turn the "are not yet" into an "are now"! I'm no expert with the code-base here, but I'd be happy to help if it's nothing too tricky to implement.

Alternatives

Just don't use absolute symlinks, I suppose, but that's a tall ask of the end-user

[MarkMcCaskey]

Thanks for the issue! That's great to hear about Zellij!

I don't think implementing the absolute symlink logic will be particularly difficult at all, the main consideration is just the sandboxing of WASI itself and making sure that there aren't ways to get access to files outside of the preopened directories beyond what's reasonable. I don't think the behavior of symlinks outside of preopened dirs has been defined clearly by WASI anywhere, but I think it's probably safe to assume that it should act as if it's in that preopened directory.

I think we can probably enable reading absolute symlinks without too many sandbox escape considerations, though I'd like to review a few things before being comfortable shipping it. Creating absolute pathed symlinks sounds a lot more complicated, although technically all paths a WASI program can see are relative to a preopened directory, so maybe the notion of creating an "absolute path" symlink from WASI doesn't even make sense...

The places that need the most scrutiny are the core Kind enum for wasi files to make sure the abstractions are coherent with symlinks to places outside of a preopened dir and the symlinking logic itself. Aside from that, anywhere in wasi/src/syscalls/mod.rs where symlinks are dealt with directly should probably be reviewed again too just in case.

[TheLostLambda]

Sounds great! Thank you for all of the pointers as well! I can probably start seriously taking a look at things in the next week or so. It sounds like a good plan to treat the symlinks the same as they would be treated outside of WASI, and perhaps creating them could be a separate PR (as I agree that would need significantly more thought and discussion).

Thanks again for the guidance and I'll keep you updated on any progress I make!

[MarkMcCaskey]

Alright, follow up: the current code assumes that all paths (including the things symlinks point to) can be made relative to one of the preopened fds. This is a good assumption for filesystem sandboxing and helps us ensure that we're never accessing files outside of what was explicitly granted.

I see two options here:

1. Keep this invariant of the system, if you want to use absolute symlinks you must preopen where they point to. We then convert absolute paths into relative ones to a preopened fd.
2. Break this invariant, introduce a new Kind to prevent bugs and call it Kind::AbsoluteSymlink this ensures that we also handle it separately and update all locations that need to be updated.

1 is definitely better for security but comes at the cost that users may start granting much wider access than they strictly need to just make programs work. So it's possible that this may be worse for security in practice. This can be mitigated with good error messages about what exactly is going wrong and what extra directories must be preopened in order to make this work. Ideally we could even do this interactively when run on the CLI. In order for that to work, we'd have to extend WASI to allow the host to add more preopened directories later.

2 is practical but I worry about the interactions of referencing absolute files and directories in other places.

Symlinks are tricky because .../symlink/.. is not the same thing as .../directory/.. and shouldn't be allowed in general.

---

I'd like to hear more about your use case and if a solution like 1 could reasonably solve your problem (with and without dynamic preopens (dynamic preopens is contingent on shipping that as a new feature in WASI itself))

[TheLostLambda]

Thanks for the follow up on this!

The plugin we are developing is essentially a simplified file-manager, and I have the odd situation where my home folder is split over two physical drives and symlinked together. Generally, this is totally transparent, but I think solution 1 would mean I'd need to premount several directories for this to work? In my case, that would be mounting two different filesystems on two different devices to browse my home directory. While this is certainly possible, I don't feel it's super intuitive. I agree that good error messages go a long way, but I'd probably prefer here for them to work as they do outside of WASI if possible.

I think 2 might be worth the extra bit of trouble in implementation and I'm more than happy to take that on :slightly_smiling_face: Having the different Kind should make it easy enough to pin down edge-cases if I understand correctly?

From a user's perspective, I think solution 2 is the most intuitive, but it is always annoying to break invariants, so I'd certainly understand if you are hesitant to do so.

Thanks again for all of the thought you've put into this!

[Zykino]

Hello, I am a user of Zellij and found out this problem too.

My experience from a pure user perspective is that starting a session with the plugin to test it result in an immediate crash (see zellij-org/zellij#526). Because I want to use this in my terminal and the default location is ~ which happen to have symlink.

I can't say I understand the WASM sandboxing model but as I understand it, this line https://github.com/wasmerio/wasmer/blob/a3daf3d1130be9b04d95911f28efd56ad23ad3eb/lib/wasi/src/state/mod.rs#L964 generate a panic from within the lib.

Isn't it possible to instead generate an Error that the parent can catch? I mean the function appear to list everything in a directory so shouldn't it not crash. Maybe returning an incomplete list to the caller (with or without Error in it). That way the caller can have a warning message like "Absolute symlink and some other exotic type of files may not be listed here", either dynamically (if an error is returned) or as a permanent warning.

I feel like a lib listing files in a random dir should never panic on that. But a real implementation (even telling "this_symlink points to /etc/passwd but you do not have access to it") can work by not giving access to pointed files. ls present them in red and it appears in all kind of occasions (drive/network not mounted, build dir being cleaned).

I could also understand that the sandbox does not want to have the "points to" filters so just an info like ls without argument may be good enough.

ls *symlink
my_broken_symlink@ # shown in red since we cannot access where does it point to.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

Feel free to reopen the issue if it has been closed by mistake.

[syrusakbary]

Not stale, also related #4062

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[TheLostLambda]

I don't know if there has been progress on that PR here, but I don't think it's stale either?

Admittedly I'm a little out of the loop though!