Harden HTTP endpoints against cross-site requests and XSS

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`.

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,
 	}
 }
 

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

html/dashboard.html

@@ -26,7 +26,7 @@ <h3>Key Management</h3>
                     {{if $key.Exists}}
                         {{if $key.IsActive}}
                             <span class="active-badge">● Active</span>
-                            <form action="/remove_keys" method="GET" style="display: inline; margin-left: 8px;">
+                            <form action="/remove_keys" method="POST" style="display: inline; margin-left: 8px;">
                                 <input type="hidden" name="role" value="{{$key.Role}}" />
                                 <button type="submit" class="remove-button small-button" onclick="return confirmRemoveKey('{{$key.DisplayName}}');">Remove</button>
                             </form>
@@ -35,14 +35,14 @@ <h3>Key Management</h3>
                                 <input type="hidden" name="role" value="{{$key.Role}}" />
                                 <button type="submit" class="save-button small-button">Activate</button>
                             </form>
-                            <form action="/remove_keys" method="GET" style="display: inline; margin-left: 8px;">
+                            <form action="/remove_keys" method="POST" style="display: inline; margin-left: 8px;">
                                 <input type="hidden" name="role" value="{{$key.Role}}" />
                                 <button type="submit" class="remove-button small-button" onclick="return confirmRemoveKey('{{$key.DisplayName}}');">Remove</button>
                             </form>
                         {{end}}
                     {{else}}
                         <span class="not-generated">Not generated</span>
-                        <form action="/gen_keys" method="GET" style="display: inline; margin-left: 12px;">
+                        <form action="/gen_keys" method="POST" style="display: inline; margin-left: 12px;">
                             <input type="hidden" name="role" value="{{$key.Role}}" />
                             <button type="submit" class="add-button small-button">Generate</button>
                         </form>

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 == "" {

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: `<script>alert(1)</script>`,
+		Type:    models.Error,
+	})
+
+	rec := httptest.NewRecorder()
+	ShowDashboard(os.DirFS("../../.."))(rec, httptest.NewRequest("GET", "/dashboard", nil))
+
+	body := rec.Body.String()
+	if strings.Contains(body, "<script>alert(1)</script>") {
+		t.Fatalf("message rendered unescaped")
+	}
+	if !strings.Contains(body, "&lt;script&gt;alert(1)&lt;/script&gt;") {
+		t.Fatalf("escaped message not found in dashboard")
+	}
+}

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"

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 <img> 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 = `<script>alert(1)</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, "&lt;script&gt;alert(1)&lt;/script&gt;") {
+		t.Fatalf("escaped input not found in dashboard")
+	}
+}

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)
 }

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