From 46bd507fd8756ea8ca943ea8dac33c637d7c5c56 Mon Sep 17 00:00:00 2001 From: m3ta-chiron Date: Sat, 24 Oct 2026 09:35:45 +0200 Subject: [PATCH] =?UTF-8?q?fix(api):=20Origin-Guard=20=E2=80=94=20Browser?= =?UTF-8?q?=20erreichen=20nur=20POST=20/speak=20von=20gelisteten=20Origins?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CORS allein verhindert nur das Lesen der Antwort, nicht die Wirkung: Ein POST mit text/plain-Body oder ein Formular-POST geht ohne Preflight raus, und talkd hat den Body trotzdem als JSON gelesen bzw. /mute, /cancel und /mcp ohne Body ausgeführt. Jede Webseite konnte so sprechen, muten und abbrechen. Neue Middleware vor dem Router: Anfragen mit Origin-Header kommen nur als OPTIONS /speak (Preflight, ohne Wirkung) oder POST /speak von gelisteter Origin durch; alles andere ist 403, bevor ein Body gelesen wird. CLI, curl und MCP-Clients senden keinen Origin und sind nicht betroffen. Prozesstests angepasst (ungelistete Origin auf /speak → 403, Steuer- Endpunkte aus dem Browser → 403) und TestBrowserKannNichtSteuern ergänzt, der den Angriff mit simple requests durchspielt und prüft, dass weder Provider noch Mute-State berührt werden. Co-Authored-By: Claude Fable 5.1 --- README.md | 13 +++++- internal/api/cors.go | 39 ++++++++++++++++-- internal/api/server.go | 6 ++- test/cors_test.go | 94 ++++++++++++++++++++++++++++++++++++------ 4 files changed, 132 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index aac9e9b..fd97291 100644 --- a/README.md +++ b/README.md @@ -126,8 +126,17 @@ allowed_origins = ["https://chat.az-gruppe.com"] # Env TALK_ALLOWED_ORIGINS `Access-Control-Allow-Origin` (die Origin bzw. `*`), `-Methods: POST, OPTIONS`, `-Headers: Content-Type`, `Max-Age: 600`. - Echter `POST /speak` mit gelisteter `Origin` trägt denselben ACAO-Header. -- Ungelistete Origins bekommen saubere `204`/`202` **ohne** CORS-Header — - die Blockade macht der Browser. +- **Die Tür ist serverseitig zu, nicht nur browserseitig.** CORS allein + verhindert nur das *Lesen* der Antwort; einen POST mit `text/plain`-Body + oder einen Formular-POST schickt jede Webseite ohne Preflight ab. Deshalb + lehnt talkd jede Anfrage mit fremdem `Origin`-Header mit `403` ab, die + nicht POST /speak von einer gelisteten Origin ist — also ungelistete + Origins auf /speak, jeden Browser-Zugriff auf `/status`, `/mute`, + `/unmute`, `/cancel` und `/mcp`, auch von gelisteten Origins. Nur der + Preflight `OPTIONS /speak` bleibt für ungelistete Origins ein `204` ohne + CORS-Header (ohne Wirkung; der Browser verwirft den geplanten POST). + CLI, curl und MCP-Clients senden keinen `Origin`-Header und sind nicht + betroffen. **MCP** (`POST /mcp`, Streamable HTTP): schlanke eigene Implementierung des JSON-RPC-Subsets ohne SDK — `initialize` (angefragte diff --git a/internal/api/cors.go b/internal/api/cors.go index ecbf297..bdd7f18 100644 --- a/internal/api/cors.go +++ b/internal/api/cors.go @@ -2,6 +2,7 @@ package api import ( "net/http" + "strings" ) // CORS-Tür für POST /speak (Task 06) — und NUR für diesen Endpunkt: Ein @@ -13,9 +14,16 @@ import ( // ausdrücklich. Bewusster Default: LEER — dann sendet der Daemon gar keine // CORS-Header und der Browser blockt (Tür zu, bis sie konfiguriert wird). // -// Ungelistete Origins bekommen bewusst trotzdem 204/202 ohne CORS-Header: -// HTTP-seitig ist die Anfrage sauber, die Blockade macht der Browser (so -// verrät der Daemon nichts über seine Erlaubnisliste). +// Die Tür ist serverseitig zu, nicht nur browserseitig (originGuard): CORS +// allein verhindert nur das LESEN der Antwort, nicht die Wirkung — ein +// Browser schickt einen POST mit text/plain-Body oder ein Formular ohne +// Preflight ab, und ein Server, der den Body dennoch als JSON liest, hätte +// gesprochen, gemutet oder abgebrochen. Deshalb lehnt talkd jede Anfrage +// mit fremdem Origin-Header ab (403), die nicht der erlaubte Weg über +// /speak ist. Nur der Preflight OPTIONS /speak bleibt für ungelistete +// Origins ein 204 ohne CORS-Header (er hat keine Wirkung; der Browser +// verwirft den geplanten POST). CLI, curl und andere Nicht-Browser-Clients +// senden keinen Origin-Header und sind nicht betroffen. // CORS-Antwort-Header (Namen zentral, damit Preflight und echter POST // dieselben setzen). @@ -76,3 +84,28 @@ func (h *handler) speakPreflight(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusNoContent) } + +// originGuard ist die Middleware vor dem Router: Anfragen mit Origin-Header +// (also aus einem Browser) kommen nur als OPTIONS /speak (Preflight, ohne +// Wirkung) oder als POST /speak von gelisteter Origin durch. Alles andere — +// ungelistete Origin auf /speak, jeder Browser-Zugriff auf Steuer-Endpunkte +// und /mcp — wird mit 403 abgelehnt, bevor ein Body gelesen oder eine +// Wirkung ausgelöst wird. Ohne Origin-Header (CLI, curl, MCP-Clients) +// passiert nichts. +func (h *handler) originGuard(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + if origin == "" { + next.ServeHTTP(w, r) + return + } + isSpeak := strings.TrimRight(r.URL.Path, "/") == "/speak" + if isSpeak && (r.Method == http.MethodOptions || (r.Method == http.MethodPost && h.corsACAO(r) != "")) { + next.ServeHTTP(w, r) + return + } + w.Header().Add(headerVary, "Origin") + writeError(w, http.StatusForbidden, + "Origin "+origin+" ist nicht erlaubt (Browser-Zugriff nur auf POST /speak von gelisteten Origins in server.allowed_origins)") + }) +} diff --git a/internal/api/server.go b/internal/api/server.go index 3a92c86..2fc744c 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -41,7 +41,9 @@ type HandlerConfig struct { } // NewHandler liefert die HTTP-Routen des Daemons. Methoden-Bindung per -// ServeMux-Muster: falsche Methode → 405, unbekannter Pfad → 404. +// ServeMux-Muster: falsche Methode → 405, unbekannter Pfad → 404. Vor dem +// Router steht der originGuard (cors.go): Browser-Anfragen (Origin-Header) +// erreichen nur POST /speak von gelisteten Origins; alles andere ist 403. func NewHandler(cfg HandlerConfig) http.Handler { h := &handler{ queue: cfg.Queue, @@ -63,7 +65,7 @@ func NewHandler(cfg HandlerConfig) http.Handler { mux.HandleFunc("POST /unmute", h.muteEndpoint(false)) mux.HandleFunc("POST /mcp", h.serveMCP) // MCP Streamable HTTP (Task 06) mux.HandleFunc("GET /mcp", h.mcpGet) // 405: keine server-initiierten Streams - return mux + return h.originGuard(mux) } // handler ist der interne Zustand der HTTP-Oberfläche. diff --git a/test/cors_test.go b/test/cors_test.go index eb756bd..ad8ae94 100644 --- a/test/cors_test.go +++ b/test/cors_test.go @@ -4,11 +4,15 @@ package test // echter POST auf /speak mit Headern aus der Config-Liste // TALK_ALLOWED_ORIGINS — beobachtet als Black Box über rohe HTTP-Requests. // Bindend: exakte Origins; "*" nur explizit; leere Liste = gar keine -// CORS-Header (der Browser blockt); CORS nur auf /speak. +// CORS-Header; CORS nur auf /speak. Die Tür ist SERVERSEITIG zu: Jede +// Anfrage mit fremdem Origin-Header außerhalb des erlaubten Wegs (OPTIONS +// /speak, POST /speak von gelisteter Origin) wird mit 403 abgelehnt, bevor +// sie Wirkung hat — CORS allein verhindert nur das Lesen der Antwort. import ( "net/http" "testing" + "time" ) const azOrigin = "https://chat.az-gruppe.com" @@ -72,16 +76,20 @@ func TestCORSPostGelisted(t *testing.T) { } fake.waitCompleted(t, 1) + // Ungelistete Origin: 403 OHNE Wirkung — der Fake-Provider darf keinen + // zweiten Treffer sehen. (Früher 202 ohne CORS-Header: der Browser + // konnte die Antwort nicht lesen, gesprochen wurde trotzdem.) raw = doRequest(t, http.MethodPost, d.baseURL+"/speak", map[string]string{ "Origin": "https://evil.example", "Content-Type": "application/json", }, `{"text":"böser browser"}`) - if raw.status != http.StatusAccepted { - t.Fatalf("POST /speak (ungelisted) Status = %d, want 202 (HTTP bleibt sauber)", raw.status) + if raw.status != http.StatusForbidden { + t.Fatalf("POST /speak (ungelisted) Status = %d, want 403", raw.status) } if got := raw.header.Get("Access-Control-Allow-Origin"); got != "" { t.Errorf("ACAO für ungeliste Origin = %q, want leer", got) } + fake.assertNoFurtherArrivals(t, 300*time.Millisecond) } // TestCORSLeereListe: Ohne allowed_origins (Default) sendet der Daemon @@ -109,12 +117,19 @@ func TestCORSLeereListe(t *testing.T) { "Origin": azOrigin, "Content-Type": "application/json", }, `{"text":"ohne cors"}`) + if raw.status != http.StatusForbidden { + t.Fatalf("POST /speak mit Origin bei leerer Liste: Status = %d, want 403", raw.status) + } + fake.assertNoHits(t, 300*time.Millisecond) + + // Ohne Origin-Header (CLI, curl) bleibt derselbe POST erlaubt. + raw = doRequest(t, http.MethodPost, d.baseURL+"/speak", map[string]string{ + "Content-Type": "application/json", + }, `{"text":"ohne origin"}`) if raw.status != http.StatusAccepted { - t.Fatalf("POST /speak Status = %d, want 202", raw.status) - } - if got := raw.header.Get("Access-Control-Allow-Origin"); got != "" { - t.Errorf("ACAO auf POST bei leerer Liste = %q, want leer", got) + t.Fatalf("POST /speak ohne Origin: Status = %d, want 202", raw.status) } + fake.waitCompleted(t, 1) } // TestCORSWildcard: Der Eintrag "*" erlaubt jede Origin ausdrücklich — @@ -137,8 +152,9 @@ func TestCORSWildcard(t *testing.T) { } } -// TestCORSNurAufSpeak: Kein anderer Endpunkt trägt CORS-Header — OPTIONS -// auf /status ist 405 (kein Preflight-Support), GET /status ohne ACAO. +// TestCORSNurAufSpeak: Kein anderer Endpunkt ist aus dem Browser erreichbar +// — auch nicht von einer gelisteten Origin: OPTIONS und GET auf /status mit +// Origin-Header sind 403 ohne CORS-Header. func TestCORSNurAufSpeak(t *testing.T) { root := t.TempDir() fake := newFakeProvider(t, 0) @@ -150,8 +166,8 @@ func TestCORSNurAufSpeak(t *testing.T) { "Origin": azOrigin, "Access-Control-Request-Method": http.MethodGet, }, "") - if raw.status != http.StatusMethodNotAllowed { - t.Fatalf("OPTIONS /status Status = %d, want 405 (kein CORS-Support)", raw.status) + if raw.status != http.StatusForbidden { + t.Fatalf("OPTIONS /status Status = %d, want 403 (kein Browser-Zugriff)", raw.status) } if got := raw.header.Get("Access-Control-Allow-Origin"); got != "" { t.Errorf("OPTIONS /status ACAO = %q, want leer", got) @@ -160,14 +176,66 @@ func TestCORSNurAufSpeak(t *testing.T) { raw = doRequest(t, http.MethodGet, d.baseURL+"/status", map[string]string{ "Origin": azOrigin, }, "") - if raw.status != http.StatusOK { - t.Fatalf("GET /status Status = %d, want 200", raw.status) + if raw.status != http.StatusForbidden { + t.Fatalf("GET /status mit Origin: Status = %d, want 403", raw.status) } if got := raw.header.Get("Access-Control-Allow-Origin"); got != "" { t.Errorf("GET /status ACAO = %q, want leer (CORS nur auf /speak)", got) } } +// TestBrowserKannNichtSteuern: Der eigentliche Angriffsfall. Eine beliebige +// Webseite kann ohne Preflight „simple requests" absetzen (POST mit +// text/plain-Body, Formular-POST ohne Body) — der Browser blockt nur das +// Lesen der Antwort, nicht das Senden. talkd darf darauf weder sprechen noch +// muten noch abbrechen noch das MCP-Tool ausführen: alles 403, Zustand +// unverändert. +func TestBrowserKannNichtSteuern(t *testing.T) { + root := t.TempDir() + fake := newFakeProvider(t, 0) + d := startDaemon(t, root, fake.srv.URL, map[string]string{ + "TALK_ALLOWED_ORIGINS": azOrigin, // Tür für EINE Origin offen — andere trotzdem zu + }) + evil := "https://evil.example" + + // Simple request: POST mit text/plain-Body, kein Preflight. + raw := doRequest(t, http.MethodPost, d.baseURL+"/speak", map[string]string{ + "Origin": evil, + "Content-Type": "text/plain", + }, `{"text":"gekapert","now":true}`) + if raw.status != http.StatusForbidden { + t.Fatalf("simple POST /speak von %s: Status = %d, want 403", evil, raw.status) + } + + // Steuer-Endpunkte brauchen keinen Body — ein Formular-POST reicht. + for _, path := range []string{"/mute", "/unmute", "/cancel"} { + raw = doRequest(t, http.MethodPost, d.baseURL+path, map[string]string{"Origin": evil}, "") + if raw.status != http.StatusForbidden { + t.Errorf("POST %s von %s: Status = %d, want 403", path, evil, raw.status) + } + } + // Auch die gelistete Origin darf NICHT steuern — nur sprechen. + raw = doRequest(t, http.MethodPost, d.baseURL+"/mute", map[string]string{"Origin": azOrigin}, "") + if raw.status != http.StatusForbidden { + t.Errorf("POST /mute von gelisteter Origin: Status = %d, want 403 (CORS nur auf /speak)", raw.status) + } + + // MCP aus dem Browser: ebenfalls zu. + raw = doRequest(t, http.MethodPost, d.baseURL+"/mcp", map[string]string{ + "Origin": evil, + "Content-Type": "text/plain", + }, `{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"speak","arguments":{"text":"gekapert"}}}`) + if raw.status != http.StatusForbidden { + t.Errorf("POST /mcp von %s: Status = %d, want 403", evil, raw.status) + } + + // Nichts davon hatte Wirkung: kein Treffer beim Provider, nicht gemutet. + fake.assertNoHits(t, 300*time.Millisecond) + if st := getStatus(t, d); st.Muted { + t.Errorf("Daemon ist gemutet — Browser-POST /mute hat gewirkt") + } +} + // TestStatusCorsFeld: GET /status trägt die CORS-Origins als cors-Feld — // gelistet als [], wenn die Tür zu ist (Compat-Anforderung Task 06). func TestStatusCorsFeld(t *testing.T) {