Integrate shadowsocks-rust #2454

madeye · 2020-02-23T12:55:24Z

This PR aims to replace shadowsocks-libev with shadowsocks-rust.

Fix issue #2452.

Nice to have:

Parallel lookup to local and remote
Unix domain socket to local DNS resolver.

Future development plan (maybe not covered by this PR)

A local DNS cache
Local DNS resolver in Rust
Fine-grained policy routing
Multiple upstreams

madeye · 2020-02-24T05:16:43Z

The naming issue should be fixed via a51ad37.

madeye · 2020-02-27T09:52:29Z

I've finished the protect() callback and get the TCP relay, UDP relay, ACL (IP rules) works now.

Further development:

Traffic stat requires changes in shadowsocks-rust's local client. I have already added the stat_path IPC interface. @zonyitoo can help to implement the traffic stat logic in local client.
ACL (domain name rules) requires reverse local DNS lookup. @Mygod can help with this part. It requires our local DNS resolver supports PTR queries for in-addr.arpa addresses. We may need a IP/domain map in our local DNS resolver for this feature.

zonyitoo · 2020-02-27T13:12:41Z

Traffic stat requires changes in shadowsocks-rust's local client. I have already added the stat_path IPC interface. @zonyitoo can help to implement the traffic stat logic in local client.

Easy peasy. I will try to do that in this weekend.

Mygod · 2020-02-27T22:55:34Z

@madeye In my proposal, I was saying that there is no need for a reverse DNS lookup -- this can be easily done using a reverse lookup map in sslocal. I think this is the better way as it also saves a RPC.

madeye · 2020-02-27T23:24:48Z

@Mygod Okay, if so, we need to implement the local resolver in sslocal. Can you help on this?

I think it can be implemented based on trust-dns. The good news is shadowsocks-rust has already used it for safe DNS resolving. What we need is to extend it to a ChinaDNS like local resolver.

Mygod · 2020-02-27T23:30:14Z

I could if y'all can wait for me to pick up rust first. :)

Mygod · 2020-02-28T20:22:56Z

Oh I was thinking of putting the code at core/src/main/rust/shadowsocks-rust and we can put other ancillary code at the rust directory... Let me see if I will have time to work on this over the weekends.

core/src/main/res/values/arrays.xml

zonyitoo · 2020-02-29T04:35:45Z

It seems that adding a flow stat for sslocal is far more complicated than I was expected. :(

I am going to merge implementation of manager stat report in ssserver to make a unified manager report module both works for clients (TCP, UDP, HTTP, ...) and servers.

madeye · 2020-02-29T04:40:31Z

No hurry... In shadowsocks-libev, I have a counter in the context and a timer to push the counter value to shadowsocks-android.

zonyitoo · 2020-02-29T04:43:01Z

Because shadowsocks-rust could be run as a library, so I have to manage these stat variables by using reference counter.

Mygod · 2020-02-29T05:23:07Z

Well it looks like this repo is going to take more than 4GB of RAM to build. Let me try again tomorrow. :)

zonyitoo · 2020-02-29T16:23:00Z

Well it looks like this repo is going to take more than 4GB of RAM to build. Let me try again tomorrow. :)

Consider disable LTO. It takes quite a lot of memory while linking. But output binary would be larger.

Mygod · 2020-02-29T21:36:22Z

build.gradle

@@ -14,6 +14,7 @@ buildscript {
        junitVersion = '4.13'
        androidTestVersion = '1.2.0'
        androidEspressoVersion = '3.2.0'
+        ndkVersion = '21.0.6113669'


Why do we need to specify ndkVersion btw?

I see the warning message that ANDROID_NDK_HOME is deprecated now. Any suggestion?

I don't think we are using that env?

Otherwise, I cannot get the project built with Android Studio 4.0...

Mygod · 2020-03-01T00:50:14Z

core/build.gradle

+        noDefaultBut "sodium", "android", "rc4", "aes-cfb", "aes-ctr", "camellia-cfb", "openssl-vendored"
+    }
+    exec { spec, toolchain ->
+        spec.environment("RUSTFLAGS", "-C link-arg=-o -C link-arg=target/${toolchain.target}/${profile}/libsslocal.so")


I would think that the plugin would take care of the output so name... Is this an upstream issue?

I think it's actually a limitation of Cargo, which cannot customize the output binary name.

Their sample app is far less involved than this. I would assume it's upstream bug (I wonder if anyone is actually using the plugin).

I got the workaround here: rust-lang/cargo#1706 (comment)

I think they already have a workaround: https://github.com/mozilla/rust-android-gradle/blob/3dc5511b2bbed4b66d03e360302b8147d7138877/plugin/src/main/kotlin/com/nishtahir/CargoBuildTask.kt#L193

mozilla/rust-android-gradle#30

Yeah as I figured nobody is actually maintaining this plugin.

madeye · 2020-03-01T01:42:20Z

I think the submodule is not correctly moved in this branch...

madeye · 2020-03-01T01:45:41Z

The latest commit should fix the issue.

Mygod · 2020-03-01T01:46:46Z

Yeah sorry for force pushing lol. @madeye

Mygod · 2020-03-01T02:44:10Z

Sadly it seems like I don't have time to understand the rust part for now, so instead I just added a JVM part for local_dns_path. You can use it just like a nonreusable TCP DNS (DNS request/response format <length:i16><payload>). The implementation is extremely drafty for now but it will serve its purpose for the time being. Thanks guys for the hard work! 👍

Mygod · 2020-03-01T02:50:06Z

P.S. It is only enabled for VpnService for now because I am not sure what is going on in shadowsocks-rust for now.

There is a possibly more efficient implementation -- we can let local_dns_path instead simply return the desired Network.getNetworkHandle(): net_handle_t and then shadowsocks-rust can handle the DNS resolving using Android NDK APIs android_getaddrinfofornet (undocumented), android_getaddrinfofornetwork on API 23+ or android_res_nsend on API 29+. (if dynamic linking is not too hard)

madeye · 2020-03-01T05:58:11Z

Currently, there's no DNS queries from shadowsocks-rust, as the SOCKS5 requests from tun2socks are all IPv4 or IPv6 types, no domain name. That's also why ACL (domain name) doesn't work now.

With the local_dns_path, I will try to implement an internal reverse DNS resolver to enable ACL (domain name) first.

Mygod · 2020-03-01T06:33:37Z

@madeye No... I am thinking that sslocal will listen on port 5450 (or local DNS port) directly to handle the DNS traffic, and use local_dns_path to do local DNS lookups. I want to minimize the packet handling in JNI and use Rust as much as possible. :)

Mygod · 2020-03-01T06:41:46Z

I would like to propose the following to-do list (based on the one in OP), with priorities.

ACL function
Traffic statistics (assigned to @zonyitoo?)
Move LocalDnsServer implementation to rust using local_dns_path (but I guess we could start with using LocalDnsServer for all the work, and change local_dns_path to use LocalDnsServer instead? but then we need to write some code that we will later throw away)

Lower priority:

Investigate the easiest way to do DNS resolving in rust in proxy/transproxy mode (in these mode we always send our traffic to default network. should we use local_dns_path vs some native approach in rust?)
Investigate possibility of using android_res_nsend directly in sslocal? Integrate shadowsocks-rust #2454 (comment)
Implement async I/O for LocalSocket in JNI (assigned to @Mygod)
Investigate binary size and make rust binary smaller? (say https://github.com/johnthagen/min-sized-rust?)
Code review & cleanup after we get things working

Lowest priority: (these are usually new features, can implement these in the future)

Fine-grained policy routing
Rust tun2socks? (https://github.com/iCepa/tun2tor)
If we have multiple rust binary, maybe we can make a master binary so that stdlib is shared, etc
Make tun2socks pass source address so that we can look up connection by UID (useful for firewall, etc)
Multiple upstreams (I wonder what an intuitive user interface would look like for this)

Thanks everyone again for the hard work!

Mygod

I haven't reviewed the rust part yet (probably next thing to do for me) but Android part looks good to me, besides the following comments:

This is related to multiple upstreams but we should implement UDP fallback support?
We should remove hosts setting or add hosts support?
We should remove TFO setting or TFO support?
Build speed is unreasonably slow. (use debug profile for debugging and CI builds? enable one target when building debug build? if only the gradle plugin is still being maintained)

For future:

Implement async I/O for LocalSocket in JNI?
Other "lowest priority" tasks mentioned in Integrate shadowsocks-rust #2454 (comment)

Mygod · 2020-04-19T00:18:14Z

.gitmodules

-	url = https://github.com/google/re2.git
+[submodule "core/src/main/rust/shadowsocks-rust"]
+	path = core/src/main/rust/shadowsocks-rust
+	url = https://github.com/madeye/shadowsocks-rust


Should this be changed to shadowsocks/shadowsocks-rust?

shadowsocks/shadowsocks-rust#211 is not merged yet.

After that, we should change the submodule to upstream.

As I mentioned I think that PR doesn't seem too important for this PR right now. 🤣

madeye · 2020-04-19T00:59:54Z

This is related to multiple upstreams but we should implement UDP fallback support?

A future plan maybe.

We should remove hosts setting or add hosts support?

Ditto. For now, I think we can remove this feature.

We should remove TFO setting or TFO support?

I suggest remove TFO as well, which is kind of broken in real usage.

Build speed is unreasonably slow. (use debug profile for debugging and CI builds? enable one target when building debug build? if only the gradle plugin is still being maintained)

Yes, I think we should do that. Previously, I tried to use rust debug target in debug build, but failed.

It's not a bad idea to do some cleanup now. IMO, many features like HOSTS and TFO are not quite useful.

Mygod · 2020-04-19T01:02:32Z

I agree except for UDP fallback because this is the only way to get UDP support for plugins. (UDP is useful for VoIP, gaming, etc and plugin is useful for apparent reasons). Maybe it's best to resolve this before merging it?

madeye · 2020-04-19T01:04:54Z

Okay, let's assume no multi-upstream for now. So, the UDP fallback logic should be the same as before.

Mygod · 2020-04-19T01:56:03Z

Addressed UDP fallback via a252f19 for now (not tested yet) but the real fix should probably be implemented via some kind of multi-upstream, as now we are putting DNS relay inside sslocal.

What's left to do is probably to address build speed and binary size... (22 minutes of CI build is really slow)

madeye · 2020-04-19T02:42:03Z

Now the CI duration dropped to 10m 8s. https://app.circleci.com/pipelines/github/shadowsocks/shadowsocks-android/8/workflows/8ad4e812-60c7-4aa5-8423-e2d944ac0b20/jobs/526

Mygod

-u does not seem to be working in shadowsocks-rust, other than that, LGTM.

(I guess this would be the best binary size we could get, good enough for me)

P.S. Once we fix UDP fallback, we can merge this and remove TFO and hosts in separate PRs.

Mygod · 2020-04-20T02:44:55Z

Oh should probably update license too due to gitmodules changes...

madeye · 2020-04-20T04:14:15Z

UDP relay should work now.

madeye · 2020-04-20T05:23:14Z

Tested locally, fallback UDP relay works now.

madeye · 2020-04-20T05:33:33Z

I think this PR is ready for merge.

Thanks for all the contributions!

This comment has been minimized.

Sign in to view

Mygod reviewed Feb 28, 2020

View reviewed changes

core/src/main/res/values/arrays.xml Show resolved Hide resolved

Mygod reviewed Feb 29, 2020

View reviewed changes

Mygod reviewed Mar 1, 2020

View reviewed changes

Mygod force-pushed the rust branch 3 times, most recently from deb5de4 to 1bd347a Compare March 1, 2020 01:38

Mygod added 2 commits April 19, 2020 08:23

Remove lintOptions

0efeff0

Remove hardcoded ndkVersion

b630bf8

Mygod reviewed Apr 19, 2020

View reviewed changes

Fix lint

871c83d

Refine code style

a12d0b0

Add hack to make UDP fallback work

a252f19

madeye and others added 2 commits April 19, 2020 10:31

Use debug cargo profile in CI

9ce5d9e

Remove outdated fix

0a1536a

Mygod added 2 commits April 20, 2020 02:38

Fix deprecation

3c7d045

Suppress local DNS resolve timeout exceptions

fb0dbaf

Mygod approved these changes Apr 19, 2020

View reviewed changes

madeye mentioned this pull request Apr 20, 2020

The remote address of UDP relay is not handled properly shadowsocks/shadowsocks-rust#230

Closed

Mygod and others added 2 commits April 20, 2020 11:01

Clean up unused jni files

98aa935

Fix the UDP relay

8fd03bf

Update shadowsocks-rust

cbbe3f0

Update Licenses

d7b8ef6

madeye merged commit 66f01b6 into master Apr 20, 2020

This was referenced Apr 20, 2020

Deprecate hosts #2502

Merged

support socks proxy without password #1046

Closed

This comment has been minimized.

Sign in to view

Mygod deleted the rust branch April 15, 2021 23:54

Integrate shadowsocks-rust #2454

Integrate shadowsocks-rust #2454

Conversation

madeye commented Feb 23, 2020 • edited by Mygod Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

madeye commented Feb 24, 2020

madeye commented Feb 27, 2020 • edited Loading

zonyitoo commented Feb 27, 2020 • edited Loading

Mygod commented Feb 27, 2020

madeye commented Feb 27, 2020

Mygod commented Feb 27, 2020

Mygod commented Feb 28, 2020

zonyitoo commented Feb 29, 2020 • edited Loading

madeye commented Feb 29, 2020

zonyitoo commented Feb 29, 2020

Mygod commented Feb 29, 2020

zonyitoo commented Feb 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madeye commented Mar 1, 2020

madeye commented Mar 1, 2020

Mygod commented Mar 1, 2020

Mygod commented Mar 1, 2020

Mygod commented Mar 1, 2020 • edited Loading

madeye commented Mar 1, 2020 • edited Loading

Mygod commented Mar 1, 2020

Mygod commented Mar 1, 2020 • edited Loading

Mygod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madeye commented Apr 19, 2020

Mygod commented Apr 19, 2020

madeye commented Apr 19, 2020

Mygod commented Apr 19, 2020 • edited Loading

madeye commented Apr 19, 2020

Mygod left a comment • edited Loading

Choose a reason for hiding this comment

Mygod commented Apr 20, 2020

madeye commented Apr 20, 2020

madeye commented Apr 20, 2020

madeye commented Apr 20, 2020

This comment has been minimized.

madeye commented Feb 23, 2020 •

edited by Mygod

Loading

madeye commented Feb 27, 2020 •

edited

Loading

zonyitoo commented Feb 27, 2020 •

edited

Loading

zonyitoo commented Feb 29, 2020 •

edited

Loading

zonyitoo commented Feb 29, 2020 •

edited

Loading

Mygod commented Mar 1, 2020 •

edited

Loading

madeye commented Mar 1, 2020 •

edited

Loading

Mygod commented Mar 1, 2020 •

edited

Loading

Mygod commented Apr 19, 2020 •

edited

Loading

Mygod left a comment •

edited

Loading