Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: move createCipher to runtime deprecation #22089

Closed

Conversation

tniessen
Copy link
Member

@tniessen tniessen commented Aug 2, 2018

createCipher is still being used, but it seems appropriate to have a runtime deprecation in one of the next major releases. Reasons include:

  • It uses static IVs: Encrypting the same message with the same password will always yield the same ciphertext. While this is generally considered bad practice and can lead to severe security issues on its own, it also means that any counter-mode cipher is more or less useless when used with createCipher, see crypto: warn if counter mode used in createCipher #13821.
  • Its KDF is non-configurable and doesn't use a salt, meaning that the same password always results in the same key. It also allows brute-force attacks since computing MD5 with a single iteration is incredibly fast on modern processors / GPUs.
  • There are instances where using createCipher is relatively secure, but its simple API makes it especially appealing to people who are new to cryptography, possibly leading to immense security risks.
  • It has been documentation-only deprected since node 10, see crypto: doc-only deprecate createCipher/Decipher #19343.
  • If people want to have this functionality for whatever reason, it is still possible by using createCipheriv (thanks to crypto: allow passing null as IV unless required #18644) and manually deriving the static key / IV. If people are using this for legitimate reasons, we can still un-deprecate it.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the crypto Issues and PRs related to the crypto subsystem. label Aug 2, 2018
@tniessen tniessen added the semver-major PRs that contain breaking changes and should be released in the next major version. label Aug 2, 2018
@tniessen
Copy link
Member Author

tniessen commented Aug 2, 2018

cc @nodejs/tsc @nodejs/crypto

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit.

/cc @nodejs/security


// Emits regular warning expected by expectWarning()
crypto.createCipher('aes-256-gcm', key);
crypto.createCipher('aes-256-cbc', key);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test checks EmitWarning in C++ code, evident from the file name. Instead of changing the message we should look for another warning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, I can make this work either way, thanks for the hint!

Copy link
Member

@TimothyGu TimothyGu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Trott
Copy link
Member

Trott commented Aug 2, 2018

@nodejs/security-wg

@tniessen tniessen added this to the 11.0.0 milestone Aug 4, 2018
@tniessen tniessen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 4, 2018
@tniessen
Copy link
Member Author

tniessen commented Aug 4, 2018

tniessen added a commit that referenced this pull request Aug 5, 2018
PR-URL: #22089
Reviewed-By: Tiancheng "Timothy" Gu <[email protected]>
Reviewed-By: Yihong Wang <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ujjwal Sharma <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@tniessen
Copy link
Member Author

tniessen commented Aug 5, 2018

Landed in 933d8eb with TSC approvals from @TimothyGu, @jasnell and @cjihrig. Thank you for reviewing, everyone.

@tniessen tniessen closed this Aug 5, 2018
@tniessen tniessen added deprecations Issues and PRs related to deprecations. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Sep 7, 2018
tniessen added a commit to tniessen/node that referenced this pull request Sep 6, 2022
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: nodejs#13821
Refs: nodejs#19343
Refs: nodejs#22089
nodejs-github-bot pushed a commit that referenced this pull request Sep 8, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Fyko pushed a commit to Fyko/node that referenced this pull request Sep 15, 2022
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: nodejs#13821
Refs: nodejs#19343
Refs: nodejs#22089
PR-URL: nodejs#44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Sep 26, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
juanarbol pushed a commit that referenced this pull request Oct 4, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
juanarbol pushed a commit that referenced this pull request Oct 7, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
juanarbol pushed a commit that referenced this pull request Oct 10, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
juanarbol pushed a commit that referenced this pull request Oct 11, 2022
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: #13821
Refs: #19343
Refs: #22089
PR-URL: #44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
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: nodejs/node#13821
Refs: nodejs/node#19343
Refs: nodejs/node#22089
PR-URL: nodejs/node#44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this pull request Jan 3, 2023
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: nodejs/node#13821
Refs: nodejs/node#19343
Refs: nodejs/node#22089
PR-URL: nodejs/node#44538
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
Reviewed-By: Filip Skokan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. deprecations Issues and PRs related to deprecations. semver-major PRs that contain breaking changes and should be released in the next major version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants