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

feat: support Android protect path #1016

Merged
merged 11 commits into from
Apr 13, 2024

Conversation

xchacha20-poly1305
Copy link
Contributor

This PR will help to use the portect path, an Android VPN option.

@haruue
Copy link
Collaborator

haruue commented Mar 31, 2024

Could you please tell us which application uses such an approach to call VpnService.protect()?

@xchacha20-poly1305
Copy link
Contributor Author

Could you please tell us which application uses such an approach to call VpnService.protect()?

Nekobox use it: Code chain: 1, 2, 3.
And they used it in their own fork

Sing-box has similar logic: code chain 1, 2, 3

@haruue haruue self-assigned this Mar 31, 2024
@xchacha20-poly1305
Copy link
Contributor Author

xchacha20-poly1305 commented Mar 31, 2024

I tested in my own application in Android arm64-v8a, and it worked.

@haruue haruue marked this pull request as draft March 31, 2024 08:10
@haruue
Copy link
Collaborator

haruue commented Mar 31, 2024

Hi there,

I am definitely interested in this idea. However, we need to make some changes to this PR before it can be merged.

Since it looks like you're still working on this PR, I've marked it as a draft. When all the work is done on your end, please undraft it to let us know it's ready for review.

conn.File() not returns real file.

Signed-off-by: HystericalDragon <[email protected]>
@xchacha20-poly1305 xchacha20-poly1305 marked this pull request as ready for review April 3, 2024 10:52
@xchacha20-poly1305
Copy link
Contributor Author

@haruue I finished my work and fix some bugs. Please review it.

haruue and others added 3 commits April 5, 2024 02:20
Android's VpnService.protect() itself is confusing, so we rename the
"protect" feature with the name `fdControlUnixSocket` and make it a
sub-option under `quic.sockopts`.

A unit test is added to make sure the protect feature works.

I also added two other common options to `quic.sockopts` that I copied
from my other projects but did not fully test here.
Signed-off-by: HystericalDragon <[email protected]>
Signed-off-by: HystericalDragon <[email protected]>
haruue and others added 4 commits April 5, 2024 10:49
Why there is no decltype() in Golang?

At least we got generics now.

ref: 9520d84
it is just no reason to use named err retval here
@haruue
Copy link
Collaborator

haruue commented Apr 5, 2024

Both fwmark and bindInterface are tested and work on my end.

@haruue haruue requested review from haruue and tobyxdd April 5, 2024 05:59
@haruue haruue assigned tobyxdd and unassigned haruue Apr 5, 2024
@haruue
Copy link
Collaborator

haruue commented Apr 5, 2024

@xchacha20-poly1305

This is still a problem because the socket for the resolver (of the server address) is still un"protect"ed.

Android apps might need to resolve the server address and generate a config file with the resolved IP address as server and the original hostname (if it is a domain name) as tls.sni.

Using a Resolver with PreferGo=true and a Dial func may fix this. However, it will cause some behavior changes on Android.

As of Android 8.0, native codes are no longer be able to get the system's DNS settings, the only way to use the system's configured DNS server to resolve addresses is to use libc's getaddrinfo(3). This is also the default behavior of Go resolver when building with GOOS=android.

To fix this problem, we will need to introduce resolver in the client config, which is used to configure the DNS address (which we cannot get from the Android system) and sockopts like quic (or just reuse the one in quic?). I think this is way beyond the scope of this PR, so I won't implement this fix as of now.

@xchacha20-poly1305
Copy link
Contributor Author

I felt confused that Hysteria just supported to set DNS for server. And it seemed that we need to refactor many things. Besides, when should we resolve the server address? When creating configuration? When connected? How to set the cache of DNS result?

Above this is what I replied before you edited your message.

Fortunately, SagerNet, the first Android GUI supported Hysteria, will automatically add DNS rule for server address (https://github.com/SagerNet/SagerNet/blob/70e684bae81d4bb4203e860ab88c4319e88f944d/app/src/main/java/io/nekohasekai/sagernet/fmt/ConfigBuilder.kt#L1274-L1282).

return fmt.Errorf("failed to send: %w", err)
}

dummy := []byte{1}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@xchacha20-poly1305

Hi, I am writing the document for the feature introduced by this PR. And I noticed this.

What's the purpose of the 1 here? Is it just a placeholder to make the dummy a 1-byte-length slice, or do you mean it should not accept any value other than 1?

I also checked the exists implementation of the unix socket server in the MatsuriDayo/libneko, and it seems like it responds with 1 if everything is OK, and 0 if any errors occurred. Should we return an error and close the UDPConn in case of 0?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Furthermore, is there any documentation or protocol specification for the protocol used in this Unix socket? Has it been named? (I plan to refer to it as "fdControl protocol" in our document if not)

@tobyxdd
Copy link
Collaborator

tobyxdd commented Apr 9, 2024

Hi @xchacha20-poly1305 mind sharing your thoughts on the above questions? We plan to include this in our next release and are currently writing docs for it

@xchacha20-poly1305
Copy link
Contributor Author

xchacha20-poly1305 commented Apr 12, 2024

@haruue

Wow, this is an interesting question. I reviewed the history of this protocol, finding it was mentioned by shadowsocks at first. In shadowsocks' Android plugin documention, it's said:

__android_vpn: VPN mode. In this case, the plugin should pass all file descriptors that needs protecting from VPN connections (i.e. its traffic will not be forwarded through the VPN) through an ancillary message to ./protect_path.

Then I found the first client implement is in v2ray-plugin, calling ControlOnConnSetup. And it is not care about 0 or 1.

And then I checked the implement of "protect server", knowning that it will send 0 if failed. So I was confused.

The question of naming it and wheather deal with the responds may ask @madeye, who invented this protocol, for more information.

@tobyxdd tobyxdd merged commit 234dc45 into apernet:master Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants