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

[NFR]: Bump up the work factor of security->hash() #14551

Closed
Ruzgfpegk opened this issue Nov 22, 2019 · 3 comments
Closed

[NFR]: Bump up the work factor of security->hash() #14551

Ruzgfpegk opened this issue Nov 22, 2019 · 3 comments
Labels
new feature request Planned Feature or New Feature Request

Comments

@Ruzgfpegk
Copy link

Ruzgfpegk commented Nov 22, 2019

Is your feature request related to a problem? Please describe.
Not a problem, rather a security enhancement.
In Security.zep, the default work factor for bcrypt is set to 8:

protected workFactor = 8 { set, get };

The Perils of the default bcrypt cost factor article by Clio Labs is what made me question this value.
As I learned through this article, 8 is the value that was suggested 20 years ago, and computing power has improved quite a bit since then.
PHP uses a default value of 10, which means that, again by default, using Phalcon's security->hash instead of PHP's password_hash leads to passwords that are 4 times weaker; +1 to the work factor leading to a twice slower, so twice more robust, hash generation.

This article advised for a value of 12 three years ago (it would probably be higher today), but it may not be suitable for everybody as a default value.

Describe the solution you'd like
Bumping up the default work factor for bcrypt to at least 10, like PHP does by default.

Describe alternatives you've considered
Well of course it's up to people to adjust this setting themselves while developing, and relying on default values that can change over time may not be a good idea if you're not aware of them, but having a more secure default value for this setting would be nice.

Additional context
I encourage people to check the benchmarks on the mentioned article, they're a well-needed reality check, not even mentioning the fact that core count increased a lot recently in consumer-level CPUs.

@Ruzgfpegk Ruzgfpegk added the new feature request Planned Feature or New Feature Request label Nov 22, 2019
@ruudboon
Copy link
Member

I don't see an issue aligning this with the PHP default factor or higher. @niden @sergeyklay any thoughts?

@sergeyklay sergeyklay added the discussion Request for comments and discussion label Nov 22, 2019
@sergeyklay
Copy link
Contributor

I'm ok with this.

@niden niden added 4.0 and removed discussion Request for comments and discussion labels Nov 22, 2019
@niden niden mentioned this issue Nov 22, 2019
5 tasks
niden added a commit that referenced this issue Nov 22, 2019
niden added a commit that referenced this issue Nov 22, 2019
niden added a commit that referenced this issue Nov 22, 2019
niden added a commit that referenced this issue Nov 22, 2019
@niden
Copy link
Member

niden commented Nov 22, 2019

Resolved in #14552

Thank you @Ruzgfpegk

@niden niden closed this as completed Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature request Planned Feature or New Feature Request
Projects
None yet
Development

No branches or pull requests

4 participants