From b97cc703d8b3a01331df138081b62d811c5f4b42 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 24 Jun 2021 13:02:07 -0700 Subject: [PATCH] Fix routing loop prevention, MagicDNS forwarding over Tailscale. Fixes tailscale/tailscale#2102 Updates tailscale/tailscale#1809 Signed-off-by: Brad Fitzpatrick --- .../java/com/tailscale/ipn/IPNService.java | 6 ----- cmd/tailscale/main.go | 25 ++++++++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/android/src/main/java/com/tailscale/ipn/IPNService.java b/android/src/main/java/com/tailscale/ipn/IPNService.java index 875adf5..7e00814 100644 --- a/android/src/main/java/com/tailscale/ipn/IPNService.java +++ b/android/src/main/java/com/tailscale/ipn/IPNService.java @@ -62,12 +62,6 @@ public class IPNService extends VpnService { .setConfigureIntent(configIntent()) .allowFamily(OsConstants.AF_INET) .allowFamily(OsConstants.AF_INET6); - try { - b.addDisallowedApplication(BuildConfig.APPLICATION_ID); - } catch (PackageManager.NameNotFoundException e) { - // This error means com.tailscale.ipn isn't - // installed. That shouldn't happen, so pretend it didn't. - } if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) b.setMetered(false); // Inherit the metered status from the underlying networks. if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) diff --git a/cmd/tailscale/main.go b/cmd/tailscale/main.go index 62a0358..2d4aa81 100644 --- a/cmd/tailscale/main.go +++ b/cmd/tailscale/main.go @@ -22,6 +22,7 @@ import ( "github.com/tailscale/tailscale-android/jni" "tailscale.com/ipn" "tailscale.com/net/dns" + "tailscale.com/net/netns" "tailscale.com/tailcfg" "tailscale.com/types/netmap" "tailscale.com/wgengine/router" @@ -221,7 +222,7 @@ func (a *App) runBackend() error { var ( cfg configPair state BackendState - service jni.Object + service jni.Object // of IPNService signingIn bool ) for { @@ -328,6 +329,27 @@ func (a *App) runBackend() error { if service != 0 { jni.DeleteGlobalRef(env, service) } + netns.SetAndroidProtectFunc(func(fd int) error { + return jni.Do(a.jvm, func(env *jni.Env) error { + // Call https://developer.android.com/reference/android/net/VpnService#protect(int) + // to mark fd as a socket that should bypass the VPN and use the underlying network. + cls := jni.GetObjectClass(env, s) + m := jni.GetMethodID(env, cls, "protect", "(I)Z") + ok, err := jni.CallBooleanMethod(env, s, m, jni.Value(fd)) + // TODO(bradfitz): return an error back up to netns if this fails, once + // we've had some experience with this and analyzed the logs over a wide + // range of Android phones. For now we're being paranoid and conservative + // and do the JNI call to protect best effort, only logging if it fails. + // The risk of returning an error is that it breaks users on some Android + // versions even when they're not using exit nodes. I'd rather the + // relatively few number of exit node users file bug reports if Tailscale + // doesn't work and then we can look for this log print. + if err != nil || !ok { + log.Printf("[unexpected] VpnService.protect(%d) = %v, %v", fd, ok, err) + } + return nil // even on error. see big TODO above. + }) + }) service = s return nil }) @@ -352,6 +374,7 @@ func (a *App) runBackend() error { jni.Do(a.jvm, func(env *jni.Env) error { defer jni.DeleteGlobalRef(env, s) if jni.IsSameObject(env, service, s) { + netns.SetAndroidProtectFunc(nil) jni.DeleteGlobalRef(env, service) service = 0 }