fix false success from SOCKS server when Dispatch() fails

[basinilya]

Postpone SOCKS server ok reply to a TCP Connect command until Dispatch() actually creates an upstream Link. Previously it was sent immediately inside Handshake(). UDP works as before.

Tested in a standalone go program that only starts a V2Ray SOCKS server.

before:

$ curl -v --socks5-hostname localhost:1080 http://localhost
* Host localhost:1080 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:1080...
* SOCKS5 connect to localhost:80 (remotely resolved)
* SOCKS5 request granted.
* Connected to localhost () port 1080
* using HTTP/1.x
* Connected to localhost (::1) port 1080
* using HTTP/1.x
> GET / HTTP/1.1
> Host: localhost
> User-Agent: curl/8.11.0
> Accept: */*
>
* Request completely sent off
* Recv failure: Connection reset by peer
* closing connection #0
curl: (56) Recv failure: Connection reset by peer

Results(V5): Succeeded (0)

after:

$ curl -v --socks5-hostname localhost:1080 http://localhost
* Host localhost:1080 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
*   Trying [::1]:1080...
* SOCKS5 connect to localhost:80 (remotely resolved)
* cannot complete SOCKS5 connection to localhost. (5)
* closing connection #0
curl: (97) cannot complete SOCKS5 connection to localhost. (5)

_
Results(V5): Connection refused (5)

[xiaokangwang]

Thanks for your contribution. Would you mind making this behavior adjustable via a configuration flag?

The reason socks inbound return an successful reply to the remote end before the link is actually setup is there to make sure that the initial content of traffic is available for 1. routing decisions, 2. bundling protocol handshake with first payload message to reduce rtt and fingerprint overhead-free.

For those reasons, the current behavior is the default and expected action for a specific reasons, and cannot be changed without impacting existing use cases. I assume you wish to know whether a connection has been successfully created before sending traffic content that lacks reentrancy, and I am happy to discuss alternative solution of the issue you are interested.

[basinilya]

I got your point. Give me some time

[database64128]

Not sure if it's easy to do in v2ray, but in my project shadowsocks-go, the behavior is determined by a config option "disableInitialPayloadWait" and client capability. For example, if a "direct" client does not have TCP Fast Open enabled, it will indicate to the relay service that it does not consume initial payloads, then the relay service will not immediately ask the server to send a successful response, and will pass the actual connection result to the server. Shadowsocks proxy clients, on the other hand, will always advertise initial payload support, and will require "disableInitialPayloadWait": true to disable it.

Of course, in v2ray this would be further complicated by the potential use of sniffers, whereas in shadowsocks-go the router does not consume payload.

[database64128]

Also you might want to implement the same thing for HTTP CONNECT requests.

[xiaokangwang]

and client capability.

Thanks @database64128. In my opinion due to V2Ray's complexity, I don't recommend any contributor to implement such feature as it would require specialized knowledge and tools to implement it correctly, and significantly increase the difficulty to make application's behavior predictable.

Also you might want to implement the same thing for HTTP CONNECT requests.

If such thing does get implemented, please submit under a separate pull request.

[basinilya]

I added a flag to ServerConfig.

[xiaokangwang]

Looks good to me! I think this pill request is ready to be merged.

[4-FLOSS-Free-Libre-Open-Source-Software]

I added a flag to ServerConfig.

Thanks, but this is v5 config only, right?

{
      "tag": "socks-1080",
      "protocol": "socks",
      "listen": "0.0.0.0",
      "port": 1080,
      "settings": {
        "auth": "noauth",
        "udp": true,
        "DeferLastReply": true
      },
      "sniffing": {
        "enabled": true,
        "destOverride": ["fakedns"],
        "metadataOnly": true
      },
      "streamSettings": {
        "sockopt": {
          "tcpFastOpen": true,
          "mptcp": true
        }
      }
    },

[basinilya]

@4-FLOSS-Free-Libre-Open-Source-Software

Thanks, but this is v5 config only, right?

No, I don't see any separate config for socks4. The server detects the desired protocol version from the first byte of the request.

Are you asking will it work with socks4? Yes. Both protocol-specific handshake functions setup their own callbacks that send the Last Reply.

[4-FLOSS-Free-Libre-Open-Source-Software]

Sorry that my message was not clear. Of course, I do NOT want to use an older version than socks5 (ipv6+hostname support). I was asking if this is only working in version 5 of v2ray configuration or in json v4 too?
Currently, there is at least one other new feature "responseTtl" that's v5 only, but not everything else I require is configurable in v5 yet, so sadly I'm stuck with v4 JSON config.

[4-FLOSS-Free-Libre-Open-Source-Software]

In handshakeFinalizingDispatcher.Dispatch, if d.serverSession.flushLastReply(false) returns an error, it is ignored.

[basinilya]

I think this error is unimportant because we're about to return the upstream error. The downstream will be closed anyway by the caller.

[4-FLOSS-Free-Libre-Open-Source-Software]

The setupLastReply function sets up a stateful callback (flushLastReply) that is intended to be called exactly once. However, if flushLastReply is called multiple times (with different boolean values), only the first call will have effect, and subsequent calls will be no-ops?

[basinilya]

yes

[basinilya]

Sorry that my message was not clear. Of course, I do NOT want to use an older version than socks5 (ipv6+hostname support). I was asking if this is only working in version 5 of v2ray configuration or in json v4 too? Currently, there is at least one other new feature "responseTtl" that's v5 only, but not everything else I require is configurable in v5 yet, so sadly I'm stuck with v4 JSON config.

Okay, I didn't dive into this because in my project I don't use config files and just configure the socks server programmatically. Can you post the v4 config you're stuck with? The v5cfg and v4 both use RawMessage for settings, maybe it will just work.