Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

fix(kv): Reorganize bcrypt password hashing outside transactions 

This ensures that transaction lifetimes are shorter to reduce lock
contention.

Additionally, this PR removes the ability to set an empty password
when running the initial on-boarding process. According to the
original developer, this behavior was required to permit Cloud
processes to complete. As this code is no longer shared, this security
issue is corrected.
pull/19730/head
Stuart Carnie 2020-10-09 17:54:05 -07:00
parent e8931bbffa
commit d28e55d3b6
4 changed files with 90 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
kv/onboarding.go
View File

@ -124,15 +124,19 @@ func (s *Service) OnboardInitialUser(ctx context.Context, req *influxdb.Onboardi
Token: req.Token,
}
var passwordHash []byte
passwordHash, err = s.generatePasswordHash(req.Password)
if err != nil {
return nil, err
}
err = s.kv.Update(ctx, func(tx Tx) error {
if err := s.createUser(ctx, tx, u); err != nil {
return err
}
if req.Password != "" {
if err := s.setPassword(ctx, tx, u.ID, req.Password); err != nil {
return err
}
if err := s.setPasswordHashTx(ctx, tx, u.ID, passwordHash); err != nil {
return err
}
if err := s.createOrganization(ctx, tx, o); err != nil {

128
kv/passwords.go
View File

@ -75,34 +75,88 @@ var _ influxdb.PasswordsService = (*Service)(nil)
// CompareAndSetPassword checks the password and if they match
// updates to the new password.
func (s *Service) CompareAndSetPassword(ctx context.Context, userID influxdb.ID, old string, new string) error {
return s.kv.Update(ctx, func(tx Tx) error {
if err := s.comparePassword(ctx, tx, userID, old); err != nil {
return err
}
return s.setPassword(ctx, tx, userID, new)
})
newHash, err := s.generatePasswordHash(new)
if err != nil {
return err
}
if err := s.compareUserPassword(ctx, userID, old); err != nil {
return err
}
return s.setPasswordHash(ctx, userID, newHash)
}
// SetPassword overrides the password of a known user.
func (s *Service) SetPassword(ctx context.Context, userID influxdb.ID, password string) error {
return s.kv.Update(ctx, func(tx Tx) error {
return s.setPassword(ctx, tx, userID, password)
})
hash, err := s.generatePasswordHash(password)
if err != nil {
return err
}
return s.setPasswordHash(ctx, userID, hash)
}
// ComparePassword checks if the password matches the password recorded.
// Passwords that do not match return errors.
func (s *Service) ComparePassword(ctx context.Context, userID influxdb.ID, password string) error {
return s.kv.View(ctx, func(tx Tx) error {
return s.comparePassword(ctx, tx, userID, password)
return s.compareUserPassword(ctx, userID, password)
}
func (s *Service) getPasswordHash(ctx context.Context, userID influxdb.ID) ([]byte, error) {
var passwordHash []byte
err := s.kv.View(ctx, func(tx Tx) error {
var err error
encodedID, err := userID.Encode()
if err != nil {
return CorruptUserIDError(userID.String(), err)
}
if _, err := s.findUserByID(ctx, tx, userID); err != nil {
return EIncorrectUser
}
b, err := tx.Bucket(userpasswordBucket)
if err != nil {
return UnavailablePasswordServiceError(err)
}
passwordHash, err = b.Get(encodedID)
if err != nil {
return EIncorrectPassword
}
return nil
})
return passwordHash, err
}
func (s *Service) compareUserPassword(ctx context.Context, userID influxdb.ID, password string) error {
passwordHash, err := s.getPasswordHash(ctx, userID)
if err != nil {
return err
}
hasher := s.Hash
if hasher == nil {
hasher = &Bcrypt{}
}
if err := hasher.CompareHashAndPassword(passwordHash, []byte(password)); err != nil {
return EIncorrectPassword
}
return nil
}
func (s *Service) setPasswordHash(ctx context.Context, userID influxdb.ID, hash []byte) error {
return s.kv.Update(ctx, func(tx Tx) error {
return s.setPasswordHashTx(ctx, tx, userID, hash)
})
}
func (s *Service) setPassword(ctx context.Context, tx Tx, userID influxdb.ID, password string) error {
if len(password) < MinPasswordLength {
return EShortPassword
}
func (s *Service) setPasswordHashTx(ctx context.Context, tx Tx, userID influxdb.ID, hash []byte) error {
encodedID, err := userID.Encode()
if err != nil {
return CorruptUserIDError(userID.String(), err)
@ -117,53 +171,27 @@ func (s *Service) setPassword(ctx context.Context, tx Tx, userID influxdb.ID, pa
return UnavailablePasswordServiceError(err)
}
hasher := s.Hash
if hasher == nil {
hasher = &Bcrypt{}
}
hash, err := hasher.GenerateFromPassword([]byte(password), DefaultCost)
if err != nil {
return InternalPasswordHashError(err)
}
if err := b.Put(encodedID, hash); err != nil {
return UnavailablePasswordServiceError(err)
}
return nil
}
func (s *Service) comparePassword(ctx context.Context, tx Tx, userID influxdb.ID, password string) error {
encodedID, err := userID.Encode()
if err != nil {
return CorruptUserIDError(userID.String(), err)
}
if _, err := s.findUserByID(ctx, tx, userID); err != nil {
return EIncorrectUser
}
b, err := tx.Bucket(userpasswordBucket)
if err != nil {
return UnavailablePasswordServiceError(err)
}
hash, err := b.Get(encodedID)
if err != nil {
// User exists but has no password has been set.
return EIncorrectPassword
func (s *Service) generatePasswordHash(password string) ([]byte, error) {
if len(password) < MinPasswordLength {
return nil, EShortPassword
}
hasher := s.Hash
if hasher == nil {
hasher = &Bcrypt{}
}
if err := hasher.CompareHashAndPassword(hash, []byte(password)); err != nil {
// User exists but the password was incorrect
return EIncorrectPassword
hash, err := hasher.GenerateFromPassword([]byte(password), DefaultCost)
if err != nil {
return nil, InternalPasswordHashError(err)
}
return nil
return hash, nil
}
// DefaultCost is the cost that will actually be set if a cost below MinCost

9
kv/passwords_test.go
View File

@ -158,9 +158,6 @@ func TestService_SetPassword(t *testing.T) {
}
return nil, kv.ErrKeyNotFound
},
PutFn: func(key, val []byte) error {
return nil
},
}, nil
},
}
@ -193,9 +190,6 @@ func TestService_SetPassword(t *testing.T) {
}
return nil, kv.ErrKeyNotFound
},
PutFn: func(key, val []byte) error {
return nil
},
}, nil
},
}
@ -231,9 +225,6 @@ func TestService_SetPassword(t *testing.T) {
}
return nil, kv.ErrKeyNotFound
},
PutFn: func(key, val []byte) error {
return nil
},
}, nil
},
}

46
testing/onboarding.go
View File

@ -124,7 +124,7 @@ func OnboardInitialUser(
},
},
{
name: "missing password should still work",
name: "missing password should fail",
fields: OnboardingFields{
IDGenerator: &loopIDGenerator{
s: []string{oneID, twoID, threeID, fourID},
@ -134,50 +134,12 @@ func OnboardInitialUser(
},
args: args{
request: &platform.OnboardingRequest{
User: "admin",
Org: "org1",
Bucket: "bucket1",
User: "admin",
Org: "org1",
},
},
wants: wants{
results: &platform.OnboardingResults{
User: &platform.User{
ID: MustIDBase16(oneID),
Name: "admin",
Status: platform.Active,
},
Org: &platform.Organization{
ID: MustIDBase16(twoID),
Name: "org1",
CRUDLog: platform.CRUDLog{
CreatedAt: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC),
UpdatedAt: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC),
},
},
Bucket: &platform.Bucket{
ID: MustIDBase16(threeID),
Name: "bucket1",
OrgID: MustIDBase16(twoID),
RetentionPeriod: 0,
CRUDLog: platform.CRUDLog{
CreatedAt: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC),
UpdatedAt: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC),
},
},
Auth: &platform.Authorization{
ID: MustIDBase16(fourID),
Token: oneToken,
Status: platform.Active,
UserID: MustIDBase16(oneID),
Description: "admin's Token",
OrgID: MustIDBase16(twoID),
Permissions: platform.OperPermissions(),
CRUDLog: platform.CRUDLog{
CreatedAt: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC),
UpdatedAt: time.Date(2006, 5, 4, 1, 2, 3, 0, time.UTC),
},
},
},
errCode: platform.EEmptyValue,
},
},
{