-
Notifications
You must be signed in to change notification settings - Fork 29
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
read_link should be allow absolute paths #353
Comments
Previously it would return an error if the link's destination were absolute. But there were three problems with this: * It wouldn't return an error if the link's destination were a relative path that uses ../ to point outside of the sandbox. That's inconsistent. * Sometimes a symlink with an absolute path doesn't escape the sandbox. For example /sandbox/link -> /sandbox/target. But read_link wouldn't allow that. * More importantly, sometimes it's important to read a link that points outside of the sandbox. For example, a backup application could copy all of the files from the root directory of one computer to a subdirectory of another. Symlinks would be broken, but would work again after a restore operation. The application that performs the restore would need to be able to read link targets, even if they point outside of the root. Or, a jail file system could contain absolute symlinks that resolve correctly for a jailed process, but not for an unjailed one. But it would still sometimes be useful for an unjailed process to read the links. Fixes bytecodealliance#353
Previously it would return an error if the link's destination were absolute. But there were three problems with this: * It wouldn't return an error if the link's destination were a relative path that uses ../ to point outside of the sandbox. That's inconsistent. * Sometimes a symlink with an absolute path doesn't escape the sandbox. For example /sandbox/link -> /sandbox/target. But read_link wouldn't allow that. * More importantly, sometimes it's important to read a link that points outside of the sandbox. For example, a backup application could copy all of the files from the root directory of one computer to a subdirectory of another. Symlinks would be broken, but would work again after a restore operation. The application that performs the restore would need to be able to read link targets, even if they point outside of the root. Or, a jail file system could contain absolute symlinks that resolve correctly for a jailed process, but not for an unjailed one. But it would still sometimes be useful for an unjailed process to read the links. Fixes bytecodealliance#353
Previously it would return an error if the link's destination were absolute. But there were three problems with this: * It wouldn't return an error if the link's destination were a relative path that uses ../ to point outside of the sandbox. That's inconsistent. * Sometimes a symlink with an absolute path doesn't escape the sandbox. For example /sandbox/link -> /sandbox/target. But read_link wouldn't allow that. * More importantly, sometimes it's important to read a link that points outside of the sandbox. For example, a backup application could copy all of the files from the root directory of one computer to a subdirectory of another. Symlinks would be broken, but would work again after a restore operation. The application that performs the restore would need to be able to read link targets, even if they point outside of the root. Or, a jail file system could contain absolute symlinks that resolve correctly for a jailed process, but not for an unjailed one. But it would still sometimes be useful for an unjailed process to read the links. Fixes bytecodealliance#353
Previously it would return an error if the link's destination were absolute. But there were three problems with this: * It wouldn't return an error if the link's destination were a relative path that uses ../ to point outside of the sandbox. That's inconsistent. * Sometimes a symlink with an absolute path doesn't escape the sandbox. For example /sandbox/link -> /sandbox/target. But read_link wouldn't allow that. * More importantly, sometimes it's important to read a link that points outside of the sandbox. For example, a backup application could copy all of the files from the root directory of one computer to a subdirectory of another. Symlinks would be broken, but would work again after a restore operation. The application that performs the restore would need to be able to read link targets, even if they point outside of the root. Or, a jail file system could contain absolute symlinks that resolve correctly for a jailed process, but not for an unjailed one. But it would still sometimes be useful for an unjailed process to read the links. Fixes bytecodealliance#353
It's not about preventing paths from being followed, because indeed, cap-std already does that. It is about presenting a view of the filesystem which isn't inherently rooted. If applications create absolute-path symlinks, they will be unwittingly leaking information about the host filesystem hierarchy outside the sandbox. It's an admittedly debatable stance, and I'm open to adding other options (I'll suggest something below).
That's true, however absolute paths that point to locations inside the sandbox still aren't permitted in general, because they leak information about the filesystem hierarchy outside the sandbox. For example, even though /home/sunfishcode/../../sandbox/target might end up inside the sandbox, if opening it succeeds, it reveals that /home/sunfishcode exists. So perhaps what we should eventually do there is make
Would it work for your purposes if we added a new |
xref #252 |
Adding a new method would work for me, as long as it doesn't need an AmbientAuthority. My application runs with Capsicum, so I can't get an AmbientAuthority except during startup. |
@cgwalters unfortunately cap_primitives::fs::read_link_contents won't work here, because that requires a |
@cgwalters Ah, thanks. I forget we added that. So yes, adding a And yeah, thinking about it more, I don't think this needs an |
Somewhat related to this...the current README contains an argument why cap-std doesn't use I would like to at least have a discussion about supporting that. This isn't the first time for me I've wanted it (it just came up again in containers/bootc#659 (comment) ). Now to be clear, it's not really onerous to just drop down to the rustix APIs and bypass
(Doesn't compile, just a sketch) |
(and to elaborate on the connection here, we'd pass down the |
I also did coreos/cap-std-ext#54 - and there I think the But as written that API hard requires Linux ( |
@cgwalters Yes, I'm open to supporting this, though I don't have much bandwidth for writing code for it at the moment. I tend to prefer a |
I'm OK to leave it in cap-std-ext for now. But let's ask @asomers - does that cap-std-ext implementation fit your needs if we added more apis like read_link to it? Are the relevant projects Linux-only? |
RESOLVE_IN_ROOT isn't really relevant for what I'm doing. I just need a read_link method that returns that literal value of the symlink and nothing else, like read_link_contents does. And for me, the relevant project isn't Linux-only, in fact it's non-Linux (specifically, FreeBSD) only. |
OK. Then indeed let's just do this:
Since I think everyone is in agreement. |
The
read_link
function will return an error if the link's destination is an absolute path. There are three problems with this:../
to point outside of the sandbox. That's inconsistent./sandbox/link -> /sandbox/target
.I suggest simply removing the absolute path check.
The text was updated successfully, but these errors were encountered: