web: avoid username enumeration on login timing

When the submitted username did not exist, the handler returned without
running the expensive password hash, so a missing account answered
noticeably faster than a wrong password. That timing difference lets an
attacker enumerate valid usernames.

Always verify the password, comparing against a fixed dummy hash when
the account is absent, so both cases take about the same time. Compute
that dummy hash lazily on the first login: it runs 870k PBKDF2
iterations, and every pw subcommand links in this package, so computing
it at init would needlessly slow down unrelated commands.

Signed-off-by: Robin Jarry <robin@jarry.cc>

Author: rjarry

pkg/web/session.go

@@ -12,6 +12,7 @@ import (
 	"encoding/hex"
 	"net/http"
 	"strings"
+	"sync"
 
 	"github.com/getpatchwork/patchwork/pkg/db"
 )
@@ -20,6 +21,16 @@ type contextKey int
 
 const webUserKey contextKey = iota
 
+// dummyPasswordHash is compared against when the submitted username has
+// no account, so the login handler spends roughly the same time as it
+// would verifying a real password. This keeps failed logins from
+// leaking whether a username exists. It is computed lazily on the first
+// login so the expensive hash does not slow down unrelated commands that
+// merely link in this package.
+var dummyPasswordHash = sync.OnceValue(func() string {
+	return db.HashPassword("patchwork-login-timing-guard")
+})
+
 func getWebUser(r *http.Request) *db.User {
 	u, _ := r.Context().Value(webUserKey).(*db.User)
 	return u
@@ -82,7 +93,17 @@ func (h *webHandler) LoginSubmit(w http.ResponseWriter, r *http.Request) {
 
 	q := db.GetQueries(r.Context())
 	user, err := q.GetUserByUsername(username)
-	if err != nil || !db.CheckPassword(password, user.Password) {
+	var hash string
+	if user != nil {
+		hash = user.Password
+	} else {
+		// Always run a password check, even for an unknown user, so
+		// that a missing account takes about as long as a wrong
+		// password and cannot be distinguished by timing (username
+		// enumeration).
+		hash = dummyPasswordHash()
+	}
+	if !db.CheckPassword(password, hash) || err != nil {
 		w.WriteHeader(http.StatusForbidden)
 		loginPage(pc, "Invalid username or password.", next).Render(r.Context(), w)
 		return