Merge pull request #177 from openziti/passwords

Password Salting and Improved Hashing (#156)

controller/login.go

@@ -26,7 +26,12 @@ func loginHandler(params account.LoginParams) middleware.Responder {
 		logrus.Errorf("error finding account '%v': %v", params.Body.Email, err)
 		return account.NewLoginUnauthorized()
 	}
-	if a.Password != hashPassword(params.Body.Password) {
+	hpwd, err := rehashPassword(params.Body.Password, a.Salt)
+	if err != nil {
+		logrus.Errorf("error hashing password for '%v': %v", params.Body.Email, err)
+		return account.NewLoginUnauthorized()
+	}
+	if a.Password != hpwd.Password {
 		logrus.Errorf("password mismatch for account '%v'", params.Body.Email)
 		return account.NewLoginUnauthorized()
 	}

controller/passwords.go

@@ -0,0 +1,43 @@
+package controller
+
+import (
+	"crypto/rand"
+	"encoding/base64"
+	"encoding/binary"
+	"github.com/michaelquigley/pfxlog"
+	"golang.org/x/crypto/argon2"
+)
+
+type hashedPassword struct {
+	Password string
+	Salt     string
+}
+
+func salt() string {
+	buf := make([]byte, binary.MaxVarintLen64)
+	_, err := rand.Read(buf)
+
+	if err != nil {
+		pfxlog.Logger().Panic(err)
+	}
+
+	return base64.StdEncoding.EncodeToString(buf)
+}
+
+func hashPassword(password string) (*hashedPassword, error) {
+	return rehashPassword(password, salt())
+}
+
+func rehashPassword(password string, salt string) (*hashedPassword, error) {
+	s, err := base64.StdEncoding.DecodeString(salt)
+	if err != nil {
+		return nil, err
+	}
+
+	hash := argon2.IDKey([]byte(password), s, 1, 3*1024, 4, 32)
+
+	return &hashedPassword{
+		Password: base64.StdEncoding.EncodeToString(hash),
+		Salt:     salt,
+	}, nil
+}

controller/register.go

@@ -38,9 +38,15 @@ func (self *registerHandler) Handle(params account.RegisterParams) middleware.Re
 		logrus.Error(err)
 		return account.NewRegisterInternalServerError()
 	}
+	hpwd, err := hashPassword(params.Body.Password)
+	if err != nil {
+		logrus.Error(err)
+		return account.NewRegisterInternalServerError()
+	}
 	a := &store.Account{
 		Email:    ar.Email,
-		Password: hashPassword(params.Body.Password),
+		Salt:     hpwd.Salt,
+		Password: hpwd.Password,
 		Token:    token,
 	}
 	if _, err := str.CreateAccount(a, tx); err != nil {

controller/resetPassword.go

@@ -37,7 +37,13 @@ func (handler *resetPasswordHandler) Handle(params account.ResetPasswordParams)
 		logrus.Error(err)
 		return account.NewResetPasswordNotFound()
 	}
-	a.Password = hashPassword(params.Body.Password)
+	hpwd, err := hashPassword(params.Body.Password)
+	if err != nil {
+		logrus.Error(err)
+		return account.NewResetPasswordRequestInternalServerError()
+	}
+	a.Salt = hpwd.Salt
+	a.Password = hpwd.Password
 
 	if _, err := str.UpdateAccount(a, tx); err != nil {
 		logrus.Error(err)

controller/store/account.go

@@ -8,18 +8,19 @@ import (
 type Account struct {
 	Model
 	Email     string
+	Salt      string
 	Password  string
 	Token     string
 	Limitless bool
 }
 
 func (self *Store) CreateAccount(a *Account, tx *sqlx.Tx) (int, error) {
-	stmt, err := tx.Prepare("insert into accounts (email, password, token, limitless) values ($1, $2, $3, $4) returning id")
+	stmt, err := tx.Prepare("insert into accounts (email, salt, password, token, limitless) values ($1, $2, $3, $4, $5) returning id")
 	if err != nil {
 		return 0, errors.Wrap(err, "error preparing accounts insert statement")
 	}
 	var id int
-	if err := stmt.QueryRow(a.Email, a.Password, a.Token, a.Limitless).Scan(&id); err != nil {
+	if err := stmt.QueryRow(a.Email, a.Salt, a.Password, a.Token, a.Limitless).Scan(&id); err != nil {
 		return 0, errors.Wrap(err, "error executing accounts insert statement")
 	}
 	return id, nil
@@ -50,12 +51,12 @@ func (self *Store) FindAccountWithToken(token string, tx *sqlx.Tx) (*Account, er
 }
 
 func (self *Store) UpdateAccount(a *Account, tx *sqlx.Tx) (int, error) {
-	stmt, err := tx.Prepare("update accounts set email=$1, password=$2, token=$3, limitless=$4 where id = $5")
+	stmt, err := tx.Prepare("update accounts set email=$1, salt=$2, password=$3, token=$4, limitless=$5 where id = $6")
 	if err != nil {
 		return 0, errors.Wrap(err, "error preparing accounts update statement")
 	}
 	var id int
-	if _, err := stmt.Exec(a.Email, a.Password, a.Token, a.Limitless, a.Id); err != nil {
+	if _, err := stmt.Exec(a.Email, a.Salt, a.Password, a.Token, a.Limitless, a.Id); err != nil {
 		return 0, errors.Wrap(err, "error executing accounts update statement")
 	}
 	return id, nil

controller/store/sql/postgresql/007_v0_3_0_salted_passwords.sql

@@ -0,0 +1,38 @@
+-- +migrate Up
+
+alter table accounts rename to accounts_old;
+alter sequence accounts_id_seq rename to accounts_id_seq_old;
+
+create table accounts (
+  id                    serial              primary key,
+  email                 varchar(1024)       not null unique,
+  salt                  varchar(16)         not null default(''),
+  password              varchar(64)         not null default(''),
+  token                 varchar(32)         not null unique,
+  limitless             boolean             not null default(false),
+  created_at            timestamp           not null default(current_timestamp),
+  updated_at            timestamp           not null default(current_timestamp),
+
+  constraint chk_email check (email <> ''),
+  constraint chk_token check(token <> '')
+);
+
+insert into accounts(id, email, token, limitless, created_at, updated_at)
+    select id, email, token, limitless, created_at, updated_at from accounts_old;
+
+alter table accounts alter column salt drop default;
+alter table accounts alter column password drop default;
+
+select setval('accounts_id_seq', (select max(id) from accounts));
+
+alter table environments drop constraint fk_accounts_id;
+alter table environments add constraint fk_accounts_id foreign key (account_id) references accounts(id);
+
+alter table password_reset_requests drop constraint fk_accounts_password_reset_requests;
+alter table password_reset_requests add constraint fk_accounts_password_reset_requests foreign key (account_id) references accounts(id);
+
+drop table accounts_old;
+
+alter index accounts_pkey1 rename to accounts_pkey;
+alter index accounts_email_key1 rename to accounts_email_key;
+alter index accounts_token_key1 rename to accounts_token_key;

controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql

@@ -0,0 +1,3 @@
+-- +migrate Up
+
+alter table accounts add column salt string;

controller/util.go

@@ -1,9 +1,7 @@
 package controller
 
 import (
-	"crypto/sha512"
 	"crypto/x509"
-	"encoding/hex"
 	errors2 "github.com/go-openapi/errors"
 	"github.com/jaevor/go-nanoid"
 	"github.com/openziti/edge/rest_management_api_client"
@@ -83,12 +81,6 @@ func createToken() (string, error) {
 	return gen(), nil
 }
 
-func hashPassword(raw string) string {
-	hash := sha512.New()
-	hash.Write([]byte(raw))
-	return hex.EncodeToString(hash.Sum(nil))
-}
-
 func realRemoteAddress(req *http.Request) string {
 	ip := strings.Split(req.RemoteAddr, ":")[0]
 	fwdAddress := req.Header.Get("X-Forwarded-For")