🔗 Dial directly without using the resolved IP from net.ResolveTCPAddr #100
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
1 similar comment
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
database64128
changed the title
- Use `net.LookupIP` instead. It returns all resolved IP addresses. - `net.ResolveTCPAddr` and `net.ResolveUDPAddr` use `forResolve`, which always prefers IPv4 for hostnames. For TCP changing to `Dial` directly restores the correct default behavior under dual-stack network conditions.
|
@googlebot I signed it! |
|
Thanks @database64128; this does look like an improved behavior. Are you using outline-ss-server with SOCKS5 named destinations? We haven't optimized for this case because the Outline clients never use this mode; instead, the client always performs DNS resolution through the proxy and then specifies an explicit IPv4 destination. |
| } | ||
|
|
||
| tgtTCPConn, err := net.DialTCP("tcp", nil, tgtTCPAddr) | ||
| tgtConn, err := net.Dial("tcp", tgtAddr.String()) |
There was a problem hiding this comment.
Unfortunately I don't think this is safe. In a DNS rebinding attack, the domain could initially resolve to a harmless destination, and then quickly be changed to resolve to a forbidden IP before being re-resolved by this call.
AFAIK, the only way to get proper dual-stack behavior is therefore to replicate the dual-stack/happy-eyeballs logic.
There was a problem hiding this comment.
Another solution would be to change the destination IP restriction to an opt-in setting. It's a lot easier to implement, maintain, and in my opinion, a lot better. By aligning our behavior with other popular Shadowsocks server implementations, there won't be any surprises to new users.
I often find myself in the need of accessing the private network of the server hosting an Outline server. Due to the restriction, I have to set up another non-Outline Shadowsocks server or resort to other VPN solutions. I'm pretty sure many other users also find it frustrating: #46.
There was a problem hiding this comment.
Opening up local host and network access is a major security flaw. The fact that other implementation are vulnerable by default means that they should change. Outline will stick to the safer setting.
We can consider having an opt-in way of enabling access to the local network. Note, however, that that's also a significant vulnerability when you are on cloud, since you can often access metadata services. On DO you would be able to retrieve the tags for the droplet and retrieve the secret for the management API.
This feels like an advanced need for advanced users that could complicate the product and increase risk. We will probably not add to the Outline Manager, but we could consider an allow list in the outline-ss-server config.
Yes. I mostly use shadowsocks-rust and v2ray-core as my shadowsocks client. |
When connecting to a TCP destination by name, go's implicit resolution behavior tries all available addresses until it finds one that works (fallback), with a preference for IPv6 if possible (happy eyeballs). This is better than our current behavior (pick one IPv4 address). The Outline client doesn't rely on named destinations, but other Shadowsocks clients do. This is an alternative to #100. This change has one key difference from the previous behavior: IP validation is enforced after the connection is established, not before. A hostile user cannot use this to send data to a private service, but they might be able to detect the existence of that service based on how long the server waits before closing the connection.
| } | ||
|
|
||
| tgtTCPConn, err := net.DialTCP("tcp", nil, tgtTCPAddr) | ||
| tgtConn, err := net.Dial("tcp", tgtAddr.String()) |
There was a problem hiding this comment.
Opening up local host and network access is a major security flaw. The fact that other implementation are vulnerable by default means that they should change. Outline will stick to the safer setting.
We can consider having an opt-in way of enabling access to the local network. Note, however, that that's also a significant vulnerability when you are on cloud, since you can often access metadata services. On DO you would be able to retrieve the tags for the droplet and retrieve the secret for the management API.
This feels like an advanced need for advanced users that could complicate the product and increase risk. We will probably not add to the Outline Manager, but we could consider an allow list in the outline-ss-server config.
|
Thanks for this suggestion, @database64128. We found a safe way to implement this and have merged it (#101). Try it out, and let us know if it's working for you! |
net.LookupIPinstead. It returns all resolved IP addresses.net.ResolveTCPAddrandnet.ResolveUDPAddruseforResolve, which always prefers IPv4 for hostnames. See https://github.com/golang/go/blob/master/src/net/ipsock.go#L70-L73. For TCP changing toDialdirectly restores the correct default behavior under dual-stack network conditions.