Place sanity checks on Username and Password length on RPC keystore.createUser method

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 to 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", args.Username[:maxUserPassLen])
+
+	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

api/keystore/service_test.go

@@ -5,13 +5,21 @@ package keystore
 
 import (
 	"bytes"
+	"fmt"
+	"math/rand"
 	"testing"
 
 	"github.com/ava-labs/gecko/database/memdb"
 	"github.com/ava-labs/gecko/ids"
 	"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)
 		}

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)
 }