Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use implicit name resolution for TCP destinations #101

Merged
merged 4 commits into from Jan 28, 2021

Conversation

bemasc
Copy link

@bemasc bemasc commented Jan 27, 2021

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.

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.
Copy link

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried about leaking private information.

Could you instead use the Dialer.Control() function instead, testing the address there and cancelling as needed?

Also, shouldn't we do that for UDP too?

@bemasc
Copy link
Author

bemasc commented Jan 28, 2021

I'm worried about leaking private information.

Could you instead use the Dialer.Control() function instead, testing the address there and cancelling as needed?

Good idea. Done!

Also, shouldn't we do that for UDP too?

No, this doesn't apply to UDP.

@bemasc bemasc requested a review from fortuna January 28, 2021 16:45

buf := make([]byte, 10)

addresses := []string{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put the errors next to the IPs for readability?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bemasc bemasc requested a review from fortuna January 28, 2021 18:54
Copy link

@fortuna fortuna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@database64128
Copy link

Thank you for the effort!

@bemasc bemasc deleted the bemasc-happy-eyeballs branch January 29, 2021 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants