v0.324.0: the database password comes back with the data (R-1106, R-127b, R-926); restic 0.19.1 (R-1107); R-1108, R-1065, R-1017; dashboard hardening (R-983, R-976)

controller/internal/web/auth.go

@@ -2,6 +2,7 @@ package web
 
 import (
 	"crypto/rand"
+	"crypto/subtle"
 	"encoding/hex"
 	"fmt"
 	"net/http"
@@ -164,31 +165,19 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	s.loginAttemptMu.Lock()
-	attempt := s.loginAttempts[ip]
-	if attempt != nil && time.Since(attempt.lastFail) > loginWindowDuration {
-		// Window expired — reset
-		attempt = nil
-		delete(s.loginAttempts, ip)
-	}
-	if attempt != nil && attempt.count >= loginMaxAttempts {
-		s.loginAttemptMu.Unlock()
-		s.logger.Printf("[WARN] [web] Login rate limited for %s (%d attempts)", ip, attempt.count)
+	// R-983 (v0.324.0): the try is RESERVED before the password is compared, under the one lock — check and count
+	// are a single step. Before, the count was taken only after the slow compare, so N parallel tries all passed the
+	// check and the 5-per-minute limit became "as many as arrive at once". A right password clears the counter below,
+	// so the household loses nothing by its own try being counted first. Pinned by TestR983_ParallelLoginsHitTheLimit.
+	if n, ok := s.reserveLoginAttempt(ip); !ok {
+		s.logger.Printf("[WARN] [web] Login rate limited for %s (%d attempts)", ip, n)
 		s.renderLogin(w, r, s.msg(r, "login.msg.rate_limited"), "")
 		return
 	}
-	s.loginAttemptMu.Unlock()
 
 	effectiveHash := s.effectivePasswordHash()
-	if err := bcrypt.CompareHashAndPassword([]byte(effectiveHash), []byte(password)); err != nil {
+	if err := s.comparePassword([]byte(effectiveHash), []byte(password)); err != nil {
 		s.logger.Printf("[WARN] [web] Failed login from %s", ip)
-		s.loginAttemptMu.Lock()
-		if s.loginAttempts[ip] == nil {
-			s.loginAttempts[ip] = &loginAttempt{}
-		}
-		s.loginAttempts[ip].count++
-		s.loginAttempts[ip].lastFail = time.Now()
-		s.loginAttemptMu.Unlock()
 		s.renderLogin(w, r, s.msg(r, "login.msg.wrong_password"), "")
 		return
 	}
@@ -221,14 +210,50 @@ func (s *Server) handleLogin(w http.ResponseWriter, r *http.Request) {
 
 	s.logger.Printf("[INFO] [web] Login from %s", ip)
 
-	// Redirect to ?next= target if provided, otherwise to dashboard
-	redirectTo := "/"
-	if nextURL != "" && strings.HasPrefix(nextURL, "/") {
-		redirectTo = nextURL
-	}
-	http.Redirect(w, r, redirectTo, http.StatusFound)
+	// Redirect to ?next= target if provided, otherwise to dashboard. R-983 (v0.324.0): through safeBackPath, the one
+	// guard for a destination taken from a request — a bare HasPrefix("/") let "//host" and "/\host" send a browser
+	// that had just signed in to another site. Pinned by TestR983_LoginNextStaysOnTheBox.
+	http.Redirect(w, r, safeBackPath(nextURL), http.StatusFound)
 }
 
+// reserveLoginAttempt counts one sign-in try for this visitor BEFORE the password is compared and reports whether it
+// may go ahead (and the count it found). Check and count happen under one lock, so parallel tries cannot all pass a
+// check that none of them has counted yet. A window that has passed starts clean.
+func (s *Server) reserveLoginAttempt(ip string) (int, bool) {
+	s.loginAttemptMu.Lock()
+	defer s.loginAttemptMu.Unlock()
+	if s.loginAttempts == nil {
+		s.loginAttempts = map[string]*loginAttempt{}
+	}
+	a := s.loginAttempts[ip]
+	if a != nil && time.Since(a.lastFail) > loginWindowDuration {
+		a = nil
+	}
+	if a == nil {
+		a = &loginAttempt{}
+		s.loginAttempts[ip] = a
+	}
+	if a.count >= loginMaxAttempts {
+		return a.count, false
+	}
+	a.count++
+	a.lastFail = time.Now()
+	return a.count, true
+}
+
+// comparePassword is bcrypt's compare; passwordCompare replaces it in tests (a slow compare makes a race measurable).
+func (s *Server) comparePassword(hash, pw []byte) error {
+	if s.passwordCompare != nil {
+		return s.passwordCompare(hash, pw)
+	}
+	return bcrypt.CompareHashAndPassword(hash, pw)
+}
+
+// handleLogout ends the presented dashboard session. It runs inside RequireAuth, BEFORE CsrfProtect, so it checks the
+// session's CSRF token itself (R-983, v0.324.0): without it any page — another site, or an app on a sibling
+// subdomain, which is same-site and so gets the Lax cookie — could sign the household out. A GET changes nothing and
+// goes to the dashboard; the dashboard's own sign-out control is a POST form carrying the token (layout.html).
+// A POST with no valid session has nothing to end and only clears the cookie. Pinned by TestR983_LogoutNeedsCSRF.
 func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodPost {
 		http.Redirect(w, r, "/", http.StatusFound)
@@ -238,6 +263,16 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
 		s.logger.Printf("[DEBUG] [web] logout from %s", r.RemoteAddr)
 	}
 	if cookie, err := r.Cookie(sessionCookieName); err == nil {
+		if expected := s.csrfTokenForSession(cookie.Value); expected != "" {
+			submitted := r.FormValue(csrfFormField)
+			if submitted == "" {
+				submitted = r.Header.Get(csrfHeaderName)
+			}
+			if submitted == "" || subtle.ConstantTimeCompare([]byte(submitted), []byte(expected)) != 1 {
+				s.csrfReject(w, r, "logout without the session's token")
+				return
+			}
+		}
 		s.sessionsMu.Lock()
 		delete(s.sessions, sessionFingerprint(cookie.Value))
 		s.saveSessionsRevokingLocked() // R-35: a signed-out cookie stays signed out across a restart
@@ -304,6 +339,23 @@ func (s *Server) invalidateAllSessions() {
 	s.logger.Printf("[INFO] [web] All sessions invalidated (cleared %d)", count)
 }
 
+// endEverySession is what a dashboard password change and a claim/reset do (R-976, v0.324.0): every dashboard session
+// ends (invalidateAllSessions), every family session — each member's and the household's — ends, and the gate key is
+// replaced, so every setup-gate pass and every family app cookie stops opening its app at the next request. Before,
+// only the dashboard sessions ended, so a session copied out of a family app kept working for up to 30 days after the
+// household had changed its password. Family members keep their own passwords and sign in again; the household's
+// browser passes again through its new dashboard session. Pinned by TestR976_PasswordChangeEndsFamilyAndGateSessions
+// and TestR976_ClaimResetEndsFamilyAndGateSessions.
+func (s *Server) endEverySession(why string) {
+	s.invalidateAllSessions()
+	n, err := s.familyStore().EndAll()
+	if err != nil {
+		s.logger.Printf("[ERROR] [web] %s: the family sessions were ended but the family list could not be saved: %v", why, err)
+	}
+	s.rotateGateKey()
+	s.logger.Printf("[INFO] [web] %s: every dashboard session, %d family session(s) and every app gate pass ended", why, n)
+}
+
 func (s *Server) cleanupSessions() {
 	ticker := time.NewTicker(15 * time.Minute)
 	defer ticker.Stop()