Repository: syncthing/syncthing (language: go)

Issue:
Hashed passwords via API are hashed again
### Include required information

About 6 years ago, a support for pre-hashed GUI passwords via API was added by fixing the issue https://github.com/syncthing/syncthing/issues/4458 in commit https://github.com/syncthing/syncthing/commit/941c9f1531351091b707803bf456cdc68b151eba

However, it seems this doesn't work anymore. If I understood correctly, this commit broke it a few years ago: https://github.com/syncthing/syncthing/commit/f0f60ba2e74737f16c9fac5e9447d1835fa3deb5#r128163305

So, currently what happens is that the hashed password is taken as the literal plain-text password and hashed. That is, the hash becomes my password.

Is there supposed to be any way of providing hashed GUI passwords to the syncthing API?

Syncthing version 1.24.0 on NixOS.


Discussion / hints:
It's supposed to work, indeed. Apparently we're missing a test for it though. 

Fix patch:
diff --git a/cmd/syncthing/generate/generate.go b/cmd/syncthing/generate/generate.go
index 6b3d0b36902..5446cb3baf0 100644
--- a/cmd/syncthing/generate/generate.go
+++ b/cmd/syncthing/generate/generate.go
@@ -69,7 +69,7 @@ func Generate(l logger.Logger, confDir, guiUser, guiPassword string, noDefaultFo
 		return err
 	}
 
-	if err := syncthing.EnsureDir(dir, 0700); err != nil {
+	if err := syncthing.EnsureDir(dir, 0o700); err != nil {
 		return err
 	}
 	locations.SetBaseDir(locations.ConfigBaseDir, dir)
@@ -127,7 +127,7 @@ func updateGUIAuthentication(l logger.Logger, guiCfg *config.GUIConfiguration, g
 	}
 
 	if guiPassword != "" && guiCfg.Password != guiPassword {
-		if err := guiCfg.HashAndSetPassword(guiPassword); err != nil {
+		if err := guiCfg.SetPassword(guiPassword); err != nil {
 			return fmt.Errorf("failed to set GUI authentication password: %w", err)
 		}
 		l.Infoln("Updated GUI authentication password.")
diff --git a/lib/api/confighandler.go b/lib/api/confighandler.go
index 5abc1f055ed..fc3e73a5988 100644
--- a/lib/api/confighandler.go
+++ b/lib/api/confighandler.go
@@ -319,7 +319,7 @@ func (c *configMuxBuilder) adjustConfig(w http.ResponseWriter, r *http.Request)
 	var status int
 	waiter, err := c.cfg.Modify(func(cfg *config.Configuration) {
 		if to.GUI.Password != cfg.GUI.Password {
-			if err := to.GUI.HashAndSetPassword(to.GUI.Password); err != nil {
+			if err := to.GUI.SetPassword(to.GUI.Password); err != nil {
 				l.Warnln("hashing password:", err)
 				errMsg = err.Error()
 				status = http.StatusInternalServerError
@@ -401,7 +401,7 @@ func (c *configMuxBuilder) adjustGUI(w http.ResponseWriter, r *http.Request, gui
 	var status int
 	waiter, err := c.cfg.Modify(func(cfg *config.Configuration) {
 		if gui.Password != oldPassword {
-			if err := gui.HashAndSetPassword(gui.Password); err != nil {
+			if err := gui.SetPassword(gui.Password); err != nil {
 				l.Warnln("hashing password:", err)
 				errMsg = err.Error()
 				status = http.StatusInternalServerError
diff --git a/lib/config/guiconfiguration.go b/lib/config/guiconfiguration.go
index cbedfffcb14..9f5df227ff5 100644
--- a/lib/config/guiconfiguration.go
+++ b/lib/config/guiconfiguration.go
@@ -9,6 +9,7 @@ package config
 import (
 	"net/url"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -115,9 +116,19 @@ 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)
+// matches a bcrypt hash and not too much else
+var bcryptExpr = regexp.MustCompile(`^\$2[aby]\$\d+\$.{50,}`)
+
+// SetPassword takes a bcrypt hash or a plaintext password and stores it.
+// Plaintext passwords are hashed. Returns an error if the password is not
+// valid.
+func (c *GUIConfiguration) SetPassword(password string) error {
+	if bcryptExpr.MatchString(password) {
+		// Already hashed
+		c.Password = password
+		return nil
+	}
+	hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.DefaultCost)
 	if err != nil {
 		return err
 	}


Test patch:
diff --git a/lib/api/api_auth_test.go b/lib/api/api_auth_test.go
index be6a0c3a800..ed3677eeb25 100644
--- a/lib/api/api_auth_test.go
+++ b/lib/api/api_auth_test.go
@@ -16,7 +16,7 @@ var guiCfg config.GUIConfiguration
 
 func init() {
 	guiCfg.User = "user"
-	guiCfg.HashAndSetPassword("pass")
+	guiCfg.SetPassword("pass")
 }
 
 func TestStaticAuthOK(t *testing.T) {
diff --git a/lib/config/config_test.go b/lib/config/config_test.go
index 241cc2b5966..d90e01fb94e 100644
--- a/lib/config/config_test.go
+++ b/lib/config/config_test.go
@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/d4l3k/messagediff"
+	"golang.org/x/crypto/bcrypt"
 
 	"github.com/syncthing/syncthing/lib/build"
 	"github.com/syncthing/syncthing/lib/events"
@@ -773,8 +774,9 @@ func TestGUIConfigURL(t *testing.T) {
 func TestGUIPasswordHash(t *testing.T) {
 	var c GUIConfiguration
 
+	// Setting a plaintext password should work
 	testPass := "pass"
-	if err := c.HashAndSetPassword(testPass); err != nil {
+	if err := c.SetPassword(testPass); err != nil {
 		t.Fatal(err)
 	}
 	if c.Password == testPass {
@@ -789,6 +791,16 @@ func TestGUIPasswordHash(t *testing.T) {
 	if err := c.CompareHashedPassword(failPass); err == nil {
 		t.Errorf("Match on different password: %v", err)
 	}
+
+	// Setting a bcrypt hash directly should also work
+	hash, err := bcrypt.GenerateFromPassword([]byte("test"), bcrypt.MinCost)
+	if err != nil {
+		t.Fatal(err)
+	}
+	c.SetPassword(string(hash))
+	if err := c.CompareHashedPassword("test"); err != nil {
+		t.Errorf("No match on hashed password: %v", err)
+	}
 }
 
 func TestDuplicateDevices(t *testing.T) {