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

Brave Shields block IPFS Companion's request hand off to localhost gateway #13641

Open
lidel opened this issue Jan 18, 2021 · 13 comments · Fixed by brave/adblock-lists#542
Open
Assignees
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop priority/P4 Planned work. We expect to get to it "soon".

Comments

@lidel
Copy link

lidel commented Jan 18, 2021

Description

When IPFS Companion redirects requests for content-addressed subresources to the local IPFS node, Brave Shields block all of them.

Steps to Reproduce

  1. Install Brave (eg. 1.19.85)
  2. Install IPFS Companion
  3. In Companion's Preferences make it us IPFS Node Type "Provided by Brave"
  4. Open https://slate.host/biodivlibrary/andrew-garrett-s-fische-der-suedsee-bd-3/
    This is a web app that stores images on IPFS+Filecoin and the web interface loads images from a public IPFS gateway.
  5. You will see requests handed off to the local IPFS node are being blocked by Brave Shields.

Actual result:

2021-01-18--19-51-01

Expected result:

Brave Shields should at the very minimum safelist the gateway URL of localhost gateway provided by go-ipfs managed by the Brave browser itself (#10220)

2021-01-18--19-56-06

Reproduces how often:

every time

Desktop Brave version:

Brave 1.19.85 Chromium: 88.0.4324.89 (Official Build) unknown (64-bit)
Revision 4534be84786f5269fb52e9bf82643af61e2fedaf-refs/branch-heads/4324@{#1721}
OS Linux

cc @bbondy

@bbondy
Copy link
Member

bbondy commented Jan 18, 2021

This should be fixed by brave/adblock-lists#542 within the next 24 hours.
Thanks @ryanbr. Closing!

@bbondy bbondy closed this as completed Jan 18, 2021
@lidel
Copy link
Author

lidel commented Jan 18, 2021

@bbondy @ryanbr I am afraid that brave/adblock-lists#542 fixed the problem only on slate.host.

Every other website leveraging content-addressing in paths remains impacted: for example https://audius.co leverages content-addressing for images and audio files and is also broken:

2021-01-18--22-24-19

Are we able to change the rule to introduce global exception and allow localhost requests for content paths?

http://localhost:*/ipfs/*
http://localhost:*/ipns/*
http://*.ipfs.localhost:*/*
http://*.ipns.localhost:*/*

Or just "bless" redirects made by specific extension via webRequest API?

@bbondy
Copy link
Member

bbondy commented Jan 18, 2021

Re-opening cc @ryanbr

@ryanbr
Copy link

ryanbr commented Jan 18, 2021

Okay @lidel brave/adblock-lists#544 should help, Have made it generic enough to cover ipfs

@lidel
Copy link
Author

lidel commented Jan 18, 2021

@ryanbr looks good thank you!
Is there a way to test it without waiting for regular blocklist update?

@ryanbr
Copy link

ryanbr commented Jan 19, 2021

@lidel sure, you could test by installing uBO extension and manually adding the following into "My filters", temp disable shields ads/trackers also.

! Specific filters (Tracking or ads) for Brave
||localhost^$third-party,domain=~127.0.0.1|~[::1]
||127.0.0.1^$third-party,domain=~localhost|~[::1]
||[::1]^$third-party,domain=~localhost|~127.0.0.1
! ipfs localhost exceptions
! https://github.com/brave/brave-browser/issues/13641
@@||127.0.0.1*/ipfs/$third-party
@@||127.0.0.1*/ipns/$third-party
@@||localhost*/ipfs/$third-party
@@||localhost*/ipns/$third-party
@@||ipfs.localhost^$third-party
@@||ipns.localhost^$third-party
@@||[::1]*/ipfs/$third-party
@@||[::1]*/ipns/$third-party

@bbondy
Copy link
Member

bbondy commented Jan 19, 2021

I think you can add them into about://adblock too but you might need to restart the browser after that.

@lidel
Copy link
Author

lidel commented Jan 20, 2021

Thanks!
Confirmed it looks good now, can see all the fish!

@lidel lidel closed this as completed Jan 20, 2021
@bbondy
Copy link
Member

bbondy commented Jan 20, 2021

This had to be reverted because Brave doesn't allow localhost access for websites, because websites could probe the available services of a user and use that for fingerprinting.
If the fish page was ipfs:// though top level, then it should allow the subresources to show.

@lidel
Copy link
Author

lidel commented Jan 21, 2021

Ouch, that's unfortunate, but understandable.
Means we need to disable redirect of HTTP subresources when running in Brave.
I will prepare fix on our end (ipfs/ipfs-companion#962)

@bbondy bbondy added the priority/P4 Planned work. We expect to get to it "soon". label Jan 30, 2021
@bbondy
Copy link
Member

bbondy commented Jan 30, 2021

@lidel I was looking at this a little more today, do you know why https://slate.textile.io/ipfs/bafybeiar2jrkjqcacbgsecu46sl6o424qa7oibgam7uqyv4u3radxmc2yy doesn't give x-ipfs-path header? That's how Brave determines IPFS resources to redirect.
What does IPFS Companion do?

I think we can consider this a bug, I did a local test page with a local http server (but served from a.com 127.0.0.1 from /etc/hosts) and a dweb.link IPFS image, and the Brave option for auto redirect IPFS resources works the same because in this case it uses the x-ipfs-path header. I think we can re-open this for now because it's the extension (or Brave) doing the redirecting, but we'd need to make sure of that.

I'm going to set it as p4 for now because I think the fix would involve translating ipfs:// later in the stack than we do now, but I think we can re-open and consider it a bug. Some other issues need to be worked out first.

@bbondy bbondy reopened this Jan 30, 2021
@lidel
Copy link
Author

lidel commented Feb 1, 2021

@bbondy x-ipfs-path is returned by go-ipfs, but support for this header is best-effort: it is sometimes filtered out when go-ipfs runs behind a reverse proxy. It is not a reliable method of identifying content-addressed content on http pages on its own.

Detection of IPFS resources done by IPFS Companion is matching URL before a request is sent to remote server, which removes surface for tracking/logging browsing history, and works even when target server is down.

Heuristic for marking a resource as IPFS one looks like this:

  • Before request is made:
    • IF URL's hostname starts with {cid}.ipfs.* or the path starts with /ipfs/{cid}/ we take string under {cid} and check if it is a real CID to eliminate false-positives like https://github.com/ipfs/foobar
    • IF URL's hostname starts with {foo}.ipns.* or path starts with /ipns/{foo} then we take {foo} and check if it is EITHER
      • a valid CID (decoding works)
      • or a hostname that has DNSLink set up
    • IF DNSLink exist for the hostname
  • When response headers are received:
    • x-ipfs-path is used only as a fallback, when the above fails and regular request was sent to HTTP server.

nightsky108 added a commit to nightsky108/adblock-lists that referenced this issue Dec 21, 2021
floresarmida added a commit to floresarmida/adblock-lists that referenced this issue Dec 22, 2021
redhawkIT added a commit to redhawkIT/adblock-lists that referenced this issue Dec 22, 2021
mixazizup added a commit to mixazizup/adblock-lists that referenced this issue Jul 13, 2022
tylerc2023 added a commit to tylerc2023/adblock-lists that referenced this issue Sep 8, 2022
stevanovicmilan839 added a commit to stevanovicmilan839/adblock-lists that referenced this issue Nov 2, 2022
@kjozwiak
Copy link
Member

Removed the closed/wontfix label as the issue is still opened and it seems like there's still some issues that need to be resolved. However, if this issue is stale, please re-add the closed/invalid or any of the other closed/[x] labels and close the issue off.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop priority/P4 Planned work. We expect to get to it "soon".
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants