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

Make bcrypt cost configurable #3677

Closed
will118 opened this issue Jul 17, 2018 · 3 comments

Comments

@will118
Copy link

commented Jul 17, 2018

Would it be possible to add something to ServerConfig to allow configuring the kBcryptCostFactor?

https://github.com/couchbase/sync_gateway/blob/master/auth/user.go#L25

I feel fairly comfortable making the change if it's something you're interested in.

@adamcfraser adamcfraser added this to the 2.5.0 milestone Jul 19, 2018

@adamcfraser

This comment has been minimized.

Copy link
Contributor

commented Jul 19, 2018

I agree - this is something we should add. We should also review whether the default should be adjusted based on typical modern processor speeds.

@bbrks

This comment has been minimized.

Copy link
Member

commented Jul 19, 2018

@adamcfraser Go will occasionally increment the default cost factor in their releases based on current hardware, although having said that, the value has not changed from 10 in 7 years.

Incrementing this to 11 will double the time it takes, but there are no strong recommendations above 10 as of right now.

@bbrks

This comment has been minimized.

Copy link
Member

commented Jul 20, 2018

We need to document the fact that password hashes will only utilise a new cost if an existing user changes their password, unless some logic is implemented to add a rolling cost upgrade... For example:

// An example using types/methods from user.go
user := userImpl{...}

ok := user.Authenticate(password)
if !ok {
    panic("login failed")
}

if bcrypt.Cost(user.PasswordHash_) < kBcryptCostFactor {
    // cost factor has since increased, so re-hash the given password
    user.SetPassword(password)
}

@adamcfraser adamcfraser added the backlog label Jul 23, 2018

@adamcfraser adamcfraser added ready and removed backlog labels Oct 1, 2018

@pasin pasin added in progress and removed ready labels Oct 9, 2018

@bbrks bbrks added the review label Oct 9, 2018

adamcfraser added a commit that referenced this issue Oct 10, 2018

#3677 configurable bcrypt cost (#3790)
* Make bcrypt cost configurable

* Rolling-upgrade

* Add unit test for disabled user Authenticate

* Make minimum bcrypt cost the current default

* Improve warning for error when rehashing password
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.