lib/config, cmd/syncthing: Enforce localhost only connections

When the GUI/API is bound to localhost, we enforce that the Host header
looks like localhost. This can be disabled by setting
insecureSkipHostCheck in the GUI config.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3558

cmd/syncthing/gui.go

@@ -313,6 +313,11 @@ func (s *apiService) Serve() {
 	// Add the CORS handling
 	handler = corsMiddleware(handler)
 
+	if addressIsLocalhost(guiCfg.Address()) && !guiCfg.InsecureSkipHostCheck {
+		// Verify source host
+		handler = localhostMiddleware(handler)
+	}
+
 	handler = debugMiddleware(handler)
 
 	srv := http.Server{
@@ -495,6 +500,17 @@ func withDetailsMiddleware(id protocol.DeviceID, h http.Handler) http.Handler {
 	})
 }
 
+func localhostMiddleware(h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if addressIsLocalhost(r.Host) {
+			h.ServeHTTP(w, r)
+			return
+		}
+
+		http.Error(w, "Host check error", http.StatusForbidden)
+	})
+}
+
 func (s *apiService) whenDebugging(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if s.cfg.GUI().Debugging {
@@ -503,7 +519,6 @@ func (s *apiService) whenDebugging(h http.Handler) http.Handler {
 		}
 
 		http.Error(w, "Debugging disabled", http.StatusBadRequest)
-		return
 	})
 }
 
@@ -1292,3 +1307,17 @@ func dirNames(dir string) []string {
 	sort.Strings(dirs)
 	return dirs
 }
+
+func addressIsLocalhost(addr string) bool {
+	host, _, err := net.SplitHostPort(addr)
+	if err != nil {
+		// There was no port, so we assume the address was just a hostname
+		host = addr
+	}
+	switch host {
+	case "127.0.0.1", "::1", "localhost":
+		return true
+	default:
+		return false
+	}
+}

cmd/syncthing/gui_test.go

@@ -16,6 +16,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -460,7 +461,6 @@ func TestHTTPLogin(t *testing.T) {
 	if resp.StatusCode != http.StatusOK {
 		t.Errorf("Unexpected non-200 return code %d for authed request (ISO-8859-1)", resp.StatusCode)
 	}
-
 }
 
 func startHTTP(cfg *mockedConfig) (string, error) {
@@ -491,7 +491,12 @@ func startHTTP(cfg *mockedConfig) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("Weird address from API service: %v", err)
 	}
-	baseURL := fmt.Sprintf("http://127.0.0.1:%d", tcpAddr.Port)
+
+	host, _, _ := net.SplitHostPort(cfg.gui.RawAddress)
+	if host == "" || host == "0.0.0.0" {
+		host = "127.0.0.1"
+	}
+	baseURL := fmt.Sprintf("http://%s", net.JoinHostPort(host, strconv.Itoa(tcpAddr.Port)))
 
 	return baseURL, nil
 }
@@ -666,3 +671,189 @@ func testConfigPost(data io.Reader) (*http.Response, error) {
 	req.Header.Set("X-API-Key", testAPIKey)
 	return cli.Do(req)
 }
+
+func TestHostCheck(t *testing.T) {
+	// An API service bound to localhost should reject non-localhost host Headers
+
+	cfg := new(mockedConfig)
+	cfg.gui.RawAddress = "127.0.0.1:0"
+	baseURL, err := startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A normal HTTP get to the localhost-bound service should succeed
+
+	resp, err := http.Get(baseURL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Regular HTTP get: expected 200 OK, not", resp.Status)
+	}
+
+	// A request with a suspicious Host header should fail
+
+	req, _ := http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusForbidden {
+		t.Error("Suspicious Host header: expected 403 Forbidden, not", resp.Status)
+	}
+
+	// A request with an explicit "localhost:8384" Host header should pass
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "localhost:8384"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Explicit localhost:8384: expected 200 OK, not", resp.Status)
+	}
+
+	// A request with an explicit "localhost" Host header (no port) should pass
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "localhost"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Explicit localhost: expected 200 OK, not", resp.Status)
+	}
+
+	// A server with InsecureSkipHostCheck set behaves differently
+
+	cfg = new(mockedConfig)
+	cfg.gui.RawAddress = "127.0.0.1:0"
+	cfg.gui.InsecureSkipHostCheck = true
+	baseURL, err = startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A request with a suspicious Host header should be allowed
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Incorrect host header, check disabled: expected 200 OK, not", resp.Status)
+	}
+
+	// A server bound to a wildcard address also doesn't do the check
+
+	cfg = new(mockedConfig)
+	cfg.gui.RawAddress = "0.0.0.0:0"
+	cfg.gui.InsecureSkipHostCheck = true
+	baseURL, err = startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A request with a suspicious Host header should be allowed
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Incorrect host header, wildcard bound: expected 200 OK, not", resp.Status)
+	}
+
+	// This should all work over IPv6 as well
+
+	cfg = new(mockedConfig)
+	cfg.gui.RawAddress = "[::1]:0"
+	baseURL, err = startHTTP(cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// A normal HTTP get to the localhost-bound service should succeed
+
+	resp, err = http.Get(baseURL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Regular HTTP get (IPv6): expected 200 OK, not", resp.Status)
+	}
+
+	// A request with a suspicious Host header should fail
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "example.com"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusForbidden {
+		t.Error("Suspicious Host header (IPv6): expected 403 Forbidden, not", resp.Status)
+	}
+
+	// A request with an explicit "localhost:8384" Host header should pass
+
+	req, _ = http.NewRequest("GET", baseURL, nil)
+	req.Host = "localhost:8384"
+	resp, err = http.DefaultClient.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		t.Error("Explicit localhost:8384 (IPv6): expected 200 OK, not", resp.Status)
+	}
+}
+
+func TestAddressIsLocalhost(t *testing.T) {
+	testcases := []struct {
+		address string
+		result  bool
+	}{
+		// These are all valid localhost addresses
+		{"localhost", true},
+		{"::1", true},
+		{"127.0.0.1", true},
+		{"localhost:8080", true},
+		{"[::1]:8080", true},
+		{"127.0.0.1:8080", true},
+
+		// These are all non-localhost addresses
+		{"example.com", false},
+		{"example.com:8080", false},
+		{"192.0.2.10", false},
+		{"192.0.2.10:8080", false},
+		{"0.0.0.0", false},
+		{"0.0.0.0:8080", false},
+		{"::", false},
+		{"[::]:8080", false},
+		{":8080", false},
+	}
+
+	for _, tc := range testcases {
+		result := addressIsLocalhost(tc.address)
+		if result != tc.result {
+			t.Errorf("addressIsLocalhost(%q)=%v, expected %v", tc.address, result, tc.result)
+		}
+	}
+}

lib/config/guiconfiguration.go

@@ -13,15 +13,16 @@ import (
 )
 
 type GUIConfiguration struct {
-	Enabled             bool   `xml:"enabled,attr" json:"enabled" default:"true"`
-	RawAddress          string `xml:"address" json:"address" default:"127.0.0.1:8384"`
-	User                string `xml:"user,omitempty" json:"user"`
-	Password            string `xml:"password,omitempty" json:"password"`
-	RawUseTLS           bool   `xml:"tls,attr" json:"useTLS"`
-	APIKey              string `xml:"apikey,omitempty" json:"apiKey"`
-	InsecureAdminAccess bool   `xml:"insecureAdminAccess,omitempty" json:"insecureAdminAccess"`
-	Theme               string `xml:"theme" json:"theme" default:"default"`
-	Debugging           bool   `xml:"debugging,attr" json:"debugging"`
+	Enabled               bool   `xml:"enabled,attr" json:"enabled" default:"true"`
+	RawAddress            string `xml:"address" json:"address" default:"127.0.0.1:8384"`
+	User                  string `xml:"user,omitempty" json:"user"`
+	Password              string `xml:"password,omitempty" json:"password"`
+	RawUseTLS             bool   `xml:"tls,attr" json:"useTLS"`
+	APIKey                string `xml:"apikey,omitempty" json:"apiKey"`
+	InsecureAdminAccess   bool   `xml:"insecureAdminAccess,omitempty" json:"insecureAdminAccess"`
+	Theme                 string `xml:"theme" json:"theme" default:"default"`
+	Debugging             bool   `xml:"debugging,attr" json:"debugging"`
+	InsecureSkipHostCheck bool   `xml:"insecureSkipHostcheck,omitempty" json:"insecureSkipHostcheck"`
 }
 
 func (c GUIConfiguration) Address() string {