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

➖ Remove dependency on PBKDF2 #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

danielberkompas
Copy link
Owner

This is a breaking change. OTP versions > 24.2 now include the pbkdf2_hmac/5 function in the :crypto module, thanks to this PR: erlang/otp#5421

The new function does not include support for all the algorithms that the old dependency supported, namely md4, md5 and ripemd. However, I think this is a valid tradeoff.

It is no longer necessary to add a :pbkdf2 dependency in order to use the PBKDF2 field type.

This is a breaking change. OTP versions > 24.2 now include the `pbkdf2_hmac/5`
function in the `:crypto` module, thanks to this PR:
erlang/otp#5421

The new function does not include support for all the algorithms that the old
dependency supported, namely `md4`, `md5` and `ripemd`. However, I think this
is a valid tradeoff.

It is no longer necessary to add a `:pbkdf2` dependency in order to use the
PBKDF2 field type.
defaults = [algorithm: :sha256, iterations: 10_000, size: 32]
@impl Cloak.Ecto.PBKDF2
def init(config) do
defaults = [algorithm: :sha256, iterations: 10_000, size: 32]
Copy link

Choose a reason for hiding this comment

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

OWASP suggests 600,000 iterations for SHA256 and 210,000 iterations for SHA512. Would you be amenable to bumping the default number of iterations and also updating the docs?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll do that in a separate PR, so that it will show up in the automatic changelog more easily.

@jc00ke
Copy link

jc00ke commented May 24, 2023

Given we're now up to Erlang 26, it'd be great to see this merged. Are you actively supporting all the way back to 23?

@danielberkompas danielberkompas added the breaking A breaking change requiring a major release label Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking change requiring a major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants