feat: add Backend that uses outline-sdk to replace tun2socks #485

jyyi1 · 2023-09-01T23:29:33Z

Actions

Implementing Intra StreamDialer and PacketProxy
Use Gradle to invoke gomobile
Apply changes to go-tun2socks first: refactor(intra): use outline-sdk as the network stack outline-go-tun2socks#125
Move go-tun2socks code to this repository (refactor: build tun2socks from source within Android Studio #487)
Refactor folder structure

fortuna

It's hard to review this code without the corresponding Android change, and the build code.
It's not clear what is actually consumed by Android.

I need to clearly see what the interface between Android and Go is. It seems like:

Tun2socks.connectIntraTunnel
Tun2socks.newDoHTransport

What else? Can you put the entire interface used somewhere we can discuss?

Let's move the backend code out of Android, so it's clear it could be used by iOS if we add support. But also to make the boundary clear.
We should rename it though. "Backend" is ambiguous.

Perhaps we put the Go code in some /go/internal root dir. The Go backend would be a specific package (or packages) in that directory. Perhaps we call it /go/internal/intrago or /go/internal/intraapi or /go/internal/gobackend. Let's discuss.

The doh code shouldn't be nested in the backend, since that's code that perhaps could be migrated to the SDK one day.

fortuna · 2023-09-05T23:05:33Z

go.mod

+
+require (
+	github.com/Jigsaw-Code/choir v1.0.1
+	github.com/Jigsaw-Code/outline-go-tun2socks v0.0.0-20230815223204-dada2652ae2c


Why do we need this?

fortuna · 2023-09-05T23:06:18Z

Android/backend/intra/internal/doh/packet_proxy.go

+	"sync/atomic"
+	"time"
+
+	intraLegacy "github.com/Jigsaw-Code/outline-go-tun2socks/intra"


Let's move the code we need to this repo instead of depending on the old code.

fortuna · 2023-09-05T23:08:23Z

Android/backend/tools.go

+package tools
+
+import (
+	_ "golang.org/x/mobile/bind"


This is technique is for binaries, not libraries.

You need the gomobile and gobind binaries. See my outline-sdk pr

gomobile still needs this package. Otherwise it will report missing package error.

The best approach I've found is to build both binaries locally to avoid that issue.
See https://github.com/Jigsaw-Code/outline-sdk/tree/fortuna-appproxy/x/examples/mobileproxy
Notice the tools.go file too.

Unfortunately gomobile calls gobind as a binary, instead of a library, so it must be on the PATH to be found. That's why the PATH trick there.

fortuna · 2023-09-05T23:10:57Z

Android/backend/intra/internal/doh/packet_proxy.go

Let's put IP-level stuff in its own package. Perhaps name it vpn?

fortuna · 2023-09-05T23:13:10Z

Android/backend/intra/tun_unix.go

Move to the vpn library

Android/backend/intra/device.go

fortuna · 2023-09-05T23:25:52Z

Android/backend/intra/internal/doh/config.go

+
+import "strings"
+
+func parseFallbackAddrs(ipList string) []string {


Perhaps move this to transport.go, since it's only used there?

fortuna

Perhaps to minimize change, we just move everything we need from outline-go-tun2socks into /go/internal/..., then make the minimal changes needed to use the SDK network APIs.

We should have no dependencies on outline-go-tun2socks.

I suggest PR #1 is just to move the code here, and figure out the build.

Then PR #2 can actually make use the SDK network apis.

jyyi1 · 2023-09-06T00:12:25Z

@fortuna No I removed all old interfaces, and only expose types in backend/intra package. Just pushed the Java change to this repo. I didn't figure out a good way to call gomobile using gradle (there was an official plugin, but it was deprecated long time ago), so you still need to call gomobile in terminal (as mentioned in backend/README.md), and then build the Android project using gradle.

fortuna · 2023-09-06T00:44:14Z

Android/app/build.gradle

-    // For go-tun2socks
-    implementation project(":tun2socks")
+    // For Golang based backend
+    implementation project(":backend")


Can we depend on the .aar file directly here if you are not automatically building it?

Suggested change

implementation project(":backend")

implementation files('backend.aar')

You can also define a task like this:

task generateGoMobileAAR(type: Exec) { workingDir '../path/to/go/package' commandLine 'gomobile', 'bind', '-o', '../path/to/android/app/libs/hello.aar', '-target=android', '.' } preBuild.dependsOn generateGoMobileAAR

fortuna · 2023-09-06T00:56:29Z

Android/backend/intra/internal/doh/protector.go

It's really hard to follow with the code in two places.

fortuna · 2023-09-06T01:09:29Z

Android/backend/README.md

+## Build
+
+```sh
+gomobile clean


It's not clear how to set up go mobile.
We need to specify it in tools.go to make sure the version is fixed.
You shouldn't use the system binary.

fortuna · 2023-09-06T01:10:39Z

Android/backend/tools.go

+package tools
+
+import (
+	_ "golang.org/x/mobile/bind"


The best approach I've found is to build both binaries locally to avoid that issue.
See https://github.com/Jigsaw-Code/outline-sdk/tree/fortuna-appproxy/x/examples/mobileproxy
Notice the tools.go file too.

Unfortunately gomobile calls gobind as a binary, instead of a library, so it must be on the PATH to be found. That's why the PATH trick there.

fortuna · 2023-09-06T01:19:42Z

Android/backend/README.md

+
+```sh
+gomobile clean
+gomobile bind -target=android -o ./Android/backend/backend.aar ./Android/backend/intra


I see the api is all in one package. Nice.

Can you move it so that there aren't things nested in it?
I was imagining something like

go/

backend/ (or some name that is clearer)

internal/

doh/

sni/

vpn/ (we should move the vpn stuff out of doh)

fortuna · 2023-09-06T01:20:44Z

Android/backend/intra/tun_unix.go

+// The returned os.File holds a separate reference to the underlying file,
+// so the file will not be closed until both `fd` and the os.File are
+// separately closed.  (UNIX only.)
+func NewTunFromFile(fd int) (IPDevice, error) {


NewDeviceFromFileDescriptor or NewDeviceFromFD?

Android/backend/intra/device.go

fortuna · 2023-09-06T01:26:27Z

Android/backend/intra/device.go

+type IntraDevice struct {
+	t2s       network.IPDevice


Since you are just extending the IPDevice, you can do this, so you don't need to specify all method wrappers:

Suggested change

type IntraDevice struct {

t2s network.IPDevice

type IntraDevice struct {

network.IPDevice

fortuna · 2023-09-06T01:31:37Z

Android/backend/intra/internal/doh/stream_dialer.go

+
+var _ DoHStreamDialer = (*dohSplitStreamDialer)(nil)
+
+func MakeDoHStreamDialer(fakeDNS netip.AddrPort, dohServer DoHTransport, protector Protector, listener intraLegacy.TCPListener, sniReporter sni.TCPSNIReporter) (DoHStreamDialer, error) {


This isn't a DoH StreamDialer. It's an IntraStreamDialer. It does a lot more than DoH. We need to rename it.

fortuna · 2023-09-06T01:32:44Z

Android/backend/intra/internal/doh/transport.go

+	"github.com/Jigsaw-Code/outline-go-tun2socks/intra/protect"
+)
+
+type DoHTransport = dohLegacy.Transport


These are really hurting readability. We should move all the code here.

fortuna

There are two main issues with this PR:

Duplication of definitions
Not clear what's original, and what's changing

It makes it really hard to review because you are moving some code, but not all, and changing it at the same time.

To make this more understandable, can you split the PR, so the move and the change are separate and clear? You can move then change, or change then move.

The move shouldn't change code. Moving to Intra first would give us the benefit of being able to build everything in one repo, though it seems we would still depend on some dependencies shared with Outline in outline-go-tun2socks, which is bad.

Perhaps it's best to change in outline-go-tun2socks first, removing all dependencies on legacy code, then move to Intra. The code in Intra should no longer depend on outline-go-tun2socks, and there shouldn't be duplication of definitions in the new code.

fortuna · 2023-09-06T07:45:26Z

Android/app/src/main/java/app/intra/net/go/GoProber.java

      try {
        // Protection isn't needed for Lollipop+, or if the VPN is not active.
-        Protector protector = VERSION.SDK_INT >= VERSION_CODES.LOLLIPOP ? null :
+        SocketProtector protector = VERSION.SDK_INT >= VERSION_CODES.LOLLIPOP ? null :


Let's get rid of the protector, it complicates things and is no longer needed.

I took a look in the Play Store, and only 0.2% of the installed audience is below Lollipop (Android 5): https://docs.google.com/spreadsheets/d/15zsfgMhJ7TsLumE6S5ZykETwMBvrBptFllX0Z02gFPw/edit?usp=sharing&resourcekey=0-inA8jaG0XS58zXNXU9YOzQ

The global stats say 0.66%: https://apilevels.com/

It's not worth the trouble.

Let's make that change separately though. Change, then move.

As we remove the protector, we should also update the minSdkVersion to 21 (Lollipop), to make sure older phones don't get the new version that will break.

intra882 · 2023-09-08T11:14:58Z

Дайте пожалуйста код -- Отправлено из Mail.ru для Android среда, 06 сентября 2023г., 12:51 +05:00 от Vinicius Fortuna ***@***.*** :

…

@fortuna commented on this pull request. ---------------------------------------------------------------------- In Android/app/src/main/java/app/intra/net/go/GoProber.java : try { // Protection isn't needed for Lollipop+, or if the VPN is not active. - Protector protector = VERSION.SDK_INT >= VERSION_CODES.LOLLIPOP ? null : + SocketProtector protector = VERSION.SDK_INT >= VERSION_CODES.LOLLIPOP ? null : As we remove the protector, we should also update the minSdkVersion to 21 (Lollipop), to make sure older phones don't get the new version that will break. — Reply to this email directly, view it on GitHub , or unsubscribe . You are receiving this because you are subscribed to this thread. Message ID: @ github . com>

Android/backend/build.gradle

fortuna · 2023-09-13T20:34:24Z

Android/backend/build.gradle

+    workingDir goBuildDir
+    environment 'ANDROID_HOME', android.sdkDirectory
+    environment 'PATH', goBuildDir.getPath() + System.getProperty("path.separator") + System.getenv("PATH")
+    commandLine "${goBuildDir}/gomobile", 'bind', '-target=android', '-o', outputAAR, "${rootDir}/backend/intra"


It would be nice to specify the -androidapi here.

Co-authored-by: Vinicius Fortuna <[email protected]>

jyyi1 · 2024-02-29T22:13:42Z

This has been partially implemented in a bunch of other PRs, such as #496, #501 . Since there has been already so many conflicts , it would be better to just create new PRs continuing the work instead of modifying this one.

jyyi1 added 4 commits September 1, 2023 19:29

feat: add Backend that uses outline-sdk to replace tun2socks

2e88e81

add intra streamdialer

2ae005e

add IntraDevice implementation

da4f950

add tun device and io copy

4b2f58b

jyyi1 requested a review from fortuna September 5, 2023 23:01

fortuna requested changes Sep 5, 2023

View reviewed changes

jyyi1 added 2 commits September 5, 2023 20:05

update java code to use the new interface

bffb7dc

add backend gradle build

1d1cdbb

fortuna requested changes Sep 6, 2023

View reviewed changes

add PoC of the gradle build script for gomobile

5efdfa0

fortuna reviewed Sep 13, 2023

View reviewed changes

jyyi1 and others added 2 commits September 15, 2023 19:24

Update Android/backend/build.gradle

1b7eacd

Co-authored-by: Vinicius Fortuna <[email protected]>

Update Android/backend/build.gradle

d97a4f1

Co-authored-by: Vinicius Fortuna <[email protected]>

jyyi1 closed this Feb 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add Backend that uses outline-sdk to replace tun2socks #485

feat: add Backend that uses outline-sdk to replace tun2socks #485

jyyi1 commented Sep 1, 2023 •

edited

Loading

fortuna left a comment

fortuna Sep 5, 2023

fortuna Sep 5, 2023

fortuna Sep 5, 2023

jyyi1 Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 5, 2023

fortuna Sep 5, 2023

fortuna Sep 5, 2023

fortuna left a comment

jyyi1 commented Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna Sep 6, 2023

fortuna left a comment

fortuna Sep 6, 2023

fortuna Sep 6, 2023

intra882 commented Sep 8, 2023 via email

fortuna Sep 13, 2023

jyyi1 commented Feb 29, 2024


		import "strings"

		func parseFallbackAddrs(ipList string) []string {

	implementation project(":backend")
	implementation files('backend.aar')


		var _ DoHStreamDialer = (*dohSplitStreamDialer)(nil)

		func MakeDoHStreamDialer(fakeDNS netip.AddrPort, dohServer DoHTransport, protector Protector, listener intraLegacy.TCPListener, sniReporter sni.TCPSNIReporter) (DoHStreamDialer, error) {

feat: add Backend that uses outline-sdk to replace tun2socks #485

feat: add Backend that uses outline-sdk to replace tun2socks #485

Conversation

jyyi1 commented Sep 1, 2023 • edited Loading

fortuna 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

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

fortuna left a comment

Choose a reason for hiding this comment

jyyi1 commented Sep 6, 2023

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

fortuna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

intra882 commented Sep 8, 2023 via email

Choose a reason for hiding this comment

jyyi1 commented Feb 29, 2024

jyyi1 commented Sep 1, 2023 •

edited

Loading