From 8c850947db341367bf0f6ce292ffecd0c04f7d1d Mon Sep 17 00:00:00 2001
From: Dmytro Shynkevych <dmytro@tailscale.com>
Date: Fri, 31 Jul 2020 01:10:14 -0400
Subject: [PATCH] router: split off sandboxed path from router_darwin (#624)

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>
---
 wgengine/router/router_darwin.go         | 57 ++++--------------------
 wgengine/router/router_darwin_support.go | 23 ----------
 wgengine/router/router_freebsd.go        | 14 ++++++
 wgengine/router/router_userspace_bsd.go  | 14 ------
 4 files changed, 22 insertions(+), 86 deletions(-)
 delete mode 100644 wgengine/router/router_darwin_support.go

diff --git a/wgengine/router/router_darwin.go b/wgengine/router/router_darwin.go
index 816cae53a..c6031d429 100644
--- a/wgengine/router/router_darwin.go
+++ b/wgengine/router/router_darwin.go
@@ -10,55 +10,14 @@ import (
 	"tailscale.com/types/logger"
 )
 
-type darwinRouter struct {
-	logf    logger.Logf
-	tunname string
-	Router
+func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, error) {
+	return newUserspaceBSDRouter(logf, wgdev, tundev)
 }
 
-func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
-	tunname, err := tundev.Name()
-	if err != nil {
-		return nil, err
-	}
+// TODO(dmytro): the following should use a macOS-specific method such as scutil.
+// This is currently not implemented. Editing /etc/resolv.conf does not work,
+// as most applications use the system resolver, which disregards it.
 
-	userspaceRouter, err := newUserspaceBSDRouter(logf, nil, tundev)
-	if err != nil {
-		return nil, err
-	}
-
-	return &darwinRouter{
-		logf:    logf,
-		tunname: tunname,
-		Router:  userspaceRouter,
-	}, nil
-}
-
-func (r *darwinRouter) Set(cfg *Config) error {
-	if cfg == nil {
-		cfg = &shutdownConfig
-	}
-
-	if SetRoutesFunc != nil {
-		return SetRoutesFunc(cfg)
-	}
-
-	return r.Router.Set(cfg)
-}
-
-func (r *darwinRouter) Up() error {
-	if SetRoutesFunc != nil {
-		return nil // bringing up the tunnel is handled externally
-	}
-	return r.Router.Up()
-}
-
-func upDNS(config DNSConfig, interfaceName string) error {
-	// Handled by IPNExtension
-	return nil
-}
-
-func downDNS(interfaceName string) error {
-	// Handled by IPNExtension
-	return nil
-}
+func upDNS(DNSConfig, string) error { return nil }
+func downDNS(string) error          { return nil }
+func cleanup(logger.Logf, string)   {}
diff --git a/wgengine/router/router_darwin_support.go b/wgengine/router/router_darwin_support.go
deleted file mode 100644
index 506a6ba17..000000000
--- a/wgengine/router/router_darwin_support.go
+++ /dev/null
@@ -1,23 +0,0 @@
-// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package router
-
-// SetRoutesFunc applies the given router settings to the OS network
-// stack. cfg is guaranteed to be non-nil.
-//
-// This is logically part of the router_darwin.go implementation, and
-// should not be used on other platforms.
-//
-// The code to reconfigure the network stack on MacOS and iOS is in
-// the non-open `ipn-go-bridge` package, which bridges between the Go
-// and Swift pieces of the application. The ipn-go-bridge sets
-// SetRoutesFunc at startup.
-//
-// So why isn't this in router_darwin.go? Because in the non-oss
-// repository, we build ipn-go-bridge when developing on Linux as well
-// as MacOS, so that we don't have to wait until the Mac CI to
-// discover that we broke it. So this one definition needs to exist in
-// both the darwin and linux builds. Hence this file and build tag.
-var SetRoutesFunc func(cfg *Config) error
diff --git a/wgengine/router/router_freebsd.go b/wgengine/router/router_freebsd.go
index 9fd8e1f41..2ccf4fad0 100644
--- a/wgengine/router/router_freebsd.go
+++ b/wgengine/router/router_freebsd.go
@@ -52,3 +52,17 @@ func downDNS(interfaceName string) error {
 	}
 	return nil
 }
+
+func cleanup(logf logger.Logf, interfaceName string) {
+	if err := downDNS(interfaceName); err != nil {
+		logf("dns down: %v", err)
+	}
+	// If the interface was left behind, ifconfig down will not remove it.
+	// In fact, this will leave a system in a tainted state where starting tailscaled
+	// will result in "interface tailscale0 already exists"
+	// until the defunct interface is ifconfig-destroyed.
+	ifup := []string{"ifconfig", interfaceName, "destroy"}
+	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
+		logf("ifconfig destroy: %v\n%s", err, out)
+	}
+}
diff --git a/wgengine/router/router_userspace_bsd.go b/wgengine/router/router_userspace_bsd.go
index 7c2aa1b88..85a29d150 100644
--- a/wgengine/router/router_userspace_bsd.go
+++ b/wgengine/router/router_userspace_bsd.go
@@ -159,17 +159,3 @@ func (r *userspaceBSDRouter) Close() error {
 	// No interface cleanup is necessary during normal shutdown.
 	return nil
 }
-
-func cleanup(logf logger.Logf, interfaceName string) {
-	if err := downDNS(interfaceName); err != nil {
-		logf("dns down: %v", err)
-	}
-	// If the interface was left behind, ifconfig down will not remove it.
-	// In fact, this will leave a system in a tainted state where starting tailscaled
-	// will result in "interface tailscale0 already exists"
-	// until the defunct interface is ifconfig-destroyed.
-	ifup := []string{"ifconfig", interfaceName, "destroy"}
-	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
-		logf("ifconfig destroy: %v\n%s", err, out)
-	}
-}