From e163933aa6393705572b741ecce01eef5c147275 Mon Sep 17 00:00:00 2001 From: Wilko Date: Sun, 18 Oct 2026 21:44:05 +0200 Subject: [PATCH 1/4] Harden HTTP endpoints against cross-site requests and XSS Any web page opened in the home network could send requests to the proxy: a plain deleted the key, and with an Owner key a simple form POST could unlock the car. - Wrap the router in http.CrossOriginProtection. Cross-site browser POSTs are rejected; evcc, curl and Home Assistant send neither Origin nor Sec-Fetch-Site and pass unchanged. - /gen_keys and /remove_keys accept POST only (dashboard forms updated). - Render dashboard and log viewer with html/template so messages carrying user input (e.g. the VIN) are escaped. - Log instead of exiting when writing a response fails. - Make MessageStack safe for concurrent handlers. Co-Authored-By: Claude Opus 5.5 --- html/dashboard.html | 6 +-- internal/api/handlers/html.go | 10 ++--- internal/api/handlers/html_test.go | 30 +++++++++++++ internal/api/handlers/logs.go | 2 +- internal/api/handlers/tesla.go | 2 +- internal/api/models/message_stack.go | 41 ++++++++++------- internal/api/routes/routes.go | 13 +++--- internal/api/routes/routes_test.go | 66 ++++++++++++++++++++++++++++ 8 files changed, 140 insertions(+), 30 deletions(-) create mode 100644 internal/api/handlers/html_test.go create mode 100644 internal/api/routes/routes_test.go diff --git a/html/dashboard.html b/html/dashboard.html index e1d991d..47895df 100644 --- a/html/dashboard.html +++ b/html/dashboard.html @@ -26,7 +26,7 @@

Key Management

{{if $key.Exists}} {{if $key.IsActive}} ● Active -
+
@@ -35,14 +35,14 @@

Key Management

-
+
{{end}} {{else}} Not generated -
+
diff --git a/internal/api/handlers/html.go b/internal/api/handlers/html.go index d5e3916..f51257a 100644 --- a/internal/api/handlers/html.go +++ b/internal/api/handlers/html.go @@ -2,10 +2,10 @@ package handlers import ( "fmt" + "html/template" "io" "io/fs" "net/http" - "text/template" "github.com/wimaha/TeslaBleHttpProxy/config" "github.com/wimaha/TeslaBleHttpProxy/internal/api/models" @@ -66,8 +66,8 @@ func ShowDashboard(html fs.FS) http.HandlerFunc { } func GenKeys(w http.ResponseWriter, r *http.Request) { - // Get role from query parameter - role := r.URL.Query().Get("role") + // Get role from the form body + role := r.FormValue("role") if role == "" { role = control.KeyRoleChargingManager // Default to charging_manager (recommended for security) } @@ -113,8 +113,8 @@ func GenKeys(w http.ResponseWriter, r *http.Request) { } func RemoveKeys(w http.ResponseWriter, r *http.Request) { - // Get role from query parameter - role := r.URL.Query().Get("role") + // Get role from the form body + role := r.FormValue("role") // Role is required (legacy keys are automatically migrated) if role == "" { diff --git a/internal/api/handlers/html_test.go b/internal/api/handlers/html_test.go new file mode 100644 index 0000000..ff0dffa --- /dev/null +++ b/internal/api/handlers/html_test.go @@ -0,0 +1,30 @@ +package handlers + +import ( + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/wimaha/TeslaBleHttpProxy/internal/api/models" +) + +// Messages can carry user input (e.g. the VIN from send_key), so the dashboard must escape them. +func TestDashboardEscapesMessages(t *testing.T) { + models.MainMessageStack.Push(models.Message{ + Title: "Error", + Message: ``, + Type: models.Error, + }) + + rec := httptest.NewRecorder() + ShowDashboard(os.DirFS("../../.."))(rec, httptest.NewRequest("GET", "/dashboard", nil)) + + body := rec.Body.String() + if strings.Contains(body, "") { + t.Fatalf("message rendered unescaped") + } + if !strings.Contains(body, "<script>alert(1)</script>") { + t.Fatalf("escaped message not found in dashboard") + } +} diff --git a/internal/api/handlers/logs.go b/internal/api/handlers/logs.go index af9f2a2..b907696 100644 --- a/internal/api/handlers/logs.go +++ b/internal/api/handlers/logs.go @@ -2,10 +2,10 @@ package handlers import ( "encoding/json" + "html/template" "io/fs" "net/http" "strconv" - "text/template" "time" "github.com/wimaha/TeslaBleHttpProxy/internal/logging" diff --git a/internal/api/handlers/tesla.go b/internal/api/handlers/tesla.go index 23b3fde..b1a3a6c 100644 --- a/internal/api/handlers/tesla.go +++ b/internal/api/handlers/tesla.go @@ -42,7 +42,7 @@ func commonDefer(w http.ResponseWriter, response *models.Response) { } w.WriteHeader(status) if err := json.NewEncoder(w).Encode(ret); err != nil { - logging.Fatal("failed to send response", "error", err) + logging.Error("failed to send response", "error", err) } logging.Debug("Response", "Command", response.Command, "Status", status, "Result", response.Result, "Reason", response.Reason) } diff --git a/internal/api/models/message_stack.go b/internal/api/models/message_stack.go index 20531c2..67052b6 100644 --- a/internal/api/models/message_stack.go +++ b/internal/api/models/message_stack.go @@ -1,5 +1,7 @@ package models +import "sync" + type MessageType int // Declare related constants for each weekday starting with index 1 @@ -25,41 +27,50 @@ type Message struct { Type MessageType } -type MessageStack []Message +// MessageStack is safe for concurrent use; HTTP handlers push and pop in parallel. +type MessageStack struct { + mu sync.Mutex + messages []Message +} // IsEmpty: check if stack is empty func (s *MessageStack) IsEmpty() bool { - return len(*s) == 0 + s.mu.Lock() + defer s.mu.Unlock() + return len(s.messages) == 0 } // Push a new value onto the stack func (s *MessageStack) Push(str Message) { - *s = append(*s, str) // Simply append the new value to the end of the stack + s.mu.Lock() + defer s.mu.Unlock() + s.messages = append(s.messages, str) // Simply append the new value to the end of the stack } // Prepend to Stack func (s *MessageStack) Prepend(str Message) { - *s = append([]Message{str}, *s...) + s.mu.Lock() + defer s.mu.Unlock() + s.messages = append([]Message{str}, s.messages...) } // Remove and return top element of stack. Return true if stack is empty. func (s *MessageStack) Pop() (Message, bool) { - if s.IsEmpty() { + s.mu.Lock() + defer s.mu.Unlock() + if len(s.messages) == 0 { return Message{}, true - } else { - index := 0 // Get the index of the top most element. - element := (*s)[index] // Index into the slice and obtain the element. - *s = (*s)[index+1:] // Remove it from the stack by slicing it off. - return element, false } + element := s.messages[0] + s.messages = s.messages[1:] + return element, false } func (s *MessageStack) PopAll() []Message { - var all []Message - for _, message := range *s { - all = append(all, message) - } - *s = (*s)[:0] + s.mu.Lock() + defer s.mu.Unlock() + all := s.messages + s.messages = nil return all } diff --git a/internal/api/routes/routes.go b/internal/api/routes/routes.go index 8e9a605..ed1e1b8 100644 --- a/internal/api/routes/routes.go +++ b/internal/api/routes/routes.go @@ -1,14 +1,17 @@ package routes import ( - "embed" + "io/fs" "net/http" "github.com/gorilla/mux" "github.com/wimaha/TeslaBleHttpProxy/internal/api/handlers" ) -func SetupRoutes(static embed.FS, html embed.FS) *mux.Router { +// SetupRoutes returns the router wrapped in cross-origin protection: state-changing requests a +// browser sends from another site are rejected, while clients that send neither Origin nor +// Sec-Fetch-Site (evcc, curl, Home Assistant) pass unchanged. +func SetupRoutes(static fs.FS, html fs.FS) http.Handler { router := mux.NewRouter() // Define the endpoints @@ -21,11 +24,11 @@ func SetupRoutes(static embed.FS, html embed.FS) *mux.Router { router.HandleFunc("/logs", handlers.ShowLogViewer(html)).Methods("GET") router.HandleFunc("/api/logs", handlers.GetLogs).Methods("GET") router.HandleFunc("/api/logs/stats", handlers.GetLogStats).Methods("GET") - router.HandleFunc("/gen_keys", handlers.GenKeys).Methods("GET") - router.HandleFunc("/remove_keys", handlers.RemoveKeys).Methods("GET") + router.HandleFunc("/gen_keys", handlers.GenKeys).Methods("POST") + router.HandleFunc("/remove_keys", handlers.RemoveKeys).Methods("POST") router.HandleFunc("/activate_key", handlers.ActivateKey).Methods("POST") router.HandleFunc("/send_key", handlers.SendKey).Methods("POST") router.PathPrefix("/static/").Handler(http.FileServer(http.FS(static))) - return router + return http.NewCrossOriginProtection().Handler(router) } diff --git a/internal/api/routes/routes_test.go b/internal/api/routes/routes_test.go new file mode 100644 index 0000000..f82e9bb --- /dev/null +++ b/internal/api/routes/routes_test.go @@ -0,0 +1,66 @@ +package routes + +import ( + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + "testing/fstest" +) + +func serve(t *testing.T, req *http.Request) int { + t.Helper() + rec := httptest.NewRecorder() + SetupRoutes(fstest.MapFS{}, fstest.MapFS{}).ServeHTTP(rec, req) + return rec.Code +} + +func commandRequest(headers map[string]string) *http.Request { + req := httptest.NewRequest("POST", "http://proxy:8080/api/1/vehicles/VIN/command/flash_lights", nil) + for k, v := range headers { + req.Header.Set(k, v) + } + return req +} + +func TestCrossOriginCommandIsRejected(t *testing.T) { + cases := map[string]map[string]string{ + "sec-fetch-site": {"Sec-Fetch-Site": "cross-site"}, + "origin": {"Origin": "http://evil.example"}, + } + for name, headers := range cases { + t.Run(name, func(t *testing.T) { + if code := serve(t, commandRequest(headers)); code != http.StatusForbidden { + t.Errorf("status = %d, want %d", code, http.StatusForbidden) + } + }) + } +} + +// evcc, curl and Home Assistant send neither Origin nor Sec-Fetch-Site and must keep working. +// Without a key the handler answers 503, which shows the request got past the protection. +func TestNonBrowserCommandPasses(t *testing.T) { + if code := serve(t, commandRequest(nil)); code != http.StatusServiceUnavailable { + t.Errorf("status = %d, want %d", code, http.StatusServiceUnavailable) + } +} + +func TestSameOriginDashboardFormPasses(t *testing.T) { + form := url.Values{"role": {"invalid"}}.Encode() + req := httptest.NewRequest("POST", "http://proxy:8080/activate_key", strings.NewReader(form)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Sec-Fetch-Site", "same-origin") + if code := serve(t, req); code != http.StatusSeeOther { + t.Errorf("status = %d, want %d", code, http.StatusSeeOther) + } +} + +// Key changes must not be reachable by GET, where a plain on any site could trigger them. +func TestKeyRoutesRejectGet(t *testing.T) { + for _, path := range []string{"/gen_keys?role=owner", "/remove_keys?role=owner"} { + if code := serve(t, httptest.NewRequest("GET", path, nil)); code != http.StatusMethodNotAllowed { + t.Errorf("GET %s: status = %d, want %d", path, code, http.StatusMethodNotAllowed) + } + } +} From ec8fe275138a4077a38e1c3a7b82b5b4bf70b8c8 Mon Sep 17 00:00:00 2001 From: Wilko Date: Sun, 18 Oct 2026 22:17:09 +0200 Subject: [PATCH 2/4] Test cross-site protection and escaping end to end against the simulated car Cross-site POSTs to door_unlock (Origin or Sec-Fetch-Site) are rejected before reaching the car, while clients without those headers still unlock it. GET /gen_keys and /remove_keys are refused, and form input from send_key renders escaped on the dashboard. tesla_test.go follows the new SetupRoutes(fs.FS, fs.FS) http.Handler signature. Co-Authored-By: Claude Opus 5.5 --- internal/api/handlers/security_test.go | 118 +++++++++++++++++++++++++ internal/api/handlers/tesla_test.go | 5 +- 2 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 internal/api/handlers/security_test.go diff --git a/internal/api/handlers/security_test.go b/internal/api/handlers/security_test.go new file mode 100644 index 0000000..c707ff4 --- /dev/null +++ b/internal/api/handlers/security_test.go @@ -0,0 +1,118 @@ +package handlers_test + +import ( + "io" + "net/http" + "net/url" + "strings" + "testing" + + "github.com/wimaha/TeslaBleHttpProxy/internal/ble/fakecar" +) + +// raw sends a request through the proxy and returns status and body without decoding JSON, +// because the cross-origin protection answers with plain text. +func (p *proxy) raw(method, path, body string, headers map[string]string) (int, string) { + p.t.Helper() + var reader io.Reader + if body != "" { + reader = strings.NewReader(body) + } + req, err := http.NewRequest(method, p.server.URL+path, reader) + if err != nil { + p.t.Fatal(err) + } + for k, v := range headers { + req.Header.Set(k, v) + } + client := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} + res, err := client.Do(req) + if err != nil { + p.t.Fatal(err) + } + defer res.Body.Close() + b, err := io.ReadAll(res.Body) + if err != nil { + p.t.Fatal(err) + } + return res.StatusCode, string(b) +} + +// A page on another site that submits a form to the proxy must not unlock the car. +func TestHTTPCrossSiteUnlockNeverReachesCar(t *testing.T) { + cases := map[string]map[string]string{ + "sec-fetch-site": {"Sec-Fetch-Site": "cross-site"}, + "origin": {"Origin": "http://evil.example"}, + } + for name, headers := range cases { + t.Run(name, func(t *testing.T) { + vin := newVin() + p := startProxy(t, vin) + + code, _ := p.raw("POST", "/api/1/vehicles/"+vin+"/command/door_unlock?wait=true", "", headers) + + if code != http.StatusForbidden { + t.Errorf("status = %d, want %d", code, http.StatusForbidden) + } + if calls := p.car.Calls(); len(calls) != 0 { + t.Errorf("car was contacted: %v", p.car.Methods()) + } + p.car.Do(func(c *fakecar.Car) { + if !c.Locked { + t.Error("car is unlocked") + } + }) + }) + } +} + +// evcc, curl and Home Assistant send neither Origin nor Sec-Fetch-Site and keep working. +func TestHTTPNonBrowserUnlockReachesCar(t *testing.T) { + vin := newVin() + p := startProxy(t, vin) + + code, resp := p.do("POST", "/api/1/vehicles/"+vin+"/command/door_unlock?wait=true", "") + + wantStatus(t, code, resp, http.StatusOK) + p.car.Do(func(c *fakecar.Car) { + if c.Locked { + t.Error("car is still locked") + } + }) +} + +// Key changes must not be reachable by GET, where a plain on any site could trigger them. +func TestHTTPKeyRoutesRejectGet(t *testing.T) { + p := startProxy(t, newVin()) + for _, path := range []string{"/gen_keys?role=owner", "/remove_keys?role=owner"} { + if code, _ := p.raw("GET", path, "", nil); code != http.StatusMethodNotAllowed { + t.Errorf("GET %s: status = %d, want %d", path, code, http.StatusMethodNotAllowed) + } + } +} + +// Form input from send_key ends up in a dashboard message, which must render escaped. +func TestHTTPDashboardEscapesSendKeyInput(t *testing.T) { + p := startProxy(t, newVin()) + const script = `` + form := url.Values{"VIN": {script}, "role": {script}}.Encode() + + code, _ := p.raw("POST", "/send_key", form, map[string]string{ + "Content-Type": "application/x-www-form-urlencoded", + "Sec-Fetch-Site": "same-origin", + }) + if code != http.StatusSeeOther { + t.Fatalf("send_key status = %d, want %d", code, http.StatusSeeOther) + } + + code, body := p.raw("GET", "/dashboard", "", nil) + if code != http.StatusOK { + t.Fatalf("dashboard status = %d", code) + } + if strings.Contains(body, script) { + t.Fatal("input rendered unescaped") + } + if !strings.Contains(body, "<script>alert(1)</script>") { + t.Fatalf("escaped input not found in dashboard") + } +} diff --git a/internal/api/handlers/tesla_test.go b/internal/api/handlers/tesla_test.go index 4f1fedb..16e43c8 100644 --- a/internal/api/handlers/tesla_test.go +++ b/internal/api/handlers/tesla_test.go @@ -2,16 +2,17 @@ package handlers_test import ( "context" - "embed" "encoding/json" "errors" "fmt" "io" "net/http" "net/http/httptest" + "os" "strings" "sync/atomic" "testing" + "testing/fstest" "time" "github.com/wimaha/TeslaBleHttpProxy/config" @@ -51,7 +52,7 @@ func startProxy(t *testing.T, vin string) *proxy { previous := control.BleControlInstance control.BleControlInstance = bc - server := httptest.NewServer(routes.SetupRoutes(embed.FS{}, embed.FS{})) + server := httptest.NewServer(routes.SetupRoutes(fstest.MapFS{}, os.DirFS("../../.."))) t.Cleanup(func() { server.Close() control.BleControlInstance = previous From 31dfc21d8e527438ca5cfb60dda8df719025f175 Mon Sep 17 00:00:00 2001 From: Wilko Date: Sun, 18 Oct 2026 22:29:53 +0200 Subject: [PATCH 3/4] Add trustedOrigins for reverse proxies; document POST-only key routes Browsers send Sec-Fetch-Site only to secure origins. Over plain HTTP the cross-origin check falls back to comparing Origin with Host, so a reverse proxy that rewrites Host (nginx default) made dashboard buttons return 403. The new trustedOrigins env var (comma-separated scheme://host[:port]) exempts such origins; invalid entries are logged and skipped. Over HTTPS nothing needs to be set. README troubleshooting now explains the 403 and that /gen_keys and /remove_keys accept POST only. Co-Authored-By: Claude Opus 5.5 --- README.md | 6 ++++++ config/config.go | 18 ++++++++++++++--- docs/environment_variables.md | 4 ++++ internal/api/handlers/tesla_test.go | 2 +- internal/api/routes/routes.go | 14 +++++++++++--- internal/api/routes/routes_test.go | 30 ++++++++++++++++++++++++++++- main.go | 2 +- 7 files changed, 67 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index acb93c0..f67aa7a 100644 --- a/README.md +++ b/README.md @@ -284,3 +284,9 @@ Due to BLE's power-saving design, Tesla vehicles may terminate connections after **Fix:** Close the Tesla app on mobile phones when not needed, especially if you're also wearing a smartwatch with the Tesla app. The proxy counts as one device, so limit other active BLE connections accordingly. This is a Tesla vehicle constraint, not a limitation of TeslaBleHttpProxy. + +### Dashboard buttons return 403 Forbidden + +The proxy rejects state-changing requests that a browser sends from another website, so a page you visit cannot change your keys or send commands to the car. If you open the dashboard through a reverse proxy over plain HTTP that rewrites the `Host` header, your own requests can look cross-site too. Set the [`trustedOrigins`](docs/environment_variables.md#trustedorigins) environment variable to the address you open in the browser, e.g. `trustedOrigins=http://nas.local:8080`. Over HTTPS this is not needed. + +The key management endpoints `/gen_keys` and `/remove_keys` accept `POST` only. Scripts that called them with `GET`, as older versions allowed, must switch to `POST`. diff --git a/config/config.go b/config/config.go index 1186f44..095400b 100644 --- a/config/config.go +++ b/config/config.go @@ -89,9 +89,10 @@ var Version = "*undefined*" type Config struct { LogLevel string HttpListenAddress string - ScanTimeout int // Seconds to scan for BLE devices - CacheMaxAge int // Seconds for HTTP Cache-Control header max-age (used for body controller state responses). If set to 0, cache headers are disabled. - VehicleDataCacheTime int // Seconds to cache VehicleData endpoint responses in memory. Each endpoint is cached separately per VIN. + ScanTimeout int // Seconds to scan for BLE devices + CacheMaxAge int // Seconds for HTTP Cache-Control header max-age (used for body controller state responses). If set to 0, cache headers are disabled. + VehicleDataCacheTime int // Seconds to cache VehicleData endpoint responses in memory. Each endpoint is cached separately per VIN. + TrustedOrigins []string // Origins (scheme://host[:port]) allowed to send state-changing requests from a browser, e.g. a reverse proxy that rewrites the Host header. } var AppConfig *Config @@ -145,12 +146,23 @@ func LoadConfig() *Config { } logging.Info("Env:", "vehicleDataCacheTime", vehicleDataCacheTimeInt) + var trustedOrigins []string + for _, origin := range strings.Split(os.Getenv("trustedOrigins"), ",") { + if origin = strings.TrimSpace(origin); origin != "" { + trustedOrigins = append(trustedOrigins, origin) + } + } + if len(trustedOrigins) > 0 { + logging.Info("Env:", "trustedOrigins", trustedOrigins) + } + return &Config{ LogLevel: envLogLevel, HttpListenAddress: addr, CacheMaxAge: cacheMaxAgeInt, ScanTimeout: scanTimeoutInt, VehicleDataCacheTime: vehicleDataCacheTimeInt, + TrustedOrigins: trustedOrigins, } } diff --git a/docs/environment_variables.md b/docs/environment_variables.md index 68f0202..c5cef19 100644 --- a/docs/environment_variables.md +++ b/docs/environment_variables.md @@ -22,6 +22,10 @@ This is the number of seconds to cache VehicleData endpoint responses in memory. This is the address and port to listen for HTTP requests. (Default: :8080) +## trustedOrigins + +Comma-separated list of origins (`scheme://host[:port]`) that may send state-changing requests (dashboard buttons, commands) from a browser. Only needed if the dashboard is reached through a reverse proxy over plain HTTP that rewrites the `Host` header (nginx does this by default). In that case the dashboard buttons answer `403 Forbidden`. Set this to the address you type in the browser, e.g. `http://nas.local:8080`. Requests from evcc, curl or Home Assistant integrations are not affected. (Default: empty) + # Example ## Docker compose diff --git a/internal/api/handlers/tesla_test.go b/internal/api/handlers/tesla_test.go index 16e43c8..f05c934 100644 --- a/internal/api/handlers/tesla_test.go +++ b/internal/api/handlers/tesla_test.go @@ -52,7 +52,7 @@ func startProxy(t *testing.T, vin string) *proxy { previous := control.BleControlInstance control.BleControlInstance = bc - server := httptest.NewServer(routes.SetupRoutes(fstest.MapFS{}, os.DirFS("../../.."))) + server := httptest.NewServer(routes.SetupRoutes(fstest.MapFS{}, os.DirFS("../../.."), nil)) t.Cleanup(func() { server.Close() control.BleControlInstance = previous diff --git a/internal/api/routes/routes.go b/internal/api/routes/routes.go index ed1e1b8..d6c396e 100644 --- a/internal/api/routes/routes.go +++ b/internal/api/routes/routes.go @@ -6,12 +6,14 @@ import ( "github.com/gorilla/mux" "github.com/wimaha/TeslaBleHttpProxy/internal/api/handlers" + "github.com/wimaha/TeslaBleHttpProxy/internal/logging" ) // SetupRoutes returns the router wrapped in cross-origin protection: state-changing requests a // browser sends from another site are rejected, while clients that send neither Origin nor -// Sec-Fetch-Site (evcc, curl, Home Assistant) pass unchanged. -func SetupRoutes(static fs.FS, html fs.FS) http.Handler { +// Sec-Fetch-Site (evcc, curl, Home Assistant) pass unchanged. trustedOrigins are exempt, for a +// reverse proxy that rewrites the Host header on plain HTTP, where browsers send no Sec-Fetch-Site. +func SetupRoutes(static fs.FS, html fs.FS, trustedOrigins []string) http.Handler { router := mux.NewRouter() // Define the endpoints @@ -30,5 +32,11 @@ func SetupRoutes(static fs.FS, html fs.FS) http.Handler { router.HandleFunc("/send_key", handlers.SendKey).Methods("POST") router.PathPrefix("/static/").Handler(http.FileServer(http.FS(static))) - return http.NewCrossOriginProtection().Handler(router) + protection := http.NewCrossOriginProtection() + for _, origin := range trustedOrigins { + if err := protection.AddTrustedOrigin(origin); err != nil { + logging.Error("Ignoring invalid trusted origin", "origin", origin, "error", err) + } + } + return protection.Handler(router) } diff --git a/internal/api/routes/routes_test.go b/internal/api/routes/routes_test.go index f82e9bb..7d446f2 100644 --- a/internal/api/routes/routes_test.go +++ b/internal/api/routes/routes_test.go @@ -10,9 +10,14 @@ import ( ) func serve(t *testing.T, req *http.Request) int { + t.Helper() + return serveTrusting(t, req, nil) +} + +func serveTrusting(t *testing.T, req *http.Request, trustedOrigins []string) int { t.Helper() rec := httptest.NewRecorder() - SetupRoutes(fstest.MapFS{}, fstest.MapFS{}).ServeHTTP(rec, req) + SetupRoutes(fstest.MapFS{}, fstest.MapFS{}, trustedOrigins).ServeHTTP(rec, req) return rec.Code } @@ -64,3 +69,26 @@ func TestKeyRoutesRejectGet(t *testing.T) { } } } + +// Behind a plain-HTTP reverse proxy that rewrites Host, browsers send no Sec-Fetch-Site and the +// Origin no longer matches Host. Dashboard forms then need the proxy's origin to be trusted. +func TestReverseProxyOriginNeedsTrust(t *testing.T) { + request := func() *http.Request { + form := url.Values{"role": {"invalid"}}.Encode() + req := httptest.NewRequest("POST", "http://127.0.0.1:8080/activate_key", strings.NewReader(form)) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Origin", "http://nas.local") + return req + } + + if code := serveTrusting(t, request(), nil); code != http.StatusForbidden { + t.Errorf("untrusted: status = %d, want %d", code, http.StatusForbidden) + } + if code := serveTrusting(t, request(), []string{"http://nas.local"}); code != http.StatusSeeOther { + t.Errorf("trusted: status = %d, want %d", code, http.StatusSeeOther) + } + // An invalid entry is logged and skipped instead of breaking startup. + if code := serveTrusting(t, request(), []string{"nas.local/path", "http://nas.local"}); code != http.StatusSeeOther { + t.Errorf("with invalid entry: status = %d, want %d", code, http.StatusSeeOther) + } +} diff --git a/main.go b/main.go index 17aed7a..0780187 100644 --- a/main.go +++ b/main.go @@ -44,7 +44,7 @@ func main() { ) } - router := routes.SetupRoutes(static, html) + router := routes.SetupRoutes(static, html, config.AppConfig.TrustedOrigins) logging.Info("TeslaBleHttpProxy is running!") if err := http.ListenAndServe(config.AppConfig.HttpListenAddress, router); err != nil { From e1f0da1a03d31caa789445ce520ecfd5eee94b57 Mon Sep 17 00:00:00 2001 From: Wilko Date: Sun, 18 Oct 2026 22:38:06 +0200 Subject: [PATCH 4/4] Normalize trustedOrigins; document 403 for browser clients on other origins The Origin header is compared verbatim, so "http://nas.local:80" never matched and "http://nas.local/" was rejected as having a path. Entries are now lowercased and stripped of a trailing slash and default ports before they are trusted. The docs now also say that custom web pages or browser tools calling the API from another origin get 403 unless listed in trustedOrigins. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 ++ docs/environment_variables.md | 9 ++++++++- internal/api/routes/routes.go | 24 +++++++++++++++++++++++- internal/api/routes/routes_test.go | 21 ++++++++++++++++++++- 4 files changed, 53 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index f67aa7a..91d6c54 100644 --- a/README.md +++ b/README.md @@ -289,4 +289,6 @@ This is a Tesla vehicle constraint, not a limitation of TeslaBleHttpProxy. The proxy rejects state-changing requests that a browser sends from another website, so a page you visit cannot change your keys or send commands to the car. If you open the dashboard through a reverse proxy over plain HTTP that rewrites the `Host` header, your own requests can look cross-site too. Set the [`trustedOrigins`](docs/environment_variables.md#trustedorigins) environment variable to the address you open in the browser, e.g. `trustedOrigins=http://nas.local:8080`. Over HTTPS this is not needed. +The same applies to custom web pages or browser tools that call the API with `POST` from another origin: they get `403` unless their origin is listed in `trustedOrigins`. + The key management endpoints `/gen_keys` and `/remove_keys` accept `POST` only. Scripts that called them with `GET`, as older versions allowed, must switch to `POST`. diff --git a/docs/environment_variables.md b/docs/environment_variables.md index c5cef19..daae937 100644 --- a/docs/environment_variables.md +++ b/docs/environment_variables.md @@ -24,7 +24,14 @@ This is the address and port to listen for HTTP requests. (Default: :8080) ## trustedOrigins -Comma-separated list of origins (`scheme://host[:port]`) that may send state-changing requests (dashboard buttons, commands) from a browser. Only needed if the dashboard is reached through a reverse proxy over plain HTTP that rewrites the `Host` header (nginx does this by default). In that case the dashboard buttons answer `403 Forbidden`. Set this to the address you type in the browser, e.g. `http://nas.local:8080`. Requests from evcc, curl or Home Assistant integrations are not affected. (Default: empty) +Comma-separated list of origins (`scheme://host[:port]`) that may send state-changing requests (dashboard buttons, commands) from a browser. Requests from evcc, curl or Home Assistant integrations are not affected. (Default: empty) + +You need it in two cases, because the proxy otherwise answers `403 Forbidden`: + +- The dashboard is reached through a reverse proxy over plain HTTP that rewrites the `Host` header (nginx does this by default). Set it to the address you type in the browser, e.g. `http://nas.local:8080`. +- A custom web page or browser tool on another origin calls the API with `POST`, e.g. a page served from `http://homeserver:3000`. Set it to that page's origin. + +Case, a trailing slash and the default ports `:80`/`:443` do not matter. Several origins: `trustedOrigins=http://nas.local:8080,https://home.example.com`. # Example diff --git a/internal/api/routes/routes.go b/internal/api/routes/routes.go index d6c396e..248cd99 100644 --- a/internal/api/routes/routes.go +++ b/internal/api/routes/routes.go @@ -3,6 +3,8 @@ package routes import ( "io/fs" "net/http" + "net/url" + "strings" "github.com/gorilla/mux" "github.com/wimaha/TeslaBleHttpProxy/internal/api/handlers" @@ -34,9 +36,29 @@ func SetupRoutes(static fs.FS, html fs.FS, trustedOrigins []string) http.Handler protection := http.NewCrossOriginProtection() for _, origin := range trustedOrigins { - if err := protection.AddTrustedOrigin(origin); err != nil { + if err := protection.AddTrustedOrigin(normalizeOrigin(origin)); err != nil { logging.Error("Ignoring invalid trusted origin", "origin", origin, "error", err) } } return protection.Handler(router) } + +// normalizeOrigin turns a configured origin into the form browsers send in the Origin header, +// which is compared verbatim: lowercase, no trailing slash, no default port. Anything it cannot +// parse is returned unchanged so AddTrustedOrigin reports it. +func normalizeOrigin(origin string) string { + u, err := url.Parse(strings.TrimRight(strings.TrimSpace(origin), "/")) + if err != nil || u.Host == "" { + return origin + } + scheme := strings.ToLower(u.Scheme) + host := strings.ToLower(u.Hostname()) + if strings.Contains(host, ":") { + host = "[" + host + "]" // IPv6 literal + } + if port := u.Port(); port != "" && !(scheme == "http" && port == "80") && !(scheme == "https" && port == "443") { + host += ":" + port + } + u.Scheme, u.Host = scheme, host + return u.String() +} diff --git a/internal/api/routes/routes_test.go b/internal/api/routes/routes_test.go index 7d446f2..7eca8d8 100644 --- a/internal/api/routes/routes_test.go +++ b/internal/api/routes/routes_test.go @@ -84,7 +84,7 @@ func TestReverseProxyOriginNeedsTrust(t *testing.T) { if code := serveTrusting(t, request(), nil); code != http.StatusForbidden { t.Errorf("untrusted: status = %d, want %d", code, http.StatusForbidden) } - if code := serveTrusting(t, request(), []string{"http://nas.local"}); code != http.StatusSeeOther { + if code := serveTrusting(t, request(), []string{"http://NAS.local:80/"}); code != http.StatusSeeOther { t.Errorf("trusted: status = %d, want %d", code, http.StatusSeeOther) } // An invalid entry is logged and skipped instead of breaking startup. @@ -92,3 +92,22 @@ func TestReverseProxyOriginNeedsTrust(t *testing.T) { t.Errorf("with invalid entry: status = %d, want %d", code, http.StatusSeeOther) } } + +func TestNormalizeOrigin(t *testing.T) { + cases := map[string]string{ + "http://nas.local": "http://nas.local", + "http://nas.local/": "http://nas.local", + "HTTP://NAS.local:80": "http://nas.local", + "https://nas.local:443/": "https://nas.local", + "https://nas.local:80": "https://nas.local:80", + "http://nas.local:8080": "http://nas.local:8080", + "http://[::1]:8080": "http://[::1]:8080", + "http://nas.local/path": "http://nas.local/path", // left for AddTrustedOrigin to reject + "bogus": "bogus", + } + for in, want := range cases { + if got := normalizeOrigin(in); got != want { + t.Errorf("normalizeOrigin(%q) = %q, want %q", in, got, want) + } + } +}