cmd/syncthing: Accept ISO-8859-1 and UTF-8 in HTTP BasicAuth header (fixes #2779)

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/2989

cmd/syncthing/gui_auth.go

@@ -78,19 +78,42 @@ func basicAuthAndSessionMiddleware(cookieName string, cfg config.GUIConfiguratio
 			return
 		}
 
+		// Check if the username is correct, assuming it was sent as UTF-8
 		username := string(fields[0])
-		if username != cfg.User {
+		if username == cfg.User {
-			emitLoginAttempt(false, username)
+			goto usernameOK
-			error()
-			return
 		}
 
-		if err := bcrypt.CompareHashAndPassword([]byte(cfg.Password), fields[1]); err != nil {
+		// ... check it again, converting it from assumed ISO-8859-1 to UTF-8
-			emitLoginAttempt(false, username)
+		username = string(iso88591ToUTF8(fields[0]))
-			error()
+		if username == cfg.User {
-			return
+			goto usernameOK
 		}
 
+		// Neither of the possible interpretations match the configured username
+		emitLoginAttempt(false, username)
+		error()
+		return
+
+	usernameOK:
+		// Check password as given (assumes UTF-8 encoding)
+		password := fields[1]
+		if err := bcrypt.CompareHashAndPassword([]byte(cfg.Password), password); err == nil {
+			goto passwordOK
+		}
+
+		// ... check it again, converting it from assumed ISO-8859-1 to UTF-8
+		password = iso88591ToUTF8(password)
+		if err := bcrypt.CompareHashAndPassword([]byte(cfg.Password), password); err == nil {
+			goto passwordOK
+		}
+
+		// Neither of the attempts to verify the password checked out
+		emitLoginAttempt(false, username)
+		error()
+		return
+
+	passwordOK:
 		sessionid := util.RandomString(32)
 		sessionsMut.Lock()
 		sessions[sessionid] = true
@@ -105,3 +128,15 @@ func basicAuthAndSessionMiddleware(cookieName string, cfg config.GUIConfiguratio
 		next.ServeHTTP(w, r)
 	})
 }
+
+// Convert an ISO-8859-1 encoded byte string to UTF-8. Works by the
+// principle that ISO-8859-1 bytes are equivalent to unicode code points,
+// that a rune slice is a list of code points, and that stringifying a slice
+// of runes generates UTF-8 in Go.
+func iso88591ToUTF8(s []byte) []byte {
+	runes := make([]rune, len(s))
+	for i := range s {
+		runes[i] = rune(s[i])
+	}
+	return []byte(string(runes))
+}

cmd/syncthing/gui_test.go

@@ -189,40 +189,11 @@ type httpTestCase struct {
 }
 
 func TestAPIServiceRequests(t *testing.T) {
-	model := new(mockedModel)
 	cfg := new(mockedConfig)
-	httpsCertFile := "../../test/h1/https-cert.pem"
+	baseURL, err := startHTTP(cfg)
-	httpsKeyFile := "../../test/h1/https-key.pem"
-	assetDir := "../../gui"
-	eventSub := new(mockedEventSub)
-	discoverer := new(mockedCachingMux)
-	relayService := new(mockedRelayService)
-	errorLog := new(mockedLoggerRecorder)
-	systemLog := new(mockedLoggerRecorder)
-
-	// Instantiate the API service
-	svc, err := newAPIService(protocol.LocalDeviceID, cfg, httpsCertFile, httpsKeyFile, assetDir, model,
-		eventSub, discoverer, relayService, errorLog, systemLog)
 	if err != nil {
 		t.Fatal(err)
 	}
-	_ = svc
-
-	// Make sure the API service is listening, and get the URL to use.
-	addr := svc.listener.Addr()
-	if addr == nil {
-		t.Fatal("Nil listening address from API service")
-	}
-	tcpAddr, err := net.ResolveTCPAddr("tcp", addr.String())
-	if err != nil {
-		t.Fatal("Weird address from API service:", err)
-	}
-	baseURL := fmt.Sprintf("http://127.0.0.1:%d", tcpAddr.Port)
-
-	// Actually start the API service
-	supervisor := suture.NewSimple("API test")
-	supervisor.Add(svc)
-	supervisor.ServeBackground()
 
 	cases := []httpTestCase{
 		// /rest/db
@@ -417,3 +388,106 @@ func testHTTPRequest(t *testing.T, baseURL string, tc httpTestCase) {
 		return
 	}
 }
+
+func TestHTTPLogin(t *testing.T) {
+	cfg := new(mockedConfig)
+	cfg.gui.User = "üser"
+	cfg.gui.Password = "$2a$10$IdIZTxTg/dCNuNEGlmLynOjqg4B1FvDKuIV5e0BB3pnWVHNb8.GSq" // bcrypt of "räksmörgås" in UTF-8
+	baseURL, err := startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Verify rejection when not using authorization
+
+	req, _ := http.NewRequest("GET", baseURL+"/rest/system/status", nil)
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusUnauthorized {
+		t.Errorf("Unexpected non-401 return code %d for unauthed request", resp.StatusCode)
+	}
+
+	// Verify that incorrect password is rejected
+
+	req.SetBasicAuth("üser", "rksmrgs")
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusUnauthorized {
+		t.Errorf("Unexpected non-401 return code %d for incorrect password", resp.StatusCode)
+	}
+
+	// Verify that incorrect username is rejected
+
+	req.SetBasicAuth("user", "räksmörgås") // string literals in Go source code are in UTF-8
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusUnauthorized {
+		t.Errorf("Unexpected non-401 return code %d for incorrect username", resp.StatusCode)
+	}
+
+	// Verify that UTF-8 auth works
+
+	req.SetBasicAuth("üser", "räksmörgås") // string literals in Go source code are in UTF-8
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Errorf("Unexpected non-200 return code %d for authed request (UTF-8)", resp.StatusCode)
+	}
+
+	// Verify that ISO-8859-1 auth
+
+	req.SetBasicAuth("\xfcser", "r\xe4ksm\xf6rg\xe5s") // escaped ISO-8859-1
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusOK {
+		t.Errorf("Unexpected non-200 return code %d for authed request (ISO-8859-1)", resp.StatusCode)
+	}
+
+}
+
+func startHTTP(cfg *mockedConfig) (string, error) {
+	model := new(mockedModel)
+	httpsCertFile := "../../test/h1/https-cert.pem"
+	httpsKeyFile := "../../test/h1/https-key.pem"
+	assetDir := "../../gui"
+	eventSub := new(mockedEventSub)
+	discoverer := new(mockedCachingMux)
+	relayService := new(mockedRelayService)
+	errorLog := new(mockedLoggerRecorder)
+	systemLog := new(mockedLoggerRecorder)
+
+	// Instantiate the API service
+	svc, err := newAPIService(protocol.LocalDeviceID, cfg, httpsCertFile, httpsKeyFile, assetDir, model,
+		eventSub, discoverer, relayService, errorLog, systemLog)
+	if err != nil {
+		return "", err
+	}
+
+	// Make sure the API service is listening, and get the URL to use.
+	addr := svc.listener.Addr()
+	if addr == nil {
+		return "", fmt.Errorf("Nil listening address from API service")
+	}
+	tcpAddr, err := net.ResolveTCPAddr("tcp", addr.String())
+	if err != nil {
+		return "", fmt.Errorf("Weird address from API service: %v", err)
+	}
+	baseURL := fmt.Sprintf("http://127.0.0.1:%d", tcpAddr.Port)
+
+	// Actually start the API service
+	supervisor := suture.NewSimple("API test")
+	supervisor.Add(svc)
+	supervisor.ServeBackground()
+
+	return baseURL, nil
+}

cmd/syncthing/mocked_config_test.go

@@ -11,10 +11,12 @@ import (
 	"github.com/syncthing/syncthing/lib/protocol"
 )
 
-type mockedConfig struct{}
+type mockedConfig struct {
+	gui config.GUIConfiguration
+}
 
 func (c *mockedConfig) GUI() config.GUIConfiguration {
-	return config.GUIConfiguration{}
+	return c.gui
 }
 
 func (c *mockedConfig) Raw() config.Configuration {