volatiletech / authboss

The boss of http auth.
MIT License
3.85k stars 207 forks source link

password brute force attack possible on locked accounts #292

Closed natefinch closed 4 years ago

natefinch commented 4 years ago

Locking accounts is supposed to prevent people from being able to brute force your password, but authboss has a bug where it actually enables brute forcing.

When you try to authenticate against a locked account, you get a different error message returned depending on if the password is correct or not. Thus you can continue to brute force an account even when it's locked by just looking at the error message. This really needs to be fixed ASAP.

If the password is incorrect, you go through this path: https://github.com/volatiletech/authboss/blob/98147bc020df3bc101817c23d2b31d90f9a2f696/lock/lock.go#L111 and get "Your account has been locked, please contact the administrator."

if the password is correct, you go through this path: https://github.com/volatiletech/authboss/blob/98147bc020df3bc101817c23d2b31d90f9a2f696/lock/lock.go#L54 and get "Your account is locked. Please contact the administrator."

My suggestion is to ensure these always send the same message, and thus should reuse the same code.

This was found by a bug bounty against our production login system at Mattel.

natefinch commented 4 years ago

The problem seems to be that lock.go's "BeforeAuth" function is uh.... not being called until after auth: https://github.com/volatiletech/authboss/blob/020487826a5df9ed53b5ff2a749432bb8fec7c30/auth/auth.go#L96

aarondl commented 4 years ago

Hi @natefinch. Hope you realize I take this very seriously and am feeling quite guilty about your company having to award a bug bounty on account of Authboss. Though it could happen due to Authboss never having had a security audit so it is what it is :( My apologies regardless.

The two paths come from here:

AfterAuthFail: https://github.com/volatiletech/authboss/blob/master/auth/auth.go#L82

BeforeAuth: https://github.com/volatiletech/authboss/blob/master/auth/auth.go#L96

BeforeAuth in Authboss terms is: "Before you actually get to be signed in by having a session created for you" not "after you enter the right password". Because you are not authenticated until a session has been established and BeforeAuth is a module's last chance to break that flow by saying "something is wrong with this user, even if they have the right password".

For AfterAuthFail (where they enter the wrong password) we have to do the logic to see if it's an attempt that requires setting locked on a user.

For BeforeAuth (where they've entered the right password) we simply check if it's locked or not and return a response based on that.

Because the messages are different it's trivial to determine if the password is correct or not currently. It's a shameful oversight. Also interesting is that despite us using bcrypt to ensure constant time comparisons it's possible that a timing attack could enable the same brute force attack since in one case the database user is being saved each time a bad attempt is made.

I've made a fix to hopefully solve both the timing attack and the response-based attack in v2.4.1.

Thanks for reporting the issue, I'm going to close it now but please comment here if you have any further concern.