search

shadowsocks / go-shadowsocks2

Modern Shadowsocks in Go
Apache License 2.0
4.45k stars 1.39k forks source link

Send iv/salt, target addr with payload altogether #164

Closed lixin9311 closed 4 years ago

lixin9311 commented 4 years ago

Alternative patch of https://github.com/shadowsocks/go-shadowsocks2/pull/160

Only tested by CI

lixin9311 commented 4 years ago

@Anonymous-Someneese Thank you for raising the issue. Please check if this patch could fix your problem.

riobard commented 4 years ago

@lixin9311 Since this PR changes the public interface of exported packages, maybe we should version it somehow? So we don't break code using this as dependency.

lixin9311 commented 4 years ago

@lixin9311 Since this PR changes the public interface of exported packages, maybe we should version it somehow? So we don't break code using this as dependency.

I think if we generate salt/iv in initWriter/newWrite, it will be ok. But the behavior of NewWriter will be changed. Create & send salt/iv before creating the Writer -> Writer will automatically create and send salt/iv.

Nevertheless, no method for creating a StreamCipher Writer was exported, and that will be the right behavior of a Writer I think.

lixin9311 commented 4 years ago

Forget it, I can't see a better way without changing NewWriter(w io.Writer, aead cipher.AEAD), as long as it takes cipher.AEAD as a parameter.

Anonymous-Someneese commented 4 years ago

My question still remains. If a client connects but expect the server to send the first byte. Wouldn't the proxy stuck at readerWithHeader.Read()?

lixin9311 commented 4 years ago

@riobard I've been thinking about it. Although manually fixing it or enabling Nagle's algorithm (SetNoDelay(false)) will fix the issue, there would be a significant latency penalty, and it's bad for some games using TCP (e.g., Minecraft). I don't think we should fix the signature problem here. Instead, I would recommend using plugins to avoid such an issue.

riobard commented 4 years ago

How much extra delay will be introduced by enabling Nagel's algo just for the first few packets?

lixin9311 commented 4 years ago

https://en.wikipedia.org/wiki/Nagle%27s_algorithm#Interactions_with_real-time_systems

Very depending on the use case, and packet size.

If we set a read timeout for a few milliseconds to the underlying TCP socket, and cancel it after the first successful read. There would be little penalty if the server was supposed to send a message first, and we don't have to use sync package (syncing goroutines will introduce more latency).

Anyway, I don't think it is the right place to solve the signature problem in the proxy.

riobard commented 4 years ago

We could probably turn off nodelay initially, and turn it back on when we receive the first piece of data from user clients.

riobard commented 4 years ago

Actually I'm wondering if turning off TCP_NODELAY works for the first packet at all?