browserify / crypto-browserify

partial implementation of node's `crypto` for the browser
MIT License
653 stars 199 forks source link

randomBytes is required from randombytes which requires from crypto #232

Open aki-cat opened 6 months ago

aki-cat commented 6 months ago

I have been having issues with browserified code using this package because it depends on non-browserified code to implement itself.

Basically, this line is the issue:

exports.randomBytes = exports.rng = exports.pseudoRandomBytes = exports.prng = require('randombytes');

When you require crypto-browserify, you're not using the non-browserified crypto package. But if you go to randombytes' implementation, it requires crypto in its code. This causes two issues:

  1. If you are resolving requires to cryptoas crypto-browserify, you get a circular dependency and the randomBytes field becomes undefined.
  2. If you are not, randombytes will just require regular crypto, which defeats the whole purpose of using a browserified package in the first place. You probably won't have it available in your environment and just end up with nothing in the randomBytes field anyway.

Maybe this is an issue for randombytes instead. But I figured that most packages that depend on randomBytes usually require crypto instead of randombytes.

ljharb commented 6 months ago

In a browser, randombytes will use https://github.com/browserify/randombytes/blob/master/browser.js (per https://github.com/browserify/randombytes/blob/master/package.json#L25), and I don't see any use of node crypto in that implementation.

aki-cat commented 6 months ago

It will use this:

image

Which will not be implemented.

ljharb commented 6 months ago

If that function doesn't exist (checked here then it uses https://github.com/browserify/randombytes/blob/master/browser.js#L12 which throws, which is by design.

I don't see any infinite loop here. Are you sure you're using browserify as a bundler?

aki-cat commented 6 months ago

I am using browserify module resolution via webpack.

And yeah, depending on how I bundle things, I'm getting that error actually. Sorry for not being clearer. This is happening to me outside of browser usage, even when running via node in CLI. My target use however is to use JavaScript as an embeded language using C# and a library that implements a JS interpreter. I'm assuming this is not an intended use-case. 😅

My concern is rather that there is no actual implementation of the actual algorithm without depending on the VM, which in my opinion defeats the purpose (I understand if you think otherwise).

I just have no options as this is an issue being caused by some other dependency of my code depending on this package, and there are frankly no alternatives to this other than implementing the algorithm itself somewhere. I assumed this would be the place for it to be implemented? I am definitely willing to help with it if that's the case, but I figured it should be discussed first.

ljharb commented 6 months ago

Bundling for "not a browser" is always an unintended use case :-)

You are correct that there is no actual implementation of the algorithm that doesn't rely on something native - whether from the browser or from node. If there is a way to implement it correctly in JS, then the place to do that would be the randombytes package, in the browser entrypoint I liked.

SValentyn commented 5 months ago

Maybe it will be useful for someone: Solving a crypto-polyfill usage error by using the Web Crypto API