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

Protocol Handler: Cross-protocol Content #52

Closed
sammacbeth opened this issue Aug 3, 2018 · 9 comments
Closed

Protocol Handler: Cross-protocol Content #52

sammacbeth opened this issue Aug 3, 2018 · 9 comments

Comments

@sammacbeth
Copy link
Contributor

sammacbeth commented Aug 3, 2018

I found that the protocol handler implementation blocks any requests for third-party origins. For my use-case I would like these resources to load as they would do on the web.

After a bit of digging I found that by changing the ProtocolHandler's protocolFlags then cross origin requests will load (see this commit). Changing the URI_LOADABLE_BY_SUBSUMERS flag to URI_LOADABLE_BY_ANYONE allows third-party scripts and images to load, but blocks cross-origin fetch (see nsiProtocolHandler docs for flag descriptions). I am not sure if there are any other implications of this change on security however.

This change also fixes links to urls with this protocol from pages on other protocols (e.g. http). With the current flags, a dat:// link like those on https://hashbase.io/pfrazee/rss-reader-pfrazee do not work.

https://github.com/sammacbeth/libdweb/blob/c8aaf40285a6fa5ed6f0f2bf3a0e6de8726ef6f6/src/protocol/router.js#L452-L460

Depending on use-cases this behaviour may be preferable, or perhaps could be configurable in the API. If cross-origin support is desired as the default, I can make a PR from my branch.

@Gozala
Copy link
Contributor

Gozala commented Aug 6, 2018

@sammacbeth Thanks for bringing this up. This was a deliberate choice as it avoids conversation about secure contexts as per https://www.w3.org/TR/secure-contexts/ mostly because I am not aware of any effort that defines cross protocol interaction.

I also personally prefer if apps did not hit the network at least without explicit user consent, although I admit that is not a best option. I did mean to ask @pfrazee what was the reason of allowing beaker to embed / request http content and this might be a good place to have this conversation.

My intention was to support CORS via CORS headers as tracked by #37 although I have to admit I'm not entirely sure what the behavior would be even if these headers were set.

I prefer to be on more conservative side here as enabling cross-protocol interactions later are likely to cause less pain than doing reverse. I suggest we define all the cross-protocol interactions defined in https://www.w3.org/TR/secure-contexts/ and then try to get a feedback from security team before enabling such interactions.

@Gozala Gozala changed the title Protocol Handler: Cross-origin requests Protocol Handler: Cross-protocol Content Aug 6, 2018
@ghost
Copy link

ghost commented Aug 6, 2018

any effort that defines cross protocol interaction.

Do you mean something like dialing a wss:// URL from e.g. an ipfs:// context?

I suggest we define all the cross-protocol interactions defined in https://www.w3.org/TR/secure-contexts/ and then try to get a feedback from security team before enabling such interactions.

Spot on as always 👍

@pfrazee
Copy link

pfrazee commented Aug 7, 2018

I also personally prefer if apps did not hit the network at least without explicit user consent, although I admit that is not a best option. I did mean to ask @pfrazee what was the reason of allowing beaker to embed / request http content and this might be a good place to have this conversation.

We initially disallowed HTTP/S requests but it was confusing and difficult for people. I figured we could revisit in the future.

@lidel
Copy link
Contributor

lidel commented Aug 15, 2018

Would be useful to have ability of opt-in whitelisting which protocols (or protocol+origin) can be embedded on pages loaded via protocol handler API.

For example in IPFS we will have the mutable ipns:// which may want to embed assets from immutable ipfs://. Perhaps dat:// site or a post on ssb:// could refer to and load some static assets from ipfs:// etc.

@Gozala
Copy link
Contributor

Gozala commented Aug 16, 2018

Would be useful to have ability of whitelisting which protocols (or protocol+origin) can be embedded on pages loaded via protocol handler API.

The main problem here is specification for cross protocol interactions similar to https://www.w3.org/TR/secure-contexts/ making such a specification open ended is going to be even more challenging. Ideally this is an exploration space that would eventually lead us to standards track. For that reason I prefer to avoid cross protocol interactions, as I fear it will bite us if we would.

Please Note: That APIs like UDP, TCP and rest are less problematic because they are protocol implementation details that would not need to be standardized even if browsers shipped ipfs, dat, ssb.

For example in IPFS we will have the mutable ipns:// which may want to embed assets from immutable ipfs://. Perhaps dat:// site or a post on ssb:// could refer to and load some static assets from ipfs:// etc.

Is referring ipfs from ipns really necessary ? Could not one just link to the relevant content at the ipfs level and refer to it as relative resource ?

@Gozala
Copy link
Contributor

Gozala commented Aug 16, 2018

To expand further on that embedding http(s) content in dat / ipfs would mean that site / app:

  1. Won't be fully functional when offline.
  2. Server content is located could track you.

Probably list is far longer, but point is issues & properties in one protocol leak to the other if cross-protocol embedding is possible, so if one turns out to be safe and other isn't allowing cross-protocol interaction would make the other one also unsafe.

ipns / ipfs interaction is somewhat special as in practice it's the same protocol just namespace separation by semantics.

@lidel
Copy link
Contributor

lidel commented Aug 16, 2018

Is referring ipfs from ipns really necessary ?

No, not necessary: one can include all static assets as relative resources and load everything over ipns:// just fine. It is just one of those "nice to have" things that people will probably try to do, just like @pfrazee mentioned, and we should have a good answer why we don't allow it. I think we do now.

[..] properties in one protocol leak to the other if cross-protocol embedding is possible [..]

Those are good arguments against allowing cross-protocol by default, fully agree with them.

Just to be clear, the ability to whitelist another protocol I had in mind was an opt-in, perhaps via options parameter passed to browser.protocol.registerProtocol.
It would be mostly useful to ipns/ipfs, but if someone decides to build foo:// that whitelists embeds from https:// then.. more data points for standards track.

@Gozala
Copy link
Contributor

Gozala commented Aug 18, 2018

No, not necessary: one can include all static assets as relative resources and load everything over ipns:// just fine. It is just one of those "nice to have" things that people will probably try to do, just like @pfrazee mentioned, and we should have a good answer why we don't allow it. I think we do now.

Yes it is going to work now. It was undesired effect of #69 but it was not possible to fix that as things stand now without this, it also enables HTTP(S) embedding, but again not something I wanted. There is also #70 to make cross-protocol embedding not possible, it's just non trivial change.

Just to be clear, the ability to whitelist another protocol I had in mind was an opt-in, perhaps via options parameter passed to browser.protocol.registerProtocol.
It would be mostly useful to ipns/ipfs, but if someone decides to build foo:// that whitelists embeds from https:// then.. more data points for standards track.

Yeah I understand what you mean, it's just I'm trying to think ahead to avoid building an ecosystem that people start depending on that will have to break.

There is another detail worth considering. While referring to ipfs from ipns is reasonable it's probably not other way round as that would sneak in mutability into not otherwise mutable space.

@Gozala
Copy link
Contributor

Gozala commented Aug 7, 2019

Not sure what to do with this issue, but in an effort to migrate effort into gecko repo & bugzilla I'm closing this. I made a not of this in https://bugzilla.mozilla.org/show_bug.cgi?id=1569733

@Gozala Gozala closed this as completed Aug 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants