luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Change ordering of user lookup vs. password hashing (#5614) 

* Change ordering of user lookup vs. password hashing

This fixes a very minor information leak where someone could brute force
the existence of a username. It's not perfect as the underlying storage
plays a part but bcrypt's slowness puts that much more in the noise.
This commit is contained in:
Jeff Mitchell 2018-10-27 10:43:08 -07:00 committed by GitHub
parent 5f88be68bc
commit a21a7e9eb4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 37 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
builtin/credential/userpass/path_login.go
View File

@ -62,9 +62,43 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew
}
// Get the user and validate auth
user, err := b.user(ctx, req.Storage, username)
if err != nil {
return nil, err
user, userError := b.user(ctx, req.Storage, username)
var userPassword []byte
var err error
var legacyPassword bool
// If there was an error or it's nil, we fake a password for the bcrypt
// check so as not to have a timing leak. Specifics of the underlying
// storage still leaks a bit but generally much more in the noise compared
// to bcrypt.
if user != nil && userError == nil {
if user.PasswordHash == nil {
userPassword = []byte(user.Password)
legacyPassword = true
} else {
userPassword = user.PasswordHash
}
} else {
// This is still acceptable as bcrypt will still make sure it takes
// a long time, it's just nicer to be random if possible
userPassword = []byte("dummy")
}
// Check for a password match. Check for a hash collision for Vault 0.2+,
// but handle the older legacy passwords with a constant time comparison.
passwordBytes := []byte(password)
if !legacyPassword {
if err := bcrypt.CompareHashAndPassword(userPassword, passwordBytes); err != nil {
return logical.ErrorResponse("invalid username or password"), nil
}
} else {
if subtle.ConstantTimeCompare(userPassword, passwordBytes) != 1 {
return logical.ErrorResponse("invalid username or password"), nil
}
}
if userError != nil {
return nil, userError
}
if user == nil {
return logical.ErrorResponse("invalid username or password"), nil
@ -75,19 +109,6 @@ func (b *backend) pathLogin(ctx context.Context, req *logical.Request, d *framew
return logical.ErrorResponse("login request originated from invalid CIDR"), nil
}
// Check for a password match. Check for a hash collision for Vault 0.2+,
// but handle the older legacy passwords with a constant time comparison.
passwordBytes := []byte(password)
if user.PasswordHash != nil {
if err := bcrypt.CompareHashAndPassword(user.PasswordHash, passwordBytes); err != nil {
return logical.ErrorResponse("invalid username or password"), nil
}
} else {
if subtle.ConstantTimeCompare([]byte(user.Password), passwordBytes) != 1 {
return logical.ErrorResponse("invalid username or password"), nil
}
}
return &logical.Response{
Auth: &logical.Auth{
Policies: user.Policies,