fix: crypto in insecure browser context

[alanshaw]

This PR adds crypto-browserify to the dependencies and replaces crypto with crypto-browserify when bundled in the browser.

Please see #105 for more context. TLDR; webcrypto is unavailable in insecure context i.e. not HTTPS or localhost. This effectively makes JS IPFS unusable when accessed in the browser from HTTP.

In files that require webcrypto we check to see if it's available. If it is not we require the Node.js implementation (which has crypto replaced with crypto-browserify) and if it is available then we use the webcrypto version (so we get fast crypto).

Shipping crypto-browserify adds to the bundle size:

Current gzipped size: 142,824 bytes
New gzipped size: 214,499 bytes

Difference: +71,675 bytes

It's not an insignificant addition so we need to decide whether this is worth it.

If not accepted, we need to add checks when libp2p-crypto methods are called and callback with an appropriate error message. JS IPFS will continue to have issues opened with confusion around this otherwise! See ipfs/js-ipfs#963 ipfs/js-ipfs#964 ipfs/js-ipfs#2017 ipfs/js-ipfs#2153

resolves #105
resolves ipfs/js-ipfs#2017
resolves ipfs/js-ipfs#2153

cc @lidel @autonome

[achingbrain]

I wonder if we couldn't log a bunch of warnings and then pull the big, slow implementation down at runtime if we detect we are running under http? Or use code splitting so as to not punish people who do the right thing?

[alanshaw]

Yep, that might be a good optimisation to make as an iteration on this. We'd have to weigh up if not being able to use IPFS in an offline environment is worth it though.

[daviddias]

What about:

• Auto detect if being run in insecure context and log a nice error that informs the users
• Make the crypto implementation a config option with the default being WebCrypto. That way, users can pass crypto-browserify for non secure origins and solve the issue themselves.

I believe that most of the pain points here are caused due to lack of information (most devs don't know the intricacies of WebCrypto) and have no option to fix it themselves.

[hugomrdias]

I support David's proposal 100%

[lidel]

"Secure Context" according to Browser Vendors

FYSA current browser vendor implementation is:

• Chrome restricts access to webcrypto to Secure Contexts since version 60.
• Firefox is planning to Require [SecureContext] for Web Crypto) as well.

This means js-ipfs is effectively broken on pages loaded from non-HTTPS or non-127.0.0.1 gateways

Examples of "insecure context" issues:

• localhost gateway at http://localhost is not a secure context(!) – super confusing for devs
  note: to be a Secure Context it needs to be accessed via raw IP
  (http://127.0.0.1:8080 or http://[::1]:8080)
• IPFS-in-a-box as a HTTP Gateway for trusted/offline LAN won't be a Secure Context
• Localhost HTTP Proxy running at http://{cid}.ipfs.localhost will be as secure as a regular gateway on 127.0.0.1 but browser won't expose webcrypto on .ipfs.localhost
• we can't run js-ipfs in DataURL-based ephemeral pages (data:text/html,<h1>hello</h1> is not a Secure Context)

"Meaningful Error" vs "Just Work"

I am personally torn here, but below opinion is formed by where IPFS is right now.

Printing meaningful error messages (multiformats/js-multihashing-async#30) will help some developers, but it is naive to think we won't have issues about this going forward. Dweb developers are unable to fully control how websites are distributes/mirrored/loaded with IPFS, missing webcrypto will continue to break things, and issues will be reported, but this time by third parties using unencrypted gateways in LAN etc.

I don't like the increased bundle size, but imo breaking things in insecure contexts is much worse. Beginners and regular users won't care about Secure Contexts, just like they did not care about intricacies of DHT or IPNS. More technical devs will google the error and find multiple issues opened over and over again. Both cases will contribute data points forming opinion that IPFS is flaky. We should make it "Just Work".

TL;DR I am leaning towards supporting insecure contexts out of the box: we should print warning about degraded performance due to lack of native webcrypto APIs, we can optimize it later and lazy-load crypto-browserify, but it should "Just Work".

[jacobheun]

I agree with @lidel, it just needs to work out of the box. We can look at adding more advanced configurations to lower bundle size for developers, but we need to make sure the barrier to entry is small, even if it bloats the bundle size a little bit.

In regards to the bundle size, does node-forge not give us the crypto support we need in the browser? We seem to have leveraged it less and less, but it's still included as a dependency. There are some other issues that would benefit from us using node-forge for everything, such as #125, so if we just use it, we could potentially keep the bundle size down. I'm going to look more at node-forge and see if there's a gap there, unless someone already knows the limitations.

[vasco-santos]

I share the same opinion as some folks before. Firstly, it should just work without any extra configurations. Then, advanced users should be able to configure IPFS, in order to reduce their bundle size

[hugomrdias]

The problem is we can't include crypto-browserify and say advanced users can reduce size with an option, once crypto-browserify it's there it's there.

Dynamic import also not an option for several reasons already discussed in many issues like this one (maybe one day).

Descriptive error messages with a link to documentation are really underrated in this crowd 😆.
This approach has been used forever and React made it the norm for browser modules and it's a better compromise than increasing size and bloat.
We made this compromise for ipld formats ipfs/js-ipfs#1980 and it was a good one.
We should do the same here!

This is obviously a problem, especially the local gateway/api situation, but browsers forced this onto us nothing we can do about it.
Refs:
https://letsencrypt.org/docs/certificates-for-localhost/
https://justmarkup.com/articles/2018-05-31-https-valid-certificate-local-domain/

We should not degrade the experience of the majority of the users for the "just works" dream especially when we have good options.

"Just works" "Works out of the box" from my experience always breaks somewhere, brings bloated code and degrade experience for the end user. On the other hand consistency, descriptive errors, documentation with copy paste solutions makes users feel comfortable and transmits maturity.

[alanshaw]

In regards to the bundle size, does node-forge not give us the crypto support we need in the browser? We seem to have leveraged it less and less, but it's still included as a dependency.

This might be a good option. I did a quick look and it seems it already has everything we're using except ECDH. We could use https://github.com/indutny/elliptic for that.

[daviddias]

I believe this to be a clear example of values in tension, things such as "Just works" "Works out of the box" "Magical" often collide with "explicit" "maintainability" "clarity" and "robustness".

Given that IPFS's mission is to make a P2P Content Addressed File System for many centuries, I'm inclined to always lean on whatever strategies will enable us to weather future storms (e.g. Clarity on what it is doing, debugability, good documentation, good tests).

Update: To clarify, I'm not advocating to go fully verbose to the point that we exhaust everyone's will tank to the point that they can not use the implementation anymore. It is great to have sane defaults that help the majority of the users and/or that prioritize the use cases for which it was designed. The tricky bit is when there is tension, one needs to know what to prioritize and in those cases, less magic is preferred.

[daviddias]

Btw, another option is to finally create the js-ipfs-core package, similar to what js-libp2p is, just a harness in which one plugs all the different components and then have js-ipfs to be the "preferred flavor of the IPFS Core developers, but you are still welcome to use ipfs-core if you want to"

[alanshaw]

My main concern is getting something done about this ASAP. It's been a very long time to not do anything about this problem and given that it completely prevents people using IPFS or libp2p in the browser without paying for an SSL cert, I think we should bump the priority slightly 😜

I don't understand what's magical or unmaintainable about using a module that is built specifically to enable crypto in the browser with an API that's identical to Node.js. We already have code to interface with Node.js's crypto module - it's a drop in replacement.

Is it magical to have crypto in the browser when in an "insecure context"? I don't feel it is magical since webcrypto hasn't been around forever and people have been bundling crypto-browserify for 3+ years before webcrypto was even standardised.

On the other hand, are the reasons that browsers have decided we can't have webcrypto in insecure contexts relevant to us? I don't know exactly the reasons or exactly what we expose ourselves to if we do enable crypto in the browser, but my gut feeling is that it's a valid reason to not make it just work by default.

Bundle size is a valid concern and is one of the reasons why I dived deeper to investigate this - before we had no idea of the overhead, now we do. 70kB feels like a lot IMHO, but if my ONLY choices are, use HTTPS or increase the bundle size then...I'd go for increase the bundle size every time. I think everyone paying a small price to enable the people who are not privileged with an SSL cert (either by cost, knowledge or experience) is worth it.

Luckily those aren't the only choices, I think the suggestion of passing your own webcrypto implementation is great - I hadn't thought of that (thanks @daviddias). That in combination with a great error message sounds like the most pragmatic solution to me. It seems like the best of both worlds: we don't lock out people using IPFS in insecure contexts, we help them to fix it, we don't have to increase our bundle size AND we don't open ourselves up to any exploits by using crypto in insecure contexts.

[daviddias]

Another concern is also performance. crypto-browserify is significantly slower than WebCrypto (hence the reason why we moved away from it in the first place. How should we go about communicating to users that "Your thing might run 10x slower if you don't get that SSL cert?", if not communicated clearly and just "resolved by shimming crypto-browserify", you will start having users that report IPFS sometimes being way slower on the browser.

[lidel]

are the reasons that browsers have decided we can't have webcrypto in insecure contexts relevant to us?

Some time ago browser vendors decided to add new WebAPIs only to Secure Contexts. The goal was to incentivize use of HTTPS. WebCrypto is an example of that. It is also especially sensitive, because if page was not loaded in secure context, the JS code using the API could be modified via MITM by a malicious third party and replaced with one that leaks private keys etc.

For IPFS this matters if a page was loaded over unencrypted HTTP from non-localhost gateway. I think we could display additional PSA warning message if window.isSecureContext is false and window.location.hostname does not point at localhost, informing user about potential for MITM due to the lack of HTTPS.

I think the suggestion of passing your own webcrypto implementation is great [..] That in combination with a great error message sounds like the most pragmatic solution to me.

I agree. This would unlock devs to use IPFS in contexts without webcrypto apis without the side effects pointed out by @hugomrdias.

We may still get issues reported by third parties, but at least there will be a path for them to ask upstream developer to provide a fix (if feasible).