shadowsocks / shadowsocks-rust

A Rust port of shadowsocks
https://shadowsocks.org/
MIT License
8.67k stars 1.18k forks source link

DnsRelay improvements #213

Open zonyitoo opened 4 years ago

zonyitoo commented 4 years ago
zonyitoo commented 4 years ago

Asking for comments @madeye @Mygod .

Reverse IP ACL lookup can be enabled by a separated feature, as suggested in #210 . Only enable it for sslocal integration.

zonyitoo commented 4 years ago

Question: Why DNS queries are proxied to remote_dns_addr in TCP protocol but UDP to local_dns_addr? @madeye

Mygod commented 4 years ago

It should be configurable :)

madeye commented 4 years ago

Many shadowsocks servers don't enable UDP relay, so we forward the DNS query via TCP to the remote server.

zonyitoo commented 4 years ago

Many shadowsocks servers don't enable UDP relay, so we forward the DNS query via TCP to the remote server.

So it should be configurable then.

zonyitoo commented 4 years ago

Ok, next step,

zonyitoo commented 4 years ago

Should we reuse trust-dns-client to implement DNS relay? @madeye

madeye commented 4 years ago

If possible, it would be very nice.

With trust-dns-client, it maybe possible to handle hosts file as well.

zonyitoo commented 4 years ago

But that will end up to link most of the trust-dns, binary size will be large.

madeye commented 4 years ago

The binary size is not a concern for me.

@Mygod what do you think?

zonyitoo commented 4 years ago

Could we reuse the global DNS resolver? @madeye , the one in src/relay/dns_resolver/

Mygod commented 4 years ago

What are you asking? If global resolver can handle the complexity of things we need to do then sure?

zonyitoo commented 4 years ago

dns_relay send queries simultaneously by direct connections and proxy connections. For direct connections, they actually work the same as trust-dns-resolver internally but lack of many other things, such as lookup strategies (ipv4only, ipv6only, ipv4thenipv6, ...). So I was thinking about we should reuse this library rather than implementing all by ourselves.

On the other hand, if dns_relay is only for proxying DNS queries without any higher level functions, then we should use trust-dns-client.

Mygod commented 4 years ago

Direct queries need to use local_dns_path on Android, no? On other platform, it is safe to use system resolver.

zonyitoo commented 4 years ago

local_dns_path is not used anywhere in this project.

Mygod commented 4 years ago

That's because it's still WIP...

zonyitoo commented 4 years ago

Okay... I will keep it until madeye finishes all his works.

Mygod commented 4 years ago

It would certainly be nice if we can reuse trust dns, e.g. connection reuse that they already have: https://github.com/bluejekyll/trust-dns/pull/1058

zonyitoo commented 4 years ago

Yes, I also highly suggest that we can use trust-dns-client.

zonyitoo commented 4 years ago

Its API allows customization of the underlying connection.

Mygod commented 4 years ago

I looked at trust-dns and it is actually less customizable than you think (without changing its source code that is). Since in rust every subclass has to be sealed via enums, we cannot extend its interface other than reusing TcpStream. Unfortunately (a) it requires us to implement trust_dns_proto::tcp::Connect which is a very restrictive interface/trait (its async fn only takes a SocketAddr, no way to pass PathBuf, let alone ProxyStream); (b) all fields of TcpStream are private so we cannot get too far.

A preliminary attempt at hacking things together: https://gist.github.com/Mygod/2532e73063986635ce1f43db4e5825dd

Mygod commented 4 years ago

Opened an upstream issue: https://github.com/bluejekyll/trust-dns/issues/1100