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

Vulnerabilites In sjcl.random #77

Open
TheRook opened this issue Jan 28, 2013 · 5 comments
Open

Vulnerabilites In sjcl.random #77

TheRook opened this issue Jan 28, 2013 · 5 comments

Comments

@TheRook
Copy link

TheRook commented Jan 28, 2013

I am a Security Analyst and I am performing a Hybrid Application Analysis for a Fortune 100 company that is using sjcl. The assessment team uncovered a weaknesses in how our client was using sjcl.random:

Problem 1:
The first vulnerability in our clients application was very simple. They instantiated sjcl and called randomWords() without calling startCollectors(), effectively turning this random number generator into a sha256 of the current time, which is not a very good secret. There should be no condition in which the user (the programmer) can use ANY entropy pool without a method of collecting entropy. startCollectors() should always be invoked in order to prevent programmers from accidentally generating weak random values. Also, if an API exposes a potentially dangerous condition, the documentation MUST warn the developer.

Problem 2:
Our client was using sjcl on a mobile device. This device lacks a mouse and there for onmousemove will be a very poor source of entorpy. Consider adding the accelerometer data (http://stackoverflow.com/questions/4378435/how-to-access-accelerometer-gyroscope-data-from-javascript) or other event handlers.

Program 3:
sjcl.random is the first entropy pool I have seen that does not save its entropy pool. This is an absolute requirement of the design. The documentation mentions that localstorage is an upcoming feature, but its trivial to implement in a compatible way (https://developer.mozilla.org/en-US/docs/DOM/Storage) Note: do not use the cookie comparability method if the browser is on HTTP page.
If this is in fact the first time sjcl.random is being used by a client then consider seeding the entropy pool with document.cookie, document.location and Math.random(). The idea behind seeding the entropy pool with Math.random() is that sjcl.random should be at least as random as Math.random(), however with the current design it is possible to misuse sjcl.random where Math.random() is a far better source of entropy.

@bitwiseshiftleft
Copy link
Owner

Hello Rook,

Thanks for your analysis! I'm surprised there are fortune 100 companies using SJCL.

If you call randomWords(n) without startCollectorts(), it should throw an exception unless:

  1. You pass an additional "paranoia" argument set to 0. This is explicitly warned against, and is for use only in the test scripts where entropy may not be available. In that case, of course, calling startCollectors() first is undesirable.
  2. You are on a browser which supports crypto.getRandomValues(). In this case, the entropy pool will be seeded from a 1024-byte getRandomValues() call, which I'm sure you'll agree is sufficient.

If there's a bug in this mechanism (there was once, but it's patched) then please tell me what it is! That would be a very serious problem, and would necessitate an announcement and an emergency patch. If, on the other hand, your clients are passing paranoia=0 to anything for reasons other than testing, please tell them not to do that. Or perhaps they've only tested on browsers that support crypto.getRandomValues().

sjcl was designed before DOM storage was portable and stable, which is why it does not save its entropy pool. There is also the issue of whether to fully trust an entropy pool saved in DOM storage, since if you do, it would provide an attack vector which might permanently compromise an app. That is, compromising the DOM storage entropy pool once (either injecting or just reading it) might make the app vulnerable forever, depending on reseed policy. Even if some entropy trickles in, unless run Fortuna via the DOM storage, the pool might never recover. This is why it's remained on the TODO list for so long. Do you have any suggestions on how much to trust a stored entropy pool?

By the way, SJCL also seeds from Math.random(), not just the current time. This is even cryptographically strong on eg some versions of Safari. But the fact that it does this doesn't change your point.

Accelerometer handling seems like a good idea, especially if the entropy rate is high enough.

Thanks again for your analysis, and please consider submitting a patch :-)
-- Mike Hamburg

On Jan 28, 2013, at 8:50 AM, TheRook notifications@github.com wrote:

I am a Security Analyst and I am performing a Hybrid Application Analysis for a Fortune 100 company that is using sjcl. The assessment team uncovered a weaknesses in how our client was using sjcl.

Problem 1:
The first vulnerability in our clients application was very simple. They instantiated sjcl and called randomWords() without calling startCollectors(), effectively turning this random number generator into a sha256 of the current time, which is not a very good secret. There should be no condition in which the user (the programmer) can use ANY entropy pool without a method of collecting entropy. startCollectors() should always be invoked in order to prevent programmers from accidentally generating weak random values. Also, if an API exposes a potentially dangerous condition, the documentation MUST warn the developer.

Problem 2:
Our client was using sjcl on a mobile device. This device lacks a mouse and there for onmousemove will be a very poor source of randomness. Consider adding the accelerometer data (http://stackoverflow.com/questions/4378435/how-to-access-accelerometer-gyroscope-data-from-javascript) or other event handlers.

Program 3:
sjcl.random is the first entropy pool I have seen that does not save its entropy pool. This is an absolute requirement of the design. The documentation mentions that localstorage is an upcoming feature, but its trivial to implement in a compatible way (https://developer.mozilla.org/en-US/docs/DOM/Storage) Note: do not use the cookie comparability method if the browser is on HTTP page.
If this is in fact the first time sjcl.random is being used by a client then consider seeding the entropy pool with document.cookie, document.location and Math.random(). The idea behind seeding the entropy pool with Math.random() is that sjcl.random should be at least as random as Math.random(), however with the current design it is possible to misuse sjcl.random where Math.random() is a far better source of entropy.


Reply to this email directly or view it on GitHub.

@ggozad
Copy link
Contributor

ggozad commented Jan 28, 2013

Was writing a long reply, but Mike got there first. Just to add:
You might be seing #60 , are you perhaps using an old version?

@TheRook
Copy link
Author

TheRook commented Jan 28, 2013

Thanks for the quick response. In short i think more should be done to prevent misuse of sjcl.random and increase its sources of entropy. If you agree with the following post, I am willing to fork this project and implement these changes.

Yes, they where setting paranoia to 0, and there was a code comment saying something like "For compatibility with chrome and firefox." Which, is probably because they didn't understand sjcl.random's construction. To be fair a developer shouldn't have to know about entropy pools to gain their benefits.

I agree that window.crypto.getRandomValues() is a better approach when available (I had forgotten about this function). However, it looks like core/random.c (http://bitwiseshiftleft.github.com/sjcl/doc/symbols/src/core_random.js.html) still relies upon Math.random(). sjcl.random should probably fall back on Math.random() instead of using it by default.

In terms of the security of DOM storage, there is a concern of JavaScript injection, or more commonly referred to as XSS. The localStorage implementation could potentially expose the entropy pool to all JavaScript running on that domain. For instance if there was a reflective XSS venerability in any page on that domain the attacker could read/write to localStorage. The real question is: How can a JavaScript based entropy pool gain the most benefit from localStorage while planning on failure? When SJCL's entropy pool is currently being used it could be stored in a variable that is only scoped to that page (which is what is currently happening), therefore the attacker would have to have XSS on the same page that SJCL is running and that is a moot point. When the window.onbeforeunload event fires, SJCL could store a snapshot of the entropy pool in localStorage. When SJCL loads again it will check the local storage to see if it can add additional entropy to its pool. If this value was modified by the attacker then its still no worse than what SJCL currently has, best case, this value was not compromised and its an additional source.

sjcl.random diverges from random.c in the Linux kernel in a few ways. random.c uses a CRC like function for mixing entropy out of perforce, but I don't see a problem with using sha256 for this. Both sjcl.random and random.c will take mouse movements into consideration, however random.c also looks at keystrokes (which could be done with window.onkeypress). In addition to mouse positioning information, random.c will also add entropy from the timestamps from various interrupts. I get a bad gut feeling when mouse movements are the predominant source of entropy. User interfaces are often used by different users in very similar ways, this can be seen with click heatmaps (http://www.google.com/search?hl=en&q=heat+click+maps&bav=on.2,or.r_gc.r_pw.r_qf.&bvm=bv.41524429,d.cGE&biw=1249&bih=1290&um=1&ie=UTF-8&tbm=isch&source=og&sa=N&tab=wi&ei=H8gGUdSYB-7yiQLT94HYAg#um=1&hl=en&tbo=d&tbm=isch&sa=1&q=click+heatmaps&oq=click+heatmaps&gs_l=img.3..0i10i24.3869.7366.0.7466.20.13.3.2.2.0.161.1181.10j3.13.0...0.0...1c.1.oAnLORlfEGU&bav=on.2,or.r_gc.r_pw.r_qf.&bvm=bv.41524429,d.cGE&fp=3f0526efed38d5a5&biw=1249&bih=1290).

Looking at mouse moments is a good idea, but there should be more sources of entropy.

@bitwiseshiftleft
Copy link
Owner

When writing SJCL, I did a user study which tracked mouse movements (not just clicks) on a few websites that a friend of mine runs. The basic notion is that even when users are clicking on the same things, there is some entropy in mouse movements. This is because the mouse is sampled at a high resolution, and you never know exactly which pixel it will be at when the driver grabs its x/y. Indeed, attempting to predict where a user's mouse will hit next based on various regression models was not especially successful. SJCL's mouse collector uses a conservative estimate from this study. Still, you are right that there should be more entropy sources, especially as many users will be on smartphones.

Keystrokes are a good idea. It would probably be a mistake to stir in the letter that was hit, because that could constitute a side channel. Even the timing could be a side channel, since timing depends on what the user is typing (I think there's a paper about snooping passwords with this?). Accelerometer data could similarly be a side channel (it can be used to deduce keypresses as well, because which softkey was pressed affects acceleration), but like keypress timing it's better than nothing and if you get enough entropy it's definitely secure.

I really ought to require sjcl.beware"Setting paranoia=0 will ruin your security; use it only for testing" or similar before setting paranoia=0, because that's a much bigger mistake than using CBC mode, and using CBC mode requires that warning.

Math.random() is always used just for simplicity of design. I mean, if you have crypto.getRandomValues() or enough entropy, it won't hurt to also call Math.random(), and it's generally a pretty fast call. That said, it did mask the terrible Issue #60.

Stored randomness is a good idea and I'd gladly accept a patch for it. The trick is that even if there is stored randomness, I'm not sure SJCL should trust it to be strong enough. Maybe it could be calibrated to paranoia level: perhaps it should count as enough entropy to overcome (default-1) paranoia, but not (default) paranoia?

Also, you can't just re-store the random pool onBeforeUnload(). This would cause two copies of the page to load with the same entropy, although the call to Math.random() probably protects SJCL in most cases, precisely because the entropy is distilled with cryptographic functions instead of linear ones. So you also have to store it after reading it, but before using it. Do you know if two pages can read from DOM storage before one of them has a chance to overwrite it?

Thanks for your feedback,
-- Mike

@TheRook
Copy link
Author

TheRook commented Jan 28, 2013

For the most part it sounds like we are in agreement, especially when it comes to the core issues. That being said I'll fork this project.

Ideally, random.js would be designed such that the entropyLevel can never be 0. Upon instantiation it should bring the entropy level to 48 or some other value using other sources of entropy. One way of doing this is mixing in a number of call to another random number generator. Forcing the library to block until a paranoia level is reached maybe problematic for some applications, however blocking is still better than an insecure state. (There may not be a perfect solution to this problem)

I think that the localStorage should be assumed to have an entropyLevel of 0, because it cannot be fully trusted. Having two pages load with the same entropy pool state can be avoided by mixing in the value obtained from localStorage with other sources of entropy during initialization. The localStorage augmentation should be seen as purely additive and in the worst case it would add an entropy of 0.

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