diff options
author | Jakob Borg <jakob@nym.se> | 2016-04-18 20:24:38 +0000 |
---|---|---|
committer | Jakob Borg <jakob@nym.se> | 2016-04-18 20:24:38 +0000 |
commit | 893cc025f985a698888ec90eb5adf68570af5b81 (patch) | |
tree | e8fc60233d3fa4361023e3280522bd9cca6e7c09 | |
parent | b81c8d2e1b5856d9493a62c816f26483f505adea (diff) |
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
-rw-r--r-- | cmd/syncthing/gui_auth.go | 51 | ||||
-rw-r--r-- | cmd/syncthing/gui_test.go | 134 | ||||
-rw-r--r-- | cmd/syncthing/mocked_config_test.go | 6 |
3 files changed, 151 insertions, 40 deletions
diff --git a/cmd/syncthing/gui_auth.go b/cmd/syncthing/gui_auth.go index 8630c4b0a5..e062185a5a 100644 --- a/cmd/syncthing/gui_auth.go +++ b/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 { - emitLoginAttempt(false, username) - error() - return + if username == cfg.User { + goto usernameOK } - if err := bcrypt.CompareHashAndPassword([]byte(cfg.Password), fields[1]); err != nil { - emitLoginAttempt(false, username) - error() - return + // ... check it again, converting it from assumed ISO-8859-1 to UTF-8 + username = string(iso88591ToUTF8(fields[0])) + if username == cfg.User { + 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)) +} diff --git a/cmd/syncthing/gui_test.go b/cmd/syncthing/gui_test.go index 195af59813..45d545ca20 100644 --- a/cmd/syncthing/gui_test.go +++ b/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" - 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) + baseURL, err := startHTTP(cfg) 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 +} diff --git a/cmd/syncthing/mocked_config_test.go b/cmd/syncthing/mocked_config_test.go index 78ac526bb4..c0bc6366c0 100644 --- a/cmd/syncthing/mocked_config_test.go +++ b/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 { |