Przeglądaj źródła

fix(sub): keep listen/bind IP out of subscription page URLs

The subscription page leaked an inbound's server-side Listen IP into the
client-facing URLs when a bind address was set:

- Per-config links: resolveInboundAddress returned the bind Listen IP
  (loopback/private/public alike) instead of the host the subscriber
  reached the panel on. It now returns the node address for node-managed
  inbounds, otherwise the subscriber host; the bind Listen is ignored
  (External Proxy remains the way to advertise a specific endpoint).

- Subscription Copy URL (SUB/JSON/CLASH): BuildURLs composed the base
  differently from the panel's Client Information page and never
  normalized the request host, so a loopback/bind request leaked the raw
  IP. The composition is extracted into the shared
  SettingService.BuildSubURIBase, used by both the panel and the sub page
  so they render identically, and fed the already-normalized subscriber
  host.
MHSanaei 7 godzin temu
rodzic
commit
fb311afa6f

+ 71 - 0
sub/build_urls_test.go

@@ -0,0 +1,71 @@
+package sub
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/mhsanaei/3x-ui/v3/database"
+)
+
+func initSubDB(t *testing.T) {
+	t.Helper()
+	if err := database.InitDB(filepath.Join(t.TempDir(), "x-ui.db")); err != nil {
+		t.Fatalf("InitDB: %v", err)
+	}
+	// Close the handle before t.TempDir cleanup so Windows doesn't refuse to
+	// remove the still-open sqlite file.
+	t.Cleanup(func() { _ = database.CloseDB() })
+}
+
+// The subscription page's Copy URL must be built from the same host the
+// subscriber reached the page on (after PrepareForRequest normalizes away a
+// loopback/bind address) — never the raw listen IP. A subscriber that hit a
+// loopback bind should see "localhost", not "127.0.0.1".
+func TestBuildURLs_NormalizesListenIP(t *testing.T) {
+	initSubDB(t)
+	s := &SubService{}
+	s.PrepareForRequest("127.0.0.1")
+
+	subURL, _, _ := s.BuildURLs("/sub/", "/json/", "/clash/", "ABC")
+
+	if strings.Contains(subURL, "127.0.0.1") {
+		t.Fatalf("listen IP leaked into Copy URL: %q", subURL)
+	}
+	if !strings.Contains(subURL, "localhost") {
+		t.Fatalf("Copy URL = %q, want a localhost host", subURL)
+	}
+	if !strings.HasSuffix(subURL, "/sub/ABC") {
+		t.Fatalf("Copy URL = %q, want it to end with /sub/ABC", subURL)
+	}
+}
+
+// A subscriber arriving on a real domain gets that exact domain in the Copy
+// URL, with the configured sub port — matching the Client Information page.
+func TestBuildURLs_UsesSubscriberDomain(t *testing.T) {
+	initSubDB(t)
+	s := &SubService{}
+	s.PrepareForRequest("sub.example.com")
+
+	subURL, jsonURL, clashURL := s.BuildURLs("/sub/", "/json/", "/clash/", "ABC")
+
+	if subURL != "http://sub.example.com:2096/sub/ABC" {
+		t.Fatalf("subURL = %q", subURL)
+	}
+	if jsonURL != "http://sub.example.com:2096/json/ABC" {
+		t.Fatalf("jsonURL = %q", jsonURL)
+	}
+	if clashURL != "http://sub.example.com:2096/clash/ABC" {
+		t.Fatalf("clashURL = %q", clashURL)
+	}
+}
+
+func TestBuildURLs_EmptySubId(t *testing.T) {
+	initSubDB(t)
+	s := &SubService{}
+	s.PrepareForRequest("sub.example.com")
+	a, b, c := s.BuildURLs("/sub/", "/json/", "/clash/", "")
+	if a != "" || b != "" || c != "" {
+		t.Fatalf("empty subId must yield empty URLs, got %q %q %q", a, b, c)
+	}
+}

+ 1 - 1
sub/subClashService.go

@@ -97,7 +97,7 @@ func (s *SubClashService) getProxies(inbound *model.Inbound, client model.Client
 	stream := s.streamData(inbound.StreamSettings)
 	// For node-managed inbounds the Clash proxy "server" must be the
 	// node's address, not the request host. resolveInboundAddress handles
-	// the node→listen→request-host fallback chain.
+	// the node→subscriber-host fallback chain.
 	defaultDest := s.SubService.resolveInboundAddress(inbound)
 	if defaultDest == "" {
 		defaultDest = host

+ 1 - 1
sub/subController.go

@@ -130,7 +130,7 @@ func (a *SUBController) subs(c *gin.Context) {
 		// If the request expects HTML (e.g., browser) or explicitly asked (?html=1 or ?view=html), render the info page here
 		accept := c.GetHeader("Accept")
 		if strings.Contains(strings.ToLower(accept), "text/html") || c.Query("html") == "1" || strings.EqualFold(c.Query("view"), "html") {
-			subURL, subJsonURL, subClashURL := a.subService.BuildURLs(scheme, hostWithPort, a.subPath, a.subJsonPath, a.subClashPath, subId)
+			subURL, subJsonURL, subClashURL := a.subService.BuildURLs(a.subPath, a.subJsonPath, a.subClashPath, subId)
 			if !a.jsonEnabled {
 				subJsonURL = ""
 			}

+ 1 - 1
sub/subJsonService.go

@@ -147,7 +147,7 @@ func (s *SubJsonService) getConfig(inbound *model.Inbound, client model.Client,
 	// synthetic one whose `dest` is the host the client connects to.
 	// For node-managed inbounds we want the node's address — request
 	// host won't reach the right xray. resolveInboundAddress already
-	// implements the node→listen→request-host fallback chain.
+	// implements the node→subscriber-host fallback chain.
 	defaultDest := s.SubService.resolveInboundAddress(inbound)
 	if defaultDest == "" {
 		defaultDest = host

+ 14 - 48
sub/subService.go

@@ -663,24 +663,17 @@ func (s *SubService) loadNodes() {
 	s.nodesByID = m
 }
 
-// resolveInboundAddress picks the host an external client should
-// connect to. Order:
-//  1. If the inbound is node-managed and the node has an address, use
-//     the node's address — central panel's hostname doesn't speak xray
-//     for that inbound.
-//  2. If the inbound binds to a non-wildcard listen address, use it.
-//  3. Otherwise fall back to the request's host (whatever the client
-//     subscribed against).
+// resolveInboundAddress returns the node's address for node-managed inbounds,
+// otherwise the subscriber's host (s.address). The inbound's bind Listen is
+// deliberately ignored: it's a server-side address, not a client-reachable
+// host, so operators advertise a specific endpoint via External Proxy instead.
 func (s *SubService) resolveInboundAddress(inbound *model.Inbound) string {
 	if inbound.NodeID != nil && s.nodesByID != nil {
 		if n, ok := s.nodesByID[*inbound.NodeID]; ok && n.Address != "" {
 			return n.Address
 		}
 	}
-	if inbound.Listen == "" || inbound.Listen == "0.0.0.0" || inbound.Listen == "::" || inbound.Listen == "::0" {
-		return s.address
-	}
-	return inbound.Listen
+	return s.address
 }
 
 func findClientIndex(clients []model.Client, email string) int {
@@ -1866,7 +1859,7 @@ func (s *SubService) ResolveRequest(c *gin.Context) (scheme string, host string,
 
 // BuildURLs constructs absolute subscription and JSON subscription URLs for a given subscription ID.
 // It prioritizes configured URIs, then individual settings, and finally falls back to request-derived components.
-func (s *SubService) BuildURLs(scheme, hostWithPort, subPath, subJsonPath, subClashPath, subId string) (subURL, subJsonURL, subClashURL string) {
+func (s *SubService) BuildURLs(subPath, subJsonPath, subClashPath, subId string) (subURL, subJsonURL, subClashURL string) {
 	if subId == "" {
 		return "", "", ""
 	}
@@ -1875,50 +1868,23 @@ func (s *SubService) BuildURLs(scheme, hostWithPort, subPath, subJsonPath, subCl
 	configuredSubJsonURI, _ := s.settingService.GetSubJsonURI()
 	configuredSubClashURI, _ := s.settingService.GetSubClashURI()
 
-	var baseScheme, baseHostWithPort string
-	if configuredSubURI == "" || configuredSubJsonURI == "" || configuredSubClashURI == "" {
-		baseScheme, baseHostWithPort = s.getBaseSchemeAndHost(scheme, hostWithPort)
-	}
+	// Same base as the panel's Client Information page; s.address is the
+	// subscriber's host already normalized away from any loopback/bind IP.
+	base := s.settingService.BuildSubURIBase(s.address)
 
-	subURL = s.buildSingleURL(configuredSubURI, baseScheme, baseHostWithPort, subPath, subId)
-	subJsonURL = s.buildSingleURL(configuredSubJsonURI, baseScheme, baseHostWithPort, subJsonPath, subId)
-	subClashURL = s.buildSingleURL(configuredSubClashURI, baseScheme, baseHostWithPort, subClashPath, subId)
+	subURL = s.buildSingleURL(configuredSubURI, base, subPath, subId)
+	subJsonURL = s.buildSingleURL(configuredSubJsonURI, base, subJsonPath, subId)
+	subClashURL = s.buildSingleURL(configuredSubClashURI, base, subClashPath, subId)
 
 	return subURL, subJsonURL, subClashURL
 }
 
-// getBaseSchemeAndHost determines the base scheme and host from settings or falls back to request values
-func (s *SubService) getBaseSchemeAndHost(requestScheme, requestHostWithPort string) (string, string) {
-	subDomain, err := s.settingService.GetSubDomain()
-	if err != nil || subDomain == "" {
-		return requestScheme, requestHostWithPort
-	}
-
-	// Get port and TLS settings
-	subPort, _ := s.settingService.GetSubPort()
-	subKeyFile, _ := s.settingService.GetSubKeyFile()
-	subCertFile, _ := s.settingService.GetSubCertFile()
-
-	// Determine scheme from TLS configuration
-	scheme := "http"
-	if subKeyFile != "" && subCertFile != "" {
-		scheme = "https"
-	}
-
-	// Build host:port, always include port for clarity
-	hostWithPort := fmt.Sprintf("%s:%d", subDomain, subPort)
-
-	return scheme, hostWithPort
-}
-
 // buildSingleURL constructs a single URL using configured URI or base components
-func (s *SubService) buildSingleURL(configuredURI, baseScheme, baseHostWithPort, basePath, subId string) string {
+func (s *SubService) buildSingleURL(configuredURI, base, basePath, subId string) string {
 	if configuredURI != "" {
 		return s.joinPathWithID(configuredURI, subId)
 	}
-
-	baseURL := fmt.Sprintf("%s://%s", baseScheme, baseHostWithPort)
-	return s.joinPathWithID(baseURL+basePath, subId)
+	return s.joinPathWithID(base+basePath, subId)
 }
 
 // joinPathWithID safely joins a base path with a subscription ID

+ 38 - 0
sub/subService_test.go

@@ -61,6 +61,44 @@ func TestIsRoutableHost(t *testing.T) {
 	}
 }
 
+func TestResolveInboundAddress(t *testing.T) {
+	const reqHost = "sub.example.com"
+
+	// A subscriber reaches the panel through reqHost; the inbound's own
+	// bind Listen IP (loopback, private, or even a public secondary IP) is
+	// a server-side detail and must never become the link's connect host.
+	t.Run("bind listen IP must not leak into the link host", func(t *testing.T) {
+		s := &SubService{address: reqHost}
+		for _, listen := range []string{"127.0.0.1", "10.0.0.5", "192.168.1.10", "1.2.3.4", "0.0.0.0", "::", "::0", ""} {
+			ib := &model.Inbound{Listen: listen}
+			if got := s.resolveInboundAddress(ib); got != reqHost {
+				t.Fatalf("listen %q: address = %q, want %q (subscriber host, not bind IP)", listen, got, reqHost)
+			}
+		}
+	})
+
+	t.Run("node-managed inbound uses the node address", func(t *testing.T) {
+		id := 7
+		s := &SubService{
+			address:   reqHost,
+			nodesByID: map[int]*model.Node{7: {Id: 7, Address: "node7.example.com"}},
+		}
+		ib := &model.Inbound{NodeID: &id, Listen: "1.2.3.4"}
+		if got := s.resolveInboundAddress(ib); got != "node7.example.com" {
+			t.Fatalf("node-managed address = %q, want node7.example.com", got)
+		}
+	})
+
+	t.Run("node id with no known node falls back to subscriber host", func(t *testing.T) {
+		id := 9
+		s := &SubService{address: reqHost, nodesByID: map[int]*model.Node{}}
+		ib := &model.Inbound{NodeID: &id, Listen: "10.0.0.1"}
+		if got := s.resolveInboundAddress(ib); got != reqHost {
+			t.Fatalf("unknown-node address = %q, want subscriber host %q", got, reqHost)
+		}
+	})
+}
+
 func TestUnmarshalStreamSettings(t *testing.T) {
 	got := unmarshalStreamSettings(`{"network":"ws","wsSettings":{"path":"/api"}}`)
 	if got["network"] != "ws" {

+ 23 - 22
web/service/setting.go

@@ -908,6 +908,28 @@ func extractHostname(host string) string {
 	return "[" + h + "]"
 }
 
+// BuildSubURIBase is shared by GetDefaultSettings (the panel's Client
+// Information page) and the subscription page so both render subscription
+// URLs identically.
+func (s *SettingService) BuildSubURIBase(host string) string {
+	subPort, _ := s.GetSubPort()
+	subDomain, _ := s.GetSubDomain()
+	subKeyFile, _ := s.GetSubKeyFile()
+	subCertFile, _ := s.GetSubCertFile()
+	subTLS := subKeyFile != "" && subCertFile != ""
+	if subDomain == "" {
+		subDomain = extractHostname(host)
+	}
+	scheme := "http"
+	if subTLS {
+		scheme = "https"
+	}
+	if (subPort == 443 && subTLS) || (subPort == 80 && !subTLS) {
+		return scheme + "://" + subDomain
+	}
+	return fmt.Sprintf("%s://%s:%d", scheme, subDomain, subPort)
+}
+
 func (s *SettingService) GetDefaultSettings(host string) (any, error) {
 	type settingFunc func() (any, error)
 	settings := map[string]settingFunc{
@@ -953,32 +975,11 @@ func (s *SettingService) GetDefaultSettings(host string) (any, error) {
 		}
 	}
 	if (subEnable && result["subURI"].(string) == "") || (subJsonEnable && result["subJsonURI"].(string) == "") || (subClashEnable && result["subClashURI"].(string) == "") {
-		subURI := ""
+		subURI := s.BuildSubURIBase(host)
 		subTitle, _ := s.GetSubTitle()
-		subPort, _ := s.GetSubPort()
 		subPath, _ := s.GetSubPath()
 		subJsonPath, _ := s.GetSubJsonPath()
 		subClashPath, _ := s.GetSubClashPath()
-		subDomain, _ := s.GetSubDomain()
-		subKeyFile, _ := s.GetSubKeyFile()
-		subCertFile, _ := s.GetSubCertFile()
-		subTLS := false
-		if subKeyFile != "" && subCertFile != "" {
-			subTLS = true
-		}
-		if subDomain == "" {
-			subDomain = extractHostname(host)
-		}
-		if subTLS {
-			subURI = "https://"
-		} else {
-			subURI = "http://"
-		}
-		if (subPort == 443 && subTLS) || (subPort == 80 && !subTLS) {
-			subURI += subDomain
-		} else {
-			subURI += fmt.Sprintf("%s:%d", subDomain, subPort)
-		}
 		if subEnable && result["subURI"].(string) == "" {
 			result["subURI"] = subURI + subPath
 		}

+ 51 - 0
web/service/sub_uri_base_test.go

@@ -0,0 +1,51 @@
+package service
+
+import "testing"
+
+// BuildSubURIBase is the single source of truth for the scheme://host[:port]
+// prefix shown both on the panel's Client Information page and on the
+// subscription page. The cases pin scheme selection (sub TLS cert/key),
+// Sub Domain preference, standard-port omission, and IPv6 bracketing.
+func TestBuildSubURIBase(t *testing.T) {
+	setupConflictDB(t)
+	s := &SettingService{}
+
+	set := func(subDomain, port, cert, key string) {
+		if err := s.saveSetting("subDomain", subDomain); err != nil {
+			t.Fatalf("set subDomain: %v", err)
+		}
+		if err := s.saveSetting("subPort", port); err != nil {
+			t.Fatalf("set subPort: %v", err)
+		}
+		if err := s.saveSetting("subCertFile", cert); err != nil {
+			t.Fatalf("set subCertFile: %v", err)
+		}
+		if err := s.saveSetting("subKeyFile", key); err != nil {
+			t.Fatalf("set subKeyFile: %v", err)
+		}
+	}
+
+	cases := []struct {
+		name                    string
+		subDomain, port         string
+		cert, key               string
+		host                    string
+		want                    string
+	}{
+		{"no domain, plain, non-standard port", "", "2096", "", "", "panel.example.com", "http://panel.example.com:2096"},
+		{"host carries a port — stripped, sub port applied", "", "2096", "", "", "panel.example.com:9999", "http://panel.example.com:2096"},
+		{"sub domain preferred over host", "sub.cdn.com", "2096", "", "", "panel.example.com", "http://sub.cdn.com:2096"},
+		{"tls + 443 omits the port", "sub.cdn.com", "443", "/c.crt", "/k.key", "panel.example.com", "https://sub.cdn.com"},
+		{"plain + 80 omits the port", "", "80", "", "", "x.com", "http://x.com"},
+		{"tls on a non-standard port keeps it", "", "2096", "/c.crt", "/k.key", "x.com", "https://x.com:2096"},
+		{"ipv6 host is bracketed", "", "2096", "", "", "::1", "http://[::1]:2096"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			set(c.subDomain, c.port, c.cert, c.key)
+			if got := s.BuildSubURIBase(c.host); got != c.want {
+				t.Fatalf("BuildSubURIBase(%q) = %q, want %q", c.host, got, c.want)
+			}
+		})
+	}
+}