cmd/tailscaled: move cleanup to an implicit action during startup

This removes a potentially increased boot delay for certain boot
topologies where they block on ExecStartPre that may have socket
activation dependencies on other system services (such as
systemd-resolved and NetworkManager).

Also rename cleanup to clean up in affected/immediately nearby places
per code review commentary.

Fixes #11599

Signed-off-by: James Tucker <[email protected]>

cmd/tailscaled/tailscaled.go

@@ -116,7 +116,7 @@ var args struct {
 	// or comma-separated list thereof.
 	tunname string
 
-	cleanup        bool
+	cleanUp        bool
 	confFile       string
 	debug          string
 	port           uint16
@@ -156,7 +156,7 @@ func main() {
 
 	printVersion := false
 	flag.IntVar(&args.verbose, "verbose", 0, "log verbosity level; 0 is default, 1 or higher are increasingly verbose")
-	flag.BoolVar(&args.cleanup, "cleanup", false, "clean up system state and exit")
+	flag.BoolVar(&args.cleanUp, "cleanup", false, "clean up system state and exit")
 	flag.StringVar(&args.debug, "debug", "", "listen address ([ip]:port) of optional debug server")
 	flag.StringVar(&args.socksAddr, "socks5-server", "", `optional [ip]:port to run a SOCK5 server (e.g. "localhost:1080")`)
 	flag.StringVar(&args.httpProxyAddr, "outbound-http-proxy-listen", "", `optional [ip]:port to run an outbound HTTP proxy (e.g. "localhost:8080")`)
@@ -207,7 +207,7 @@ func main() {
 		os.Exit(0)
 	}
 
-	if runtime.GOOS == "darwin" && os.Getuid() != 0 && !strings.Contains(args.tunname, "userspace-networking") && !args.cleanup {
+	if runtime.GOOS == "darwin" && os.Getuid() != 0 && !strings.Contains(args.tunname, "userspace-networking") && !args.cleanUp {
 		log.SetFlags(0)
 		log.Fatalf("tailscaled requires root; use sudo tailscaled (or use --tun=userspace-networking)")
 	}
@@ -387,12 +387,16 @@ func run() (err error) {
 	}
 	logf = logger.RateLimitedFn(logf, 5*time.Second, 5, 100)
 
-	if args.cleanup {
-		if envknob.Bool("TS_PLEASE_PANIC") {
-			panic("TS_PLEASE_PANIC asked us to panic")
-		}
-		dns.Cleanup(logf, args.tunname)
-		router.Cleanup(logf, args.tunname)
+	if envknob.Bool("TS_PLEASE_PANIC") {
+		panic("TS_PLEASE_PANIC asked us to panic")
+	}
+	// Always clean up, even if we're going to run the server. This covers cases
+	// such as when a system was rebooted without shutting down, or tailscaled
+	// crashed, and would for example restore system DNS configuration.
+	dns.CleanUp(logf, args.tunname)
+	router.CleanUp(logf, args.tunname)
+	// If the cleanUp flag was passed, then exit.
+	if args.cleanUp {
 		return nil
 	}
 

cmd/tailscaled/tailscaled.service

@@ -6,7 +6,6 @@ After=network-pre.target NetworkManager.service systemd-resolved.service
 
 [Service]
 EnvironmentFile=/etc/default/tailscaled
-ExecStartPre=/usr/sbin/tailscaled --cleanup
 ExecStart=/usr/sbin/tailscaled --state=/var/lib/tailscale/tailscaled.state --socket=/run/tailscale/tailscaled.sock --port=${PORT} $FLAGS
 ExecStopPost=/usr/sbin/tailscaled --cleanup
 

net/dns/manager.go

@@ -449,10 +449,10 @@ func (m *Manager) FlushCaches() error {
 	return flushCaches()
 }
 
-// Cleanup restores the system DNS configuration to its original state
+// CleanUp restores the system DNS configuration to its original state
 // in case the Tailscale daemon terminated without closing the router.
 // No other state needs to be instantiated before this runs.
-func Cleanup(logf logger.Logf, interfaceName string) {
+func CleanUp(logf logger.Logf, interfaceName string) {
 	oscfg, err := NewOSConfigurator(logf, interfaceName)
 	if err != nil {
 		logf("creating dns cleanup: %v", err)

util/linuxfw/iptables_runner.go

@@ -561,9 +561,9 @@ func (i *iptablesRunner) DelMagicsockPortRule(port uint16, network string) error
 	return nil
 }
 
-// IPTablesCleanup removes all Tailscale added iptables rules.
+// IPTablesCleanUp removes all Tailscale added iptables rules.
 // Any errors that occur are logged to the provided logf.
-func IPTablesCleanup(logf logger.Logf) {
+func IPTablesCleanUp(logf logger.Logf) {
 	err := clearRules(iptables.ProtocolIPv4, logf)
 	if err != nil {
 		logf("linuxfw: clear iptables: %v", err)

wgengine/router/router.go

@@ -49,11 +49,11 @@ func New(logf logger.Logf, tundev tun.Device, netMon *netmon.Monitor) (Router, e
 	return newUserspaceRouter(logf, tundev, netMon)
 }
 
-// Cleanup restores the system network configuration to its original state
+// CleanUp restores the system network configuration to its original state
 // in case the Tailscale daemon terminated without closing the router.
 // No other state needs to be instantiated before this runs.
-func Cleanup(logf logger.Logf, interfaceName string) {
-	cleanup(logf, interfaceName)
+func CleanUp(logf logger.Logf, interfaceName string) {
+	cleanUp(logf, interfaceName)
 }
 
 // Config is the subset of Tailscale configuration that is relevant to

wgengine/router/router_darwin.go

@@ -13,6 +13,6 @@ func newUserspaceRouter(logf logger.Logf, tundev tun.Device, netMon *netmon.Moni
 	return newUserspaceBSDRouter(logf, tundev, netMon)
 }
 
-func cleanup(logger.Logf, string) {
+func cleanUp(logger.Logf, string) {
 	// Nothing to do.
 }

wgengine/router/router_default.go

@@ -18,6 +18,6 @@ func newUserspaceRouter(logf logger.Logf, tunDev tun.Device, netMon *netmon.Moni
 	return nil, fmt.Errorf("unsupported OS %q", runtime.GOOS)
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
+func cleanUp(logf logger.Logf, interfaceName string) {
 	// Nothing to do here.
 }

wgengine/router/router_freebsd.go

@@ -18,7 +18,7 @@ func newUserspaceRouter(logf logger.Logf, tundev tun.Device, netMon *netmon.Moni
 	return newUserspaceBSDRouter(logf, tundev, netMon)
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
+func cleanUp(logf logger.Logf, interfaceName string) {
 	// 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"

wgengine/router/router_linux.go

@@ -1396,12 +1396,12 @@ func normalizeCIDR(cidr netip.Prefix) string {
 	return cidr.Masked().String()
 }
 
-// cleanup removes all the rules and routes that were added by the linux router.
-// The function calls cleanup for both iptables and nftables since which ever
-// netfilter runner is used, the cleanup function for the other one doesn't do anything.
-func cleanup(logf logger.Logf, interfaceName string) {
+// cleanUp removes all the rules and routes that were added by the linux router.
+// The function calls cleanUp for both iptables and nftables since which ever
+// netfilter runner is used, the cleanUp function for the other one doesn't do anything.
+func cleanUp(logf logger.Logf, interfaceName string) {
 	if interfaceName != "userspace-networking" {
-		linuxfw.IPTablesCleanup(logf)
+		linuxfw.IPTablesCleanUp(logf)
 		linuxfw.NfTablesCleanUp(logf)
 	}
 }

wgengine/router/router_openbsd.go

@@ -236,11 +236,11 @@ func (r *openbsdRouter) UpdateMagicsockPort(_ uint16, _ string) error {
 }
 
 func (r *openbsdRouter) Close() error {
-	cleanup(r.logf, r.tunname)
+	cleanUp(r.logf, r.tunname)
 	return nil
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
+func cleanUp(logf logger.Logf, interfaceName string) {
 	out, err := cmd("ifconfig", interfaceName, "down").CombinedOutput()
 	if err != nil {
 		logf("ifconfig down: %v\n%s", err, out)

wgengine/router/router_windows.go

@@ -120,7 +120,7 @@ func (r *winRouter) Close() error {
 	return nil
 }
 
-func cleanup(logf logger.Logf, interfaceName string) {
+func cleanUp(logf logger.Logf, interfaceName string) {
 	// Nothing to do here.
 }