OWASP/NodeGoat

Guidance on A6 does not follow Node.js best practices for the Crypto module

jboyer2012 opened this issue · 2 comments

According to Node's Crypto API docs, the default initialization vector used by the createCipher function is not random enough to protect against brute-force attacks.
Here is the appropriate quote from the Crypto API docs:

The implementation of crypto.createCipher() derives keys using the OpenSSL function EVP_BytesToKey with the digest algorithm set to MD5, one iteration, and no salt. The lack of salt allows dictionary attacks as the same password always creates the same key. The low iteration count and non-cryptographically secure hash algorithm allow passwords to be tested very rapidly.

In line with OpenSSL's recommendation to use pbkdf2 instead of EVP_BytesToKey it is recommended that developers derive a key and IV on their own using crypto.pbkdf2() and to use crypto.createCipheriv() to create the Cipher object.

Based on this, we should update the code samples and tutorial for A6 to include this logic. Developers may be looking to simply copy the code there which will result in their encryption not being as strong as expected due to poor implementation. I have submitted PR #104 to resolve this issue.

I updated the A6 tutorial code to create an IV first using the config.cryptoKey and PBKDF2 and then use createCipheriv to create the cipher and decipher objects. See the PR for more details.

@jboyer2012 Thanks for proactively reaching out and providing the context for the PR. Really appreciate your contribution. I will review and merge the PR or get back to you if any questions 👍

Merged the PR. Thanks.