Security(Go): Login rate limit + WS origin allowlist + REST bearer auth

server/go/wsauth/ratelimit.go

@@ -0,0 +1,110 @@
+package wsauth
+
+import (
+	"sync"
+	"time"
+)
+
+// RateLimiter is a sliding-window per-key counter used to throttle login
+// attempts. Two instances are typically created: one keyed by client IP
+// (to slow distributed brute force), one keyed by username (to slow
+// targeted attacks against a known account).
+//
+// Design notes:
+//   - Denied attempts are NOT recorded — the window slides naturally and a
+//     legitimate user who fat-fingers their password recovers as soon as
+//     the oldest attempt ages out, while a determined attacker is capped
+//     at `limit` successful attempts per `window` indefinitely.
+//   - Lazy cleanup: stale timestamps for a key are pruned on every Allow()
+//     call. Truly idle keys are GC'd by Sweep(), which callers should run
+//     periodically from a background goroutine.
+//   - Map size is bounded by the count of recently-active keys; for the
+//     web UI's expected load (a handful of users + occasional scanners),
+//     no extra GC pressure considerations needed.
+type RateLimiter struct {
+	mu      sync.Mutex
+	limit   int
+	window  time.Duration
+	entries map[string][]time.Time
+}
+
+// NewRateLimiter returns a limiter that allows up to `limit` events per
+// `window` duration per key. Zero or negative limit/window disables the
+// limiter (Allow always returns true) — useful for tests / dev mode.
+func NewRateLimiter(limit int, window time.Duration) *RateLimiter {
+	return &RateLimiter{
+		limit:   limit,
+		window:  window,
+		entries: make(map[string][]time.Time),
+	}
+}
+
+// Allow records an attempt for `key` if and only if the caller is under
+// the per-key limit. Returns true when allowed, false when over limit.
+// Empty key is treated as "no throttle" (returns true without recording)
+// so the caller can fall through when the IP/username is unavailable.
+func (r *RateLimiter) Allow(key string) bool {
+	if r == nil || r.limit <= 0 || r.window <= 0 || key == "" {
+		return true
+	}
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	cutoff := time.Now().Add(-r.window)
+	times := r.entries[key]
+	// Compact in place — keep only timestamps within the window.
+	keep := times[:0]
+	for _, t := range times {
+		if t.After(cutoff) {
+			keep = append(keep, t)
+		}
+	}
+
+	if len(keep) >= r.limit {
+		// Update the map even when denying so the compacted slice doesn't
+		// keep stale entries forever. Don't append the new attempt: that
+		// would let attackers extend the window arbitrarily.
+		r.entries[key] = keep
+		return false
+	}
+
+	r.entries[key] = append(keep, time.Now())
+	return true
+}
+
+// Reset clears state for a key. Call on successful login to give the user
+// a fresh budget — otherwise a string of failed attempts followed by a
+// correct one still leaves the budget partially consumed.
+func (r *RateLimiter) Reset(key string) {
+	if r == nil || key == "" {
+		return
+	}
+	r.mu.Lock()
+	delete(r.entries, key)
+	r.mu.Unlock()
+}
+
+// Sweep removes entries whose timestamps have all aged out of the window.
+// Safe to call concurrently with Allow. Intended for periodic invocation
+// from a background ticker (e.g. every window-length) to bound the map.
+func (r *RateLimiter) Sweep() {
+	if r == nil {
+		return
+	}
+	r.mu.Lock()
+	defer r.mu.Unlock()
+	cutoff := time.Now().Add(-r.window)
+	for key, times := range r.entries {
+		keep := times[:0]
+		for _, t := range times {
+			if t.After(cutoff) {
+				keep = append(keep, t)
+			}
+		}
+		if len(keep) == 0 {
+			delete(r.entries, key)
+		} else {
+			r.entries[key] = keep
+		}
+	}
+}

server/go/wsauth/wsauth.go

@@ -95,13 +95,26 @@ func (a *Authenticator) AddUser(u User) {
 	a.mu.Unlock()
 }
 
-// AddAdminFromPlainPassword is a convenience for the bootstrap admin: salt is
-// empty (matching the C++ admin record), hash is SHA256(password).
+// AddAdminFromPlainPassword is a convenience for the bootstrap admin.
+// Unlike legacy convention, the admin record is given a real per-instance
+// salt — exposing an empty salt for admin while everyone else has a real
+// 16-hex one would let an unauthenticated probe distinguish admin from
+// other accounts via /get_salt alone. The cost is a tiny break in
+// users.json schema compat: admin is never persisted to users.json
+// anyway (snapshotPersistableLocked excludes it), so this is in-memory
+// only.
 func (a *Authenticator) AddAdminFromPlainPassword(username, plainPassword string) {
+	salt, err := NewSalt()
+	if err != nil {
+		// Fall back to deterministic salt derived from the password hash
+		// rather than empty — preserves the uniform-shape property even
+		// if crypto/rand briefly errors at startup.
+		salt = ComputeSHA256(plainPassword)[:saltBytes*2]
+	}
 	a.AddUser(User{
 		Username:     username,
-		PasswordHash: ComputeSHA256(plainPassword),
-		Salt:         "",
+		PasswordHash: HashPassword(plainPassword, salt),
+		Salt:         salt,
 		Role:         "admin",
 	})
 }
@@ -200,17 +213,43 @@ func (a *Authenticator) ListUsers() []User {
 	return out
 }
 
-// GetSalt returns the per-user salt. If the user does not exist, returns ("", false).
-// Note: the C++ admin uses an empty salt — that is still considered "found"
-// and the empty string is returned with ok=true.
+// GetSalt returns the per-user salt for an existing user, or a
+// deterministic 16-hex pseudo-salt for an unknown user. The ok flag
+// reports which case occurred, so callers can decide whether to update
+// rate-limit / audit state — but the returned salt itself is shaped
+// identically (16 hex chars) in both cases, defeating the user-existence
+// probe an attacker would otherwise mount via /get_salt.
+//
+// The pseudo-salt is derived from a server-instance secret (the admin
+// password hash, taken at first call) mixed with the username, so the
+// same unknown user always sees the same fake salt across requests.
+// Without this, an attacker could fingerprint the "fake-salt branch"
+// by submitting the same username twice and watching for differences.
 func (a *Authenticator) GetSalt(username string) (string, bool) {
 	a.mu.RLock()
 	u, ok := a.users[username]
 	a.mu.RUnlock()
-	if !ok {
-		return "", false
+	if ok {
+		return u.Salt, true
 	}
-	return u.Salt, true
+	return a.fakeSalt(username), false
+}
+
+// fakeSalt derives a deterministic 16-hex value for unknown usernames.
+// The secret pepper is the bootstrap admin's password hash — present as
+// long as the server has any admin, deterministic per deployment, never
+// transmitted. Reveals nothing useful to an attacker even if reverse-
+// engineered: the only thing they can do with it is reproduce the fake
+// salt, which they already see in the response.
+func (a *Authenticator) fakeSalt(username string) string {
+	a.mu.RLock()
+	pepper := ""
+	if admin, ok := a.users["admin"]; ok {
+		pepper = admin.PasswordHash
+	}
+	a.mu.RUnlock()
+	digest := ComputeSHA256("yama-fake-salt|" + pepper + "|" + username)
+	return digest[:saltBytes*2]
 }
 
 // VerifyLogin checks a challenge-response login. The browser sends

server/go/wsauth/wsauth_test.go

@@ -14,19 +14,31 @@ func TestSHA256Vector(t *testing.T) {
 	}
 }
 
-func TestLoginRoundTripAdminEmptySalt(t *testing.T) {
+// adminLoginResponse helps tests compute the right login response for an
+// admin account that now uses a real per-instance salt.
+func adminLoginResponse(t *testing.T, a *Authenticator, username, password, nonce string) string {
+	t.Helper()
+	salt, ok := a.GetSalt(username)
+	if !ok {
+		t.Fatalf("admin %s not registered", username)
+	}
+	return ComputeSHA256(HashPassword(password, salt) + nonce)
+}
+
+func TestLoginRoundTripAdmin(t *testing.T) {
 	a := New()
 	a.AddAdminFromPlainPassword("admin", "hunter2")
 
 	salt, ok := a.GetSalt("admin")
-	if !ok || salt != "" {
-		t.Fatalf("admin salt: ok=%v salt=%q", ok, salt)
+	if !ok {
+		t.Fatal("admin should be found")
+	}
+	if len(salt) != 2*saltBytes {
+		t.Fatalf("admin salt should be a real 16-hex value, got %q (len=%d)", salt, len(salt))
 	}
 
-	// Simulate the browser: nonce = "abc123", response = SHA256(passwordHash + nonce)
 	nonce := "abc123"
-	passwordHash := ComputeSHA256("hunter2")
-	response := ComputeSHA256(passwordHash + nonce)
+	response := adminLoginResponse(t, a, "admin", "hunter2", nonce)
 
 	token, role, err := a.VerifyLogin("admin", response, nonce)
 	if err != nil {
@@ -71,6 +83,33 @@ func TestLoginRoundTripViewerWithSalt(t *testing.T) {
 	}
 }
 
+// TestGetSaltUnknownUserShape verifies the salt-probe mitigation: an
+// unknown user must get back a value that's shape-identical to a real
+// salt, so an attacker can't tell from /get_salt alone whether a
+// username exists.
+func TestGetSaltUnknownUserShape(t *testing.T) {
+	a := New()
+	a.AddAdminFromPlainPassword("admin", "pw")
+
+	fake, ok := a.GetSalt("nobody")
+	if ok {
+		t.Fatal("ok should be false for unknown user")
+	}
+	if len(fake) != 2*saltBytes {
+		t.Fatalf("fake salt should be %d hex chars; got %q (len=%d)", 2*saltBytes, fake, len(fake))
+	}
+	// Determinism: repeated probes for the same username get the same fake.
+	fake2, _ := a.GetSalt("nobody")
+	if fake != fake2 {
+		t.Fatalf("fake salt should be deterministic for repeated probes; got %q vs %q", fake, fake2)
+	}
+	// Different usernames get different fake salts.
+	other, _ := a.GetSalt("ghost")
+	if fake == other {
+		t.Fatalf("fake salts should differ across usernames; both = %q", fake)
+	}
+}
+
 func TestLoginRejectsWrongResponse(t *testing.T) {
 	a := New()
 	a.AddAdminFromPlainPassword("admin", "x")
@@ -89,7 +128,7 @@ func TestTokenExpiry(t *testing.T) {
 	a.SetTokenExpire(50 * time.Millisecond)
 	a.AddAdminFromPlainPassword("admin", "x")
 	nonce, _ := NewNonce()
-	response := ComputeSHA256(ComputeSHA256("x") + nonce)
+	response := adminLoginResponse(t, a, "admin", "x", nonce)
 	token, _, err := a.VerifyLogin("admin", response, nonce)
 	if err != nil {
 		t.Fatal(err)
@@ -107,10 +146,57 @@ func TestRevoke(t *testing.T) {
 	a := New()
 	a.AddAdminFromPlainPassword("admin", "x")
 	nonce, _ := NewNonce()
-	response := ComputeSHA256(ComputeSHA256("x") + nonce)
+	response := adminLoginResponse(t, a, "admin", "x", nonce)
 	token, _, _ := a.VerifyLogin("admin", response, nonce)
 	a.RevokeToken(token)
 	if _, err := a.ValidateToken(token); err == nil {
 		t.Fatal("revoked token should not validate")
 	}
 }
+
+func TestRateLimiterAllowsBurstThenBlocks(t *testing.T) {
+	r := NewRateLimiter(3, time.Minute)
+	for i := 0; i < 3; i++ {
+		if !r.Allow("ip-a") {
+			t.Fatalf("attempt %d should be allowed", i+1)
+		}
+	}
+	if r.Allow("ip-a") {
+		t.Fatal("4th attempt should be denied")
+	}
+	// Different key has independent budget.
+	if !r.Allow("ip-b") {
+		t.Fatal("different key should still be allowed")
+	}
+}
+
+func TestRateLimiterReset(t *testing.T) {
+	r := NewRateLimiter(2, time.Minute)
+	r.Allow("k")
+	r.Allow("k")
+	if r.Allow("k") {
+		t.Fatal("3rd should be denied")
+	}
+	r.Reset("k")
+	if !r.Allow("k") {
+		t.Fatal("after Reset, should be allowed again")
+	}
+}
+
+func TestRateLimiterDisabledWhenZeroLimit(t *testing.T) {
+	r := NewRateLimiter(0, time.Minute)
+	for i := 0; i < 100; i++ {
+		if !r.Allow("k") {
+			t.Fatalf("limit=0 should never deny, denied at i=%d", i)
+		}
+	}
+}
+
+func TestRateLimiterNilSafe(t *testing.T) {
+	var r *RateLimiter
+	if !r.Allow("anything") {
+		t.Fatal("nil limiter should allow")
+	}
+	r.Reset("anything")
+	r.Sweep()
+}