From a3360b1f4f9b6b8c837f558bcc08bd454c19673e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Thu, 8 Sep 2022 20:36:07 +0200 Subject: [PATCH] doc: emphasize that createCipher is never secure The current documentation clearly states that createCipher() and createDecipher() should not be used with ciphers in counter mode, but (1) this is an understatement, and (2) these functions are (semantically) insecure for ciphers in any other supported block cipher mode as well. Semantic security requires IND-CPA, but a deterministic cipher with fixed key and IV, such as those generated by these functions, does not fulfill IND-CPA. Are there justified use cases for createCipher() and createDecipher()? Yes and no. The only case in which these functions can be used in a semantically secure manner arises only when the password argument is not actually a password but rather a random or pseudo-random sequence that is unpredictable and that is never reused (e.g., securely derived from a password with a proper salt). Insofar, it is possible to use these APIs without immediately creating a vulnerability. However, - any application that manages to fulfill this requirement should also be able to fulfill the similar requirements of crypto.createCipheriv() and those of crypto.createDecipheriv(), which give much more control over key and initialization vector, and - the MD5-based key derivation step generally does not help and might even reduce the overall security due to its many weaknesses. Refs: https://github.com/nodejs/node/pull/13821 Refs: https://github.com/nodejs/node/pull/19343 Refs: https://github.com/nodejs/node/pull/22089 PR-URL: https://github.com/nodejs/node/pull/44538 Reviewed-By: Anna Henningsen Reviewed-By: Rich Trott Reviewed-By: Mohammed Keyvanzadeh Reviewed-By: Filip Skokan --- doc/api/crypto.md | 8 ++++++++ doc/api/deprecations.md | 4 ++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/api/crypto.md b/doc/api/crypto.md index 385dff2b808ded..5e8736bde40d73 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -3005,6 +3005,10 @@ The `password` is used to derive the cipher key and initialization vector (IV). The value must be either a `'latin1'` encoded string, a [`Buffer`][], a `TypedArray`, or a `DataView`. +This function is semantically insecure for all +supported ciphers and fatally flawed for ciphers in counter mode (such as CTR, +GCM, or CCM). + 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 @@ -3124,6 +3128,10 @@ cipher in CCM or OCB mode (e.g. `'aes-128-ccm'`) is used. In that case, the authentication tag in bytes, see [CCM mode][]. For `chacha20-poly1305`, the `authTagLength` option defaults to 16 bytes. +This function is semantically insecure for all +supported ciphers and fatally flawed for ciphers in counter mode (such as CTR, +GCM, or CCM). + The implementation of `crypto.createDecipher()` 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 diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 25efb8f0a9e9f9..b0203d552503c7 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -2114,10 +2114,10 @@ changes: Type: Runtime -Using [`crypto.createCipher()`][] and [`crypto.createDecipher()`][] should be +Using [`crypto.createCipher()`][] and [`crypto.createDecipher()`][] must be avoided as they use a weak key derivation function (MD5 with no salt) and static initialization vectors. It is recommended to derive a key using -[`crypto.pbkdf2()`][] or [`crypto.scrypt()`][] and to use +[`crypto.pbkdf2()`][] or [`crypto.scrypt()`][] with random salts and to use [`crypto.createCipheriv()`][] and [`crypto.createDecipheriv()`][] to obtain the [`Cipher`][] and [`Decipher`][] objects respectively.