Proposal: crypto: Make the digest parameter required for pbkdf2.#3292
Closed
tomgco wants to merge 1 commit intonodejs:masterfrom
Closed
Proposal: crypto: Make the digest parameter required for pbkdf2.#3292tomgco wants to merge 1 commit intonodejs:masterfrom
tomgco wants to merge 1 commit intonodejs:masterfrom
Conversation
This change will remove the default behaviour of defaulting to SHA1 as its digest function, instead this will break backwards compatibility and force implementors to choose a secure digest that is suited for them at that time.
Member
|
Deprecating the 'default digest' overload might be reasonable (discussion needed) but it has to go through a deprecation cycle first to give people time to upgrade, i.e., v5 prints a warning, v6 throws. |
bnoordhuis
added
crypto
Contributor
Author
Shall I create a new PR with the deprecation notice and link it to this issue? |
Member
|
I'll leave that up to you. If your reasoning is that this PR can go into v6 once the merge window for that opens, keep in mind that the chances of it applying cleanly will be slim. |
tomgco
added a commit
to tomgco/node
that referenced
this pull request
Jan 22, 2016
As per nodejs#3292, this PR introduces a deprecation notice about removing the 'default digest' overload which currently defaults to the soon to be defunct SHA1 digest. Instead it should be left up to the documentation and implementor to suggest a suitable digest function.
shigeki
pushed a commit
that referenced
this pull request
Jan 22, 2016
As per #3292, this PR introduces a deprecation notice about removing the 'default digest' overload which currently defaults to the soon to be defunct SHA1 digest. Instead it should be left up to the documentation and implementor to suggest a suitable digest function. Ref: #3292 PR-URL: #4047 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
Contributor
scovetta
pushed a commit
to scovetta/node
that referenced
this pull request
Apr 2, 2016
As per nodejs#3292, this PR introduces a deprecation notice about removing the 'default digest' overload which currently defaults to the soon to be defunct SHA1 digest. Instead it should be left up to the documentation and implementor to suggest a suitable digest function. Ref: nodejs#3292 PR-URL: nodejs#4047 Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor@indutny.com> Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
This was referenced Feb 20, 2022
This was referenced Sep 29, 2022
This was referenced Oct 5, 2022
This was referenced Oct 8, 2022
This was referenced May 13, 2024
This was referenced May 22, 2024
This was referenced May 23, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change will remove the default behaviour of defaulting to SHA1
as its digest function, instead this will break backwards compatibility
and force implementers to choose a secure digest that is suited for them
at that time.
At this moment of time SHA1 is fine to be used as an HMAC, but this statement
will eventually not be true, as is the nature of cryptography. Instead of changing the default
hash algorithm when we deem one to be not secure (and thus break backwards
compatibility at each change in the future), implementing this fix now (v5?) will remove any future
breakages as it pushes the choice onto the consumer of this api.
I would like to hear your thoughts on this, I understand that this sub-module is at the stability index of 2: stable, however I still think that this should be changed now, instead of waiting till it has to be changed.
Thanks.