From 45d83d15214e364d466a58b7d7b1111e4e537721 Mon Sep 17 00:00:00 2001 From: Michael Quigley Date: Mon, 23 Jan 2023 11:49:22 -0500 Subject: [PATCH 1/3] salted password migrations (#156) --- .../007_v0_3_0_salted_passwords.sql | 38 +++++++++++++++++++ .../sqlite3/007_v0_3_0_salted_passwords.sql | 3 ++ 2 files changed, 41 insertions(+) create mode 100644 controller/store/sql/postgresql/007_v0_3_0_salted_passwords.sql create mode 100644 controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql diff --git a/controller/store/sql/postgresql/007_v0_3_0_salted_passwords.sql b/controller/store/sql/postgresql/007_v0_3_0_salted_passwords.sql new file mode 100644 index 00000000..4e514626 --- /dev/null +++ b/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; \ No newline at end of file diff --git a/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql b/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql new file mode 100644 index 00000000..1c3fc52b --- /dev/null +++ b/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql @@ -0,0 +1,3 @@ +-- +migrate up + +alter table accounts add column salt string; \ No newline at end of file From b32ee6350ea5f3a3091d00c02f00af21e69e6257 Mon Sep 17 00:00:00 2001 From: Michael Quigley Date: Mon, 23 Jan 2023 12:50:24 -0500 Subject: [PATCH 2/3] implement the new password hashing approach (#156) --- controller/login.go | 7 +++++- controller/passwords.go | 43 +++++++++++++++++++++++++++++++++++++ controller/register.go | 8 ++++++- controller/resetPassword.go | 8 ++++++- controller/store/account.go | 9 ++++---- controller/util.go | 8 ------- 6 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 controller/passwords.go diff --git a/controller/login.go b/controller/login.go index 75693cd8..0e048225 100644 --- a/controller/login.go +++ b/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() } diff --git a/controller/passwords.go b/controller/passwords.go new file mode 100644 index 00000000..9e87e92c --- /dev/null +++ b/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 +} diff --git a/controller/register.go b/controller/register.go index d312489d..f3481cc2 100644 --- a/controller/register.go +++ b/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 { diff --git a/controller/resetPassword.go b/controller/resetPassword.go index 187e1822..1a202d50 100644 --- a/controller/resetPassword.go +++ b/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) diff --git a/controller/store/account.go b/controller/store/account.go index c9584ffd..1125b32b 100644 --- a/controller/store/account.go +++ b/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 diff --git a/controller/util.go b/controller/util.go index 750defed..c1b74974 100644 --- a/controller/util.go +++ b/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") From 789532fff9f6a1e0b281a13556964a1c748f517e Mon Sep 17 00:00:00 2001 From: Michael Quigley Date: Mon, 23 Jan 2023 12:52:47 -0500 Subject: [PATCH 3/3] fix sqlite migration (#156) --- controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql b/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql index 1c3fc52b..f30b0732 100644 --- a/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql +++ b/controller/store/sql/sqlite3/007_v0_3_0_salted_passwords.sql @@ -1,3 +1,3 @@ --- +migrate up +-- +migrate Up alter table accounts add column salt string; \ No newline at end of file