From 7ef1fb113d5dd2908795436e808ed9a5ee22a602 Mon Sep 17 00:00:00 2001
From: Nick Khyl <nickk@tailscale.com>
Date: Fri, 23 Feb 2024 19:55:08 -0600
Subject: [PATCH] cmd/tailscaled, ipn/ipnlocal, wgengine: shutdown tailscaled
 if wgdevice is closed

Tailscaled becomes inoperative if the Tailscale Tunnel wintun adapter is abruptly removed.
wireguard-go closes the device in case of a read error, but tailscaled keeps running.
This adds detection of a closed WireGuard device, triggering a graceful shutdown of tailscaled.
It is then restarted by the tailscaled watchdog service process.

Fixes #11222

Signed-off-by: Nick Khyl <nickk@tailscale.com>
---
 cmd/tailscaled/tailscaled.go | 26 ++++++++++++++++++++------
 ipn/ipnlocal/local.go        |  2 +-
 net/dns/manager_windows.go   |  2 +-
 wgengine/userspace.go        | 19 +++++++++++++++++--
 wgengine/watchdog.go         |  4 ++--
 wgengine/wgengine.go         |  9 +++++----
 6 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go
index 8c4365548..8c100f3be 100644
--- a/cmd/tailscaled/tailscaled.go
+++ b/cmd/tailscaled/tailscaled.go
@@ -432,13 +432,26 @@ func startIPNServer(ctx context.Context, logf logger.Logf, logID logid.PublicID,
 	if sigPipe != nil {
 		signal.Ignore(sigPipe)
 	}
+	wgEngineCreated := make(chan struct{})
 	go func() {
-		select {
-		case s := <-interrupt:
-			logf("tailscaled got signal %v; shutting down", s)
-			cancel()
-		case <-ctx.Done():
-			// continue
+		var wgEngineClosed <-chan struct{}
+		wgEngineCreated := wgEngineCreated // local shadow
+		for {
+			select {
+			case s := <-interrupt:
+				logf("tailscaled got signal %v; shutting down", s)
+				cancel()
+				return
+			case <-wgEngineClosed:
+				logf("wgengine has been closed; shutting down")
+				cancel()
+				return
+			case <-wgEngineCreated:
+				wgEngineClosed = sys.Engine.Get().Done()
+				wgEngineCreated = nil
+			case <-ctx.Done():
+				return
+			}
 		}
 	}()
 
@@ -464,6 +477,7 @@ func startIPNServer(ctx context.Context, logf logger.Logf, logID logid.PublicID,
 		if err == nil {
 			logf("got LocalBackend in %v", time.Since(t0).Round(time.Millisecond))
 			srv.SetLocalBackend(lb)
+			close(wgEngineCreated)
 			return
 		}
 		lbErr.Store(err) // before the following cancel
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index d99ce189c..8b372e878 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -678,7 +678,7 @@ func (b *LocalBackend) Shutdown() {
 	}
 	b.ctxCancel()
 	b.e.Close()
-	b.e.Wait()
+	<-b.e.Done()
 }
 
 func stripKeysFromPrefs(p ipn.PrefsView) ipn.PrefsView {
diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go
index 55e2660cf..603de8c2e 100644
--- a/net/dns/manager_windows.go
+++ b/net/dns/manager_windows.go
@@ -92,7 +92,7 @@ func (m *windowsManager) openInterfaceKey(pfx winutil.RegistryPathPrefix) (regis
 func (m *windowsManager) muteKeyNotFoundIfClosing(err error) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	if !m.closing || (err != windows.ERROR_FILE_NOT_FOUND && err != windows.ERROR_PATH_NOT_FOUND) {
+	if !m.closing || (!errors.Is(err, windows.ERROR_FILE_NOT_FOUND) && !errors.Is(err, windows.ERROR_PATH_NOT_FOUND)) {
 		return err
 	}
 
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index a8b2719f3..dc92be1f9 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -425,6 +425,21 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 		}
 	}()
 
+	go func() {
+		select {
+		case <-e.wgdev.Wait():
+			e.mu.Lock()
+			closing := e.closing
+			e.mu.Unlock()
+			if !closing {
+				e.logf("Closing the engine because the WireGuard device has been closed...")
+				e.Close()
+			}
+		case <-e.waitCh:
+			// continue
+		}
+	}()
+
 	e.logf("Bringing WireGuard device up...")
 	if err := e.wgdev.Up(); err != nil {
 		return nil, fmt.Errorf("wgdev.Up: %w", err)
@@ -1112,8 +1127,8 @@ func (e *userspaceEngine) Close() {
 	}
 }
 
-func (e *userspaceEngine) Wait() {
-	<-e.waitCh
+func (e *userspaceEngine) Done() <-chan struct{} {
+	return e.waitCh
 }
 
 func (e *userspaceEngine) linkChange(delta *netmon.ChangeDelta) {
diff --git a/wgengine/watchdog.go b/wgengine/watchdog.go
index 75bc1c0ea..fd8a2372c 100644
--- a/wgengine/watchdog.go
+++ b/wgengine/watchdog.go
@@ -150,8 +150,8 @@ func (e *watchdogEngine) PeerForIP(ip netip.Addr) (ret PeerForIP, ok bool) {
 	return ret, ok
 }
 
-func (e *watchdogEngine) Wait() {
-	e.wrap.Wait()
+func (e *watchdogEngine) Done() <-chan struct{} {
+	return e.wrap.Done()
 }
 
 func (e *watchdogEngine) InstallCaptureHook(cb capture.Callback) {
diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go
index e21987f93..d96df05fc 100644
--- a/wgengine/wgengine.go
+++ b/wgengine/wgengine.go
@@ -89,10 +89,11 @@ type Engine interface {
 	// new Engine.
 	Close()
 
-	// Wait waits until the Engine's Close method is called or the
-	// engine aborts with an error. You don't have to call this.
-	// TODO: return an error?
-	Wait()
+	// Done returns a channel that is closed when the Engine's
+	// Close method is called, the engine aborts with an error,
+	// or it shuts down due to the closure of the underlying device.
+	// You don't have to call this.
+	Done() <-chan struct{}
 
 	// SetNetworkMap informs the engine of the latest network map
 	// from the server. The network map's DERPMap field should be