net/netns: indicate when android protect func is no longer nilcatzkorn/netcheckuout

We have been getting into routing loops due to the timing of when we
bind sockets on starting the tailscale android app. At this point, we do
not have access to `VpnService.protect()` and are unable to protect
the magicsock sockets, which causes a routing loop issue until we
forcibly rebind about 10 minutes into the service being started.

This change returns a bool from SetAndroidProtectFunc we get access to
a func.

Updates tailscale/corp#13814

Signed-off-by: Charlotte Brandhorst-Satzkorn <charlotte@tailscale.com>

2 files changed, 37 insertions, 1 deletions

diff --git a/net/netns/netns_android.go b/net/netns/netns_android.go
index 162e5c79a..50a2e539a 100644
--- a/net/netns/netns_android.go
+++ b/net/netns/netns_android.go
@@ -44,10 +44,23 @@ func UseSocketMark() bool {
 // fwmark return errors on Android. The actual implementation of
 // VpnService.protect ends up doing an IPC to another process on
 // Android, asking for the fwmark to be set.
-func SetAndroidProtectFunc(f func(fd int) error) {
+//
+// When we start the android application we bind sockets before we
+// have access to the VpnService.protect() causes us to set a nil func.
+// When we activate the service we get access to the protect, but do
+// not retrospectively protect the sockets which are already opened.
+// This breaks connectivity until a rebind occurs.
+// As a temporary fix, we return a bool that indicates if we now have
+// access to the protect func which should be used to determine if
+// we require a rebind.
+// See https://github.com/tailscale/corp/issues/13814
+func SetAndroidProtectFunc(f func(fd int) error) bool {
 	androidProtectFuncMu.Lock()
 	defer androidProtectFuncMu.Unlock()
+	hasPrevProtectFunc := androidProtectFunc != nil
 	androidProtectFunc = f
+
+	return hasPrevProtectFunc
 }
 
 func control(logger.Logf, *netmon.Monitor) func(network, address string, c syscall.RawConn) error {
diff --git a/net/netns/netns_android_test.go b/net/netns/netns_android_test.go
new file mode 100644
index 000000000..4300c3ae9
--- /dev/null
+++ b/net/netns/netns_android_test.go
@@ -0,0 +1,23 @@
+// Copyright (c) Tailscale Inc & AUTHORS
+// SPDX-License-Identifier: BSD-3-Clause
+
+//go:build android
+
+package netns
+
+import "testing"
+
+func TestSetAndroidProtectFunc(t *testing.T) {
+	// No function has previously been set.
+	hasPrevProtectFunc := SetAndroidProtectFunc(func(fd int) error { return nil })
+
+	if hasPrevProtectFunc {
+		t.Fatal("SetAndroidProtectFunc returned true, should be false")
+	}
+
+	hasPrevProtectFunc = SetAndroidProtectFunc(func(fd int) error { return nil })
+
+	if !hasPrevProtectFunc {
+		t.Fatal("SetAndroidProtectFunc returned false, should be true")
+	}
+}