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

Cannot read property 'randomBytes' of undefined #35

Open
mpetrunic opened this issue Nov 21, 2019 · 8 comments
Open

Cannot read property 'randomBytes' of undefined #35

mpetrunic opened this issue Nov 21, 2019 · 8 comments

Comments

@mpetrunic
Copy link

mpetrunic commented Nov 21, 2019

Bcrypto panics when rebuilded by electron.
From my investigation it seems that electron replaces openssl with boressl and this causes some incompatibilities. It seems that requiring of native random passes and doesn't fallback to node version.
I've mitigated it with (ELECTRON env is set by user , not electron itself):

  if(process && process.env.ELECTRON) {
    return require("bcrypto/lib/node/random").randomBytes(length);
  } else {
    return require("bcrypto/lib/random").randomBytes(length);
  }

bcrypto 4.2.8

@mpetrunic
Copy link
Author

This is when pbkdf2 native is invoked on electron rebuilded bcrypto:

backend.load is not a function
      at derive (node_modules/@nodefactory/bls-keystore/node_modules/bcrypto/lib/native/pbkdf2.js:35:11)

@tynes
Copy link
Member

tynes commented Nov 23, 2019

@mpetrunic Have you seen this issue? #27

At runtime, NODE_ENV can be set as js and that might fix the problem. The require of the module looks like this:

bcrypto/lib/pbkdf2.js

Lines 9 to 16 in 1d257bb

try {
module.exports = require('./native/pbkdf2');
} catch (e) {
if (process.env.NODE_BACKEND === 'js')
module.exports = require('./js/pbkdf2');
else
module.exports = require('./node/pbkdf2');
}

There is also pbkdf2-browser.js.
https://github.com/bcoin-org/bcrypto/blob/master/lib/pbkdf2-browser.js

@mpetrunic
Copy link
Author

Well code never reaches catch clause. It successfully requires native module and fails to execute function there. /node/pbkdf2 works perfectly so that means it would work if native throws exception,

@mpetrunic
Copy link
Author

mpetrunic commented Nov 25, 2019

@tynes
I debug this a lot, good thing it's not related to electron. It's jest that's causing this, apparently, moduleRegistry inside jest (they have custom loader because of mocking and stubbing) stores empty object({}) if module require throws. So in our tests in jest with electron rebuilded bcrypto, loading of sha256 throws with module did not self-register. and all other native modules load successfully because require('./bindings') returns "{}" instead of throwing.

Would you be willing to accept PR where all native modules have instead of

const assert = require('bsert');
const binding = require('./binding').random;

this

const assert = require('bsert');
const binding = require('./binding').random;

assert(binding);

?
Which prevents during requiring binding to be undefined and it would throw causing bcrypto to fallback on js or node version.

@alexbarnsley
Copy link

Is there any update on this? I also get the same error during jest as @mpetrunic described. Thanks

@mpetrunic
Copy link
Author

Bug is releated to jest and how it handles require calls. Aparently if require trows, jest will same empty object to modules cache and return it on all subsequent calls. Havent found solution yet

@tynes
Copy link
Member

tynes commented Mar 25, 2020

@alexbarnsley @mpetrunic Have you tried the latest version of bcrypto? A lot has changed including the removal of OpenSSL as a dependency.

@braydonf
Copy link
Contributor

A lot has changed including the removal of OpenSSL as a dependency.

OpenSSL is still used, however is via the existing Node.js crypto API for example aes and random. Also sha{256,384,512}, hash160 and hash256 use OpenSSL via libtorsion, except on electron (which has a fallback).

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

4 participants