mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 04:33:17 +00:00
Stop websocket upgrade headers exempting demo-mode writes (#2666)
`DemoModeMiddleware` let any request carrying `Upgrade: websocket`
through, whatever its method or path. On a `DEMO_MODE` instance a
`POST`/`PUT`/`PATCH`/`DELETE` with that header reached its handler, and
the read-only guard did nothing.
## How exposed the public demo was
- The public demo (`demo.pulserelay.pro`, v6.5.0) runs `DEMO_MODE=true`
with `PULSE_AUTH_USER=demo`, and its login page signs visitors in as
demo/demo. No proxy auth is configured. Caddy is a plain `reverse_proxy`
with no method filtering, and it forwards `Connection: Upgrade` and
`Upgrade: websocket` unchanged.
- That session is the configured admin.
`sessionUserHasSettingsAdminPrivileges` matches `cfg.AuthUser`, so
`RequireAdmin` accepts it, and session requests skip token scope checks.
Nothing in the handlers checks demo mode on its own, so this middleware
was the only write guard.
- Anyone using the public credentials could therefore make any admin
write by adding the two headers. That covers switching the demo out of
mock mode (`POST /api/system/mock-mode`), changing system settings,
creating organisations, importing config, changing the password and
adding notification targets. A CSRF token was needed, but the attacker
gets one from their own login.
- On the old code, the new contract test reproduces this end to end. A
demo/demo session's mock-mode POST with upgrade headers returned 200 and
turned mock mode off.
- I found no sign it was used. Over the last 30 days the demo journal
shows every "mock mode disabled" line paired with a process restart (new
PID). Config file mtimes match deploys. 5,782 ordinary writes were
blocked over the same period. Caddy keeps no access log for the demo, so
this is the absence of evidence, not proof.
## Change
The exemption is deleted rather than narrowed. RFC 6455 handshakes are
`GET` requests, gorilla's `Upgrader` refuses any other method, and `GET`
already passes the read-only branch above it. A narrowed `GET`-only
exemption would never run. `/ws` and `/api/agent/ws` keep connecting,
and the request method alone decides.
## Tests
- `TestContract_DemoModeUpgradeHeadersDoNotExemptWrites` (registered
api-contracts proof) runs against the full router. It signs in as the
configured demo admin and checks that the session passes `RequireAdmin`
for a read. A mock-mode POST carrying `Connection: Upgrade` and
`Upgrade: websocket` now gets the demo 403 and leaves mock mode on, and
a real `/ws` handshake from the same session still upgrades.
- `demo_middleware_test.go`: the table row that asserted the bypass now
expects 403. POST/PUT/PATCH/DELETE with handshake headers to mock-mode,
settings, orgs, `/ws` and `/api/agent/ws` all get 403. Real gorilla
handshakes to `/ws` and `/api/agent/ws` through the middleware still
upgrade and echo.
- Negative control: run against the old middleware via `go test
-overlay`, the new contract test and the upgrade-header test fail, and
the handshake test passes on both versions.
## Not changed here
- `auth.go` `isWebSocketUpgrade` and `configapi`
`explicitAPITokenFromRequest` still accept a `?token=` query credential
on any request with `Upgrade: websocket`. A valid, scoped token is still
required, so no access is gained, but the credential can travel in a
URL. The `ErrorHandler` skip in `middleware.go` only bypasses recovery
and metrics.
- Older and separate:
`/api/ai/sessions/{id}/{abort,summarize,fork,undo,redo}` change state on
`GET`, which the demo guard admits as reads. Tracked separately.
This commit is contained in:
@@ -8910,3 +8910,12 @@ agent from its last report instead of from the PVE poll, and mark it stale at
|
||||
their own heartbeat cutoff (`fleethealth.AgentStaleThreshold`, which is
|
||||
separate from the monitoring reporting lease). Agent registration, enrolment,
|
||||
install, update, removal and report identity are unchanged.
|
||||
|
||||
### Demo write guard ignores websocket upgrade headers
|
||||
|
||||
`internal/api/demo_middleware.go` changed only so a `POST`, `PUT`, `PATCH` or
|
||||
`DELETE` carrying `Upgrade: websocket` is judged by the demo read-only guard
|
||||
like any other write instead of always reaching its handler. Agent command
|
||||
websocket handshakes to `/api/agent/ws` are HTTP/1.1 `GET` requests, the only
|
||||
transport its gorilla upgrader serves, and still connect on a demo instance.
|
||||
No agent registration, enrolment, install, update or removal path changed.
|
||||
|
||||
@@ -7651,6 +7651,17 @@ read-only demo account. That same hidden read-side boundary includes `GET` and
|
||||
`HEAD` reads for `/api/admin/users` and manual discovery at `/api/discover`;
|
||||
public demo mode may block writes generically, but it must not reveal that
|
||||
admin-user inventory or manual-discovery read routes exist.
|
||||
That generic write block keys on the request method alone. The demo
|
||||
middleware admits `GET`, `HEAD` and `OPTIONS` plus the login, logout, AI
|
||||
execute and OIDC login/callback allowlist; HTTP/1.1 websocket handshakes for
|
||||
`/ws` and `/api/agent/ws` are `GET` requests and connect through that read
|
||||
branch, so an `Upgrade: websocket` or `Connection: Upgrade` header never
|
||||
carries a `POST`, `PUT`, `PATCH` or `DELETE` past the guard. A demo deployment that signs
|
||||
visitors in as the configured admin, as the public demo does, has only this
|
||||
guard between that session and admin writes such as `/api/system/mock-mode`.
|
||||
`TestContract_DemoModeUpgradeHeadersDoNotExemptWrites` in
|
||||
`internal/api/contract_test.go` signs in as that admin through the full router
|
||||
and pins both halves.
|
||||
That shared infrastructure install boundary now also preserves copied shell
|
||||
command payload continuity: any privilege-escalation wrapper applied at
|
||||
`frontend-modern/src/components/Settings/InfrastructureInstallerSection.tsx`
|
||||
|
||||
@@ -6895,3 +6895,14 @@ migration or recovery authority is added or moved.
|
||||
The performance report and reporting runtime snapshot handlers apply the same
|
||||
test before tabulating a disk temperature. That changes only which held value a
|
||||
report shows, not any storage or recovery path.
|
||||
|
||||
### Demo write guard ignores websocket upgrade headers
|
||||
|
||||
`internal/api/demo_middleware.go` no longer exempts every request carrying
|
||||
`Upgrade: websocket` from the demo read-only guard. On a `DEMO_MODE` instance a
|
||||
`POST`, `PUT`, `PATCH` or `DELETE` with websocket handshake headers is now
|
||||
judged like any other write: outside the login, logout, AI execute and OIDC
|
||||
allowlist it gets the generic demo `403`, or `404` on a hidden route, and that
|
||||
includes config import at `/api/config/import`. HTTP/1.1 websocket handshakes
|
||||
are `GET` requests and still connect. No storage, retention, backup, migration
|
||||
or recovery path is added or moved.
|
||||
|
||||
@@ -60,6 +60,7 @@ import (
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/updates"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/vmware"
|
||||
pulsews "github.com/rcourtman/pulse-go-rewrite/internal/websocket"
|
||||
agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host"
|
||||
"github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts"
|
||||
"github.com/rcourtman/pulse-go-rewrite/pkg/audit"
|
||||
@@ -9261,6 +9262,119 @@ func TestContract_DemoModeCommercialSurfacePolicy(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// The public demo signs every visitor in as the configured admin, so admin
|
||||
// routes accept a demo session and the demo guard is the only thing between it
|
||||
// and a write. Websocket handshake headers must not carry a write past that
|
||||
// guard, while a genuine handshake from the same session must still connect.
|
||||
func TestContract_DemoModeUpgradeHeadersDoNotExemptWrites(t *testing.T) {
|
||||
setMockModeForTest(t, true)
|
||||
|
||||
dataDir := t.TempDir()
|
||||
hashedPass, err := authpkg.HashPassword("demo")
|
||||
if err != nil {
|
||||
t.Fatalf("hash password: %v", err)
|
||||
}
|
||||
cfg := &config.Config{
|
||||
DataPath: dataDir,
|
||||
ConfigPath: dataDir,
|
||||
DemoMode: true,
|
||||
AuthUser: "demo",
|
||||
AuthPass: hashedPass,
|
||||
}
|
||||
|
||||
monitor, err := monitoring.New(cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("new monitor: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { monitor.Stop() })
|
||||
if err := monitor.SetMockMode(true); err != nil {
|
||||
t.Fatalf("set monitor mock mode: %v", err)
|
||||
}
|
||||
|
||||
hub := pulsews.NewHub(nil)
|
||||
go hub.Run()
|
||||
t.Cleanup(hub.Stop)
|
||||
router := NewRouter(cfg, monitor, nil, hub, nil, "1.0.0")
|
||||
cleanupTestRouter(t, router)
|
||||
server := newIPv4HTTPServer(t, router.Handler())
|
||||
t.Cleanup(server.Close)
|
||||
|
||||
loginResp, err := http.Post(server.URL+"/api/login", "application/json", strings.NewReader(`{"username":"demo","password":"demo"}`))
|
||||
if err != nil {
|
||||
t.Fatalf("demo login: %v", err)
|
||||
}
|
||||
loginResp.Body.Close()
|
||||
if loginResp.StatusCode != http.StatusOK {
|
||||
t.Fatalf("demo login status = %d, want %d", loginResp.StatusCode, http.StatusOK)
|
||||
}
|
||||
var cookiePairs []string
|
||||
csrfToken := ""
|
||||
for _, cookie := range loginResp.Cookies() {
|
||||
cookiePairs = append(cookiePairs, cookie.Name+"="+cookie.Value)
|
||||
if cookie.Name == CookieNameCSRF {
|
||||
csrfToken = cookie.Value
|
||||
}
|
||||
}
|
||||
if csrfToken == "" {
|
||||
t.Fatal("expected CSRF cookie after demo login")
|
||||
}
|
||||
sessionCookies := strings.Join(cookiePairs, "; ")
|
||||
|
||||
send := func(method, path, body string, headers map[string]string) (int, string) {
|
||||
t.Helper()
|
||||
req, err := http.NewRequest(method, server.URL+path, strings.NewReader(body))
|
||||
if err != nil {
|
||||
t.Fatalf("build %s %s: %v", method, path, err)
|
||||
}
|
||||
req.Header.Set("Cookie", sessionCookies)
|
||||
for key, value := range headers {
|
||||
req.Header.Set(key, value)
|
||||
}
|
||||
resp, err := http.DefaultClient.Do(req)
|
||||
if err != nil {
|
||||
t.Fatalf("%s %s: %v", method, path, err)
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
var payload bytes.Buffer
|
||||
_, _ = payload.ReadFrom(resp.Body)
|
||||
return resp.StatusCode, payload.String()
|
||||
}
|
||||
|
||||
if status, body := send(http.MethodGet, "/api/system/mock-mode", "", nil); status != http.StatusOK {
|
||||
t.Fatalf("demo session read of admin route status = %d, want %d: %s", status, http.StatusOK, body)
|
||||
}
|
||||
|
||||
status, body := send(http.MethodPost, "/api/system/mock-mode", `{"enabled":false}`, map[string]string{
|
||||
"Content-Type": "application/json",
|
||||
"X-CSRF-Token": csrfToken,
|
||||
"Connection": "Upgrade",
|
||||
"Upgrade": "websocket",
|
||||
"Sec-WebSocket-Version": "13",
|
||||
"Sec-WebSocket-Key": "dGhlIHNhbXBsZSBub25jZQ==",
|
||||
})
|
||||
if status != http.StatusForbidden || !strings.Contains(body, "Demo mode enabled") {
|
||||
t.Fatalf("demo write with websocket upgrade headers status = %d, want %d demo block: %s", status, http.StatusForbidden, body)
|
||||
}
|
||||
if !mock.IsMockEnabled() {
|
||||
t.Fatal("demo write with websocket upgrade headers switched mock mode off")
|
||||
}
|
||||
|
||||
wsHeaders := wsHeadersForHTTP(t, server.URL)
|
||||
wsHeaders.Set("Cookie", sessionCookies)
|
||||
conn, resp, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL)+"/ws", wsHeaders)
|
||||
if err != nil {
|
||||
status := 0
|
||||
if resp != nil {
|
||||
status = resp.StatusCode
|
||||
}
|
||||
t.Fatalf("demo session websocket handshake failed: %v (status %d)", err, status)
|
||||
}
|
||||
defer conn.Close()
|
||||
if resp.StatusCode != http.StatusSwitchingProtocols {
|
||||
t.Fatalf("demo session websocket handshake status = %d, want %d", resp.StatusCode, http.StatusSwitchingProtocols)
|
||||
}
|
||||
}
|
||||
|
||||
func TestContract_ReleaseDemoFixtureRuntimeGuardrailsRemainCanonical(t *testing.T) {
|
||||
requiredSnippets := map[string][]string{
|
||||
"licensing_handlers.go": {
|
||||
|
||||
@@ -33,18 +33,15 @@ func DemoModeMiddleware(cfg *config.Config, next http.Handler) http.Handler {
|
||||
}
|
||||
}
|
||||
|
||||
// Allow GET and HEAD requests (read-only)
|
||||
// Allow GET and HEAD requests (read-only). WebSocket handshakes are GET
|
||||
// requests (RFC 6455 section 4.1), so /ws and /api/agent/ws connect
|
||||
// through this branch. An Upgrade header never exempts another method:
|
||||
// a POST carrying "Upgrade: websocket" is still a write.
|
||||
if r.Method == http.MethodGet || r.Method == http.MethodHead || r.Method == http.MethodOptions {
|
||||
next.ServeHTTP(w, r)
|
||||
return
|
||||
}
|
||||
|
||||
// Allow WebSocket upgrades
|
||||
if strings.ToLower(r.Header.Get("Upgrade")) == "websocket" {
|
||||
next.ServeHTTP(w, r)
|
||||
return
|
||||
}
|
||||
|
||||
// Allow authentication endpoints (login is read-only - verifies credentials)
|
||||
authPaths := []string{
|
||||
"/api/login",
|
||||
|
||||
@@ -4,8 +4,11 @@ import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/websocket"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/config"
|
||||
)
|
||||
|
||||
@@ -44,8 +47,8 @@ func TestDemoModeMiddleware(t *testing.T) {
|
||||
{"demo on websocket GET", true, http.MethodGet, "/api/ws", "websocket", true, http.StatusOK, true},
|
||||
{"demo on websocket case insensitive", true, http.MethodGet, "/api/ws", "WebSocket", true, http.StatusOK, true},
|
||||
{"demo on websocket uppercase", true, http.MethodGet, "/api/ws", "WEBSOCKET", true, http.StatusOK, true},
|
||||
// WebSocket upgrade with POST method (tests websocket branch after GET/HEAD/OPTIONS check)
|
||||
{"demo on websocket POST", true, http.MethodPost, "/api/ws", "websocket", true, http.StatusOK, true},
|
||||
// An Upgrade header does not turn a write into a websocket handshake
|
||||
{"demo on websocket POST", true, http.MethodPost, "/api/ws", "websocket", false, http.StatusForbidden, true},
|
||||
|
||||
// Demo mode enabled - auth endpoints allowed (POST)
|
||||
{"demo on login", true, http.MethodPost, "/api/login", "", true, http.StatusOK, true},
|
||||
@@ -172,3 +175,79 @@ func TestDemoModeMiddleware_BlockedResponse(t *testing.T) {
|
||||
t.Error("message should not be empty")
|
||||
}
|
||||
}
|
||||
|
||||
// A write that carries websocket handshake headers is still a write. The demo
|
||||
// guard used to wave through any request with "Upgrade: websocket", so a demo
|
||||
// session could switch mock mode off or change settings by adding two headers.
|
||||
func TestDemoModeMiddlewareUpgradeHeadersDoNotExemptWrites(t *testing.T) {
|
||||
handler := DemoModeMiddleware(&config.Config{DemoMode: true}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
t.Errorf("write with websocket upgrade headers reached the handler: %s %s", r.Method, r.URL.Path)
|
||||
}))
|
||||
|
||||
methods := []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete}
|
||||
paths := []string{"/api/system/mock-mode", "/api/system/settings/update", "/api/orgs", "/ws", "/api/agent/ws"}
|
||||
for _, method := range methods {
|
||||
for _, path := range paths {
|
||||
req := httptest.NewRequest(method, path, strings.NewReader(`{"enabled":false}`))
|
||||
req.Header.Set("Connection", "Upgrade")
|
||||
req.Header.Set("Upgrade", "websocket")
|
||||
req.Header.Set("Sec-WebSocket-Version", "13")
|
||||
req.Header.Set("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==")
|
||||
rec := httptest.NewRecorder()
|
||||
handler.ServeHTTP(rec, req)
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Errorf("%s %s with websocket upgrade headers: status = %d, want %d", method, path, rec.Code, http.StatusForbidden)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestDemoModeMiddlewareAllowsWebSocketHandshake(t *testing.T) {
|
||||
upgrader := websocket.Upgrader{CheckOrigin: func(*http.Request) bool { return true }}
|
||||
echo := func(w http.ResponseWriter, r *http.Request) {
|
||||
conn, err := upgrader.Upgrade(w, r, nil)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
defer conn.Close()
|
||||
messageType, payload, err := conn.ReadMessage()
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
_ = conn.WriteMessage(messageType, payload)
|
||||
}
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("/ws", echo)
|
||||
mux.HandleFunc("/api/agent/ws", echo)
|
||||
|
||||
server := newIPv4HTTPServer(t, DemoModeMiddleware(&config.Config{DemoMode: true}, mux))
|
||||
defer server.Close()
|
||||
|
||||
for _, path := range []string{"/ws", "/api/agent/ws"} {
|
||||
conn, resp, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL)+path, nil)
|
||||
if err != nil {
|
||||
status := 0
|
||||
if resp != nil {
|
||||
status = resp.StatusCode
|
||||
}
|
||||
t.Fatalf("websocket handshake to %s failed in demo mode: %v (status %d)", path, err, status)
|
||||
}
|
||||
if resp.StatusCode != http.StatusSwitchingProtocols {
|
||||
conn.Close()
|
||||
t.Fatalf("websocket handshake to %s: status = %d, want %d", path, resp.StatusCode, http.StatusSwitchingProtocols)
|
||||
}
|
||||
if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil {
|
||||
conn.Close()
|
||||
t.Fatalf("set read deadline for %s: %v", path, err)
|
||||
}
|
||||
if err := conn.WriteMessage(websocket.TextMessage, []byte("ping")); err != nil {
|
||||
conn.Close()
|
||||
t.Fatalf("write to %s: %v", path, err)
|
||||
}
|
||||
_, payload, err := conn.ReadMessage()
|
||||
conn.Close()
|
||||
if err != nil || string(payload) != "ping" {
|
||||
t.Fatalf("echo from %s = %q, %v; want %q", path, payload, err, "ping")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user