SOCKS5 UDP route issue

[dyhkwong]

Operating system

Windows

System version

Windows 11 26100.2454

Installation type

Original sing-box Command Line

If you are using a graphical client, please provide the version of the client.

No response

Version

sing-box version 1.11.0-beta.8

Environment: go1.23.4 windows/amd64
Tags: with_gvisor,with_dhcp,with_clash_api,with_quic,with_utls
Revision: 8a138e34ccac59295e90a3885e8225631aa26311
CGO: disabled

Description

Since the route refactoring of v1.11, SOCKS5 UDP can't be route correctly. Previously only scenario in #1370 is affected, but this gets worse since version 1.11.

https://github.com/SagerNet/sing/blob/809d8eca139712f6c833cea813674a1cb1154ba5/protocol/socks/handshake.go#L270
A line of destination = request.Destination is missing. However, this violated related RFCs, and a correct fix should always fill the destination with 0.0.0.0:0 as described in #1370.

sing-box/route/route.go

Lines 464 to 475 in 8a138e3

	if !preMatch && metadata.Destination.Addr.IsUnspecified() {
	newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, &rule.RuleActionSniff{}, inputConn, inputPacketConn)
	if newErr != nil {
	fatalErr = newErr
	return
	}
	if newBuffer != nil {
	buffers = append(buffers, newBuffer)
	} else if len(newPacketBuffers) > 0 {
	packetBuffers = append(packetBuffers, newPacketBuffers...)
	}
	}

These lines are for handling 0.0.0.0:0, but they should be moved to a location before :match.

sing-box/route/route.go

Line 368 in 8a138e3

match:

Reproduction

Use software that support SOCKS5 UDP ASSOCIATE to test. e.g. configure NATTypeTester to use SOCKS5 proxy 127.0.0.1:1080 and do UDP STUN test.

{
    "log": {
        "level": "debug"
    },
    "inbounds": [
        {
            "type": "socks",
            "listen": "127.0.0.1",
            "listen_port": 1080,
        }
    ],
    "outbounds": [
        {
            "type": "direct"
        }
    ],
    "route": {
        "rules": [
            {
                "ip_cidr": [
                    "0.0.0.0/0"
                ],
                "action": "reject"
            }
        ]
    }
}

Logs

>sing-box.exe run -c config.json
INFO[0000] network: updated default interface WLAN, index 15
INFO[0000] inbound/socks[0]: tcp server started at 127.0.0.1:1080
INFO[0000] sing-box started (0.11s)
INFO[0021] [4133872565 0ms] inbound/socks[0]: inbound connection from 127.0.0.1:62351
INFO[0021] [4133872565 15ms] inbound/socks[0]: inbound packet connection to :0
INFO[0021] [4133872565 15ms] outbound/direct[0]: outbound packet connection
INFO[0043] [564257197 0ms] inbound/socks[0]: inbound connection from 127.0.0.1:62470
INFO[0043] [564257197 14ms] inbound/socks[0]: inbound packet connection to :0
INFO[0043] [564257197 14ms] outbound/direct[0]: outbound packet connection

Supporter

• I am a sponsor

Integrity requirements

• I confirm that I have read the documentation, understand the meaning of all the configuration items I wrote, and did not pile up seemingly useful options or default values.
• I confirm that I have provided the server and client configuration files and process that can be reproduced locally, instead of a complicated client configuration file that has been stripped of sensitive data.
• I confirm that I have provided the simplest configuration that can be used to reproduce the error I reported, instead of depending on remote servers, TUN, graphical interface clients, or other closed-source software.
• I confirm that I have provided the complete configuration files and logs, rather than just providing parts I think are useful out of confidence in my own intelligence.

[mazzz1y]

The same issue in v1.11.0-beta.9:

config.json

{
  "log": {
    "disabled": false,
    "level": "debug"
  },
  "inbounds": [
    {
      "type": "mixed",
      "tag": "mixed-in",
      "listen": "127.0.0.1",
      "listen_port": 10888
    }
  ],
  "outbounds": [
    {
      "type": "direct",
      "tag": "direct-out"
    }
  ],
  "route": {
    "auto_detect_interface": true,
    "final": "direct-out",
    "rules": [
      {
        "port": 53,
        "action": "reject"
      }
    ]
  }
}

Here I'm sending a DNS request over a SOCKS proxy:

» docker run --net=host -v $PWD/config.json:/config.json -it ghcr.io/sagernet/sing-box:v1.11.0-beta.9 -c /config.json run
INFO[0000] network: updated default interface wlp0s20f3, index 2
INFO[0000] inbound/mixed[mixed-in]: tcp server started at 127.0.0.1:10888
INFO[0000] sing-box started (0.00s)
INFO[0002] [3058626039 0ms] inbound/mixed[mixed-in]: inbound connection from 127.0.0.1:41076
INFO[0002] [3058626039 0ms] inbound/mixed[mixed-in]: inbound packet connection to :0
DEBUG[0002] [3058626039 0ms] router: sniffed packet protocol: dns
INFO[0002] [3058626039 1ms] outbound/direct[direct-out]: outbound packet connection
ERROR[0002] [3058626039 1ms] inbound/mixed[mixed-in]: process connection from 127.0.0.1:41076: invalid argument

» docker run --net=host -v $PWD/config.json:/config.json -it ghcr.io/sagernet/sing-box:v1.11.0-beta.8 -c /config.json run
INFO[0000] network: updated default interface wlp0s20f3, index 2
INFO[0000] inbound/mixed[mixed-in]: tcp server started at 127.0.0.1:10888
INFO[0000] sing-box started (0.00s)
INFO[0001] [1992170862 0ms] inbound/mixed[mixed-in]: inbound connection from 127.0.0.1:42362
INFO[0001] [1992170862 0ms] inbound/mixed[mixed-in]: inbound packet connection to :0
INFO[0001] [1992170862 0ms] outbound/direct[direct-out]: outbound packet connection

I see two differences between these two versions: in beta 9, I see an "invalid arguments" error, and the sniffer detected a DNS packet, even though I didn't enable it.

[mazzz1y]

Behavior in v1.11.0-beta.11 hasn't changed. It currently looks impossible to create a custom route for UDP packets. The destination address is missing in the metadata.

config.json

{
  "log": {
    "disabled": false,
    "level": "debug"
  },
  "inbounds": [
    {
      "type": "mixed",
      "tag": "mixed-in",
      "listen": "127.0.0.1",
      "listen_port": 10888
    }
  ],
  "outbounds": [
    {
      "type": "direct",
      "tag": "direct-out"
    }
  ],
  "route": {
    "auto_detect_interface": true,
    "final": "direct-out",
    "rules": [
      {
        "port": 53,
        "action": "reject"
      }
    ]
  }
}

Test script

# virtualenv venv
# source ./venv/bin/activate
# pip install pysocks dnslib
# python main.py

import socket
import socks
import dnslib

socks.set_default_proxy(socks.SOCKS5, "127.0.0.1", 10888)
socket.socket = socks.socksocket

def send_dns_request(domain, dns_server="8.8.8.8"):
    request = dnslib.DNSRecord.question(domain)

    sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
    sock.sendto(request.pack(), (dns_server, 53))

    response, _ = sock.recvfrom(1024)
    dns_response = dnslib.DNSRecord.parse(response)
    
    return dns_response

response = send_dns_request("example.com")
print(response)

» docker run --net=host -v $PWD/config.json:/config.json -it ghcr.io/sagernet/sing-box:v1.11.0-beta.11 -c /config.json run
INFO[0000] network: updated default interface wlp0s20f3, index 2
INFO[0000] inbound/mixed[mixed-in]: tcp server started at 127.0.0.1:10888
INFO[0000] sing-box started (0.00s)
INFO[0007] [1026707425 0ms] inbound/mixed[mixed-in]: inbound connection from 127.0.0.1:51940
INFO[0007] [1026707425 0ms] inbound/mixed[mixed-in]: inbound packet connection
DEBUG[0007] [1026707425 1ms] router: sniffed packet protocol: dns
INFO[0007] [1026707425 1ms] outbound/direct[direct-out]: outbound packet connection

[dyhkwong]

sing-box/route/route.go

Lines 474 to 489 in a3278af

	if !preMatch && inputPacketConn != nil && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() {
	var timeout time.Duration
	if metadata.InboundType == C.TypeSOCKS {
	timeout = C.TCPTimeout
	}
	newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, &rule.RuleActionSniff{Timeout: timeout}, inputConn, inputPacketConn)
	if newErr != nil {
	fatalErr = newErr
	return
	}
	if newBuffer != nil {
	buffers = append(buffers, newBuffer)
	} else if len(newPacketBuffers) > 0 {
	packetBuffers = append(packetBuffers, newPacketBuffers...)
	}
	}

These need to be before match:

And currently actionSniff with no sniffer means all sniffer enabled, so even if no sniff enabled, SOCKS5 UDP will do a sniffing.

[mazzz1y]

I removed the previous comment since the code I posted didn't really work.

@dyhkwong

After briefly reviewing the code, I can see that the sniffing for all SOCKS UDP packets is performed to determine the destination address of these packets. This happens for every SOCKS packet, whether sniffing is enabled or not. Since the sniff action doesn't have specified sniffers, it's applying all sniffers to each packet, as you mentioned.

To fix this, I've added a new action that processes UDP packets without sniffing them.

While it works in my basic testing, I'm aware this is a hack. I might have misunderstood the logic, and in general, it probably shouldn't be implemented as a "route" action.

Patch

diff --git a/route/route.go b/route/route.go
index 55a83d15..9471420d 100644
--- a/route/route.go
+++ b/route/route.go
@@ -493,8 +493,11 @@ match:
                      break match
              }
      }
-       if !preMatch && inputPacketConn != nil && (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() {
-               newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, &rule.RuleActionSniff{Timeout: C.TCPTimeout}, inputConn, inputPacketConn)
+
+       if !preMatch && inputPacketConn != nil &&
+               (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) &&
+               !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() {
+               newBuffer, newPacketBuffers, newErr := r.actionProcessPacketConn(ctx, metadata, inputPacketConn)
              if newErr != nil {
                      fatalErr = newErr
                      return
@@ -655,6 +658,48 @@ func (r *Router) actionSniff(
      return
}

+func (r *Router) actionProcessPacketConn(
+       ctx context.Context, metadata *adapter.InboundContext, inputPacketConn N.PacketConn,
+) (buffer *buf.Buffer, packetBuffers []*N.PacketBuffer, fatalErr error) {
+
+       var (
+               destination M.Socksaddr
+               done        = make(chan struct{})
+               err         error
+       )
+       sniffBuffer := buf.NewPacket()
+
+       go func() {
+               inputPacketConn.SetReadDeadline(time.Now().Add(C.ReadPayloadTimeout))
+               destination, err = inputPacketConn.ReadPacket(sniffBuffer)
+               inputPacketConn.SetReadDeadline(time.Time{})
+               close(done)
+       }()
+
+       select {
+       case <-done:
+       case <-ctx.Done():
+               inputPacketConn.Close()
+               fatalErr = ctx.Err()
+               return
+       }
+
+       if err != nil {
+               sniffBuffer.Release()
+               fatalErr = err
+               return
+       }
+
+       metadata.Destination = destination
+       packetBuffer := N.NewPacketBuffer()
+       *packetBuffer = N.PacketBuffer{
+               Buffer:      sniffBuffer,
+               Destination: destination,
+       }
+       packetBuffers = append(packetBuffers, packetBuffer)
+       return
+}
+
func (r *Router) actionResolve(ctx context.Context, metadata *adapter.InboundContext, action *rule.RuleActionResolve) error {
      if metadata.Destination.IsFqdn() {
              var transport adapter.DNSTransport

[mazzz1y]

Please disregard my previous message, this is not optimal and broke processing flow.

I'll stick with this solution for now, which forces sniffing for socks/udp before processing any rules and then skips subsequent sniffing rules if they've already been performed for the connection. While it's still a workaround, it's much clearer when we can see the reason for what's happening in the logs.

Patch

diff --git a/route/route.go b/route/route.go
index edf6063e..0ffe8f28 100644
--- a/route/route.go
+++ b/route/route.go
@@ -294,6 +294,8 @@ func (r *Router) matchRule(
        selectedRule adapter.Rule, selectedRuleIndex int,
        buffers []*buf.Buffer, packetBuffers []*N.PacketBuffer, fatalErr error,
 ) {
+       var isSniffed bool
+
        if r.processSearcher != nil && metadata.ProcessInfo == nil {
                var originDestination netip.AddrPort
                if metadata.OriginDestination.IsValid() {
@@ -363,6 +365,7 @@ func (r *Router) matchRule(
                                fatalErr = newErr
                                return
                        }
+                       isSniffed = true
                        if newBuffer != nil {
                                buffers = []*buf.Buffer{newBuffer}
                        } else if len(newPackerBuffers) > 0 {
@@ -383,6 +386,25 @@ func (r *Router) matchRule(
                metadata.InboundOptions = option.InboundOptions{}
        }
 
+       if !preMatch && inputPacketConn != nil && (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() {
+               r.logger.DebugContext(ctx, "socks packet connection: forcing sniff")
+
+               newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, &rule.RuleActionSniff{Timeout: C.TCPTimeout}, inputConn, inputPacketConn)
+               if newErr != nil {
+                       fatalErr = newErr
+                       return
+               }
+               if newBuffer != nil {
+                       buffers = append(buffers, newBuffer)
+               } else if len(newPacketBuffers) > 0 {
+                       packetBuffers = append(packetBuffers, newPacketBuffers...)
+               }
+               if metadata.Destination.IsValid() {
+                       r.logger.DebugContext(ctx, "sniffed destination: ", metadata.Destination)
+               }
+               isSniffed = true
+       }
+
 match:
        for currentRuleIndex, currentRule := range r.rules {
                metadata.ResetRuleCache()
@@ -457,6 +479,10 @@ match:
                }
                switch action := currentRule.Action().(type) {
                case *rule.RuleActionSniff:
+                       if isSniffed {
+                               r.logger.DebugContext(ctx, "already sniffed")
+                               continue
+                       }
                        if !preMatch {
                                newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, action, inputConn, inputPacketConn)
                                if newErr != nil {
@@ -489,18 +515,6 @@ match:
                        break match
                }
        }
-       if !preMatch && inputPacketConn != nil && (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() {
-               newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, &rule.RuleActionSniff{Timeout: C.TCPTimeout}, inputConn, inputPacketConn)
-               if newErr != nil {
-                       fatalErr = newErr
-                       return
-               }
-               if newBuffer != nil {
-                       buffers = append(buffers, newBuffer)
-               } else if len(newPacketBuffers) > 0 {
-                       packetBuffers = append(packetBuffers, newPacketBuffers...)
-               }
-       }
        return
 }

[nekohasekai]

Do we have any issues here?

[mazzz1y]

Yes, this is not fixed in v1.12.0-alpha.13

config.json

{
  "log": {
    "disabled": false,
    "level": "debug"
  },
  "dns": {
    "servers": [
      {
        "type": "local",
        "tag": "local"
      }
    ]
  },
  "inbounds": [
    {
      "type": "mixed",
      "tag": "mixed-in",
      "listen": "127.0.0.1",
      "listen_port": 10888
    }
  ],
  "outbounds": [
    {
      "type": "direct",
      "tag": "direct-out"
    }
  ],
  "route": {
    "auto_detect_interface": true,
    "default_domain_resolver": {
      "server": "local"
    },
    "final": "direct-out",
    "rules": [
      {
        "port": 53,
        "action": "reject"
      }
    ]
  }
}

Test script

# virtualenv venv
# source ./venv/bin/activate
# pip install pysocks dnslib
# python main.py

import socket
import socks
import dnslib

socks.set_default_proxy(socks.SOCKS5, "127.0.0.1", 10888)
socket.socket = socks.socksocket

def send_dns_request(domain, dns_server="8.8.8.8"):
    request = dnslib.DNSRecord.question(domain)

    sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
    sock.sendto(request.pack(), (dns_server, 53))

    response, _ = sock.recvfrom(1024)
    dns_response = dnslib.DNSRecord.parse(response)
    
    return dns_response

response = send_dns_request("example.com")
print(response)

~ docker run --net=host -v $PWD/config.json:/config.json -it ghcr.io/sagernet/sing-box:v1.12.0-alpha.13 -c /config.json run
INFO[0000] network: updated default interface wlp0s20f3, index 2
INFO[0000] inbound/mixed[mixed-in]: tcp server started at 127.0.0.1:10888
INFO[0000] sing-box started (0.00s)
INFO[0022] [149032334 0ms] inbound/mixed[mixed-in]: inbound connection from 127.0.0.1:36566
INFO[0022] [149032334 0ms] inbound/mixed[mixed-in]: inbound packet connection
DEBUG[0022] [149032334 0ms] router: sniffed packet protocol: dns
INFO[0022] [149032334 0ms] outbound/direct[direct-out]: outbound packet connection

Issues:

1. Sniffing is enabled for all SOCKS/UDP connections regardless of the sniffer's settings.
2. It's impossible to route SOCKS/UDP connections based on the destination (host/port).

Both of these issues were reproduced in the provided example. I logically expected that the connection wouldn't be sniffed and would be blocked by the reject rule based on the dest port.

[dyhkwong]

It is better to fix this issue in sing, otherwise we need a patch like this. I don't know a good fix without using bufio.NewCachedPacketConn.

• We need to do this before rule matching.
• We only need to read the destination of inputPacketConn instead of doing a full sniffing, and the real sniffing should not be affected by this operation. Achieve this by using bufio.NewCachedPacketConn. Is there a better solution?
• The original metadata.Destination.Addr is always :0 because of M.Socksaddr{} in https://github.com/dyhkwong/sing/blob/1c3b777fe509bde0b8c451c68a054a182f9a5ba2/protocol/socks/handshake.go#L221, so !metadata.Destination.Addr.IsValid() should be used.
• Timeout does not matter because it is still handshaking and the initial payload has not been sent yet?

diff --git a/route/route.go b/route/route.go
index bb484efd..2956e6b5 100644
--- a/route/route.go
+++ b/route/route.go
@@ -294,6 +294,21 @@ func (r *Router) matchRule(
 	selectedRule adapter.Rule, selectedRuleIndex int,
 	buffers []*buf.Buffer, packetBuffers []*N.PacketBuffer, fatalErr error,
 ) {
+	if !preMatch && inputPacketConn != nil && (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsValid() {
+		buffer := buf.NewPacket()
+		if destination, err := inputPacketConn.ReadPacket(buffer); err == nil {
+			metadata.Destination = destination
+			inputPacketConn = bufio.NewCachedPacketConn(inputPacketConn, buffer, destination)
+			packetBuffers = append(packetBuffers, &N.PacketBuffer{
+				Buffer:      buffer,
+				Destination: destination,
+			})
+		} else {
+			buffer.Release()
+			return
+		}
+	}
+
 	if r.processSearcher != nil && metadata.ProcessInfo == nil {
 		var originDestination netip.AddrPort
 		if metadata.OriginDestination.IsValid() {
@@ -489,18 +504,6 @@ match:
 			break match
 		}
 	}
-	if !preMatch && inputPacketConn != nil && (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() {
-		newBuffer, newPacketBuffers, newErr := r.actionSniff(ctx, metadata, &rule.RuleActionSniff{Timeout: C.TCPTimeout}, inputConn, inputPacketConn)
-		if newErr != nil {
-			fatalErr = newErr
-			return
-		}
-		if newBuffer != nil {
-			buffers = append(buffers, newBuffer)
-		} else if len(newPacketBuffers) > 0 {
-			packetBuffers = append(packetBuffers, newPacketBuffers...)
-		}
-	}
 	return
 }
 
@@ -586,9 +589,6 @@ func (r *Router) actionSniff(
 					return
 				}
 			} else {
-				if (metadata.InboundType == C.TypeSOCKS || metadata.InboundType == C.TypeMixed) && !metadata.Destination.IsFqdn() && !metadata.Destination.Addr.IsGlobalUnicast() && !metadata.RouteOriginalDestination.IsValid() {
-					metadata.Destination = destination
-				}
 				if len(packetBuffers) > 0 {
 					err = sniff.PeekPacket(
 						ctx,

[nekohasekai]

Try 9aca54d

[dyhkwong]

Fixed in 9aca54d.