ipfs / ipfs-webui

A frontend for an IPFS Kubo and IPFS Desktop
https://webui.ipfs.io
MIT License
1.56k stars 489 forks source link

Share Link incorrectly gives path routed instead of subdomain routed URL. #2244

Closed MicahZoltu closed 1 month ago

MicahZoltu commented 9 months ago

Describe the bug When you right click on a file in Files and choose "Share Link" you are given a URL like https://<host>/ipfs/<cid>. This should be of the form https://<cid>.ipfs.<host> for security reasons.

To Reproduce Steps to reproduce the behavior:

  1. Go to Files tab.
  2. Right click on any file.
  3. Choose "Share Link"
  4. Notice the link provided uses path routing.

Expected behavior Subdomain routing is always used.

Additional context Path routing is known to be insecure for websites that use cookies, local storage, etc. This is well documented in the IPFS documentation and the documentation and security experts all recommend using subdomain routing whenever possible (which is almost always possible). These share links are encouraging people to share URLs that are insecure by default, and we should instead be using subdomain by default.

whizzzkid commented 9 months ago

Thanks @milahu this sounds like a good feature to have, the reason it doesn't have it today is because not all gateways support subdomain gateways. The default gateway does, but that's not the norm. I think we can implement a simple check to validate if the server supports subdomain gatways and then generate those links.

I'll mark this as a backlog item.

acul71 commented 2 months ago

I'm working on fixing this issue

I'm currently working on a fork of ipfs-webui https://github.com/ipfs/ipfs-webui at https://github.com/acul71/ipfs-webui-fork.

Should there be a preference setting to enable or disable Subdomain Gateway URLs? Should this be shown as a preference info item? Thanks!

lidel commented 2 months ago

@acul71 thank you for looking into this.

Note that sending https://<host>/ipfs/<cid> to subdomain gw like dweb.link returns HTTP 301 to correct URL at https://<cid>.ipfs.<host>, there is no security issue, and user always ends up in unique origin based on root CID.

Is your intention to avoid this redirect when default is changed to subdomain gateway like dweb.link? If so, you need to check that <cid> is no longer than 63 characters (reason below).

Note that the current default in ipfs-webui is still ipfs.io (path gw):

https://github.com/ipfs/ipfs-webui/blob/27bea32930deabf518bbc015be5b7e7307cb2c43/src/bundles/gateway.js#L3

So the users by default will not benefit from your check. Changing the default here would do the trick.

But, there is a problem with using dweb.link by default: we did not switch default to subdomain gateways because not every asset is website, not everything needs origin isolation, and some long CIDs/hashes are not compatible with DNS label length (https://github.com/ipfs/kubo/issues/7318).

As prior art, IPFS Companion is bit further in the migration, and has two settings:

image

Perhaps ipfs-webui should have something similar, with extra check to decide which one to use, based on shared CID:

dweb.link (subdomain isolation) being listed first, and being the new default, but ipfs.io (path gateway fallback) being listed also, used for CIDs that can't be represented in 63 character DNS label. This way user can adjust both subdomain and path gateway they use, but by default we use subdomain whenever possible.

Thoughts?

acul71 commented 2 months ago

@lidel Thank you for your detailed response.

I agree with your suggestion that "ipfs-webui should have something similar, with an extra check to decide which gateway to use based on the shared CID." I'll proceed in this direction.

Thanks for the guidance!

acul71 commented 2 months ago

@lidel

Hello there. I've modified the setting page like this:

image

Are the labels ok? Should I add some tests, which ones do you suggest? In this case how do I get a CID v1 with lengh > 63 chars? Thanks.

SgtPooki commented 2 months ago

Are the labels ok?

I think the labels are decent, but they feel a little too similar. Maybe we need a disclaimer saying that certain content that doesn't need origin-isolation for the subdomain gateway input.

Should I add some tests, which ones do you suggest?

I think a simple test on the use of those config items would be most important. Then two basic tests ensuring the reset and submit buttons work.

In this case how do I get a CID v1 with lengh > 63 chars? Thanks.

Here is a very long CID v1 (using sha3-512): https://explore.ipld.io/#/explore/bagaaifcavabu6fzheerrmtxbbwv7jjhc3kaldmm7lbnvfopyrthcvod4m6ygpj3unrcggkzhvcwv5wnhc5ufkgzlsji7agnmofovc2g4a3ui7ja

acul71 commented 2 months ago

@lidel @SgtPooki https://github.com/acul71/ipfs-webui-fork/pull/1

lidel commented 2 months ago

@acul71 are you using AI or some GUI tools? That PR makes no sense (it merges from main in this repo to a branch in your repo, and changes described in PR description are not in git).

What you want to do is to open PR from your form into this repository.

acul71 commented 2 months ago

@lidel Sorry did submit too late in the night.... Thanks, is this ok: https://github.com/ipfs/ipfs-webui/pull/2255