search

MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.66k stars 4.78k forks source link

ENS IPFS redirect doesn't work with localhost gateway. #10121

Open MicahZoltu opened 3 years ago

MicahZoltu commented 3 years ago

Describe the bug If you set the IPFS gateway to localhost:8080 it fails to redirect ENS queries and instead sends you to the ENS management app.

Steps to reproduce (REQUIRED)

  1. Install IPFS Desktop.
  2. Run IPFS Desktop.
  3. Set IPFS Gateway in MetaMask to localhost:8080
  4. Attempt to navigate to http://augur2.eth/
  5. Notice that you get taken to ENS management page.
  6. Set IPFS Gateway in MetaMask to cf-ipfs.com
  7. Attempt to navigate to http://augur2.eth/
  8. Notice that you get taken to Augur.

Expected behavior Localhost IPFS gateway actually works so people can browse privately and securely.

Browser details (please complete the following information):

Additional context (Error Messages, etc.) ENS to IPFS redirects to localhost work with IPFS Companion browser extension.

jacobc-eth commented 3 years ago

@MicahZoltu could you check if this is working if you change the url from http:// to https:// ?

Gudahtt commented 3 years ago

I was able to reproduce this locally by installing IPFS Desktop from npm, and testing this with a dev build on Firefox (curiously, our address bar ENS resolution seems entirely non-functional on dev builds on Chrome).

The problem here is two-fold. First, our IPFS ENS resolution doesn't seem to work well with IPFS Desktop. I am not yet sure why. I can see in the debugger that it does make a request to the local gateway, but it always fails.

Second, if we fail to resolve an .eth address, we fallback to navigating to the ENS management page. This fallback behaviour is convenient for many users, but I can see why you'd find it undesirable if you're using a custom gateway (particularly if it's a local gateway).

Gudahtt commented 3 years ago

The problem here is two-fold. First, our IPFS ENS resolution doesn't seem to work well with IPFS Desktop. I am not yet sure why. I can see in the debugger that it does make a request to the local gateway, but it always fails.

I investigated this a bit further, and was able to get this to work on Firefox by using http for the resolved URL instead of HTTPS. Additionally, I used localhost:8080 instead of 127.0.0.1:8080. I don't really know why this worked though.

Maybe we could add special handling of localhost, to use http rather than https? I'd like to understand better why https failed first before considering something like that. I don't know a lot about IPFS, or how TLS is typically handled with local gateways. Similarly, we could normalize 127.0.0.1 to localhost if necessary, but I'd like to understand better why it fails first before considering that.

MicahZoltu commented 3 years ago

@MicahZoltu could you check if this is working if you change the url from http:// to https:// ?

Do you mean change the URL I try to navigate to to https://augur2.eth/ or change the IPFS host to https://localhost:8080? In either case, neither are served via TLS so that shouldn't work and having that be a requirement would be a bug in itself. IPFS doesn't serve TLS and localhost rarely is served via TLS (because it is a huge PITA and unnecessary).

MicahZoltu commented 3 years ago

Ah, it sounds like the bug here is that MetaMask is trying to force TLS connections to the provided gateway. This is an incorrect assumption/behavior for 127.0.0.1 and (as of recent versions of browsers) also for localhost. TLS is necessary for ensuring you are communicating with the host you think you are communicating with. 127.0.0.1 is by definition the loopback device and will always be local, so there is no need for TLS (it is purely wasteful, and adds significant complexity to connections).

localhost technically is resolved via DNS resolution, and historically has resolved to 127.0.0.1 by default but this behavior can be changed on any given host such that localhost resolves to some other (potentially non-local) address. However, recently browsers have changed their behavior such that they always resolve localhost to 127.0.0.1. This allows them to treat localhost and localhost subdomains as trusted sources (similar to TLS connected hosts). If you are using a modern version of chrome, firefox, or brave (I'm not sure about Safari), then localhost and *.localhost should behave the same as 127.0.0.1 on all hosts (with the exception of passing along a Host: localhost header or not).

Gudahtt commented 3 years ago

Thanks, that makes sense! Special handling of localhost to allow http seems like the clear solution for this then. And I can look into the localhost vs. 127.0.0.1 issue further - it seemed like it was the platform treating them differently, not us. But I will confirm.

Any thoughts on whether it still makes sense to fallback to the ENS Management page if it fails to resolve using a local gateway? That is hard-coded as our fallback behaviour despite the gateway being used at the moment, but from a privacy perspective I can see why that might be undesirable.

MicahZoltu commented 3 years ago

Any thoughts on whether it still makes sense to fallback to the ENS Management page if it fails to resolve using a local gateway? That is hard-coded as our fallback behaviour despite the gateway being used at the moment, but from a privacy perspective I can see why that might be undesirable.

If there is no IPFS address attached to the name, then falling back to ENS management page may make sense. Ideally, you would fallback to an IPFS hosted version of the page though, rather than a centralized version of the page.

If there is an IPFS address attached to the name, I think falling back is a bad idea for the privacy reasons you eluded to.

Gudahtt commented 3 years ago

Thanks, that makes sense.

Ideally, you would fallback to an IPFS hosted version of the page though, rather than a centralized version of the page.

I've created a separate issue for this: #10351

If there is an IPFS address attached to the name, I think falling back is a bad idea for the privacy reasons you eluded to.

So this is how the current implementation is supposed to work. Unfortunately it doesn't distinguish between a complete failure to resolve the ENS address, and a successful resolution that doesn't find an address.

I'll address that as part of this bug. If our attempt to resolve completely fails, in an unexpected way, we shouldn't assume there is no address attached to the name.