From 979549448ce759fc5041bea363a53a40ed0a8de0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ferdinand=20M=C3=BCtsch?= Date: Sun, 31 Jan 2021 14:34:54 +0100 Subject: [PATCH] chore: remove legacy support for md5 hashed passwords chore: remove password from encoded cookie content as not used anyway --- middlewares/authenticate.go | 24 ++++-------------------- routes/login.go | 7 +++---- routes/settings.go | 2 +- utils/auth.go | 26 ++++---------------------- 4 files changed, 12 insertions(+), 47 deletions(-) diff --git a/middlewares/authenticate.go b/middlewares/authenticate.go index 6332418..4939e63 100644 --- a/middlewares/authenticate.go +++ b/middlewares/authenticate.go @@ -2,9 +2,7 @@ package middlewares import ( "context" - "errors" "fmt" - "github.com/emvi/logbuch" conf "github.com/muety/wakapi/config" "github.com/muety/wakapi/models" "github.com/muety/wakapi/services" @@ -78,32 +76,18 @@ func (m *AuthenticateMiddleware) tryGetUserByApiKey(r *http.Request) (*models.Us } func (m *AuthenticateMiddleware) tryGetUserByCookie(r *http.Request) (*models.User, error) { - login, err := utils.ExtractCookieAuth(r, m.config) + username, err := utils.ExtractCookieAuth(r, m.config) if err != nil { return nil, err } - user, err := m.userSrvc.GetUserById(login.Username) + user, err := m.userSrvc.GetUserById(*username) if err != nil { return nil, err } - if !CheckAndMigratePassword(user, login, m.config.Security.PasswordSalt, &m.userSrvc) { - return nil, errors.New("invalid password") - } + // no need to check password here, as securecookie decoding will fail anyway, + // if cookie is not properly signed return user, nil } - -// migrate old md5-hashed passwords to new salted bcrypt hashes for backwards compatibility -func CheckAndMigratePassword(user *models.User, login *models.Login, salt string, userServiceRef *services.IUserService) bool { - if utils.IsMd5(user.Password) { - if utils.CompareMd5(user.Password, login.Password, "") { - logbuch.Info("migrating old md5 password to new bcrypt format for user '%s'", user.ID) - (*userServiceRef).MigrateMd5Password(user, login) - return true - } - return false - } - return utils.CompareBcrypt(user.Password, login.Password, salt) -} diff --git a/routes/login.go b/routes/login.go index 70d9322..8b786e4 100644 --- a/routes/login.go +++ b/routes/login.go @@ -4,10 +4,10 @@ import ( "fmt" "github.com/gorilla/mux" conf "github.com/muety/wakapi/config" - "github.com/muety/wakapi/middlewares" "github.com/muety/wakapi/models" "github.com/muety/wakapi/models/view" "github.com/muety/wakapi/services" + "github.com/muety/wakapi/utils" "net/http" "time" ) @@ -76,14 +76,13 @@ func (h *LoginHandler) PostLogin(w http.ResponseWriter, r *http.Request) { return } - // TODO: depending on middleware package here is a hack - if !middlewares.CheckAndMigratePassword(user, &login, h.config.Security.PasswordSalt, &h.userSrvc) { + if !utils.CompareBcrypt(user.Password, login.Password, h.config.Security.PasswordSalt) { w.WriteHeader(http.StatusUnauthorized) templates[conf.LoginTemplate].Execute(w, h.buildViewModel(r).WithError("invalid credentials")) return } - encoded, err := h.config.Security.SecureCookie.Encode(models.AuthCookieKey, login) + encoded, err := h.config.Security.SecureCookie.Encode(models.AuthCookieKey, login.Username) if err != nil { w.WriteHeader(http.StatusInternalServerError) templates[conf.LoginTemplate].Execute(w, h.buildViewModel(r).WithError("internal server error")) diff --git a/routes/settings.go b/routes/settings.go index 7ff65be..f242988 100644 --- a/routes/settings.go +++ b/routes/settings.go @@ -113,7 +113,7 @@ func (h *SettingsHandler) PostCredentials(w http.ResponseWriter, r *http.Request Username: user.ID, Password: user.Password, } - encoded, err := h.config.Security.SecureCookie.Encode(models.AuthCookieKey, login) + encoded, err := h.config.Security.SecureCookie.Encode(models.AuthCookieKey, login.Username) if err != nil { w.WriteHeader(http.StatusInternalServerError) templates[conf.SettingsTemplate].Execute(w, h.buildViewModel(r).WithError("internal server error")) diff --git a/utils/auth.go b/utils/auth.go index e30c18e..111f0da 100644 --- a/utils/auth.go +++ b/utils/auth.go @@ -1,9 +1,7 @@ package utils import ( - "crypto/md5" "encoding/base64" - "encoding/hex" "errors" "github.com/muety/wakapi/config" "github.com/muety/wakapi/models" @@ -46,21 +44,17 @@ func ExtractBearerAuth(r *http.Request) (key string, err error) { return string(keyBytes), err } -func ExtractCookieAuth(r *http.Request, config *config.Config) (login *models.Login, err error) { +func ExtractCookieAuth(r *http.Request, config *config.Config) (username *string, err error) { cookie, err := r.Cookie(models.AuthCookieKey) if err != nil { return nil, errors.New("missing authentication") } - if err := config.Security.SecureCookie.Decode(models.AuthCookieKey, cookie.Value, &login); err != nil { - return nil, errors.New("invalid parameters") + if err := config.Security.SecureCookie.Decode(models.AuthCookieKey, cookie.Value, &username); err != nil { + return nil, errors.New("cookie is invalid") } - return login, nil -} - -func IsMd5(hash string) bool { - return md5Regex.Match([]byte(hash)) + return username, nil } func CompareBcrypt(wanted, actual, pepper string) bool { @@ -69,11 +63,6 @@ func CompareBcrypt(wanted, actual, pepper string) bool { return err == nil } -// deprecated, only here for backwards compatibility -func CompareMd5(wanted, actual, pepper string) bool { - return HashMd5(actual, pepper) == wanted -} - func HashBcrypt(plain, pepper string) (string, error) { plainPepperedPassword := []byte(strings.TrimSpace(plain) + pepper) bytes, err := bcrypt.GenerateFromPassword(plainPepperedPassword, bcrypt.DefaultCost) @@ -82,10 +71,3 @@ func HashBcrypt(plain, pepper string) (string, error) { } return "", err } - -func HashMd5(plain, pepper string) string { - plainPepperedPassword := []byte(strings.TrimSpace(plain) + pepper) - hash := md5.Sum(plainPepperedPassword) - hashStr := hex.EncodeToString(hash[:]) - return hashStr -}