shadowsocks-org icon indicating copy to clipboard operation
shadowsocks-org copied to clipboard

[SIP] Add SOCKS Reply Field

Open fortuna opened this issue 6 years ago • 19 comments

The SOCKS5 protocol specifies a Reply Field that can be very useful to tell the client what happened to that connection to the destination. From the RFC:

Reply field:
             o  X'00' succeeded
             o  X'01' general SOCKS server failure
             o  X'02' connection not allowed by ruleset
             o  X'03' Network unreachable
             o  X'04' Host unreachable
             o  X'05' Connection refused
             o  X'06' TTL expired
             o  X'07' Command not supported
             o  X'08' Address type not supported
             o  X'09' to X'FF' unassigned

With this information the server can tell the client application the proper error. This is very useful in split horizon networks (to tell whether the destination is reachable and fallback to local) or to implement selective proxying with rules (and fallback to local access)

It would be great if Shadowsocks added that.

fortuna avatar Apr 05 '19 00:04 fortuna

It's a bit tricky to retrofit SOCKS semantics to Shadowsocks.

SOCKS is two-legged:

[User agent] --1--> [SOCKS server] --2--> [Target]

Shadowsocks is three-legged:

[User agent] --1--> [SS client] --2--> [SS server] --3--> [Target]

The challenge is how to properly map errors occurred on leg 2 and 3 to those defined in RFC 1928, most likely using the undefined range 0x09 to 0xFF. For errors occurred on leg 3, we also need to change SS protocol to carry them over leg 2.

At this point it becomes rather pointless:

  • Many (most?) SS clients do not even utilize SOCKS protocol at all, instead using system-specific mechanisms (TUN, TPROXY, REDIRECT, NetworkExtension, etc) to intercept connections (aka transparent proxy).
  • For the remaining minor SOCKS-speakers, no user agents have correct error prompts for the undefined range, and there's no way to provide custom messages due to limitations of SOCKS protocol.

It'll be a tough sell. 🤪

riobard avatar Apr 06 '19 03:04 riobard

Not sure for others, but for tuns there are ICMP messages equivalent of those SOCKS replies. I think the toughest part is to maintain backward compatibility.

Mygod avatar Apr 06 '19 10:04 Mygod

Shadowsocks is a lot like SOCKS, and is also two-legged:

[User agent] --1--> [Shadwosocks server] --2--> [Target]

There's nothing preventing an app from speaking Shadowsocks directly

It's straightforward for the Shadowsocks server (as it is for a SOCKS server) to return the SOCKS error codes based on the errors returned by the socket to the Target or a ruleset.

It's common to see architectures that run a local SOCKS server:

[User agent] ---> [SOCKS server (ss-local)]  --> [Shadwosocks server] ---> [Target]

In that case, the local SOCKS server could simply forward return the SOCKS error returned by Shadowsocks server. Maybe add extra errors in case of connection failure with the shadowsocks server.

Outline runs a tun2socks architecture in order to proxy all the TCP/UDP traffic:

[User agent] ---> [Network stack]  --> [tun device] -->  [tun2socks] --> [SOCKS server (ss-local)] --> [Shadwosocks server] ---> [Target]

In this case, tun2socks can read the SOCKS errors, and write the appropriate TCP or IP errors back to the tun device. For example, in case of "connection refused", we could write a TCP reset. In case of host or network unreachable, we can write one of the destination unreachable ICMP messages.

Selective routing could happen with rules that live in the server by returning something like "Host administratively prohibited", so tun2socks knows to route it locally. This could be cached for performance.

fortuna avatar Apr 12 '19 04:04 fortuna

@Mygod: I agree, it's not clear how this can be backward compatible. I thought about maybe setting a bit in the SOCKS address type byte, to indicate you expect a SOCKS reply, but that would break with old servers. I don't see a way to add data that is simply ignored by old servers. Extensibility is something to keep in mind for future versions of the protocol

fortuna avatar Apr 12 '19 04:04 fortuna

I agree. So maybe the better question to ask here is whether it is worth it to add this breaking change?

Mygod avatar Apr 13 '19 03:04 Mygod

@fortuna Have you checked the SOCKS6 draft? It looks intriguing as the basis of a major Shadowsocks protocol revision, but I'm worried about the complexity…

riobard avatar Jul 03 '19 17:07 riobard

I've seen the draft. I'm excited about it, as it addresses many of the issues that Shadowsocks had to address, like adding encryption and enabling 0-RTT connection by leveraging TFO, TLS-false start and streamlining the SOCKS handshake. It's nice that the flow is TLS and hard to differentiate from web traffic.

I think the only missing piece is support for UDP, which is still in plain text. Maybe it's waiting for DTLS 1.3?

fortuna avatar Aug 02 '19 21:08 fortuna

I put some more thought on this, since I'd really like to have it happen. We are looking into introducing usage quotas to Outline, and it would be really helpful to differentiate a out of quota error from a connection that fails to authenticate or other errors.

I think I have something that can work in a backward compatible way. I'm assuming AEAD ciphers only, since no one should be using the stream ciphers by now.

Proposal 1 - Play with the record format

Successful Connections You get the regular AEAD protocol: TCP: [salt]([2 bytes payload size][tag][payload][tag])* UDP: [salt][payload][tag]

Authentication Errors Don't return anything and end the connection (immediately, or after a time out). We don't want to return unencrypted errors and give the server away.

Other Errors For both protocols, return [salt-1][1 byte error][tag] I'm returning a salt that is 1 byte shorter than usual, followed by one encrypted byte representing the error code, then the AEAD tag.

This format will fail to decrypt in both the standard TCP and UDP formats, so existing clients will just ignore the connection. However, new clients will have the option to parse the data again in the error format, in order to extract the error code.

The reason why I'm using a 1-byte shorter salt is so I can differentiate

Proposal 2 - Change the HKDF info

Perhaps a much more flexible approach is for the server return an error structure encrypting using the standard TCP or UDP formats, but using ss-error as the info for the HKDF function, instead of ss-subkey.

Old clients will fail to parse the error message and close the connection. New clients can re-parse the message using the ss-error info and extract the error from the payload.

The payload can be any structure we want, which is a lot more flexible than a single byte. For example, an "out of quota" error could include what the data limit is. You could also leverage this to deliver a new ss:// link, in case you want to deprecate an existing one and move users over to a new server.

@mygod @riobard What do you think?

@alalamav @bemasc FYI

fortuna avatar Aug 09 '19 23:08 fortuna

@fortuna In Proposal 1 with the 1-byte-omission how do you plan to decrypt the chunk? Exploring all 256 possibilities of the missing byte? If true I think it would be quite ugly…

In Proposal 2 I'd suggest you use ss-info so you can throw all kinds of auxiliary data into it. But changing the crypto behavior comes with its own risk regarding replay attack, timing differences, etc and I'm not so sure if it's completely safe.

There is actually a Proposal 3: when I originally proposed the AEAD chunk structure I left a door for such thing: of the 2-byte chunk length, only the lower 14 bits are utilized (thus we cap the chunk length to 16383 bytes). The two higher bits are reserved for future upgrades, for example you could flip one of the two bits to signal that the chunk right after it contains auxiliary control info, not actual payload data. This would be similar to how SSL works. Backward compatibility should be OK because if the client/server implementation follows the spec correctly it should detect that reserved bits are used and drop the connection if they don't know how to handle the situation. If the implementation naively accepts more than 16383-byte chunk length, the actual chunk would be shorter than that and will cause a decryption error anyway.

riobard avatar Aug 12 '19 05:08 riobard

@riobard I think your proposal would work best. Let's take out a bit as the error flag and indicate the following message is an error and we can put the error there (using some format from socks protocol or json etc.)

Mygod avatar Aug 12 '19 06:08 Mygod

@riobard That's a good idea, it's definitely simpler. I had forgotten about those two bits. Unfortunately in UDP we don't have those two bits available, so it wouldn't help us in that case.

Going back to my proposals, let me clarify some things that were not clear.

Proposal 1 I'm deliberately making the salt shorter. The server would provide a shorter salt and encrypt with the shorter salt when reporting the error. The client would retry the decryption for error considering a shorter salt. There's no guessing of what the missing byte is.

Does the salt have to be the same size as the key? If so, we can assume the last byte is zero.

This makes the crypto slightly weaker, but 32 -> 31 bytes doesn't seem to be significant. We could give away less bits if needed.

Proposal 2 Between 1 and 2, this is my favorite option. We are not really changing the crypto behavior much. The crypto is pretty much the same, the big difference is what you do with the decrypted payload.

The second decryption would only happen in case of connection errors. I can see how that could potentially be vulnerable to timing attacks, but it doesn't seem very feasible, and I'd rather worry about that if there's real potential to be a threat.

As far as replay attacks, that's a protocol issue that the proposal doesn't make worse. You'll still have the salt that you can use for mitigation.

Using ss-info is fine with me, though the server can only answer with that message in case of errors, so I think ss-error would be more explicit. But it really doesn't matter to me, as long as it's different from ss-subkey.

fortuna avatar Aug 12 '19 16:08 fortuna

To prevent replay attacks, you can bind the ss-error response to the request that generated it, e.g. by including the request's salt in the response's AEAD additional data (or in its payload).

bemasc avatar Aug 12 '19 16:08 bemasc

@fortuna The second proposal sounds like a good option.

@bemasc Can you be more specific on your proposal?

Mygod avatar Aug 13 '19 07:08 Mygod

With an AEAD cipher, on TCP, the client opens a connection by sending something like

[client salt][encrypted length][length-tag][encrypted address][address-tag]

A simple version of my proposal would have the server construct a reply plaintext that consists of

[error response] = [client salt][ss-error-info]

The server would of course encrypt this, so the full reply would be

[server salt][length of error response][length-tag][encrypted error response][error-tag]

Where the length and error response are encrypted with the error key as derived in Proposal 2.

After decrypting [error response], the client would verify that the salt matches and discard the response on mismatch.

Technically this implementation is inefficient, because AEAD ciphers accept "additional data", which I believe is not currently used in Shadowsocks. Setting the additional data to the client's salt would create this replay resistance without making the server's response longer. If the main Shadowsocks implementations can easily make use of AEAD additional data, this seems like it would be a preferable solution.

bemasc avatar Aug 14 '19 22:08 bemasc

@riobard I think we can definitely include this for shadowsocks v2 as well.

Kind of similar to #156, I am also thinking of transmitting some ICMP packets over shadowsocks-udp via address 0 (as mentioned in #156) and port 0 (I bet the socks6 folks have not thought of this!), following rfc5508, which would be useful for echo/traceroute or maybe more for socksifier VPN (note that there are still advantages to using a socksifier than a stateless VPN like WireGuard).

Mygod avatar Feb 20 '20 17:02 Mygod

Sure. I'm going to open a new issue to keep track of all the improvements needed for v2.

riobard avatar Feb 20 '20 17:02 riobard

It could be good to make a list of things but no rush. I think all of these changes (inc. #156) are not urgent and we can certainly wait for socks6 to go stable first.

P.S. I also proposed the idea of doing ICMP over udp:0.0.0.0:0 to the socks6 mailing list. Let's see what they think.

Mygod avatar Feb 20 '20 17:02 Mygod

@Mygod could you add a link to your mailing list message?

bemasc avatar Feb 21 '20 19:02 bemasc

I would if I know where to find such a link.

Mygod avatar Feb 22 '20 00:02 Mygod