-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
RFC: Handle Symbolic links in the Gateway #3508
Conversation
While I do like this approach, i don't think its quite right. This will solve the problem of reading paths like |
@whyrusleeping yeah your right, this was a first stab at the problem. Do you think |
I think the answer to the question is if we fix the resolver to follow the links, |
So it looks like to do this in the resolver ResolveLinks (in
I see (1) is unavoidable unless we want to introduce some abstract notion of a symbolic link. For (2) we can either explicitly detect cycles or use a length limit, for (3) for now, the safest thing would be to refuse to resolve symbolic links with ".." in it. For (4) I think it should be configurable, otherwise we might resolve symbolic links in places where it might not be desirable. |
Note, if we disallow ".." is the symbolic name, I think it will avoid cycles, but i am not 100% sure. Especially if the path is "/ipns" or something else where the id is not the hash of the content. If the id is the hash of the content I don't think there is a way to create a cycle as back links are highly improbable (but I suppressible possible, just next to impossible to create). |
I was moving towards this with my refactor of the resolver that added the
For this, the resolver should simply count the number of symlinks its resolved so far, and error out after a certain number (I think
We should allow '..', it should be relative to the paths being resolved, and the 'root' should be
I think we should always follow symlinks in the resolver. (at least in a unixfs context, via the ResolveOnce function discussed above) |
@whyrusleeping rather than limit the number of symbolic links, what about limiting the number of path components resolved to say 255. It might be easier to implement that way. |
On The intent here was to provide a contextual transformation of the graph. For unixfs sharding, the raw graph looked weird, the raw paths would look something like:
But when browsing this in a unixfs context, we wanted it to look like:
So to transparently perform this transformation, we are going to add context to the resolver in the form of the The dummy 'unixfs resolve once' function is here: https://github.com/ipfs/go-ipfs/blob/master/unixfs/io/resolve.go Thinking about this in the context of symlinks makes me realize that each resolution is very likely going to want some degree of state, so that we can do things like keep track of the number of symlinks traversed during this resolution, and total path length, and so on. To that end, i think we should change it from a |
@whyrusleeping if we change |
|
If Qm20 is constructed to be malicious, but some user trusts it anyways, then there are easier ways to make them download things (like linking directly to the executable). We are supporting ".." in symbolic links. |
@whyrusleeping I am fine with that. |
We still need to decide if we want to handle absolute and what the semantics of them will be if we do. |
@whyrusleeping I'm fine with that. But I was more referring to absolute paths in symbolic links and what it should mean. For example should a link be |
It should be |
Hi folks! What is the status of this PR ( and the corresponding https://github.com/ipfs/go-ipfs-gateway/issues/6 )? I put together an example of an ipfs-ified tarball: https://ipfs.io/ipfs/QmbWphVroGYPVAkRGNHdzYJCrJHcwbd35ygvy32gBUKNpB, and the following still won't work as of writing this message: https://ipfs.io/ipfs/QmbWphVroGYPVAkRGNHdzYJCrJHcwbd35ygvy32gBUKNpB/linkz.tar.extracted/linkz/pointer/sym Thanks! |
I just wanted to ask: Would it be generally possible that if a path |
License: MIT Signed-off-by: Kevin Atkinson <[email protected]>
54e15e6
to
0fe423c
Compare
@jcaesar that could work, but it doesn't really simply much, it might makes things more complicated I am working on this now. Getting the resolver to support symbolic links is messy and I had several false starts. Getting it to return enough information so that the symbolic links can be handed outside of the resolver is equally messy. Will keep working on it until I get something I am happy with. |
As far as absolute paths, by forcing a I still think As a longer term solution there should be a way for a symbolic link to refer the a parent directly. One idea is to assign each parent a random id that is stored with the directory, when traversing a directory the resolver remembers this id so you can refer to it using a special namespace, maybe |
After talking with @whyrusleeping we agreed this will likely involve some API changes, I will figure out what those all and likely propose it separately. |
Closing as this was created before we had proper security isolation on the web (subdomain gateways, unique origin per content root). Continued in #8437 (comment) |
Per @Kubuxu @lgierth request.
Closes https://github.com/ipfs/go-ipfs-gateway/issues/6
This is one way to handle it.
Another would be to just have the
Cat
API call resolve the symbolic link. Another would be to have the gateway resolve the link instead of using a HTTP Redirect.I like the HTTP redirect because it is simple and better interacts with the browsers cache. It also avoids any possible security problems from a malicious symbolic link.