From d64fee60c88222835c9c1358b3475d662c9fc368 Mon Sep 17 00:00:00 2001 From: Knut Ahlers Date: Sun, 24 Dec 2017 11:55:52 +0100 Subject: [PATCH] Replace insecure password hashing Prior this commit passwords were hashed with a static salt and using the SHA1 hashing function. This could lead to passwords being attackable in case someone gets access to the raw data stored inside the database. This commit introduces password hashing using bcrypt hashing function which addresses this issue. Old passwords are not automatically re-hashed as they are unknown. Replacing the old password scheme is not that easy and needs #10 to be solved. Therefore the old hashing scheme is kept for compatibility reason. Signed-off-by: Knut Ahlers --- README.md | 4 ++-- login.go | 10 +++++++--- register.go | 19 +++++++++++++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 8eff69e..bbd48aa 100644 --- a/README.md +++ b/README.md @@ -24,8 +24,8 @@ What you definitely should set when starting the server: - `cookie-authkey` - This flag protects the encrypted cookies you're putting on the users computers containing the session. If you don't set it yourself it will be randomly generated. In that case your users will get logged out every time you restart the server. You need to use a key with length of 16 characters (AES128) or 32 characters (AES256). - `cookie-encryptkey` - This flag is the encryption key itself. Like the authkey it will get autogenerated with the same result. You need to use a key with length of 16 characters (AES128) or 32 characters (AES256). -- `password-salt` - The login password of your users are stored in the database for comparison when they log in. Though the passwords are hashed this salt gives you more confidence nobody can use a hash table to simply decrypt the passwords. -- `username-salt` - The usernames are the keys in the database. Like the passwords they are also hashed but you can put an additional salt to them to make it way harder to break them. You should use another salt than for the passwords. +- `password-salt` - [deprecated] In version <=v1.6.1 the password was hashed with a static salt. You only need to provide this if you started using Cloudkeys in one of those versions. +- `username-salt` - The usernames are the keys in the database. They are hashed but you can put an additional salt to them to make it harder to decipher them. If you don't want to define the secrets using command line flags you also can use environment variables to set those flags: diff --git a/login.go b/login.go index 2124942..b51a138 100644 --- a/login.go +++ b/login.go @@ -7,6 +7,8 @@ import ( "strconv" "strings" + "golang.org/x/crypto/bcrypt" + "github.com/flosch/pongo2" "github.com/gorilla/mux" "github.com/gorilla/sessions" @@ -14,8 +16,9 @@ import ( func loginHandler(res http.ResponseWriter, r *http.Request, session *sessions.Session, ctx *pongo2.Context) (*string, error) { var ( - username = strings.ToLower(r.FormValue("username")) - password = fmt.Sprintf("%x", sha1.Sum([]byte(cfg.PasswordSalt+r.FormValue("password")))) + username = strings.ToLower(r.FormValue("username")) + password = r.FormValue("passsword") + deprecatedPassword = fmt.Sprintf("%x", sha1.Sum([]byte(cfg.PasswordSalt+r.FormValue("password")))) // Here for backwards compatibility ) if !storage.IsPresent(createUserFilename(username)) { @@ -32,7 +35,8 @@ func loginHandler(res http.ResponseWriter, r *http.Request, session *sessions.Se userFile, _ := readDataObject(userFileRaw) - if userFile.MetaData.Password != password { + bcryptValidationError := bcrypt.CompareHashAndPassword([]byte(userFile.MetaData.Password), []byte(password)) + if bcryptValidationError != nil && userFile.MetaData.Password != deprecatedPassword { (*ctx)["error"] = true return stringPointer("login.html"), nil } diff --git a/register.go b/register.go index ed8cd2c..6afd2d9 100644 --- a/register.go +++ b/register.go @@ -1,21 +1,21 @@ package main import ( - "crypto/sha1" "fmt" "net/http" "strings" + "golang.org/x/crypto/bcrypt" + "github.com/flosch/pongo2" "github.com/gorilla/sessions" ) func registerHandler(res http.ResponseWriter, r *http.Request, session *sessions.Session, ctx *pongo2.Context) (*string, error) { var ( - username = strings.ToLower(r.FormValue("username")) - password = r.FormValue("password") - passwordCheck = r.FormValue("password_repeat") - hashedPassword = fmt.Sprintf("%x", sha1.Sum([]byte(cfg.PasswordSalt+password))) + username = strings.ToLower(r.FormValue("username")) + password = r.FormValue("password") + passwordCheck = r.FormValue("password_repeat") ) if username == "" || password == "" || password != passwordCheck { @@ -27,8 +27,15 @@ func registerHandler(res http.ResponseWriter, r *http.Request, session *sessions return stringPointer("register.html"), nil } + hashedPassword, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost) + if err != nil { + fmt.Printf("ERR: Unable to hash users password: %s\n", err) + (*ctx)["error"] = true + return stringPointer("register.html"), nil + } + d := dataObject{} - d.MetaData.Password = hashedPassword + d.MetaData.Password = string(hashedPassword) data, _ := d.GetData() if err := storage.Write(createUserFilename(username), data); err != nil {