HTTP transports: Require trustedXForwardedFor for X-Forwarded-For

common/protocol/http/headers.go

@@ -1,19 +1,17 @@
 package http
 
 import (
+	"context"
+	gonet "net"
 	"net/http"
 	"strconv"
 	"strings"
 
+	"github.com/xtls/xray-core/common/errors"
 	"github.com/xtls/xray-core/common/net"
 )
 
-// ParseXForwardedFor parses X-Forwarded-For header in http headers, and return the IP list in it.
+func parseXForwardedFor(xff string) []net.Address {
-func ParseXForwardedFor(header http.Header) []net.Address {
-	xff := header.Get("X-Forwarded-For")
-	if xff == "" {
-		return nil
-	}
 	list := strings.Split(xff, ",")
 	addrs := make([]net.Address, 0, len(list))
 	for _, proxy := range list {
@@ -22,6 +20,25 @@ func ParseXForwardedFor(header http.Header) []net.Address {
 	return addrs
 }
 
+// ParseTrustedXForwardedFor parses X-Forwarded-For only when a configured trusted header is present.
+func ParseTrustedXForwardedFor(header http.Header, trusted []string, clientAddr gonet.Addr) []net.Address {
+	xff := header.Get("X-Forwarded-For")
+	if xff == "" {
+		return nil
+	}
+	for _, key := range trusted {
+		if len(header.Values(key)) > 0 {
+			return parseXForwardedFor(xff)
+		}
+	}
+	if len(trusted) == 0 {
+		errors.LogWarning(context.Background(), `received "X-Forwarded-For" from `, clientAddr, ` but "sockopt.trustedXForwardedFor" is not configured; ignoring it and using the real remote address`)
+	} else {
+		errors.LogError(context.Background(), `ignored potentially forged "X-Forwarded-For" from `, clientAddr, `: `, xff)
+	}
+	return nil
+}
+
 // RemoveHopByHopHeaders removes hop by hop headers in http header list.
 func RemoveHopByHopHeaders(header http.Header) {
 	// Strip hop-by-hop header based on RFC:

common/protocol/http/headers_test.go

@@ -2,6 +2,7 @@ package http_test
 
 import (
 	"bufio"
+	gonet "net"
 	"net/http"
 	"strings"
 	"testing"
@@ -12,10 +13,17 @@ import (
 	. "github.com/xtls/xray-core/common/protocol/http"
 )
 
-func TestParseXForwardedFor(t *testing.T) {
+func TestParseTrustedXForwardedFor(t *testing.T) {
 	header := http.Header{}
 	header.Add("X-Forwarded-For", "129.78.138.66, 129.78.64.103")
-	addrs := ParseXForwardedFor(header)
+	clientAddr := &gonet.TCPAddr{IP: gonet.ParseIP("127.0.0.1"), Port: 12345}
+
+	if addrs := ParseTrustedXForwardedFor(header, nil, clientAddr); addrs != nil {
+		t.Fatalf("unexpected trusted X-Forwarded-For: %v", addrs)
+	}
+
+	header.Add("X-Trusted-CDN", "")
+	addrs := ParseTrustedXForwardedFor(header, []string{"X-Trusted-CDN"}, clientAddr)
 	if r := cmp.Diff(addrs, []net.Address{net.ParseAddress("129.78.138.66"), net.ParseAddress("129.78.64.103")}); r != "" {
 		t.Error(r)
 	}

infra/conf/xray.go

@@ -173,27 +173,6 @@ func (c *InboundDetourConfig) Build() (*core.InboundHandlerConfig, error) {
 			return nil, err
 		}
 		receiverSettings.StreamSettings = ss
-		// TODO: Actually implement this breaking change
-		protocol := ss.GetEffectiveProtocol()
-		if (protocol == "websocket" || protocol == "httpupgrade" || protocol == "splithttp") &&
-			(c.StreamSetting.SocketSettings == nil || len(c.StreamSetting.SocketSettings.TrustedXForwardedFor) == 0) {
-			errors.LogWarning(
-				context.Background(),
-				`====== SECURITY WARNING ======`,
-				"\n",
-				`inbound "`, c.Tag, `" using `, protocol, ` has not configured "sockopt.trustedXForwardedFor".`,
-				"\n",
-				`THIS IS VERY INSECURE!!!`,
-				"\n",
-				`For compatibility, Xray still allows this for now and still trusts X-Forwarded-For implicitly.`,
-				"\n",
-				`Please configure "sockopt.trustedXForwardedFor" immediately.`,
-				"\n",
-				`In future versions, this option must be explicitly set.`,
-				"\n",
-				`====== SECURITY WARNING ======`,
-			)
-		}
 		if strings.Contains(ss.SecurityType, "reality") && (receiverSettings.PortList == nil ||
 			len(receiverSettings.PortList.Ports()) != 1 || receiverSettings.PortList.Ports()[0] != 443) {
 			errors.LogWarning(context.Background(), `REALITY: Listening on non-443 ports may get your IP blocked by the GFW`)

transport/internet/httpupgrade/httpupgrade_test.go

@@ -138,6 +138,9 @@ func TestDialWithRemoteAddr(t *testing.T) {
 		ProtocolSettings: &Config{
 			Path: "httpupgrade",
 		},
+		SocketSettings: &internet.SocketConfig{
+			TrustedXForwardedFor: []string{"X-Forwarded-For"},
+		},
 	}, func(conn stat.Connection) {
 		go func(c stat.Connection) {
 			defer c.Close()

transport/internet/httpupgrade/hub.go

@@ -80,18 +80,12 @@ func (s *server) upgrade(conn net.Conn) (stat.Connection, error) {
 		return nil, err
 	}
 
-	var forwardedAddrs []net.Address
-	if s.socketSettings != nil && len(s.socketSettings.TrustedXForwardedFor) > 0 {
-		for _, key := range s.socketSettings.TrustedXForwardedFor {
-			if len(req.Header.Values(key)) > 0 {
-				forwardedAddrs = http_proto.ParseXForwardedFor(req.Header)
-				break
-			}
-		}
-	} else {
-		forwardedAddrs = http_proto.ParseXForwardedFor(req.Header)
-	}
 	remoteAddr := conn.RemoteAddr()
+	var trustedXFF []string
+	if s.socketSettings != nil {
+		trustedXFF = s.socketSettings.TrustedXForwardedFor
+	}
+	forwardedAddrs := http_proto.ParseTrustedXForwardedFor(req.Header, trustedXFF, remoteAddr)
 	if len(forwardedAddrs) > 0 && forwardedAddrs[0].Family().IsIP() {
 		remoteAddr = &net.TCPAddr{
 			IP:   forwardedAddrs[0].IP(),

transport/internet/splithttp/hub.go

@@ -155,17 +155,6 @@ func (h *requestHandler) ServeHTTP(writer http.ResponseWriter, request *http.Req
 		return
 	}
 
-	var forwardedAddrs []net.Address
-	if h.socketSettings != nil && len(h.socketSettings.TrustedXForwardedFor) > 0 {
-		for _, key := range h.socketSettings.TrustedXForwardedFor {
-			if len(request.Header.Values(key)) > 0 {
-				forwardedAddrs = http_proto.ParseXForwardedFor(request.Header)
-				break
-			}
-		}
-	} else {
-		forwardedAddrs = http_proto.ParseXForwardedFor(request.Header)
-	}
 	var remoteAddr net.Addr
 	var err error
 	remoteAddr, err = net.ResolveTCPAddr("tcp", request.RemoteAddr)
@@ -181,6 +170,11 @@ func (h *requestHandler) ServeHTTP(writer http.ResponseWriter, request *http.Req
 			Port: remoteAddr.(*net.TCPAddr).Port,
 		}
 	}
+	var trustedXFF []string
+	if h.socketSettings != nil {
+		trustedXFF = h.socketSettings.TrustedXForwardedFor
+	}
+	forwardedAddrs := http_proto.ParseTrustedXForwardedFor(request.Header, trustedXFF, remoteAddr)
 	if len(forwardedAddrs) > 0 && forwardedAddrs[0].Family().IsIP() {
 		remoteAddr = &net.TCPAddr{
 			IP:   forwardedAddrs[0].IP(),

transport/internet/splithttp/splithttp_test.go

@@ -88,6 +88,9 @@ func TestDialWithRemoteAddr(t *testing.T) {
 		ProtocolSettings: &Config{
 			Path: "sh",
 		},
+		SocketSettings: &internet.SocketConfig{
+			TrustedXForwardedFor: []string{"X-Forwarded-For"},
+		},
 	}, func(conn stat.Connection) {
 		go func(c stat.Connection) {
 			defer c.Close()

transport/internet/websocket/hub.go

@@ -65,18 +65,12 @@ func (h *requestHandler) ServeHTTP(writer http.ResponseWriter, request *http.Req
 		return
 	}
 
-	var forwardedAddrs []net.Address
-	if h.socketSettings != nil && len(h.socketSettings.TrustedXForwardedFor) > 0 {
-		for _, key := range h.socketSettings.TrustedXForwardedFor {
-			if len(request.Header.Values(key)) > 0 {
-				forwardedAddrs = http_proto.ParseXForwardedFor(request.Header)
-				break
-			}
-		}
-	} else {
-		forwardedAddrs = http_proto.ParseXForwardedFor(request.Header)
-	}
 	remoteAddr := conn.RemoteAddr()
+	var trustedXFF []string
+	if h.socketSettings != nil {
+		trustedXFF = h.socketSettings.TrustedXForwardedFor
+	}
+	forwardedAddrs := http_proto.ParseTrustedXForwardedFor(request.Header, trustedXFF, remoteAddr)
 	if len(forwardedAddrs) > 0 && forwardedAddrs[0].Family().IsIP() {
 		remoteAddr = &net.TCPAddr{
 			IP:   forwardedAddrs[0].IP(),

transport/internet/websocket/ws_test.go

@@ -79,6 +79,9 @@ func TestDialWithRemoteAddr(t *testing.T) {
 		ProtocolSettings: &Config{
 			Path: "ws",
 		},
+		SocketSettings: &internet.SocketConfig{
+			TrustedXForwardedFor: []string{"X-Forwarded-For"},
+		},
 	}, func(conn stat.Connection) {
 		go func(c stat.Connection) {
 			defer c.Close()