From 439c6c5b7c64eb8f06d2cd4d8f66e27d0e277d2d Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Tue, 14 Nov 2023 11:57:39 +0100 Subject: [PATCH] lib/api: Add cache busting for basic auth (ref #9208) (#9215) This adds our short device ID to the basic auth realm. This has at least two consequences: - It is different from what's presented by another device on the same address (e.g., if I use SSH forwards to different dives on the same local address), preventing credentials for one from being sent to another. - It is different from what we did previously, meaning we avoid cached credentials from old versions interfering with the new login flow. I don't *think* there should be things that depend on our precise realm string, so this shouldn't break any existing setups... Sneakily this also changes the session cookie and CSRF name, because I think `id.Short().String()` is nicer than `id.String()[:5]` and the short ID is two characters longer. That's also not a problem... --- gui/default/syncthing/app.js | 5 ++--- lib/api/api.go | 7 ++++--- lib/api/api_auth.go | 8 ++++---- lib/protocol/deviceid.go | 13 +++++++++---- next-gen-gui/src/app/api-utils.ts | 3 +-- test/http_test.go | 2 +- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/gui/default/syncthing/app.js b/gui/default/syncthing/app.js index 80e9a20d7..0a961f508 100644 --- a/gui/default/syncthing/app.js +++ b/gui/default/syncthing/app.js @@ -39,9 +39,8 @@ syncthing.config(function ($httpProvider, $translateProvider, LocaleServiceProvi return; } - var deviceIDShort = metadata.deviceID.substr(0, 5); - $httpProvider.defaults.xsrfHeaderName = 'X-CSRF-Token-' + deviceIDShort; - $httpProvider.defaults.xsrfCookieName = 'CSRF-Token-' + deviceIDShort; + $httpProvider.defaults.xsrfHeaderName = 'X-CSRF-Token-' + metadata.deviceIDShort; + $httpProvider.defaults.xsrfCookieName = 'CSRF-Token-' + metadata.deviceIDShort; }); // @TODO: extract global level functions into separate service(s) diff --git a/lib/api/api.go b/lib/api/api.go index 1f2fd3a55..065b634d4 100644 --- a/lib/api/api.go +++ b/lib/api/api.go @@ -365,15 +365,15 @@ func (s *service) Serve(ctx context.Context) error { // Wrap everything in CSRF protection. The /rest prefix should be // protected, other requests will grant cookies. - var handler http.Handler = newCsrfManager(s.id.String()[:5], "/rest", guiCfg, mux, locations.Get(locations.CsrfTokens)) + var handler http.Handler = newCsrfManager(s.id.Short().String(), "/rest", guiCfg, mux, locations.Get(locations.CsrfTokens)) // Add our version and ID as a header to responses handler = withDetailsMiddleware(s.id, handler) // Wrap everything in basic auth, if user/password is set. if guiCfg.IsAuthEnabled() { - sessionCookieName := "sessionid-" + s.id.String()[:5] - handler = basicAuthAndSessionMiddleware(sessionCookieName, guiCfg, s.cfg.LDAP(), handler, s.evLogger) + sessionCookieName := "sessionid-" + s.id.Short().String() + handler = basicAuthAndSessionMiddleware(sessionCookieName, s.id.Short().String(), guiCfg, s.cfg.LDAP(), handler, s.evLogger) handlePasswordAuth := passwordAuthHandler(sessionCookieName, guiCfg, s.cfg.LDAP(), s.evLogger) restMux.Handler(http.MethodPost, "/rest/noauth/auth/password", handlePasswordAuth) @@ -719,6 +719,7 @@ func (*service) getSystemPaths(w http.ResponseWriter, _ *http.Request) { func (s *service) getJSMetadata(w http.ResponseWriter, _ *http.Request) { meta, _ := json.Marshal(map[string]interface{}{ "deviceID": s.id.String(), + "deviceIDShort": s.id.Short().String(), "authenticated": true, }) w.Header().Set("Content-Type", "application/javascript") diff --git a/lib/api/api_auth.go b/lib/api/api_auth.go index 7f9610068..7af4faacb 100644 --- a/lib/api/api_auth.go +++ b/lib/api/api_auth.go @@ -42,8 +42,8 @@ func antiBruteForceSleep() { time.Sleep(time.Duration(rand.Intn(100)+100) * time.Millisecond) } -func unauthorized(w http.ResponseWriter) { - w.Header().Set("WWW-Authenticate", "Basic realm=\"Authorization Required\"") +func unauthorized(w http.ResponseWriter, shortID string) { + w.Header().Set("WWW-Authenticate", fmt.Sprintf(`Basic realm="Authorization Required (%s)"`, shortID)) http.Error(w, "Not Authorized", http.StatusUnauthorized) } @@ -78,7 +78,7 @@ func isNoAuthPath(path string) bool { }) } -func basicAuthAndSessionMiddleware(cookieName string, guiCfg config.GUIConfiguration, ldapCfg config.LDAPConfiguration, next http.Handler, evLogger events.Logger) http.Handler { +func basicAuthAndSessionMiddleware(cookieName, shortID string, guiCfg config.GUIConfiguration, ldapCfg config.LDAPConfiguration, next http.Handler, evLogger events.Logger) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if hasValidAPIKeyHeader(r, guiCfg) { next.ServeHTTP(w, r) @@ -117,7 +117,7 @@ func basicAuthAndSessionMiddleware(cookieName string, guiCfg config.GUIConfigura // Some browsers don't send the Authorization request header unless prompted by a 401 response. // This enables https://user:pass@localhost style URLs to keep working. if guiCfg.SendBasicAuthPrompt { - unauthorized(w) + unauthorized(w, shortID) return } diff --git a/lib/protocol/deviceid.go b/lib/protocol/deviceid.go index 5effb8e38..fee2f6d65 100644 --- a/lib/protocol/deviceid.go +++ b/lib/protocol/deviceid.go @@ -13,10 +13,15 @@ import ( "github.com/syncthing/syncthing/lib/sha256" ) -const DeviceIDLength = 32 +const ( + DeviceIDLength = 32 + ShortIDStringLength = 7 +) -type DeviceID [DeviceIDLength]byte -type ShortID uint64 +type ( + DeviceID [DeviceIDLength]byte + ShortID uint64 +) var ( LocalDeviceID = repeatedDeviceID(0xff) @@ -94,7 +99,7 @@ func (s ShortID) String() string { } var bs [8]byte binary.BigEndian.PutUint64(bs[:], uint64(s)) - return base32.StdEncoding.EncodeToString(bs[:])[:7] + return base32.StdEncoding.EncodeToString(bs[:])[:ShortIDStringLength] } func (n *DeviceID) UnmarshalText(bs []byte) error { diff --git a/next-gen-gui/src/app/api-utils.ts b/next-gen-gui/src/app/api-utils.ts index 50d0a874d..13d4137c7 100644 --- a/next-gen-gui/src/app/api-utils.ts +++ b/next-gen-gui/src/app/api-utils.ts @@ -1,8 +1,7 @@ import { environment } from '../environments/environment' export const deviceID = (): String => { - const dID: String = environment.production ? globalThis.metadata['deviceID'] : '12345'; - return dID.substring(0, 5) + return environment.production ? globalThis.metadata['deviceIDShort'] : '1234567'; } export const apiURL: String = '/' diff --git a/test/http_test.go b/test/http_test.go index 4541c75b4..3dd9cf751 100644 --- a/test/http_test.go +++ b/test/http_test.go @@ -173,7 +173,7 @@ func TestHTTPPOSTWithoutCSRF(t *testing.T) { } res.Body.Close() hdr := res.Header.Get("Set-Cookie") - id := res.Header.Get("X-Syncthing-ID")[:5] + id := res.Header.Get("X-Syncthing-ID")[:protocol.ShortIDStringLength] if !strings.Contains(hdr, "CSRF-Token") { t.Error("Missing CSRF-Token in", hdr) }