search

klml / msgsplit

message split allowes you to send messages (passwords etc.) to another person without having the message decrypted on the server or in the email.
MIT License
3 stars 1 forks source link

Math.random() is not cryptographically secure #12

Open hacklschorsch opened 3 years ago

hacklschorsch commented 3 years ago

Math.random() is used for cryptography, but it is not a suitable source of randomness:

https://github.com/klml/msgsplit/blob/e15c42f29d75e938b4739f88406ca4358481817b/static/msgsplit.js#L5

These two articles illustrate the problem quite well:

TIFU by using Math.random() by Mike Malone, Betable CTO, 2015-11-19

Many random number generators in use today are not very good. — Donald Knuth

The current algorithm, which appears to have been passed down from one programmer to another, is comparatively unsatisfactory (and arguably completely broken) due to subtle, non-intuitive degenerate behavior that is likely to be encountered under realistic circumstances.

and

There’s Math.random(), and then there’s Math.random() by Yang Guo, v8 Engineer at Google, 2015-12-17

For use cases such as hashing, signature generation, and encryption/decryption, ordinary PRNGs are unsuitable. The Web Cryptography API introduces window.crypto.getRandomValues, a method that returns cryptographically secure random values, at a performance cost.

I thus propose to use the Web Cryptography API instead.

klml commented 3 years ago

@hacklschorsch what do you think about 74b5e1ae23ee65eef4bb7538157970106fecdfec

hacklschorsch commented 2 years ago

Hi! Sorry for the delay, I overlooked your activity here. Even though I think invoking this API once per byte is a bit much overhead - it can yield up to 64 KB of randomness in one go - I think we shouldn't be worrying about performance yet.

More of a worry to me is this documentation on getRandomValues() on MDN:

Don't use getRandomValues() to generate encryption keys. Instead, use the generateKey() method. There are a few reasons for this; for example, getRandomValues() is not guaranteed to be running in a secure context.

There is no minimum degree of entropy mandated by the Web Cryptography specification.

Maybe you can use a key derivation function like PBKDF2 to get some more bits out of this generateKey() method and not do the whole key generation shebang for every byte?

Since I am treading on rather thin ice here - my own crypto kung foo isn't very strong - I would like refer to the warning from the beginning of the docs to the SubtleCrypto interface:

Even assuming you use the basic cryptographic functions correctly, secure key management and overall security system design are extremely hard to get right, and are generally the domain of specialist security experts.

And of course there's to note that you cannot generate a one time pad with a pseudo random number generator.

So maybe this is good as a demonstration - but it's not a real one time pad, but rather a not very efficient stream cipher?

Maybe there is some silver bullet? Quick Googling yielded this paper: Implementation of One-Time Pad Cryptography but quick skimming looks like it's all bull?