Skip to content

Add iteration count parameter#3

Open
LoonyPandora wants to merge 11 commits intoamiri:masterfrom
LoonyPandora:master
Open

Add iteration count parameter#3
LoonyPandora wants to merge 11 commits intoamiri:masterfrom
LoonyPandora:master

Conversation

@LoonyPandora
Copy link
Contributor

The iteration count is analogous to the cost parameter in Digest::Bcrypt and I think it would be useful to able to set it in this module. The default imposed by Crypt::PBKDF2 is only 1,000 - which is rather low in 2012 (f.ex iOS uses 10,000 iterations by default)

I also believe that it should be a required parameter, rather than defaulting it. If someone relies on the default being 1,000 and that changes upstream in the future, with no way to set it via this module, a whole bunch of user passwords could become unretrievable.

Please consider this pull request the start of a discussion, I think this parameter would be useful - but is not essential, and it does have downsides (the breaking of backwards compatibility)

First draft of changes, still need to write tests and update documentation
Also revert to using the VERSION number provided when building via Dist::Zilla
Slight refactor to improve creation of Crypt::PBKDF2 object. It's a shame the encoding method is readonly in that module, else this would be even more simple.
Since we output the raw digest without the salt and iteration count, we should die if they are not specified, rather than using some unknowable default.
@esskar
Copy link

esskar commented Dec 12, 2012

that is a good request.

@danielluke
Copy link

+1, this functionality would be very useful (and would close https://rt.cpan.org/Public/Bug/Display.html?id=89570 :) )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants