search

ProtonMail / WebClients

Monorepo hosting the proton web clients
GNU General Public License v3.0
4.44k stars 561 forks source link

As MDN says crypto.getRandomValues() should not be used to generate encryption keys #358

Closed bruncanepa closed 10 months ago

bruncanepa commented 10 months ago

For creating encryption keys you are using a method that is not recommended by MDN Web Docs.

https://github.com/ProtonMail/WebClients/blob/5a77c345342affe53be81f59457bce46a42eee1a/packages/pass/lib/crypto/utils/crypto-helpers.ts#L12

image

Here MDN explicitly says that:

image

twiss commented 10 months ago

Hey :wave: Web Crypto spec editor here. This advice makes sense in cases where Web Crypto keys are generated and kept in the browser only, and the key material doesn't need to be accessible by JavaScript - in that case, generateKey with exportable = false can be used instead. However, in Proton's use case, the key would need to be exported anyway, in order to be encrypted with the user key. There's no significant functional difference between generating a key using generateKey and then exporting it, and generating a key using getRandomValues and then importing it. Both use a cryptographically secure random number generator. The concern mentioned by MDN (getRandomValues doesn't require a secure context) also doesn't apply because Proton Mail does run in a secure context.

We could of course still switch to generateKey + exportKey, to appease MDN, but I don't think there's a significant reason to do so, so I'll close as wontfix. Let us know if you have any other concerns, though!