diff --git a/common/protocol/http/headers.go b/common/protocol/http/headers.go index db3aa670..61a07005 100644 --- a/common/protocol/http/headers.go +++ b/common/protocol/http/headers.go @@ -1,25 +1,41 @@ package http import ( + "context" "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(header http.Header) []net.Address { - xff := header.Get("X-Forwarded-For") - if xff == "" { - return nil +// ApplyTrustedXForwardedFor returns remoteAddr overridden by X-Forwarded-For only when a configured trusted header is present. +func ApplyTrustedXForwardedFor(header http.Header, trusted []string, remoteAddr net.Addr) net.Addr { + value := header.Get("X-Forwarded-For") + if value == "" { + return remoteAddr } - list := strings.Split(xff, ",") - addrs := make([]net.Address, 0, len(list)) - for _, proxy := range list { - addrs = append(addrs, net.ParseAddress(proxy)) + for _, t := range trusted { + if len(header.Values(t)) > 0 { + if idx := strings.IndexByte(value, ','); idx >= 0 { + value = value[:idx] + } + if addr := net.ParseAddress(value); addr.Family().IsIP() { + return &net.TCPAddr{ + IP: addr.IP(), + Port: 0, + } + } + return remoteAddr + } } - return addrs + if len(trusted) == 0 { + errors.LogWarning(context.Background(), `received "X-Forwarded-For" from `, remoteAddr, ` 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 `, remoteAddr, `: `, value) + } + return remoteAddr } // RemoveHopByHopHeaders removes hop by hop headers in http header list. diff --git a/common/protocol/http/headers_test.go b/common/protocol/http/headers_test.go index 80e243ae..bad395d0 100644 --- a/common/protocol/http/headers_test.go +++ b/common/protocol/http/headers_test.go @@ -2,23 +2,48 @@ package http_test import ( "bufio" + gonet "net" "net/http" "strings" "testing" - "github.com/google/go-cmp/cmp" "github.com/xtls/xray-core/common" "github.com/xtls/xray-core/common/net" . "github.com/xtls/xray-core/common/protocol/http" ) -func TestParseXForwardedFor(t *testing.T) { - header := http.Header{} - header.Add("X-Forwarded-For", "129.78.138.66, 129.78.64.103") - addrs := ParseXForwardedFor(header) - if r := cmp.Diff(addrs, []net.Address{net.ParseAddress("129.78.138.66"), net.ParseAddress("129.78.64.103")}); r != "" { - t.Error(r) - } +func TestApplyTrustedXForwardedFor(t *testing.T) { + remoteAddr := &gonet.TCPAddr{IP: gonet.ParseIP("127.0.0.1"), Port: 12345} + + t.Run("ignore X-Forwarded-For without trusted header", func(t *testing.T) { + header := http.Header{} + header.Add("X-Forwarded-For", "129.78.138.66, 129.78.64.103") + + if addr := ApplyTrustedXForwardedFor(header, nil, remoteAddr); addr != remoteAddr { + t.Fatalf("unexpected remote address: %v", addr) + } + }) + + t.Run("trust X-Forwarded-For", func(t *testing.T) { + header := http.Header{} + header.Add("X-Forwarded-For", "129.78.138.66, 129.78.64.103") + header.Add("X-Trusted-CDN", "") + + addr := ApplyTrustedXForwardedFor(header, []string{"X-Trusted-CDN"}, remoteAddr) + if addr.String() != "129.78.138.66:0" { + t.Fatalf("unexpected remote address: %v", addr) + } + }) + + t.Run("ignore non-IP X-Forwarded-For", func(t *testing.T) { + header := http.Header{} + header.Add("X-Forwarded-For", "example.com") + header.Add("X-Trusted-CDN", "") + + if addr := ApplyTrustedXForwardedFor(header, []string{"X-Trusted-CDN"}, remoteAddr); addr != remoteAddr { + t.Fatalf("unexpected remote address: %v", addr) + } + }) } func TestHopByHopHeadersRemoving(t *testing.T) { diff --git a/infra/conf/xray.go b/infra/conf/xray.go index 3e6e4371..c8ff1627 100644 --- a/infra/conf/xray.go +++ b/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`) diff --git a/transport/internet/grpc/dial.go b/transport/internet/grpc/dial.go index 9ffbbfd1..4883f30a 100644 --- a/transport/internet/grpc/dial.go +++ b/transport/internet/grpc/dial.go @@ -62,7 +62,7 @@ func dialgRPC(ctx context.Context, dest net.Destination, streamSettings *interne if err != nil { return nil, errors.New("Cannot dial gRPC").Base(err) } - return encoding.NewMultiHunkConn(grpcService, nil), nil + return encoding.NewMultiHunkConn(grpcService, nil, nil), nil } errors.LogDebug(ctx, "using gRPC tun mode service name: `"+grpcSettings.getServiceName()+"` stream name: `"+grpcSettings.getTunStreamName()+"`") @@ -71,7 +71,7 @@ func dialgRPC(ctx context.Context, dest net.Destination, streamSettings *interne return nil, errors.New("Cannot dial gRPC").Base(err) } - return encoding.NewHunkConn(grpcService, nil), nil + return encoding.NewHunkConn(grpcService, nil, nil), nil } func getGrpcClient(ctx context.Context, dest net.Destination, streamSettings *internet.MemoryStreamConfig) (*grpc.ClientConn, error) { diff --git a/transport/internet/grpc/encoding/hunkconn.go b/transport/internet/grpc/encoding/hunkconn.go index f1155de8..92769065 100644 --- a/transport/internet/grpc/encoding/hunkconn.go +++ b/transport/internet/grpc/encoding/hunkconn.go @@ -9,8 +9,6 @@ import ( "github.com/xtls/xray-core/common/net" "github.com/xtls/xray-core/common/net/cnc" "github.com/xtls/xray-core/common/signal/done" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/peer" ) type HunkConn interface { @@ -38,31 +36,8 @@ func NewHunkReadWriter(hc HunkConn, cancel context.CancelFunc) *HunkReaderWriter return &HunkReaderWriter{hc, cancel, done.New(), nil, 0} } -func NewHunkConn(hc HunkConn, cancel context.CancelFunc) net.Conn { - var rAddr net.Addr - pr, ok := peer.FromContext(hc.Context()) - if ok { - rAddr = pr.Addr - } else { - rAddr = &net.TCPAddr{ - IP: []byte{0, 0, 0, 0}, - Port: 0, - } - } - - md, ok := metadata.FromIncomingContext(hc.Context()) - if ok { - header := md.Get("x-real-ip") - if len(header) > 0 { - realip := net.ParseAddress(header[0]) - if realip.Family().IsIP() { - rAddr = &net.TCPAddr{ - IP: realip.IP(), - Port: 0, - } - } - } - } +func NewHunkConn(hc HunkConn, cancel context.CancelFunc, trustedXForwardedFor []string) net.Conn { + rAddr := remoteAddrFromContext(hc.Context(), trustedXForwardedFor) wrc := NewHunkReadWriter(hc, cancel) return cnc.NewConnection( cnc.ConnectionInput(wrc), diff --git a/transport/internet/grpc/encoding/multiconn.go b/transport/internet/grpc/encoding/multiconn.go index fa95cba8..f27a1c35 100644 --- a/transport/internet/grpc/encoding/multiconn.go +++ b/transport/internet/grpc/encoding/multiconn.go @@ -3,15 +3,12 @@ package encoding import ( "context" "io" - "net" "github.com/xtls/xray-core/common/buf" "github.com/xtls/xray-core/common/errors" - xnet "github.com/xtls/xray-core/common/net" + "github.com/xtls/xray-core/common/net" "github.com/xtls/xray-core/common/net/cnc" "github.com/xtls/xray-core/common/signal/done" - "google.golang.org/grpc/metadata" - "google.golang.org/grpc/peer" ) type MultiHunkConn interface { @@ -34,31 +31,8 @@ func NewMultiHunkReadWriter(hc MultiHunkConn, cancel context.CancelFunc) *MultiH return &MultiHunkReaderWriter{hc, cancel, done.New(), nil} } -func NewMultiHunkConn(hc MultiHunkConn, cancel context.CancelFunc) net.Conn { - var rAddr net.Addr - pr, ok := peer.FromContext(hc.Context()) - if ok { - rAddr = pr.Addr - } else { - rAddr = &net.TCPAddr{ - IP: []byte{0, 0, 0, 0}, - Port: 0, - } - } - - md, ok := metadata.FromIncomingContext(hc.Context()) - if ok { - header := md.Get("x-real-ip") - if len(header) > 0 { - realip := xnet.ParseAddress(header[0]) - if realip.Family().IsIP() { - rAddr = &net.TCPAddr{ - IP: realip.IP(), - Port: 0, - } - } - } - } +func NewMultiHunkConn(hc MultiHunkConn, cancel context.CancelFunc, trustedXForwardedFor []string) net.Conn { + rAddr := remoteAddrFromContext(hc.Context(), trustedXForwardedFor) wrc := NewMultiHunkReadWriter(hc, cancel) return cnc.NewConnection( cnc.ConnectionInputMulti(wrc), diff --git a/transport/internet/grpc/encoding/remoteaddr.go b/transport/internet/grpc/encoding/remoteaddr.go new file mode 100644 index 00000000..a28d9193 --- /dev/null +++ b/transport/internet/grpc/encoding/remoteaddr.go @@ -0,0 +1,58 @@ +package encoding + +import ( + "context" + "strings" + + "github.com/xtls/xray-core/common/errors" + "github.com/xtls/xray-core/common/net" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" +) + +func remoteAddrFromContext(ctx context.Context, trusted []string) net.Addr { + var remoteAddr net.Addr + if pr, ok := peer.FromContext(ctx); ok { + remoteAddr = pr.Addr + } else { + remoteAddr = &net.TCPAddr{ + IP: []byte{0, 0, 0, 0}, + Port: 0, + } + } + + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return remoteAddr + } + + if forwardedAddr := parseTrustedXForwardedFor(md, trusted, remoteAddr); forwardedAddr != nil && forwardedAddr.Family().IsIP() { + remoteAddr = &net.TCPAddr{ + IP: forwardedAddr.IP(), + Port: 0, + } + } + return remoteAddr +} + +func parseTrustedXForwardedFor(md metadata.MD, trusted []string, remoteAddr net.Addr) net.Address { + values := md.Get("X-Forwarded-For") + if len(values) == 0 || values[0] == "" { + return nil + } + value := values[0] + for _, t := range trusted { + if len(md.Get(t)) > 0 { + if idx := strings.IndexByte(value, ','); idx >= 0 { + value = value[:idx] + } + return net.ParseAddress(value) + } + } + if len(trusted) == 0 { + errors.LogWarning(context.Background(), `received "X-Forwarded-For" from `, remoteAddr, ` 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 `, remoteAddr, `: `, value) + } + return nil +} diff --git a/transport/internet/grpc/encoding/remoteaddr_test.go b/transport/internet/grpc/encoding/remoteaddr_test.go new file mode 100644 index 00000000..81e40aa9 --- /dev/null +++ b/transport/internet/grpc/encoding/remoteaddr_test.go @@ -0,0 +1,53 @@ +package encoding + +import ( + "context" + "net" + "testing" + + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/peer" +) + +func TestRemoteAddrFromContext(t *testing.T) { + tests := []struct { + name string + metadata metadata.MD + trustedXForwardedFor []string + expectedRemoteAddress string + }{ + { + name: "trust X-Forwarded-For when configured", + metadata: metadata.Pairs("X-Forwarded-For", "2.2.2.2, 3.3.3.3"), + trustedXForwardedFor: []string{"X-Forwarded-For"}, + expectedRemoteAddress: "2.2.2.2:0", + }, + { + name: "trust X-Forwarded-For with trusted marker", + metadata: metadata.Pairs("X-Forwarded-For", "4.4.4.4", "X-Trusted-CDN", "1"), + trustedXForwardedFor: []string{"X-Trusted-CDN"}, + expectedRemoteAddress: "4.4.4.4:0", + }, + { + name: "ignore X-Forwarded-For without trusted marker", + metadata: metadata.Pairs("X-Forwarded-For", "5.5.5.5"), + trustedXForwardedFor: []string{"X-Trusted-CDN"}, + expectedRemoteAddress: "127.0.0.1:12345", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + ctx := peer.NewContext(metadata.NewIncomingContext(context.Background(), test.metadata), &peer.Peer{ + Addr: &net.TCPAddr{ + IP: net.ParseIP("127.0.0.1"), + Port: 12345, + }, + }) + remoteAddr := remoteAddrFromContext(ctx, test.trustedXForwardedFor) + if remoteAddr.String() != test.expectedRemoteAddress { + t.Fatalf("unexpected remote address: %s", remoteAddr.String()) + } + }) + } +} diff --git a/transport/internet/grpc/hub.go b/transport/internet/grpc/hub.go index 34f8f1c0..91bd2ab6 100644 --- a/transport/internet/grpc/hub.go +++ b/transport/internet/grpc/hub.go @@ -19,24 +19,25 @@ import ( type Listener struct { encoding.UnimplementedGRPCServiceServer - ctx context.Context - handler internet.ConnHandler - local net.Addr - config *Config + ctx context.Context + handler internet.ConnHandler + local net.Addr + config *Config + trustedXForwardedFor []string s *grpc.Server } func (l Listener) Tun(server encoding.GRPCService_TunServer) error { tunCtx, cancel := context.WithCancel(l.ctx) - l.handler(encoding.NewHunkConn(server, cancel)) + l.handler(encoding.NewHunkConn(server, cancel, l.trustedXForwardedFor)) <-tunCtx.Done() return nil } func (l Listener) TunMulti(server encoding.GRPCService_TunMultiServer) error { tunCtx, cancel := context.WithCancel(l.ctx) - l.handler(encoding.NewMultiHunkConn(server, cancel)) + l.handler(encoding.NewMultiHunkConn(server, cancel, l.trustedXForwardedFor)) <-tunCtx.Done() return nil } @@ -74,6 +75,9 @@ func Listen(ctx context.Context, address net.Address, port net.Port, settings *i } listener.ctx = ctx + if settings.SocketSettings != nil { + listener.trustedXForwardedFor = settings.SocketSettings.TrustedXForwardedFor + } config := tls.ConfigFromStreamSettings(settings) diff --git a/transport/internet/httpupgrade/httpupgrade_test.go b/transport/internet/httpupgrade/httpupgrade_test.go index c0310b95..5b420ed6 100644 --- a/transport/internet/httpupgrade/httpupgrade_test.go +++ b/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() diff --git a/transport/internet/httpupgrade/hub.go b/transport/internet/httpupgrade/hub.go index 8e70ad08..cb9a79d9 100644 --- a/transport/internet/httpupgrade/hub.go +++ b/transport/internet/httpupgrade/hub.go @@ -80,24 +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() - if len(forwardedAddrs) > 0 && forwardedAddrs[0].Family().IsIP() { - remoteAddr = &net.TCPAddr{ - IP: forwardedAddrs[0].IP(), - Port: int(0), - } + var trustedXFF []string + if s.socketSettings != nil { + trustedXFF = s.socketSettings.TrustedXForwardedFor } + remoteAddr = http_proto.ApplyTrustedXForwardedFor(req.Header, trustedXFF, remoteAddr) return stat.Connection(newConnection(conn, remoteAddr)), nil } diff --git a/transport/internet/splithttp/hub.go b/transport/internet/splithttp/hub.go index 535549bf..a28866cf 100644 --- a/transport/internet/splithttp/hub.go +++ b/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,12 +170,11 @@ func (h *requestHandler) ServeHTTP(writer http.ResponseWriter, request *http.Req Port: remoteAddr.(*net.TCPAddr).Port, } } - if len(forwardedAddrs) > 0 && forwardedAddrs[0].Family().IsIP() { - remoteAddr = &net.TCPAddr{ - IP: forwardedAddrs[0].IP(), - Port: 0, - } + var trustedXFF []string + if h.socketSettings != nil { + trustedXFF = h.socketSettings.TrustedXForwardedFor } + remoteAddr = http_proto.ApplyTrustedXForwardedFor(request.Header, trustedXFF, remoteAddr) var currentSession *httpSession if sessionId != "" { diff --git a/transport/internet/splithttp/splithttp_test.go b/transport/internet/splithttp/splithttp_test.go index ab02b619..62ca858f 100644 --- a/transport/internet/splithttp/splithttp_test.go +++ b/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() diff --git a/transport/internet/websocket/hub.go b/transport/internet/websocket/hub.go index 73799174..42dba880 100644 --- a/transport/internet/websocket/hub.go +++ b/transport/internet/websocket/hub.go @@ -65,24 +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() - if len(forwardedAddrs) > 0 && forwardedAddrs[0].Family().IsIP() { - remoteAddr = &net.TCPAddr{ - IP: forwardedAddrs[0].IP(), - Port: int(0), - } + var trustedXFF []string + if h.socketSettings != nil { + trustedXFF = h.socketSettings.TrustedXForwardedFor } + remoteAddr = http_proto.ApplyTrustedXForwardedFor(request.Header, trustedXFF, remoteAddr) h.ln.addConn(NewConnection(conn, remoteAddr, extraReader, h.ln.config.HeartbeatPeriod)) } diff --git a/transport/internet/websocket/ws_test.go b/transport/internet/websocket/ws_test.go index aab28af6..08cd2751 100644 --- a/transport/internet/websocket/ws_test.go +++ b/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()