Add security hardening

2026-09-04 14:54:07 +03:00
1 parent 4669bf10c0
commit e592930728
@@ -0,0 +1,74 @@
+package httpapi
+
+import (
+	"net/http"
+	"net/url"
+	"strings"
+)
+
+// Browser-driven cross-site attacks are the realistic threat here. The
+// dashboard is reached with a browser session, so any page the operator
+// happens to visit can make their browser issue requests to the pool
+// with whatever credentials that session carries. Two shapes matter:
+//
+//   - Cross-Site WebSocket Hijacking. The WebSocket handshake is not
+//     subject to the same-origin policy, so without a check any site
+//     could open /api/ws and read the whole snapshot: payout addresses,
+//     per-worker hashrates, block history.
+//   - CSRF against the state-changing POSTs. The admin endpoints take no
+//     body, so a plain <form> submission reaches them with no preflight
+//     to stop it, which is enough to zero the latency counters or kick
+//     off a rescan of a multi-million-line log.
+//
+// Both are closed by checking Origin, which browsers attach to every
+// WebSocket handshake and to cross-site POSTs, and which page script
+// cannot forge.
+
+// originAllowed reports whether a request may proceed. A request with no
+// Origin header is not browser-driven (curl, the StartOS actions, a
+// health probe) and is allowed: an attacker able to make arbitrary
+// direct requests already has network access and does not need the
+// operator's browser to carry them.
+func originAllowed(r *http.Request) bool {
+	origin := r.Header.Get("Origin")
+	if origin == "" {
+		return true
+	}
+	u, err := url.Parse(origin)
+	if err != nil || u.Host == "" {
+		return false
+	}
+	return strings.EqualFold(u.Host, r.Host)
+}
+
+// sameOrigin wraps a handler so cross-origin browser requests are
+// refused. Applied to every state-changing route and to the WebSocket.
+func (s *Server) sameOrigin(h http.HandlerFunc) http.HandlerFunc {
+	return func(w http.ResponseWriter, r *http.Request) {
+		if !originAllowed(r) {
+			s.Log.Warn("refused cross-origin request",
+				"path", r.URL.Path, "origin", r.Header.Get("Origin"), "host", r.Host)
+			writeJSON(w, http.StatusForbidden, map[string]any{
+				"error": "cross-origin request refused",
+			})
+			return
+		}
+		h(w, r)
+	}
+}
+
+// securityHeaders sets the response headers that cost nothing and close
+// off framing and MIME-sniffing. No CSP: the dashboard is a bundled
+// Svelte app with inline styles, and a policy loose enough to run it
+// would not be worth the claim of having one.
+func securityHeaders(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		head := w.Header()
+		head.Set("X-Content-Type-Options", "nosniff")
+		// The dashboard has nothing to gain from being framed, and
+		// framing it invites clickjacking of the admin controls.
+		head.Set("X-Frame-Options", "DENY")
+		head.Set("Referrer-Policy", "no-referrer")
+		h.ServeHTTP(w, r)
+	})
+}
@@ -0,0 +1,107 @@
+package httpapi
+
+import (
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestOriginAllowed(t *testing.T) {
+	tests := []struct {
+		name   string
+		host   string
+		origin string
+		want   bool
+	}{
+		// No Origin: not browser-driven, so not a cross-site vector.
+		{"no origin header", "pool.local:8080", "", true},
+		{"same origin", "pool.local:8080", "http://pool.local:8080", true},
+		{"same origin over https", "pool.local", "https://pool.local", true},
+		{"case-insensitive host", "Pool.Local:8080", "http://pool.local:8080", true},
+
+		// The attacks this closes.
+		{"attacker site", "pool.local:8080", "https://evil.example", false},
+		{"different port on same name", "pool.local:8080", "http://pool.local:9999", false},
+		{"subdomain of the host", "pool.local:8080", "http://x.pool.local:8080", false},
+		{"suffix trick", "pool.local:8080", "http://notpool.local:8080", false},
+		{"null origin from a sandboxed frame", "pool.local:8080", "null", false},
+		{"malformed origin", "pool.local:8080", "::::", false},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			r := httptest.NewRequest("POST", "http://"+tc.host+"/api/admin/reset-latency", nil)
+			r.Host = tc.host
+			if tc.origin != "" {
+				r.Header.Set("Origin", tc.origin)
+			}
+			if got := originAllowed(r); got != tc.want {
+				t.Errorf("originAllowed(host=%q origin=%q) = %v, want %v",
+					tc.host, tc.origin, got, tc.want)
+			}
+		})
+	}
+}
+
+// A cross-origin POST must be refused before the handler runs, so a page
+// the operator happens to be visiting cannot drive the admin routes with
+// their session.
+func TestSameOriginRefusesCrossSite(t *testing.T) {
+	s := &Server{Log: discardLogger()}
+	called := false
+	h := s.sameOrigin(func(w http.ResponseWriter, r *http.Request) {
+		called = true
+		w.WriteHeader(http.StatusOK)
+	})
+
+	r := httptest.NewRequest("POST", "http://pool.local:8080/api/admin/reset-latency", nil)
+	r.Host = "pool.local:8080"
+	r.Header.Set("Origin", "https://evil.example")
+	w := httptest.NewRecorder()
+	h(w, r)
+
+	if called {
+		t.Error("handler ran for a cross-origin request")
+	}
+	if w.Code != http.StatusForbidden {
+		t.Errorf("status = %d, want 403", w.Code)
+	}
+}
+
+func TestSameOriginAllowsSameSite(t *testing.T) {
+	s := &Server{Log: discardLogger()}
+	called := false
+	h := s.sameOrigin(func(w http.ResponseWriter, r *http.Request) { called = true })
+
+	r := httptest.NewRequest("POST", "http://pool.local:8080/api/admin/ack-best", nil)
+	r.Host = "pool.local:8080"
+	r.Header.Set("Origin", "http://pool.local:8080")
+	h(httptest.NewRecorder(), r)
+
+	if !called {
+		t.Error("handler did not run for a same-origin request")
+	}
+}
+
+func TestSecurityHeaders(t *testing.T) {
+	h := securityHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest("GET", "/", nil))
+
+	for k, want := range map[string]string{
+		"X-Content-Type-Options": "nosniff",
+		"X-Frame-Options":        "DENY",
+		"Referrer-Policy":        "no-referrer",
+	} {
+		if got := w.Header().Get(k); got != want {
+			t.Errorf("%s = %q, want %q", k, got, want)
+		}
+	}
+}
+
+func discardLogger() *slog.Logger {
+	return slog.New(slog.NewTextHandler(io.Discard, nil))
+}
@@ -38,18 +38,20 @@ func (s *Server) Handler() http.Handler {
 	mux.HandleFunc("GET /api/clients", s.clients)
 	mux.HandleFunc("GET /api/blocks", s.blocks)
 	mux.HandleFunc("GET /api/snapshot", s.snapshot)
-	mux.HandleFunc("GET /api/ws", s.handleWS)
+	// The WebSocket handshake is exempt from the same-origin policy, so
+	// it is origin-checked like the mutating routes below.
+	mux.HandleFunc("GET /api/ws", s.sameOrigin(s.handleWS))
 	mux.HandleFunc("GET /api/admin/debug-blocks", s.debugBlocks)
-	mux.HandleFunc("POST /api/admin/reset-latency", s.resetLatency)
-	mux.HandleFunc("POST /api/admin/ack-best", s.ackBest)
-	mux.HandleFunc("POST /api/admin/reset-ack-best", s.resetAckBest)
-	mux.HandleFunc("POST /api/admin/rebuild-share-stats", s.rebuildShareStats)
-	mux.HandleFunc("POST /api/accelerate", s.accelerate)
-	mux.HandleFunc("POST /api/accelerate/cancel", s.accelerateCancel)
-	mux.HandleFunc("POST /api/accelerate/max", s.accelerateMax)
+	mux.HandleFunc("POST /api/admin/reset-latency", s.sameOrigin(s.resetLatency))
+	mux.HandleFunc("POST /api/admin/ack-best", s.sameOrigin(s.ackBest))
+	mux.HandleFunc("POST /api/admin/reset-ack-best", s.sameOrigin(s.resetAckBest))
+	mux.HandleFunc("POST /api/admin/rebuild-share-stats", s.sameOrigin(s.rebuildShareStats))
+	mux.HandleFunc("POST /api/accelerate", s.sameOrigin(s.accelerate))
+	mux.HandleFunc("POST /api/accelerate/cancel", s.sameOrigin(s.accelerateCancel))
+	mux.HandleFunc("POST /api/accelerate/max", s.sameOrigin(s.accelerateMax))
 	mux.HandleFunc("GET /api/accelerate/list", s.accelerateList)
 	mux.Handle("/", spaHandler(webui.FS()))
-	return mux
+	return securityHeaders(mux)
 }

 // spaHandler serves static files from the embedded dist tree and
@@ -11,6 +11,7 @@ import (
 	"log/slog"
 	"strconv"
 	"sync"
+	"sync/atomic"
 	"time"

 	"github.com/kamadopool/kamado-api/internal/bitcoind"
@@ -368,6 +369,11 @@ type Aggregator struct {
 	// reconstructible header. Never mutated after assignment, so the
 	// pointer is safe to hand out in snapshots.
 	bestPow *BestSharePow
+	// Set while a log rescan is running. The scan walks every line of a
+	// log that can hold millions, so overlapping runs would multiply the
+	// I/O for no benefit and give a caller a cheap way to load the box.
+	rebuilding atomic.Bool
+
 	// PoW data lines arrive on the share channel just before their
 	// matching "Accepted client" line. Park them here keyed by header
 	// hash until the accepted event confirms the share, so data from a
@@ -1007,6 +1013,10 @@ func (a *Aggregator) RebuildShareStatsFromLog() (adopted bool, scanned int64, er
 	if a.LogFilePath == "" {
 		return false, 0, errors.New("no ckpool log path configured")
 	}
+	if !a.rebuilding.CompareAndSwap(false, true) {
+		return false, 0, errors.New("a share statistics rebuild is already running")
+	}
+	defer a.rebuilding.Store(false)
 	st, err := logmon.ScanShareStats(a.LogFilePath)
 	if err != nil {
 		return false, 0, err