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

gateway redirect messes up CORS headers on firefox #436

Closed
fahrradflucht opened this issue Mar 31, 2018 · 10 comments · Fixed by #511 or #771
Closed

gateway redirect messes up CORS headers on firefox #436

fahrradflucht opened this issue Mar 31, 2018 · 10 comments · Fixed by #511 or #771
Assignees
Labels
area/firefox Issues related to Mozilla Firefox status/blocked/upstream-bug Blocked by upstream bugs status/blocked Unable to be worked further until needs are met status/in-progress In progress

Comments

@fahrradflucht
Copy link

fahrradflucht commented Mar 31, 2018

Browser: Firefox 59.0.2
AddOn Version: IPFS Companion (RC1@fb21b6d) 2.2.0.8540 or IPFS Companion 2.1.0

Given this webpage:

<!DOCTYPE html>
<html>

<body>
    <button onclick="window.fetchFromPublicGateway()">Fetch from public gateway.</button>
    <button onclick="window.fetchFromLocalGateway()">Fetch from local gateway.</button>
    <script>
        window.fetchFromPublicGateway = () => {
            fetch("https://ipfs.io/ipfs/QmejvEPop4D7YUadeGqYWmZxHhLc4JBUCzJJHWMzdcMe2y")
                .then(res => res.text())
                .then(console.log)
                .catch(console.log)
        }

        window.fetchFromLocalGateway = () => {
            fetch("http://localhost:8080/ipfs/QmejvEPop4D7YUadeGqYWmZxHhLc4JBUCzJJHWMzdcMe2y")
                .then(res => res.text())
                .then(console.log)
                .catch(console.log)
        }
    </script>
</body>

</html>

when clicking on the public gateway button firefox will cancel the request and issue a same-origin-policy warning due to missing CORS headers even though the local gateway has the necessary CORS-headers set which can be checked clicking the other button (be sure to check which origin you are accessing the document when using the IPFS link).

I checked Chrome 65 for comparison and the issue isn't present.

@lidel lidel self-assigned this Mar 31, 2018
@lidel lidel added the kind/bug A bug in existing code (including security flaws) label Mar 31, 2018
@lidel
Copy link
Member

lidel commented Mar 31, 2018

Thank you for the sample app, it made reproducing it much easier 👍

Upon inspection of the onBeforeRequest hook Firefox indeed blocks XHR right after redirect:

screenshot

Chrome does not block redirect, second call has updated url field:

screenshot_1

I'll look into it this week.

@lidel
Copy link
Member

lidel commented Apr 3, 2018

It seems to be a bug in Firefox (see findings below).

I filled an upstream bug:

Steps to Reproduce

I created a minimal PoC extension to confirm that the problem is not companion-specific: ipfs-companion-cors-issue-436-mvp.zip

  1. Make sure IPFS Companion is disabled at about:addons (we don't want to have race conditions)
  2. Load poc extension into Firefox and Chrome
  3. Extension opens the test page at ipfs.io
    1. page provides two buttons for fetching the same resource from two different gateways: ipfs.io (the host used for loading test page) and ipfs.infura.io (3rd party host)
    2. the extension registers onBeforeRequest hook that redirects fetch for the resource from ipfs.infura.io to ipfs.io (this will trigger the bug)

Chrome: Success, No Error

Both buttons result in a successful fetch and This is a string downloaded via JS fetch being printed to the console:

mvp-chrome-screenshot

Firefox: CORS Error

Only the first button results in a successful fetch, second button triggers CORS error.
Firefox blocks JS fetch due to CORS.

Screenshot

@lidel lidel added status/blocked Unable to be worked further until needs are met status/blocked/upstream-bug Blocked by upstream bugs and removed kind/bug A bug in existing code (including security flaws) labels Apr 3, 2018
@Mithgol
Copy link

Mithgol commented Apr 13, 2018

Note to self: for the same bogus reason Firefox prevents Pannellum from using IPFS-hosted images (rerouted by IPFS Companion) as WebGL textures.

@lidel
Copy link
Member

lidel commented Apr 18, 2018

Another thing that is broken by this issue: https://ipfs.github.io/public-gateway-checker/
(gateway checker got updated to opt-out from redirects since then)

@lidel
Copy link
Member

lidel commented Jun 4, 2018

Due to discrepancy between how Chrome and Firefox handle CORS validation in webRequest.onBeforeRequest context the upstream fix may take longer than expected.

To mitigate the situation, I decided to disable redirect for cross-origin XHRs that would fail anyway, restoring original functionality to websites impacted by this issue.

See PR #494 – it is an interesting edge case, so feedback would be appreciated.

lidel added a commit that referenced this issue Jun 5, 2018
This PR disables gateway redirect for cross-origin XHRs.

- It fixes all the websites broken by [CORS false-positive bug in Firefox](#436 (comment))
  -  It is a "lesser evil". There will be no gateway redirect for such requests, but at least "IPFS-enabled" websites that do cross-origin requests to different gateways will work.
- The full context is in #436 (comment) including a link to the upstream bug.
@lidel
Copy link
Member

lidel commented Jun 5, 2018

Merged Firefox-only workaround from #494 and released it to beta channel as v2.3.0.9590 and stable as v2.3.1.
It no longer breaks valid websites. Will promote it to stable in a few days.

Let's keep this issue open until https://bugzilla.mozilla.org/show_bug.cgi?id=1450965 is closed.

lidel added a commit that referenced this issue Jul 2, 2018
Context for CORS XHR problems in Firefox:
#436

In short, onBeforeRequest should not change anything, as it will trigger
false-positive CORS error.  onHeadersReceived is after CORS validation
happens, so its ok to cancel and redirect late.

This is not ideal, as there is an outgoing request to the public
gateway, and we need to read response headers before connection is
aborted, but we can't do better than that until
https://bugzilla.mozilla.org/show_bug.cgi?id=1450965 is resolved.
@lidel
Copy link
Member

lidel commented Jul 2, 2018

To restore feature parity between browsers, I decided to enable gateway redirect for CORS XHRs in Firefox using late redirects at a price of small overhead. Details in PR #511.

@lidel lidel closed this as completed in #511 Jul 3, 2018
lidel added a commit that referenced this issue Jul 3, 2018
This PR restores gateway redirect for CORS XHR in Firefox via late redirect in `onHeadersReceived` and closes #436.

Context for why CORS XHR were not redirected until now can be found in #436 (comment) and upstream [Bug #1450965](https://bugzilla.mozilla.org/show_bug.cgi?id=1450965). 

In short, `onBeforeRequest` can't redirect anything when CORS XHR is processed, otherwise it will trigger false-positive CORS error.  Good news is that `onHeadersReceived` is executed  long after CORS validation happens in Firefox, and allows us to cancel original connection and do a late redirect right after response headers arrive. 

This is the best we can do until upstream [Bug #1450965](https://bugzilla.mozilla.org/show_bug.cgi?id=1450965) is addressed.

Additional Notes: 
- The original outgoing request to the public gateway has to happen :( 
  I [raised privacy concerns](https://bugzilla.mozilla.org/show_bug.cgi?id=1450965#c4) in the upstream bug.
-  Good news is that we only need to read response headers before connection is  aborted. The overhead is minimal and I believe it is worth it: enables us to reach feature-parity with Chrome and removes dapp developer confusion that was caused by different redirect behaviors in Firefox and Chrome.
@lidel
Copy link
Member

lidel commented Jul 3, 2018

Merged #511 and released to beta channel as v2.4.0.10190.
If no issues, we will promote it to stable in a few days.

@lidel lidel added status/blocked/upstream-bug Blocked by upstream bugs area/firefox Issues related to Mozilla Firefox and removed status/blocked/upstream-bug Blocked by upstream bugs labels Jul 2, 2019
@lidel
Copy link
Member

lidel commented Jul 2, 2019

According to https://bugzilla.mozilla.org/show_bug.cgi?id=1450965#c33 the bug will be fixed in Firefox 69.

I am reopening this, so we don't forget to remove workaround introduced in #511 when FF69 lands to the stable channel in September (2019-09-03 according to this page)

@lidel lidel reopened this Jul 2, 2019
@KrasnayaPloshchad
Copy link

Firefox 69 is released.

lidel added a commit that referenced this issue Sep 30, 2019
This change removes woraround introduced in PR #494
and restores redirect in onBeforeRequest.

The original bug was fixed in Firefox 69,
that is why we also bump minimal version.

More info at:
#436 (comment)
lidel added a commit that referenced this issue Sep 30, 2019
This change removes woraround introduced in PR #494
and restores redirect in onBeforeRequest.

The original bug was fixed in Firefox 69,
that is why we also bump minimal version.

More info at:
#436 (comment)
@lidel lidel closed this as completed in #771 Oct 1, 2019
lidel added a commit that referenced this issue Oct 1, 2019
This change removes workaround introduced in PR #494
and restores redirect in onBeforeRequest.

The original bug was fixed in Firefox 69,
that is why we also bump minimal version.

More info at:
#436 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/firefox Issues related to Mozilla Firefox status/blocked/upstream-bug Blocked by upstream bugs status/blocked Unable to be worked further until needs are met status/in-progress In progress
Projects
None yet
4 participants