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
Dubious rng #225
Dubious rng #225
Conversation
BTW, I'm thinking we bump the major version for this, since it may throw in cases where it didn't previously. |
rnds[i] = r >>> ((i & 0x03) << 3) & 0xff; | ||
} | ||
function mathRNG() { | ||
if (!rng._dubious) throw new Error('No good RNG available. See https://goo.gl/vz3J87'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lint?
I would probably consider allowing the user to specify the crypto, or honestly, maybe recommend they polyfill the crypto functions the library needs if their browser does not have them? I do like the idea of a library that ships with the support it needs so another option is to not change anything and simply let the user polyfill the crypto function to throw if they don't want to suppose "weak" crypto browsers. |
Oh, I like that idea. Can't really think of any downsides to this at the moment.
This is basically asking users to fix the problem themselves. I don't think we can say we've fixed this bug if naive users are still vulnerable to the issue. I think the spirit of this change is to complain loudly if there's an issue, and then inform the user about the problem and what options are available for fixing it. |
[rant]npm is filled with prng modules that are just wrappers around window.crypto || window.msCrypto. Boo![/rant] 'Would really like to have a code snippet demonstrating the preferred method for shimming crypto in legacy browsers. |
I don't have the required knowledge to review this sorry, maybe @defunctzombie can do another pass? |
@vvo: No worries - I'm not sure we've resolved how best to address this. As @defunctzombie says, asking the user to find a polyfill may be the best solution here, but it's up for debate. |
Apparently I don't have time/interest to push this through. |
#173 points out that quietly falling back to Math.random raises security concerns. This diff causes
uuid
to throw an error if/when Math.random would be used to create UUIDs, provides a hook for suppressing that error, and links to a wiki page (https://goo.gl/vz3J87) that explains the error, the hook, and a possible workaround for generating high-quality UUIDs using SJCL crypto library.