fix(client/linux): delete TUN device and NM connections when disconnect

client/go/outline/vpn/nmconn_linux.go

@@ -16,6 +16,8 @@ package vpn
 
 import (
 	"encoding/binary"
+	"fmt"
+	"io"
 	"log/slog"
 	"net"
 	"time"
@@ -24,6 +26,11 @@ import (
 	"golang.org/x/sys/unix"
 )
 
+const (
+	nmAPIRetryCount = 20
+	nmAPIRetryDelay = 50 * time.Millisecond
+)
+
 type nmConnectionOptions struct {
 	Name            string
 	TUNName         string
@@ -34,73 +41,110 @@ type nmConnectionOptions struct {
 	RoutingPriority uint32
 }
 
-func establishNMConnection(nm gonm.NetworkManager, opts *nmConnectionOptions) (ac gonm.ActiveConnection, err error) {
+type nmConnection struct {
+	nm   gonm.NetworkManager
+	name string
+	ac   gonm.ActiveConnection
+}
+
+func newNMConnection(nm gonm.NetworkManager, opts *nmConnectionOptions) (_ io.Closer, err error) {
 	if nm == nil {
 		panic("a NetworkManager must be provided")
 	}
+
+	c := &nmConnection{
+		nm:   nm,
+		name: opts.Name,
+	}
 	defer func() {
 		if err != nil {
-			closeNMConnection(nm, ac)
-			ac = nil
+			c.Close()
 		}
 	}()
 
-	dev, err := waitForTUNDeviceToBeAvailable(nm, opts.TUNName)
-	if err != nil {
-		return nil, errSetupVPN("failed to find tun device", err, "tun", opts.TUNName, "api", "NetworkManager")
-	}
-	slog.Debug("located tun device in NetworkManager", "tun", opts.TUNName, "dev", dev.GetPath())
-
-	if err = dev.SetPropertyManaged(true); err != nil {
-		return nil, errSetupVPN("failed to manage tun device", err, "dev", dev.GetPath(), "api", "NetworkManager")
-	}
-	slog.Debug("NetworkManager now manages the tun device", "dev", dev.GetPath())
-
 	props := make(map[string]map[string]interface{})
 	configureCommonProps(props, opts)
 	configureTUNProps(props)
 	configureIPv4Props(props, opts)
 	slog.Debug("populated NetworkManager connection settings", "settings", props)
 
-	// The previous SetPropertyManaged call needs some time to take effect (typically within 50ms)
-	for retries := 20; retries > 0; retries-- {
+	dev, err := nm.GetDeviceByIpIface(opts.TUNName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to locate TUN device in NetworkManager: %w", err)
+	}
+
+	for retries := nmAPIRetryCount; retries > 0; retries-- {
 		slog.Debug("trying to create NetworkManager connection for tun device...", "dev", dev.GetPath())
-		ac, err = nm.AddAndActivateConnection(props, dev)
+		c.ac, err = nm.AddAndActivateConnection(props, dev)
 		if err == nil {
+			slog.Info("successfully created NetworkManager connection", "conn", c.ac.GetPath())
 			break
 		}
 		slog.Debug("failed to create NetworkManager connection, will retry later", "err", err)
-		time.Sleep(50 * time.Millisecond)
+		time.Sleep(nmAPIRetryDelay)
 	}
-	if err != nil {
-		return ac, errSetupVPN("failed to create connection", err, "dev", dev.GetPath(), "api", "NetworkManager")
+	return c, err
+}
+
+func (c *nmConnection) Close() error {
+	if c.ac != nil {
+		if err := c.nm.DeactivateConnection(c.ac); err != nil {
+			slog.Warn("failed to deactivate NetworkManager connection", "err", err, "conn", c.ac.GetPath())
+		}
+		slog.Debug("deactivated NetworkManager connection", "conn", c.ac.GetPath())
 	}
-	return
+	return clearNMConnections(c.nm, c.name)
 }
 
-func closeNMConnection(nm gonm.NetworkManager, ac gonm.ActiveConnection) error {
+// clearNMConnections removes all NetworkManager connections with a given name.
+func clearNMConnections(nm gonm.NetworkManager, name string) error {
 	if nm == nil {
 		panic("a NetworkManager must be provided")
 	}
-	if ac == nil {
+	if name == "" {
 		return nil
 	}
 
-	if err := nm.DeactivateConnection(ac); err != nil {
-		slog.Warn("failed to deactivate NetworkManager connection", "err", err, "conn", ac.GetPath())
-	}
-	slog.Debug("deactivated NetworkManager connection", "conn", ac.GetPath())
-
-	conn, err := ac.GetPropertyConnection()
-	if err == nil {
-		err = conn.Delete()
-	}
+	slog.Debug("removing all NetworkManager connections with name ...", "name", name)
+	nmSettings, err := gonm.NewSettings()
 	if err != nil {
-		return errCloseVPN("failed to delete NetworkManager connection", err, "conn", ac.GetPath())
+		return fmt.Errorf("failed to connect to NetworkManager settings: %w", err)
 	}
-	slog.Info("NetworkManager connection deleted", "conn", ac.GetPath())
-
-	return nil
+	for retries := nmAPIRetryCount; retries > 0; retries-- {
+		if conns, err := nmSettings.ListConnections(); err != nil {
+			slog.Debug("failed to list NetworkManager connections, will retry later", "err", err)
+		} else {
+			// Find all connections with the given name, and delete them all
+			found := false
+			for _, conn := range conns {
+				props, err := conn.GetSettings()
+				if err != nil {
+					slog.Debug("failed to read connection properties", "conn", conn.GetPath())
+					continue
+				}
+				connProps, ok := props["connection"]
+				if !ok {
+					slog.Debug("basic connection properties not found", "conn", conn.GetPath())
+					continue
+				}
+				if connProps["id"] == name {
+					found = true
+					slog.Debug("deleting NetworkManager connection", "conn", conn.GetPath(), "uuid", connProps["uuid"])
+					if err := conn.Delete(); err != nil {
+						slog.Debug("failed to delete connection, will rety later", "conn", conn.GetPath())
+						continue
+					}
+					slog.Debug("deleted NetworkManager connection", "conn", conn.GetPath())
+				}
+			}
+			if !found {
+				slog.Info("all NetworkManager connections deleted", "name", name)
+				return nil
+			}
+		}
+		time.Sleep(nmAPIRetryDelay)
+	}
+	return fmt.Errorf("failed to delete NetworkManager connection: %s", name)
 }
 
 // NetworkManager settings reference:

client/go/outline/vpn/tun_linux.go

@@ -24,9 +24,23 @@ import (
 	"github.com/songgao/water"
 )
 
+const (
+	tunAPIRetryCount = 20
+	tunAPIRetryDelay = 50 * time.Millisecond
+)
+
+type tunDevice struct {
+	*water.Interface
+	nm    gonm.NetworkManager
+	nmDev gonm.Device
+}
+
 // newTUNDevice creates a non-persist layer 3 TUN device with the given name.
-func newTUNDevice(name string) (io.ReadWriteCloser, error) {
-	tun, err := water.New(water.Config{
+func newTUNDevice(nm gonm.NetworkManager, name string) (_ io.ReadWriteCloser, err error) {
+	tun := &tunDevice{nm: nm}
+
+	// Create TUN device file
+	tun.Interface, err = water.New(water.Config{
 		DeviceType: water.TUN,
 		PlatformSpecificParams: water.PlatformSpecificParams{
 			Name:    name,
@@ -36,23 +50,74 @@ func newTUNDevice(name string) (io.ReadWriteCloser, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer func() {
+		if err != nil {
+			tun.Interface.Close()
+		}
+	}()
+
 	if tun.Name() != name {
-		return nil, fmt.Errorf("tun device name mismatch: requested `%s`, created `%s`", name, tun.Name())
+		return nil, fmt.Errorf("TUN device name mismatch: requested `%s`, created `%s`", name, tun.Name())
+	}
+
+	// Wait for the TUN device to be available in NetworkManager
+	for retries := tunAPIRetryCount; retries > 0; retries-- {
+		slog.Debug("trying to locate TUN device in NetworkManager...", "tun", name)
+		tun.nmDev, err = nm.GetDeviceByIpIface(name)
+		if tun.nmDev != nil && err == nil {
+			break
+		}
+		slog.Debug("waiting for TUN device to be available in NetworkManager", "err", err)
+		time.Sleep(tunAPIRetryDelay)
+	}
+	if tun.nmDev == nil {
+		return nil, fmt.Errorf("failed to locate the TUN device `%s` in NetworkManager", tun.Name())
+	}
+	slog.Debug("found TUN device in NetworkManager", "dev", tun.nmDev.GetPath())
+
+	// Let NetworkManager take care of the TUN device
+	if err = setTUNDeviceManaged(tun.nmDev, true); err != nil {
+		return nil, err
 	}
+
+	slog.Info("TUN device successfully created", "name", tun.Name(), "dev", tun.nmDev.GetPath())
 	return tun, nil
 }
 
-// waitForTUNDeviceToBeAvailable waits for the TUN device with the given name to be available
-// in the specific NetworkManager.
-func waitForTUNDeviceToBeAvailable(nm gonm.NetworkManager, name string) (dev gonm.Device, err error) {
-	for retries := 20; retries > 0; retries-- {
-		slog.Debug("trying to find tun device in NetworkManager...", "tun", name)
-		dev, err = nm.GetDeviceByIpIface(name)
-		if dev != nil && err == nil {
-			return
+func (tun *tunDevice) Close() (err error) {
+	tun.Interface.Close()
+	if err = deleteTUNDevice(tun.nm, tun.Name()); err == nil {
+		slog.Info("TUN device deleted", "name", tun.Name())
+	}
+	return
+}
+
+func setTUNDeviceManaged(dev gonm.Device, value bool) error {
+	for retries := tunAPIRetryCount; retries > 0; retries-- {
+		if err := dev.SetPropertyManaged(value); err != nil {
+			return fmt.Errorf("NetworkManager failed to set TUN device Managed=%v: %w", value, err)
+		}
+		if managed, err := dev.GetPropertyManaged(); err == nil && managed == value {
+			slog.Debug("NetworkManager updated TUN device Managed", "dev", dev.GetPath(), "managed", value)
+			return nil
+		}
+		time.Sleep(tunAPIRetryDelay)
+	}
+	return fmt.Errorf("NetworkManager failed to set TUN device Managed=%v after retries", value)
+}
+
+func deleteTUNDevice(nm gonm.NetworkManager, name string) error {
+	for retries := tunAPIRetryCount; retries > 0; retries-- {
+		dev, err := nm.GetDeviceByIpIface(name)
+		if err != nil {
+			slog.Debug("TUN device deleted", "name", name, "msg", err)
+			return nil
+		}
+		slog.Debug("deleting TUN device ...", "dev", dev.GetPath(), "name", name)
+		if err := dev.Delete(); err != nil {
+			slog.Debug("failed to delete TUN device, will retry later", "dev", dev.GetPath(), "err", err)
 		}
-		slog.Debug("waiting for tun device to be available in NetworkManager", "err", err)
-		time.Sleep(50 * time.Millisecond)
+		time.Sleep(tunAPIRetryDelay)
 	}
-	return nil, errSetupVPN("failed to find tun device in NetworkManager", err, "tun", name)
+	return fmt.Errorf("failed to delete TUN device %s", name)
 }

client/go/outline/vpn/vpn_linux.go

@@ -29,7 +29,7 @@ type linuxVPNConn struct {
 	tun    io.ReadWriteCloser
 	nmOpts *nmConnectionOptions
 	nm     gonm.NetworkManager
-	ac     gonm.ActiveConnection
+	nmConn io.Closer
 }
 
 var _ platformVPNConn = (*linuxVPNConn)(nil)
@@ -82,16 +82,12 @@ func (c *linuxVPNConn) Establish(ctx context.Context) (err error) {
 	if ctx.Err() != nil {
 		return perrs.PlatformError{Code: perrs.OperationCanceled}
 	}
-
-	if c.tun, err = newTUNDevice(c.nmOpts.TUNName); err != nil {
+	if c.tun, err = newTUNDevice(c.nm, c.nmOpts.TUNName); err != nil {
 		return errSetupVPN("failed to create tun device", err, "name", c.nmOpts.TUNName)
 	}
-	slog.Info("tun device created", "name", c.nmOpts.TUNName)
-
-	if c.ac, err = establishNMConnection(c.nm, c.nmOpts); err != nil {
-		return
+	if c.nmConn, err = newNMConnection(c.nm, c.nmOpts); err != nil {
+		return errSetupVPN("failed to configure NetworkManager connection", err, "name", c.nmOpts.TUNName)
 	}
-	slog.Info("successfully configured NetworkManager connection", "conn", c.ac.GetPath())
 	return nil
 }
 
@@ -101,13 +97,12 @@ func (c *linuxVPNConn) Close() (err error) {
 		return nil
 	}
 
-	closeNMConnection(c.nm, c.ac)
+	if c.nmConn != nil {
+		c.nmConn.Close()
+	}
 	if c.tun != nil {
-		// this is the only error that matters
 		if err = c.tun.Close(); err != nil {
 			err = errCloseVPN("failed to delete tun device", err, "name", c.nmOpts.TUNName)
-		} else {
-			slog.Info("tun device deleted", "name", c.nmOpts.TUNName)
 		}
 	}