diff --git a/api/internal/httpapi/security.go b/api/internal/httpapi/security.go new file mode 100644 index 0000000..d369082 --- /dev/null +++ b/api/internal/httpapi/security.go @@ -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
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) + }) +} diff --git a/api/internal/httpapi/security_test.go b/api/internal/httpapi/security_test.go new file mode 100644 index 0000000..829d98d --- /dev/null +++ b/api/internal/httpapi/security_test.go @@ -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)) +} diff --git a/api/internal/httpapi/server.go b/api/internal/httpapi/server.go index 42877c8..440618a 100644 --- a/api/internal/httpapi/server.go +++ b/api/internal/httpapi/server.go @@ -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 diff --git a/api/internal/state/aggregator.go b/api/internal/state/aggregator.go index 8598aa2..4ea0498 100644 --- a/api/internal/state/aggregator.go +++ b/api/internal/state/aggregator.go @@ -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