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...
This commit is contained in:
Jakob Borg 2023-11-14 11:57:39 +01:00 committed by GitHub
parent aaee0c126b
commit 439c6c5b7c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 21 additions and 17 deletions

View File

@ -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)

View File

@ -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")

View File

@ -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
}

View File

@ -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 {

View File

@ -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 = '/'

View File

@ -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)
}