Skip to content

Commit

Permalink
Merge pull request #202 from Shashank-In/patch-1
Browse files Browse the repository at this point in the history
Fix for KeyStore DoS vulnerability. Resolves #195
  • Loading branch information
StephenButtolph committed Jun 15, 2020
2 parents 6abf3a8 + 91852fe commit 58c9093
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions api/keystore/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,17 @@ const (
// maxUserPassLen is the maximum length of the username or password allowed
maxUserPassLen = 1024

// requiredPassScore defines the score a password must achieve to be accepted
// as a password with strong characteristics by the zxcvbn package
// maxCheckedPassLen limits the length of the password that should be
// strength checked.
//
// As per issue https://github.com/ava-labs/gecko/issues/195 it was found
// the longer the length of password the slower zxcvbn.PasswordStrength()
// performs. To avoid performance issues, and a DoS vector, we only check
// the first 50 characters of the password.
maxCheckedPassLen = 50

// requiredPassScore defines the score a password must achieve to be
// accepted as a password with strong characteristics by the zxcvbn package
//
// The scoring mechanism defined is as follows;
//
Expand Down Expand Up @@ -138,10 +147,10 @@ func (ks *Keystore) CreateUser(_ *http.Request, args *CreateUserArgs, reply *Cre
defer ks.lock.Unlock()

ks.log.Verbo("CreateUser called with %.*s", maxUserPassLen, args.Username)

if err := ks.AddUser(args.Username, args.Password); err != nil {
return err
}

reply.Success = true
return nil
}
Expand Down Expand Up @@ -380,6 +389,8 @@ func (ks *Keystore) GetDatabase(bID ids.ID, username, password string) (database
return encDB, nil
}

// AddUser attempts to register this username and password as a new user of the
// keystore.
func (ks *Keystore) AddUser(username, password string) error {
if len(username) > maxUserPassLen || len(password) > maxUserPassLen {
return errUserPassMaxLength
Expand All @@ -392,7 +403,12 @@ func (ks *Keystore) AddUser(username, password string) error {
return fmt.Errorf("user already exists: %s", username)
}

if zxcvbn.PasswordStrength(password, nil).Score < requiredPassScore {
checkPass := password
if len(password) > maxCheckedPassLen {
checkPass = password[:maxCheckedPassLen]
}

if zxcvbn.PasswordStrength(checkPass, nil).Score < requiredPassScore {
return errWeakPassword
}

Expand All @@ -414,6 +430,7 @@ func (ks *Keystore) AddUser(username, password string) error {
return nil
}

// CreateTestKeystore returns a new keystore that can be utilized for testing
func CreateTestKeystore(t *testing.T) *Keystore {
ks := &Keystore{}
ks.Initialize(logging.NoLog{}, memdb.New())
Expand Down

0 comments on commit 58c9093

Please sign in to comment.