diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go index d7d7d5a25..70add9a95 100644 --- a/lib/api/api_auth.go +++ b/lib/api/api_auth.go @@ -19,7 +19,6 @@ import ( "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/rand" "github.com/syncthing/syncthing/lib/sync" - "golang.org/x/crypto/bcrypt" ) var ( @@ -117,14 +116,12 @@ func auth(username string, password string, guiCfg config.GUIConfiguration, ldap if guiCfg.AuthMode == config.AuthModeLDAP { return authLDAP(username, password, ldapCfg) } else { - return authStatic(username, password, guiCfg.User, guiCfg.Password) + return authStatic(username, password, guiCfg) } } -func authStatic(username string, password string, configUser string, configPassword string) bool { - configPasswordBytes := []byte(configPassword) - passwordBytes := []byte(password) - return bcrypt.CompareHashAndPassword(configPasswordBytes, passwordBytes) == nil && username == configUser +func authStatic(username string, password string, guiCfg config.GUIConfiguration) bool { + return guiCfg.CompareHashedPassword(password) == nil && username == guiCfg.User } func authLDAP(username string, password string, cfg config.LDAPConfiguration) bool { diff --git a/lib/api/api_auth_test.go b/lib/api/api_auth_test.go index 4735378a5..046b06a07 100644 --- a/lib/api/api_auth_test.go +++ b/lib/api/api_auth_test.go @@ -9,19 +9,20 @@ package api import ( "testing" - "golang.org/x/crypto/bcrypt" + "github.com/syncthing/syncthing/lib/config" ) -var passwordHashBytes []byte +var guiCfg config.GUIConfiguration func init() { - passwordHashBytes, _ = bcrypt.GenerateFromPassword([]byte("pass"), 0) + guiCfg.User = "user" + guiCfg.HashAndSetPassword("pass") } func TestStaticAuthOK(t *testing.T) { t.Parallel() - ok := authStatic("user", "pass", "user", string(passwordHashBytes)) + ok := authStatic("user", "pass", guiCfg) if !ok { t.Fatalf("should pass auth") } @@ -30,7 +31,7 @@ func TestStaticAuthOK(t *testing.T) { func TestSimpleAuthUsernameFail(t *testing.T) { t.Parallel() - ok := authStatic("userWRONG", "pass", "user", string(passwordHashBytes)) + ok := authStatic("userWRONG", "pass", guiCfg) if ok { t.Fatalf("should fail auth") } @@ -39,7 +40,7 @@ func TestSimpleAuthUsernameFail(t *testing.T) { func TestStaticAuthPasswordFail(t *testing.T) { t.Parallel() - ok := authStatic("user", "passWRONG", "user", string(passwordHashBytes)) + ok := authStatic("user", "passWRONG", guiCfg) if ok { t.Fatalf("should fail auth") } diff --git a/lib/api/confighandler.go b/lib/api/confighandler.go index 8f03acfdf..24e6b118c 100644 --- a/lib/api/confighandler.go +++ b/lib/api/confighandler.go @@ -13,7 +13,6 @@ import ( "net/http" "github.com/julienschmidt/httprouter" - "golang.org/x/crypto/bcrypt" "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/protocol" @@ -290,11 +289,13 @@ func (c *configMuxBuilder) adjustConfig(w http.ResponseWriter, r *http.Request) var errMsg string var status int waiter, err := c.cfg.Modify(func(cfg *config.Configuration) { - if to.GUI.Password, err = checkGUIPassword(cfg.GUI.Password, to.GUI.Password); err != nil { - l.Warnln("bcrypting password:", err) - errMsg = err.Error() - status = http.StatusInternalServerError - return + if to.GUI.Password != cfg.GUI.Password { + if err := to.GUI.HashAndSetPassword(to.GUI.Password); err != nil { + l.Warnln("hashing password:", err) + errMsg = err.Error() + status = http.StatusInternalServerError + return + } } *cfg = to }) @@ -370,11 +371,13 @@ func (c *configMuxBuilder) adjustGUI(w http.ResponseWriter, r *http.Request, gui var errMsg string var status int waiter, err := c.cfg.Modify(func(cfg *config.Configuration) { - if gui.Password, err = checkGUIPassword(oldPassword, gui.Password); err != nil { - l.Warnln("bcrypting password:", err) - errMsg = err.Error() - status = http.StatusInternalServerError - return + if gui.Password != oldPassword { + if err := gui.HashAndSetPassword(gui.Password); err != nil { + l.Warnln("hashing password:", err) + errMsg = err.Error() + status = http.StatusInternalServerError + return + } } cfg.GUI = gui }) @@ -418,14 +421,6 @@ func unmarshalToRawMessages(body io.ReadCloser) ([]json.RawMessage, error) { return data, err } -func checkGUIPassword(oldPassword, newPassword string) (string, error) { - if newPassword == oldPassword { - return newPassword, nil - } - hash, err := bcrypt.GenerateFromPassword([]byte(newPassword), 0) - return string(hash), err -} - func (c *configMuxBuilder) finish(w http.ResponseWriter, waiter config.Waiter) { waiter.Wait() if err := c.cfg.Save(); err != nil { diff --git a/lib/config/config_test.go b/lib/config/config_test.go index e49aa9776..5da13207c 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -739,6 +739,27 @@ func TestGUIConfigURL(t *testing.T) { } } +func TestGUIPasswordHash(t *testing.T) { + var c GUIConfiguration + + testPass := "pass" + if err := c.HashAndSetPassword(testPass); err != nil { + t.Fatal(err) + } + if c.Password == testPass { + t.Error("Password hashing resulted in plaintext") + } + + if err := c.CompareHashedPassword(testPass); err != nil { + t.Errorf("No match on same password: %v", err) + } + + failPass := "different" + if err := c.CompareHashedPassword(failPass); err == nil { + t.Errorf("Match on different password: %v", err) + } +} + func TestDuplicateDevices(t *testing.T) { // Duplicate devices should be removed diff --git a/lib/config/guiconfiguration.go b/lib/config/guiconfiguration.go index 7b9830b30..147ed9fdc 100644 --- a/lib/config/guiconfiguration.go +++ b/lib/config/guiconfiguration.go @@ -12,6 +12,8 @@ import ( "strconv" "strings" + "golang.org/x/crypto/bcrypt" + "github.com/syncthing/syncthing/lib/rand" ) @@ -113,6 +115,23 @@ func (c GUIConfiguration) URL() string { return u.String() } +// SetHashedPassword hashes the given plaintext password and stores the new hash. +func (c *GUIConfiguration) HashAndSetPassword(password string) error { + hash, err := bcrypt.GenerateFromPassword([]byte(password), 0) + if err != nil { + return err + } + c.Password = string(hash) + return nil +} + +// CompareHashedPassword returns nil when the given plaintext password matches the stored hash. +func (c GUIConfiguration) CompareHashedPassword(password string) error { + configPasswordBytes := []byte(c.Password) + passwordBytes := []byte(password) + return bcrypt.CompareHashAndPassword(configPasswordBytes, passwordBytes) +} + // IsValidAPIKey returns true when the given API key is valid, including both // the value in config and any overrides func (c GUIConfiguration) IsValidAPIKey(apiKey string) bool {