RNG used for CSRF tokens is seeded using system time
Closed this issue · 10 comments
NOTE: I am not a cryptographer! Just someone studying this code to develop another auth library and found this mildly concerning.
I noticed that this package uses random using the StdGen interface, which after some further digging in the source to random, and initSMGen, it appears that the random numbers used for this are using an RNG seeded by the system time (admittedly including a picoseconds CPU time component). It might be possible to mount an offline attack against this.
I am not sure if the random generator is correct for this use case. mwc-random (also explicitly not cryptographically secure) or cryptonite may be more appropriate for the purpose, and neither uses system time as a seed except as a last-resort fallback.
Thank you for bringing this up! I will switch to one of those other options as soon as possible.
It feels like some time should be dedicated to understanding this more deeply, but I'm not sure I have the bandwidth right now. I wouldn't want to be too alarmist about the versions in the wild, if there's no actual attack possible (or if successful attack is very unlikely), but I also don't want to silently fix an actual security issue without appropriate notification to users (or, for example, pulling versions down from Hackage). If anyone has thoughts or suggestions about this, please let me know.
I ended up implementing mine with base64 encoded randomness from the system provider via cryptonite, to absolutely make sure it's not a problem. getRandomBytes in an IO monad I think? :)
Same trouble on my side with the lack of background to appropriately evaluate options. I simply do not know if this attack is plausible or the scope of exposure, just that the RNG is definitely weak.
Also, mwc-random is not "cryptographically secure" but it's unclear whether that means it's still susceptible to hypothetical attacks, how many outputs do such possible attacks require, what's the exposure, etc? I'm not sure.
Yeah, so I was (imperfectly) trying to assess things as:
- We're using a thing that uses system time for seed -- that's definitely not good and may cause a vulnerability
- We're using a thing that's not "cryptographically secure", and is only (or at least will be) "sufficiently random"
The first point is clear enough to fix directly (which either option does), but the second point is nebulous. What is the delta between "cryptographically secure" and "sufficiently random"? I elected to take the more easily-dropped-in / fewer-transitive-deps option.
I'm less into that choice after thinking about it more, though. So maybe I'll slot cryptonite into that PR next and see how it goes.
I really wish there was a tiny package that just offered good bindings to the OS random infrastructure, which is what I think I heard the best practice is. It's unfortunate that the current ecosystem makes the wrong choice the easy choice due to the dependency graph and popularity issues with doing it right.
good bindings to the OS random infrastructure
When looking at mwc-random, it said it reads /dev/urandom, which is definitely the best practice OS-wise.
That makes me feel pretty good about this package for this use-case actually 🤷
It's unfortunate that the current ecosystem makes the wrong choice the easy choice due
Strong agree!
Yeah, they have their seeding done correctly but it is unknown whether there's weaknesses that let the seed be found by observing multiple requests. I seem to recall the best practice might have been using the OS random number generator for the entire token (what I did with cryptonite), reducing the possibility of implementation issues on the Haskell side.
I seem to recall the best practice might have been using the OS random number generator for the entire token
That makes sense. I think you could do it (albeit ill-performantly) with either library. In my PR I would replace
withSystemRandom $ replicateM 30 $ ...with
replicateM 30 $ withSystemRandom ...You could use acquireSeedSystem directly: https://hackage.haskell.org/package/mwc-random-0.14.0.0/docs/System-Random-MWC-SeedSource.html
Fixed (with cryptonite) in yesod-auth-oauth2-0.6.1.3.