diff --git a/api/keystore/service.go b/api/keystore/service.go index 604ec5d..dad7073 100644 --- a/api/keystore/service.go +++ b/api/keystore/service.go @@ -21,10 +21,30 @@ import ( "github.com/ava-labs/gecko/vms/components/codec" jsoncodec "github.com/ava-labs/gecko/utils/json" + zxcvbn "github.com/nbutton23/zxcvbn-go" +) + +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 + // + // 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 ) var ( - errEmptyUsername = errors.New("username can't be the empty string") + errEmptyUsername = errors.New("username can't be the empty string") + errUserPassMaxLength = fmt.Errorf("CreateUser call rejected due to username or password exceeding maximum length of %d chars", maxUserPassLen) + errWeakPassword = errors.New("Failed to create user as the given password is too weak. A stronger password is one of 8 or more characters containing attributes of upper and lowercase letters, numbers, and/or special characters") ) // KeyValuePair ... @@ -114,7 +134,11 @@ func (ks *Keystore) CreateUser(_ *http.Request, args *CreateUserArgs, reply *Cre ks.lock.Lock() defer ks.lock.Unlock() - ks.log.Verbo("CreateUser called with %s", args.Username) + ks.log.Verbo("CreateUser called with %.*s", maxUserPassLen, args.Username) + + if len(args.Username) > maxUserPassLen || len(args.Password) > maxUserPassLen { + return errUserPassMaxLength + } if args.Username == "" { return errEmptyUsername @@ -123,6 +147,10 @@ func (ks *Keystore) CreateUser(_ *http.Request, args *CreateUserArgs, reply *Cre return fmt.Errorf("user already exists: %s", args.Username) } + if zxcvbn.PasswordStrength(args.Password, nil).Score < requiredPassScore { + return errWeakPassword + } + usr := &User{} if err := usr.Initialize(args.Password); err != nil { return err diff --git a/api/keystore/service_test.go b/api/keystore/service_test.go index ab2a096..0868a29 100644 --- a/api/keystore/service_test.go +++ b/api/keystore/service_test.go @@ -5,6 +5,8 @@ package keystore import ( "bytes" + "fmt" + "math/rand" "testing" "github.com/ava-labs/gecko/database/memdb" @@ -12,6 +14,12 @@ import ( "github.com/ava-labs/gecko/utils/logging" ) +var ( + // strongPassword defines a password used for the following tests that + // scores high enough to pass the password strength scoring system + strongPassword = "N_+=_jJ;^(<;{4,:*m6CET}'&N;83FYK.wtNpwp-Jt" +) + func TestServiceListNoUsers(t *testing.T) { ks := Keystore{} ks.Initialize(logging.NoLog{}, memdb.New()) @@ -33,7 +41,7 @@ func TestServiceCreateUser(t *testing.T) { reply := CreateUserReply{} if err := ks.CreateUser(nil, &CreateUserArgs{ Username: "bob", - Password: "launch", + Password: strongPassword, }, &reply); err != nil { t.Fatal(err) } @@ -56,6 +64,78 @@ func TestServiceCreateUser(t *testing.T) { } } +// genStr returns a string of given length +func genStr(n int) string { + b := make([]byte, n) + rand.Read(b) + return fmt.Sprintf("%x", b)[:n] +} + +// TestServiceCreateUserArgsChecks generates excessively long usernames or +// passwords to assure the santity checks on string length are not exceeded +func TestServiceCreateUserArgsCheck(t *testing.T) { + ks := Keystore{} + ks.Initialize(logging.NoLog{}, memdb.New()) + + { + reply := CreateUserReply{} + err := ks.CreateUser(nil, &CreateUserArgs{ + Username: genStr(maxUserPassLen + 1), + Password: strongPassword, + }, &reply) + + if reply.Success || err != errUserPassMaxLength { + t.Fatal("User was created when it should have been rejected due to too long a Username, err =", err) + } + } + + { + reply := CreateUserReply{} + err := ks.CreateUser(nil, &CreateUserArgs{ + Username: "shortuser", + Password: genStr(maxUserPassLen + 1), + }, &reply) + + if reply.Success || err != errUserPassMaxLength { + t.Fatal("User was created when it should have been rejected due to too long a Password, err =", err) + } + } + + { + reply := ListUsersReply{} + if err := ks.ListUsers(nil, &ListUsersArgs{}, &reply); err != nil { + t.Fatal(err) + } + + if len(reply.Users) > 0 { + t.Fatalf("A user exists when there should be none") + } + } +} + +// TestServiceCreateUserWeakPassword tests creating a new user with a weak +// password to ensure the password strength check is working +func TestServiceCreateUserWeakPassword(t *testing.T) { + ks := Keystore{} + ks.Initialize(logging.NoLog{}, memdb.New()) + + { + reply := CreateUserReply{} + err := ks.CreateUser(nil, &CreateUserArgs{ + Username: "bob", + Password: "weak", + }, &reply) + + if err != errWeakPassword { + t.Error("Unexpected error occurred when testing weak password:", err) + } + + if reply.Success { + t.Fatal("User was created when it should have been rejected due to weak password") + } + } +} + func TestServiceCreateDuplicate(t *testing.T) { ks := Keystore{} ks.Initialize(logging.NoLog{}, memdb.New()) @@ -64,7 +144,7 @@ func TestServiceCreateDuplicate(t *testing.T) { reply := CreateUserReply{} if err := ks.CreateUser(nil, &CreateUserArgs{ Username: "bob", - Password: "launch", + Password: strongPassword, }, &reply); err != nil { t.Fatal(err) } @@ -77,7 +157,7 @@ func TestServiceCreateDuplicate(t *testing.T) { reply := CreateUserReply{} if err := ks.CreateUser(nil, &CreateUserArgs{ Username: "bob", - Password: "launch!", + Password: strongPassword, }, &reply); err == nil { t.Fatalf("Should have errored due to the username already existing") } @@ -90,7 +170,7 @@ func TestServiceCreateUserNoName(t *testing.T) { reply := CreateUserReply{} if err := ks.CreateUser(nil, &CreateUserArgs{ - Password: "launch", + Password: strongPassword, }, &reply); err == nil { t.Fatalf("Shouldn't have allowed empty username") } @@ -104,7 +184,7 @@ func TestServiceUseBlockchainDB(t *testing.T) { reply := CreateUserReply{} if err := ks.CreateUser(nil, &CreateUserArgs{ Username: "bob", - Password: "launch", + Password: strongPassword, }, &reply); err != nil { t.Fatal(err) } @@ -114,7 +194,7 @@ func TestServiceUseBlockchainDB(t *testing.T) { } { - db, err := ks.GetDatabase(ids.Empty, "bob", "launch") + db, err := ks.GetDatabase(ids.Empty, "bob", strongPassword) if err != nil { t.Fatal(err) } @@ -124,7 +204,7 @@ func TestServiceUseBlockchainDB(t *testing.T) { } { - db, err := ks.GetDatabase(ids.Empty, "bob", "launch") + db, err := ks.GetDatabase(ids.Empty, "bob", strongPassword) if err != nil { t.Fatal(err) } @@ -144,7 +224,7 @@ func TestServiceExportImport(t *testing.T) { reply := CreateUserReply{} if err := ks.CreateUser(nil, &CreateUserArgs{ Username: "bob", - Password: "launch", + Password: strongPassword, }, &reply); err != nil { t.Fatal(err) } @@ -154,7 +234,7 @@ func TestServiceExportImport(t *testing.T) { } { - db, err := ks.GetDatabase(ids.Empty, "bob", "launch") + db, err := ks.GetDatabase(ids.Empty, "bob", strongPassword) if err != nil { t.Fatal(err) } @@ -166,7 +246,7 @@ func TestServiceExportImport(t *testing.T) { exportReply := ExportUserReply{} if err := ks.ExportUser(nil, &ExportUserArgs{ Username: "bob", - Password: "launch", + Password: strongPassword, }, &exportReply); err != nil { t.Fatal(err) } @@ -178,7 +258,7 @@ func TestServiceExportImport(t *testing.T) { reply := ImportUserReply{} if err := newKS.ImportUser(nil, &ImportUserArgs{ Username: "bob", - Password: "launch", + Password: strongPassword, User: exportReply.User, }, &reply); err != nil { t.Fatal(err) @@ -189,7 +269,7 @@ func TestServiceExportImport(t *testing.T) { } { - db, err := newKS.GetDatabase(ids.Empty, "bob", "launch") + db, err := newKS.GetDatabase(ids.Empty, "bob", strongPassword) if err != nil { t.Fatal(err) } diff --git a/utils/logging/log.go b/utils/logging/log.go index 04d69fb..a9e6aec 100644 --- a/utils/logging/log.go +++ b/utils/logging/log.go @@ -171,7 +171,7 @@ func (l *Log) format(level Level, format string, args ...interface{}) string { return fmt.Sprintf("%s[%s]%s %s\n", level, - time.Now().Format("01-02|15:04:05.000"), + time.Now().Format("01-02|15:04:05"), prefix, text) }