Fix legacy API exceptions for persistent requests

http-proxy/main.go

@@ -97,9 +97,9 @@ var (
 	banditCallbackURL   = flag.String("banditcallbackurl", "", "Full URL of the /v1/bandit/callback endpoint")
 	banditCallbackTTL   = flag.Duration("banditcallbackttl", 60*time.Second, "Per-device dedup window and heartbeat cadence for bandit callback emission. The API's absence-reaper expects a callback within ArmCallbackHeartbeatWindow (~90s) of the previous one; values much smaller than that just amplify callback traffic, values larger risk false-positive negative rewards on idle clients.")
 
-	// Defaults cover the legacy hosts so this fixes itself on upgrade;
-	// the provisioner can still override it later. See eng#3695.
-	legacyAPIHosts = flag.String("legacyapihosts", "api.getiantem.org,geo.getiantem.org", "Comma-separated hostnames exempted from the BlockLocal filter")
+	// Additional legacy API hosts beyond the mandatory compatibility hosts
+	// built into the proxy package. See eng#3695.
+	legacyAPIHosts = flag.String("legacyapihosts", "", "Comma-separated additional hostnames exempted from the BlockLocal filter")
 
 	throttleRefreshInterval = flag.Duration("throttlerefresh", throttle.DefaultRefreshInterval, "Specifies how frequently to refresh throttling configuration from redis. Defaults to 5 minutes.")
 

http_proxy.go

@@ -85,6 +85,11 @@ var (
 	log = golog.LoggerFor("lantern-proxy")
 
 	proxyNameRegex = regexp.MustCompile(`(fp-([a-z0-9]+-)?([a-z0-9]+)-[0-9]{8}-[0-9]+)(-.+)?`)
+
+	// Legacy clients require these hosts for account and payment operations.
+	// Keep them mandatory so deployment configuration can add exceptions but
+	// cannot accidentally remove the compatibility baseline.
+	requiredLegacyAPIHosts = [...]string{"api.getiantem.org", "geo.getiantem.org"}
 )
 
 // Proxy is an HTTP proxy.
@@ -176,9 +181,9 @@ type Proxy struct {
 	BanditCallbackTTL     time.Duration
 	banditCallbackEmitter *banditcallback.Emitter
 
-	// LegacyAPIHosts are extra hostnames exempted from BlockLocal, comma
-	// separated. Used for legacy pre-9.x clients hitting api.getiantem.org
-	// directly (see eng#3695).
+	// LegacyAPIHosts are additional comma-separated hostnames exempted from
+	// BlockLocal. The compatibility hosts required by legacy pre-9.x clients
+	// are always included separately (see eng#3695).
 	LegacyAPIHosts string
 
 	MultiplexProtocol             string
@@ -735,15 +740,26 @@ func (p *Proxy) loadThrottleConfig() {
 }
 
 func (p *Proxy) legacyAPIHostExceptions() []string {
-	if p.LegacyAPIHosts == "" {
-		return nil
+	hosts := make([]string, 0, len(requiredLegacyAPIHosts)+1)
+	seen := make(map[string]struct{}, len(requiredLegacyAPIHosts)+1)
+	add := func(host string) {
+		host = strings.TrimSpace(host)
+		key := strings.ToLower(host)
+		if host == "" {
+			return
+		}
+		if _, ok := seen[key]; ok {
+			return
+		}
+		seen[key] = struct{}{}
+		hosts = append(hosts, host)
+	}
+
+	for _, host := range requiredLegacyAPIHosts {
+		add(host)
 	}
-	var hosts []string
 	for _, h := range strings.Split(p.LegacyAPIHosts, ",") {
-		h = strings.TrimSpace(h)
-		if h != "" {
-			hosts = append(hosts, h)
-		}
+		add(h)
 	}
 	return hosts
 }

http_proxy_test.go

@@ -689,6 +689,22 @@ func basicServer(maxConns uint64, idleTimeout time.Duration) *server.Server {
 	return srv
 }
 
+func TestLegacyAPIHostExceptions(t *testing.T) {
+	t.Run("required hosts cannot be removed", func(t *testing.T) {
+		p := &Proxy{}
+		assert.Equal(t, []string{"api.getiantem.org", "geo.getiantem.org"}, p.legacyAPIHostExceptions())
+	})
+
+	t.Run("configuration adds unique hosts", func(t *testing.T) {
+		p := &Proxy{LegacyAPIHosts: " extra.example.org, API.GETIANTEM.ORG, "}
+		assert.Equal(t, []string{
+			"api.getiantem.org",
+			"geo.getiantem.org",
+			"extra.example.org",
+		}, p.legacyAPIHostExceptions())
+	})
+}
+
 func setupNewHTTPServer(maxConns uint64, idleTimeout time.Duration, https bool) (addr string, err error) {
 	var (
 		s     = basicServer(maxConns, idleTimeout)

proxyfilters/blocklocal.go

@@ -33,15 +33,22 @@ func BlockLocal(exceptions []string, r resolver) filters.Filter {
 	}
 
 	return filters.FilterFunc(func(cs *filters.ConnectionState, req *http.Request, next filters.Next) (*http.Response, *filters.ConnectionState, error) {
-		host, port, err := net.SplitHostPort(req.URL.Host)
+		targetHost := req.URL.Host
+		if targetHost == "" {
+			// Origin-form requests carry the authority in Request.Host. This is
+			// how legacy clients send HTTP requests over persistent proxy tunnels.
+			targetHost = req.Host
+		}
+
+		host, port, err := net.SplitHostPort(targetHost)
 		if err != nil {
 			// host didn't have a port, thus splitting didn't work
-			host = req.URL.Host
+			host = targetHost
 		}
 
 		// Check the bare host too, so a hostname exception matches with
 		// or without the default port.
-		if isException(req.URL.Host) || isException(host) {
+		if isException(targetHost) || isException(host) {
 			return next(cs, req)
 		}
 

proxyfilters/blocklocal_test.go

@@ -46,6 +46,20 @@ func TestBlockLocalExceptionIgnoresDefaultPort(t *testing.T) {
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 }
 
+func TestBlockLocalExceptionForOriginFormRequest(t *testing.T) {
+	// Legacy clients use origin-form requests over persistent proxy tunnels,
+	// which leaves URL.Host empty and puts the authority in Request.Host.
+	req, err := http.NewRequest(http.MethodGet, "/plans-v4", nil)
+	if !assert.NoError(t, err) {
+		return
+	}
+	req.Host = "api.getiantem.org:80"
+	assert.Empty(t, req.URL.Host)
+
+	_, resp := doTestBlockLocalRequest(t, []string{"api.getiantem.org"}, req, &testResolver{127, 0, 0, 1})
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}
+
 func TestBlockLocalNotLocal(t *testing.T) {
 	modifiedReq, resp := doTestBlockLocal(t, []string{"localhost"}, "http://example.com/index.html", &testResolver{93, 184, 215, 16})
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
@@ -55,14 +69,20 @@ func TestBlockLocalNotLocal(t *testing.T) {
 }
 
 func doTestBlockLocal(t *testing.T, exceptions []string, urlStr string, r resolver) (*http.Request, *http.Response) {
+	t.Helper()
+	req, _ := http.NewRequest(http.MethodGet, urlStr, nil)
+	return doTestBlockLocalRequest(t, exceptions, req, r)
+}
+
+func doTestBlockLocalRequest(t *testing.T, exceptions []string, req *http.Request, r resolver) (*http.Request, *http.Response) {
+	t.Helper()
 	next := func(cs *filters.ConnectionState, req *http.Request) (*http.Response, *filters.ConnectionState, error) {
 		return &http.Response{
 			StatusCode: http.StatusOK,
 		}, cs, nil
 	}
 
 	filter := BlockLocal(exceptions, r)
-	req, _ := http.NewRequest(http.MethodGet, urlStr, nil)
 	log.Debug(req.URL.Host)
 	cs := filters.NewConnectionState(req, nil, nil)
 	resp, _, _ := filter.Apply(cs, req, next)