diff --git a/README.md b/README.md index acb93c0..91d6c54 100644 --- a/README.md +++ b/README.md @@ -284,3 +284,11 @@ 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 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/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..daae937 100644 --- a/docs/environment_variables.md +++ b/docs/environment_variables.md @@ -22,6 +22,17 @@ 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. 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 ## Docker compose 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/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.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/handlers/tesla_test.go b/internal/api/handlers/tesla_test.go index 4f1fedb..f05c934 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("../../.."), nil)) t.Cleanup(func() { server.Close() control.BleControlInstance = previous 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..248cd99 100644 --- a/internal/api/routes/routes.go +++ b/internal/api/routes/routes.go @@ -1,14 +1,21 @@ package routes import ( - "embed" + "io/fs" "net/http" + "net/url" + "strings" "github.com/gorilla/mux" "github.com/wimaha/TeslaBleHttpProxy/internal/api/handlers" + "github.com/wimaha/TeslaBleHttpProxy/internal/logging" ) -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. 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 @@ -21,11 +28,37 @@ 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 + protection := http.NewCrossOriginProtection() + for _, origin := range trustedOrigins { + 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 new file mode 100644 index 0000000..7eca8d8 --- /dev/null +++ b/internal/api/routes/routes_test.go @@ -0,0 +1,113 @@ +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() + 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{}, trustedOrigins).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) + } + } +} + +// 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: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. + 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) + } +} + +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) + } + } +} 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 {