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

Support async random number generators #180

Closed
wants to merge 2 commits into from

Conversation

ikhemissi
Copy link

@ikhemissi ikhemissi commented Oct 27, 2019

Make randomBytes and its dependant functions async.
This will allow using the library with custom secure random number generators (e.g. on mobile via Expo's Random module).

Change the signature of the following methods to make them async (return a Promise):

  • nacl.randomBytes()
  • nacl.box.keyPair()
  • nacl.sign.keyPair()
  • nacl.sign.keyPair.fromSeed()

The signature of nacl.setPRNG also changed to allow passing either a regular function (like in tweetnacl@1) or a function that returns a Promise like async functions.

I also switched uglifyjs to terser in order to support ES6+ syntax.

BREAKING CHANGE: this change breaks compatibility with tweetnacl@1 as the signature of few public functions have changed to return Promises and the minimum required runtime becomes ES 2017 (no IE11 support).


I dedicate any and all copyright interest in this software to the
public domain. I make this dedication for the benefit of the public at
large and to the detriment of my heirs and successors. I intend this
dedication to be an overt act of relinquishment in perpetuity of all
present and future rights to this software under copyright law.

Anyone is free to copy, modify, publish, use, compile, sell, or
distribute this software, either in source code form or as a compiled
binary, for any purpose, commercial or non-commercial, and by any
means.

Make `randomBytes` and its dependant functions `async` to support using asynchronous random byte generators.
This will allow using the library with mobile's native random bytes.

Change the signature of the following methods to make them async (return a Promise):
- nacl.randomBytes()
- nacl.box.keyPair()
- nacl.sign.keyPair()
- nacl.sign.keyPair.fromSeed()

I also switched uglifyjs to terser in order to support ES6+ syntax.

BREAKING CHANGE: this change breaks compatibility with tweetnacl@1 as the signature of few public functions have changed to return Promises and the minimum required runtime becomes ES 2017 (no IE11 support).
Describe which functions need to be update, the minimum runtime, and how to specify an async random bytes generator function.
@dchest
Copy link
Owner

dchest commented Oct 27, 2019

Thanks, however I don't think I'm ready to merge it, as this is a major change for little benefit of most users (browsers and Node do have synchronous PRNG). If you need such functionality, you can keep it as a fork — since I consider TweetNaCl-js pretty much done (not in "unmaintained" sense — just that there are no major features planned, only bug and compatibility fixes, to avoid typical JavaScript library churn, which is especially bad for crypto libraries). I would like to see an async version of the whole library, because it will allow adding WebAssembly support as discussed in #141, so your fork may be a good starting point.

BTW, the way we worked around the requirement for PRNG to be synchronous in React Native is to initialize synchronous JavaScript-level CSPRNG with a 32-byte seed provided by system's PRNG before using the library.

@ikhemissi
Copy link
Author

ikhemissi commented Oct 27, 2019

Thank you very much @dchest for reviewing the code and for the tip regarding RN 👍
Would it help to add the newly updated functions as a separate async versions while keeping the existing ones (e.g. randomBytesAsync, keyPairAsync, fromSeedAsync, and setPRNGAsync) ?
This will still require ES6 support for Promises (or using a polyfill like Bluebird).

If you think that can be a good addition then I am happy to do it, otherwise I will keep the fork and do my changes there. Any suggestions for the fork are more than welcome 😄

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

Successfully merging this pull request may close these issues.

None yet

2 participants