poanetwork
/
gecko
mirror of https://github.com/poanetwork/gecko.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

factor out password strength checking and enfore password strength checking for auth token password

This commit is contained in:
Dan Laine 2020-06-30 10:54:17 -04:00
parent fe3e1a319e
commit a4ed688533
6 changed files with 123 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
api/auth/auth.go
View File

@ -12,6 +12,7 @@ import (
"time"
"github.com/ava-labs/gecko/utils/hashing"
"github.com/ava-labs/gecko/utils/password"
"github.com/ava-labs/gecko/utils/timer"
jwt "github.com/dgrijalva/jwt-go"
@ -23,6 +24,8 @@ const (
// Endpoint is the base of the auth URL
Endpoint = "auth"
maxPasswordLen = 1024
// RequiredPasswordStrength defines the minimum strength of a password
RequiredPasswordStrength = password.OK
)
var (
@ -129,10 +132,14 @@ func (auth *Auth) changePassword(oldPassword, newPassword string) error {
defer auth.lock.Unlock()
if !bytes.Equal(auth.HashedPassword, hashing.ComputeHash256([]byte(oldPassword))) {
return errWrongPassword
} else if len(newPassword) == 0 || len(newPassword) > maxPasswordLen {
} else if len(newPassword) == 0 {
return errors.New("newPassword can't be empty")
} else if len(newPassword) > maxPasswordLen {
return fmt.Errorf("new password length exceeds maximum length, %d", maxPasswordLen)
} else if oldPassword == newPassword {
return errors.New("new password can't be same as old password")
} else if !password.SufficientlyStrong(newPassword, RequiredPasswordStrength) {
return errors.New("new password isn't strong enough. Add more characters")
}
auth.HashedPassword = hashing.ComputeHash256([]byte(newPassword))
auth.revoked = []string{} // All the revoked tokens are now invalid; no need to mark specifically as revoked

36
api/auth/auth_test.go
View File

@ -18,8 +18,8 @@ import (
)
var (
password = "password"
hashedPassword = hashing.ComputeHash256([]byte(password))
testPassword = "password!@#$%$#@!"
hashedPassword = hashing.ComputeHash256([]byte(testPassword))
)
var (
@ -49,7 +49,7 @@ func TestNewTokenHappyPath(t *testing.T) {
// Make a token
endpoints := []string{"endpoint1", "endpoint2", "endpoint3"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
@ -84,7 +84,7 @@ func TestTokenHasWrongSig(t *testing.T) {
// Make a token
endpoints := []string{"endpoint1", "endpoint2", "endpoint3"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
@ -114,14 +114,14 @@ func TestChangePassword(t *testing.T) {
HashedPassword: hashedPassword,
}
password2 := "password2"
password2 := "fejhkefjhefjhefhje"
if err := auth.changePassword("", password2); err == nil {
t.Fatal("should have failed because old password is wrong")
} else if err := auth.changePassword("notThePassword", password2); err == nil {
t.Fatal("should have failed because old password is wrong")
} else if err := auth.changePassword(password, ""); err == nil {
} else if err := auth.changePassword(testPassword, ""); err == nil {
t.Fatal("should have failed because new password is empty")
} else if err := auth.changePassword(password, password2); err != nil {
} else if err := auth.changePassword(testPassword, password2); err != nil {
t.Fatal("should have succeeded")
}
@ -129,8 +129,8 @@ func TestChangePassword(t *testing.T) {
t.Fatal("password should have been changed")
}
password3 := "password3"
if err := auth.changePassword(password, password3); err == nil {
password3 := "ufwhwohwfohawfhwdwd"
if err := auth.changePassword(testPassword, password3); err == nil {
t.Fatal("should have failed because old password is wrong")
} else if err := auth.changePassword(password2, password3); err != nil {
t.Fatal("should have succeeded")
@ -171,12 +171,12 @@ func TestRevokeToken(t *testing.T) {
// Make a token
endpoints := []string{"/ext/info", "/ext/bc/X", "/ext/metrics"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
if err := auth.revokeToken(tokenStr, password); err != nil {
if err := auth.revokeToken(tokenStr, testPassword); err != nil {
t.Fatal("should have succeeded")
} else if len(auth.revoked) != 1 || auth.revoked[0] != tokenStr {
t.Fatal("revoked token list is incorrect")
@ -191,7 +191,7 @@ func TestWrapHandlerHappyPath(t *testing.T) {
// Make a token
endpoints := []string{"/ext/info", "/ext/bc/X", "/ext/metrics"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
@ -217,11 +217,11 @@ func TestWrapHandlerRevokedToken(t *testing.T) {
// Make a token
endpoints := []string{"/ext/info", "/ext/bc/X", "/ext/metrics"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
if err := auth.revokeToken(tokenStr, password); err != nil {
if err := auth.revokeToken(tokenStr, testPassword); err != nil {
t.Fatalf("should have been able to revoke token but got: %s", err)
}
@ -248,7 +248,7 @@ func TestWrapHandlerExpiredToken(t *testing.T) {
// Make a token that expired well in the past
endpoints := []string{"/ext/info", "/ext/bc/X", "/ext/metrics"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
@ -292,7 +292,7 @@ func TestWrapHandlerUnauthorizedEndpoint(t *testing.T) {
// Make a token
endpoints := []string{"/ext/info"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
@ -319,7 +319,7 @@ func TestWrapHandlerAuthEndpoint(t *testing.T) {
// Make a token
endpoints := []string{"/ext/info", "/ext/bc/X", "/ext/metrics", "", "/foo", "/ext/info/foo"}
tokenStr, err := auth.newToken(password, endpoints)
tokenStr, err := auth.newToken(testPassword, endpoints)
if err != nil {
t.Fatal(err)
}
@ -342,7 +342,7 @@ func TestWrapHandlerAccessAll(t *testing.T) {
// Make a token that allows access to all endpoints
endpoints := []string{"/ext/info", "/ext/bc/X", "/ext/metrics", "", "/foo", "/ext/foo/info"}
tokenStr, err := auth.newToken(password, []string{"*"})
tokenStr, err := auth.newToken(testPassword, []string{"*"})
if err != nil {
t.Fatal(err)
}

33
api/keystore/service.go
View File

@ -10,9 +10,9 @@ import (
"sync"
"testing"
"github.com/gorilla/rpc/v2"
pw "github.com/ava-labs/gecko/utils/password"
zxcvbn "github.com/nbutton23/zxcvbn-go"
"github.com/gorilla/rpc/v2"
"github.com/ava-labs/gecko/chains/atomic"
"github.com/ava-labs/gecko/database"
@ -32,26 +32,8 @@ const (
// maxUserPassLen is the maximum length of the username or password allowed
maxUserPassLen = 1024
// 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;
//
// 0 # too guessable: risky password. (guesses < 10^3)
// 1 # very guessable: protection from throttled online attacks. (guesses < 10^6)
// 2 # somewhat guessable: protection from unthrottled online attacks. (guesses < 10^8)
// 3 # safely unguessable: moderate protection from offline slow-hash scenario. (guesses < 10^10)
// 4 # very unguessable: strong protection from offline slow-hash scenario. (guesses >= 10^10)
requiredPassScore = 2
// required strength of a keystore password
requiredPassStrength = pw.OK
)
var (
@ -404,12 +386,7 @@ func (ks *Keystore) AddUser(username, password string) error {
return fmt.Errorf("user already exists: %s", username)
}
checkPass := password
if len(password) > maxCheckedPassLen {
checkPass = password[:maxCheckedPassLen]
}
if zxcvbn.PasswordStrength(checkPass, nil).Score < requiredPassScore {
if !pw.SufficientlyStrong(password, requiredPassStrength) {
return errWeakPassword
}

6
main/params.go
View File

@ -13,6 +13,7 @@ import (
"path/filepath"
"strings"
"github.com/ava-labs/gecko/api/auth"
"github.com/ava-labs/gecko/database/leveldb"
"github.com/ava-labs/gecko/database/memdb"
"github.com/ava-labs/gecko/genesis"
@ -25,6 +26,7 @@ import (
"github.com/ava-labs/gecko/utils/formatting"
"github.com/ava-labs/gecko/utils/hashing"
"github.com/ava-labs/gecko/utils/logging"
"github.com/ava-labs/gecko/utils/password"
"github.com/ava-labs/gecko/utils/random"
"github.com/ava-labs/gecko/utils/wrappers"
)
@ -413,6 +415,10 @@ func init() {
errs.Add(errors.New("api-auth-password must be provided if api-require-auth is true"))
return
}
if !password.SufficientlyStrong(Config.APIAuthPassword, auth.RequiredPasswordStrength) {
errs.Add(errors.New("api-auth-password is not strong enough. Add more characters"))
return
}
// Logging:
if *logsDir != "" {

51
utils/password/password.go Normal file
View File

@ -0,0 +1,51 @@
package password
import "github.com/nbutton23/zxcvbn-go"
const (
// 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
)
// Strength is the strength of a password
type Strength int
// The scoring mechanism of the zxcvbn package is defined as follows:
// 0 # too guessable: risky password. (guesses < 10^3)
// 1 # very guessable: protection from throttled online attacks. (guesses < 10^6)
// 2 # somewhat guessable: protection from unthrottled online attacks. (guesses < 10^8)
// 3 # safely unguessable: moderate protection from offline slow-hash scenario. (guesses < 10^10)
// 4 # very unguessable: strong protection from offline slow-hash scenario. (guesses >= 10^10)
// Note: We could use the iota keyword to define each of the below consts, but I think in this case
// it's better to be explicit
const (
// VeryWeak password
VeryWeak = 0
// Weak password
Weak = 1
// OK password
OK = 2
// Strong password
Strong = 3
// VeryStrong password
VeryStrong = 4
)
// SufficientlyStrong returns true if [password] has strength
// greater than or equal to [desiredStrength]
func SufficientlyStrong(password string, desiredStrength Strength) bool {
checkPass := password
if len(password) > maxCheckedPassLen {
checkPass = password[:maxCheckedPassLen]
}
if zxcvbn.PasswordStrength(checkPass, nil).Score < int(desiredStrength) {
return false
}
return true
}

35
utils/password/password_test.go Normal file
View File

@ -0,0 +1,35 @@
package password
import "testing"
type test struct {
password string
expected Strength
}
func TestSufficientlyStrong(t *testing.T) {
tests := []test{
test{
"",
VeryWeak,
},
test{
"a",
VeryWeak,
},
test{
"password",
VeryWeak,
},
test{
"thisisareallylongandpresumablyverystrongpassword",
VeryStrong,
},
}
for _, tt := range tests {
if !SufficientlyStrong(tt.password, tt.expected) {
t.Fatalf("expected %s to be rated stronger", tt.password)
}
}
}