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

randomBytes is required from randombytes which requires from crypto #232

Open
aki-cat opened this issue Mar 7, 2024 · 6 comments
Open

Comments

@aki-cat
Copy link

aki-cat commented Mar 7, 2024

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
Copy link
Member

ljharb commented Mar 7, 2024

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
Copy link
Author

aki-cat commented Mar 7, 2024

It will use this:

image

Which will not be implemented.

@ljharb
Copy link
Member

ljharb commented Mar 7, 2024

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
Copy link
Author

aki-cat commented Mar 8, 2024

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
Copy link
Member

ljharb commented Mar 8, 2024

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants