Bad cryptographic implementation

[kaosdynamics]

Hi @RoxasShadow,

I quickly reviewed your project and there are a couple of issues regarding your cryptographic implementation. I didn't really have enough time to give a broader look at it, but as far as I read:

Issue 1 - Relying on the unsafe Math.random function

https://github.com/RoxasShadow/Pasteling/blob/master/routes/api.js#L58 Pasteling seems to rely on Math.random when a password is not specified by the user. This is a bad practice as Math.random is not a cryptographic-grade random number generator.

You should instead rely on the NodeJS Crypto APIs to obtain a CSPRNG for your app.

Issue 2 - Relying on user password

Pasteling relies on user password. This is a bad practice in the real world as it's subject to brute force attack (often to dictionary-based brute force attack). I would recommend to teach user to choose a strong password and then run a key derivation function on the specified password such as PBKDF2, Bcrypt or Scrypt to slowdown potential attacks. A PBKDF2 implementation is available in the NodeJS Crypto APIs.

Issue 3 - Encryption is done on server-side

This can't guarantee any confidentiality, so encryption is pretty much useless: instead it should be done on client-side or you should warn users of that. (look at the Lavabit case for instance..)

Issue 4 - Pasteling doesn't guarantee any data integrity

Pasteling doesn't authenticate\verify data after encryption (and before decryption). This is a bad practice, I think this answer can provide more details that I can highlight here: http://crypto.stackexchange.com/a/12192

[RoxasShadow]

Hi, and thank you for reporting these security issues. At some of them I'm already working, while for the others I will spend the next hours. Thanks again!

The first two issues have been solved in a00f4e2f83. I'll work at remaining ones in next few days.

[RoxasShadow]

Hi @kaosdynamics, some times ago I started to work on the client-side version of Pasteling.

It peforms all the encryption in the client side, so the server receives only encrypted data. By default, it uses pbkdf2 to hash encryption keys and AES to cipher pastes. It's also very simple switching to other algorithms just writing adapters for both ciphering and hashing modules.

[kaosdynamics]

Hi, didn't notice that new branch, has it been there the whole time? :o

I gave a quick look at it and it looks like a good start, except for The Return of the Living Dead ;) CryptoJS/src/core.js?r=529#336 > Pasteling/public/javascripts/algorithms/hashing/pbkdf2.js#L12

CryptoJS isn't that bad, but if you want something properly design I'd suggest you to use sjcl which doesn't rely on Math.random() and doesn't, random fact, store the encryption key in the ciphertext object to be stripped only when you call the method to convert it to a string.

As for the WordArray.random() issue I would suggest you to just rely on window.crypto.getRandomValues() or either move to sjcl which has its own PRNG (it relies on window.crypto.getRandomValues() and, if not found, has several other methods to gather entropy).

Keep on going ;)