service: find our own tunnel among any number of foreign ones (#13)

debian/changelog

@@ -1,3 +1,17 @@
+network-manager-gpclient (1.4.1-1) unstable; urgency=medium
+
+  * Fix the connection never leaving "activating" when two other tun-based
+    VPNs are already up (closes #13): tunnel detection no longer polls a
+    fixed gpd0/tun0/tun1 list, which could not see our own tunnel once it
+    landed on tun2 or higher, so NetworkManager's vpn.timeout tore the
+    connection down. The candidates are read from /sys/class/net instead,
+    and any number of foreign tunnels may be active
+  * Attribute the tunnel to gpclient by the file descriptors it holds
+    (iff: in /proc/PID/fdinfo), so another VPN activated during
+    authentication is not adopted as ours
+
+ -- WMP <example@example.org>  Wed, 26 Aug 2026 12:00:00 +0200
+
 network-manager-gpclient (1.4.0-1) unstable; urgency=medium
 
   * Fix portal addresses: --gateway is no longer forced to the server address,

docs/PYTHON_SERVICE.md

@@ -12,7 +12,7 @@ The `nm-gpclient-service` is a Python3-based VPN service that implements the Net
    - Implements D-Bus interface `org.freedesktop.NetworkManager.VPN.Plugin`
    - Communicates with NetworkManager via System Bus
    - Manages the `gpclient` process
-   - Monitors tunnel interfaces (gpd0, tun0, tun1)
+   - Monitors tunnel interfaces (`gpd0` and any `tunN`, discovered at runtime)
 
 2. **D-Bus Interface**
    - Service name: `org.freedesktop.NetworkManager.gpclient`
@@ -104,12 +104,26 @@ Background thread reads gpclient stdout and detects messages:
 After detecting message, immediately checks for tunnel interface.
 
 ### Tunnel Interface Detection
-Every 500ms checks for:
-```
-/sys/class/net/gpd0
-/sys/class/net/tun0
-/sys/class/net/tun1
-```
+Every 500ms `/sys/class/net` is scanned for `gpdN`/`tunN` devices (`gpd0`
+first, then `tunN` in numeric order). The candidate list is *not* hardcoded:
+openconnect falls back to a kernel-assigned `tunN` name whenever `gpd0` is
+unavailable, and N is the first free number - with two other tun-based VPNs
+already up, our own tunnel lands on `tun2`
+([#13](https://github.com/WMP/GlobalProtect-SAML-NetworkManager/issues/13)).
+
+A candidate is accepted only when all of the following hold:
+
+1. It has an IPv4 address.
+2. It is not one of the interfaces that already existed with the same address
+   when `Connect()` started (a stale `gpd0`, or another VPN client's tunnel -
+   [#7](https://github.com/WMP/GlobalProtect-SAML-NetworkManager/issues/7)).
+   That snapshot is taken from the same scan, so any number of foreign
+   tunnels may be active.
+3. If gpclient's own file descriptors name a tunnel interface
+   (`iff:` in `/proc/PID/fdinfo/N`, looked up across gpclient and its
+   children), the candidate is that interface. This keeps another VPN that
+   comes up mid-connect from being adopted; when the information is
+   unavailable the snapshot decides on its own.
 
 After finding interface:
 1. Builds IP configuration (interface name + DNS)

service/nm-gpclient-service.py

@@ -61,9 +61,29 @@
 NM_VPN_PLUGIN_FAILURE_CONNECT_FAILED = 1
 NM_VPN_PLUGIN_FAILURE_BAD_IP_CONFIG = 2
 
-# Interface names the tunnel detection looks for. gpd0 is created
-# exclusively by gpclient; tun0/tun1 may also belong to other VPN clients.
-TUNNEL_INTERFACES = ["gpd0", "tun0", "tun1"]
+# --- Tunnel interface detection ---------------------------------------------
+#
+# The tunnel our gpclient brings up is either gpd0 (created exclusively by
+# gpclient) or a kernel-assigned tunN device - openconnect falls back to the
+# latter, and N is simply the first free number. A fixed candidate list used
+# to be enough, but it caps how many *foreign* tunN VPNs may be up at once:
+# with two other tun-based VPNs already holding tun0 and tun1, our own tunnel
+# lands on tun2, which no fixed list predicted, so detection polled forever
+# until NetworkManager's vpn.timeout killed the connection (issue #13).
+#
+# So the candidates are discovered at runtime instead. The safety work is done
+# by the pre-existing-interface snapshot (issue #7) and by matching the tunnel
+# against the file descriptors our own gpclient process holds - not by the
+# length of a hardcoded list.
+NET_SYSFS_PATH = "/sys/class/net"
+PROC_PATH = "/proc"
+
+# gpdN / tunN, and nothing else: "tunl0" (the always-present IPIP tunnel
+# device) must not be mistaken for a VPN tunnel.
+TUNNEL_INTERFACE_RE = re.compile(r"^(?P<kind>gpd|tun)(?P<index>\d+)$")
+
+# gpd first (only gpclient creates those), then tunN by number
+TUNNEL_INTERFACE_KIND_ORDER = ("gpd", "tun")
 
 GPCLIENT_BINARY = "/usr/bin/gpclient"
 
@@ -442,6 +462,104 @@ def resolve_browser(value: str) -> Tuple[str, Optional[str]]:
     return target, None
 
 
+def tunnel_candidate_order(iface: str) -> Tuple[int, int]:
+    """Sort key putting gpd0 first and tunN in numeric (not lexical) order."""
+    match = TUNNEL_INTERFACE_RE.match(iface)
+    if not match:
+        return len(TUNNEL_INTERFACE_KIND_ORDER), 0
+    kind = match.group("kind")
+    return TUNNEL_INTERFACE_KIND_ORDER.index(kind), int(match.group("index"))
+
+
+def list_tunnel_candidates() -> List[str]:
+    """Tunnel interfaces present right now, in the order detection tries them.
+
+    Replaces the old fixed ["gpd0", "tun0", "tun1"] list, which could not see
+    a tunnel that landed on tun2 or higher because other VPNs held the lower
+    numbers (issue #13).
+    """
+    try:
+        names = os.listdir(NET_SYSFS_PATH)
+    except OSError as e:
+        logger.warning(f"Cannot list {NET_SYSFS_PATH}: {e}")
+        return []
+
+    candidates = [name for name in names if TUNNEL_INTERFACE_RE.match(name)]
+    return sorted(candidates, key=tunnel_candidate_order)
+
+
+def tunnel_ifaces_held_by(pids: List[int]) -> set:
+    r"""Tunnel interfaces whose file descriptor is held by one of `pids`.
+
+    The kernel names the interface behind a tun file descriptor in
+    /proc/PID/fdinfo/N ("iff:\ttun2"), which identifies our own tunnel
+    directly instead of inferring it from what appeared since Connect()
+    started. Returns an empty set when the information is unavailable
+    (process already gone, /proc unreadable); the caller then falls back to
+    the pre-existing-interface snapshot.
+    """
+    ifaces = set()
+    for pid in pids:
+        fdinfo_dir = f"{PROC_PATH}/{pid}/fdinfo"
+        try:
+            fds = os.listdir(fdinfo_dir)
+        except OSError:
+            continue
+        for fd in fds:
+            try:
+                with open(f"{fdinfo_dir}/{fd}", "r") as handle:
+                    content = handle.read()
+            except OSError:
+                continue
+            for line in content.splitlines():
+                if line.startswith("iff:"):
+                    name = line.split(":", 1)[1].strip()
+                    if name:
+                        ifaces.add(name)
+    return ifaces
+
+
+def process_tree(root_pid: int) -> List[int]:
+    """A PID plus every descendant, so a tunnel opened by a forked helper of
+    gpclient is still recognised as ours."""
+    try:
+        pids = [int(entry) for entry in os.listdir(PROC_PATH) if entry.isdigit()]
+    except OSError:
+        return [root_pid]
+
+    children: Dict[int, List[int]] = {}
+    for pid in pids:
+        try:
+            with open(f"{PROC_PATH}/{pid}/stat", "r") as handle:
+                stat = handle.read()
+        except OSError:
+            continue
+        # Field 2 is the command name in parentheses and may itself contain
+        # spaces and parentheses, so the parent PID is read after the last ')'
+        close = stat.rfind(")")
+        if close < 0:
+            continue
+        fields = stat[close + 1 :].split()
+        if len(fields) < 2:
+            continue
+        try:
+            children.setdefault(int(fields[1]), []).append(pid)
+        except ValueError:
+            continue
+
+    tree: List[int] = []
+    seen = set()
+    queue = [root_pid]
+    while queue:
+        pid = queue.pop()
+        if pid in seen:
+            continue
+        seen.add(pid)
+        tree.append(pid)
+        queue.extend(children.get(pid, []))
+    return tree
+
+
 class OutputScanner:
     """Split a raw PTY output stream into complete lines and a pending tail.
 
@@ -2049,31 +2167,46 @@ async def _snapshot_tunnel_interfaces(self) -> Dict[str, Any]:
 
         An interface recorded here (with an unchanged IP) is never accepted
         by _check_tunnel_loop: it is either a stale gpd0 from a crashed
-        session or another VPN client's tunnel (issue #7).
+        session or another VPN client's tunnel (issue #7). Any number of
+        foreign tunnels may be up - the snapshot grows with them (issue #13).
         """
         snapshot = {}
-        for iface in TUNNEL_INTERFACES:
-            if os.path.exists(f"/sys/class/net/{iface}"):
-                ip_addr, _ = await self._get_iface_ipv4(iface)
-                snapshot[iface] = ip_addr
-                logger.info(
-                    f"Interface {iface} (IP: {ip_addr}) already exists before "
-                    "gpclient start - it will be ignored by tunnel detection "
-                    "unless its address changes"
-                )
+        for iface in list_tunnel_candidates():
+            ip_addr, _ = await self._get_iface_ipv4(iface)
+            snapshot[iface] = ip_addr
+            logger.info(
+                f"Interface {iface} (IP: {ip_addr}) already exists before "
+                "gpclient start - it will be ignored by tunnel detection "
+                "unless its address changes"
+            )
         return snapshot
 
+    def _tunnel_ifaces_owned_by_gpclient(self) -> set:
+        """Tunnel interfaces held open by our own gpclient process tree.
+
+        Empty when the answer is unknown (gpclient already exited, /proc
+        unreadable) - the caller then relies on the snapshot alone.
+        """
+        process = self.gpclient_process
+        if process is None or process.returncode is not None:
+            return set()
+        try:
+            return tunnel_ifaces_held_by(process_tree(process.pid))
+        except Exception as e:
+            logger.debug(f"Could not determine gpclient's tunnel interfaces: {e}")
+            return set()
+
     async def _cleanup_stale_gpd0(self) -> None:
         """Remove a leftover gpd0 interface from a previous session.
 
         gpd0 is created exclusively by gpclient and gpclient enforces a
         single session via its lock file, so a gpd0 with no running gpclient
         process is always stale. A stale gpd0 blackholes routing (the portal
         becomes unreachable) and used to be picked up by tunnel detection as
-        a live connection (issue #7). tun0/tun1 may belong to other VPN
+        a live connection (issue #7). tunN devices may belong to other VPN
         clients and are never touched.
         """
-        if not os.path.exists("/sys/class/net/gpd0"):
+        if not os.path.exists(f"{NET_SYSFS_PATH}/gpd0"):
             return
 
         try:
@@ -2110,7 +2243,7 @@ async def _cleanup_stale_gpd0(self) -> None:
         except Exception as e:
             logger.debug(f"'gpclient disconnect' during cleanup failed: {e}")
 
-        if os.path.exists("/sys/class/net/gpd0"):
+        if os.path.exists(f"{NET_SYSFS_PATH}/gpd0"):
             try:
                 proc = await asyncio.create_subprocess_exec(
                     "ip", "link", "del", "gpd0"
@@ -2124,18 +2257,19 @@ async def _cleanup_stale_gpd0(self) -> None:
             except Exception as e:
                 logger.error(f"Failed to delete stale gpd0: {e}")
 
-        if not os.path.exists("/sys/class/net/gpd0"):
+        if not os.path.exists(f"{NET_SYSFS_PATH}/gpd0"):
             logger.info("Stale gpd0 interface removed")
 
     async def _check_tunnel_loop(self) -> None:
         """Periodically check for tunnel interface"""
         try:
             while True:
-                for iface in TUNNEL_INTERFACES:
-                    iface_path = f"/sys/class/net/{iface}"
-                    if not os.path.exists(iface_path):
-                        continue
+                # Which interfaces gpclient itself holds open. Looked up at
+                # most once per round, and only once a new candidate actually
+                # turned up, so the /proc walk stays off the idle path.
+                owned_ifaces = None
 
+                for iface in list_tunnel_candidates():
                     # Check if interface has an IP address (not just exists)
                     ip_addr, prefix = await self._get_iface_ipv4(iface)
 
@@ -2159,6 +2293,21 @@ async def _check_tunnel_loop(self) -> None:
                         )
                         continue
 
+                    # A tunnel that appeared after Connect() is normally ours,
+                    # but another VPN may well have been started at the same
+                    # moment. When gpclient's own file descriptors tell us
+                    # which interface is ours, trust that over the timing
+                    # (issue #13).
+                    if owned_ifaces is None:
+                        owned_ifaces = self._tunnel_ifaces_owned_by_gpclient()
+                    if owned_ifaces and iface not in owned_ifaces:
+                        logger.debug(
+                            f"Interface {iface} is new but not held by "
+                            f"gpclient (it holds {sorted(owned_ifaces)}), "
+                            "skipping"
+                        )
+                        continue
+
                     logger.info(
                         f"VPN connected - tunnel interface {iface} detected with IP {ip_addr}!"
                     )

tests/README.md

@@ -173,6 +173,7 @@ tests/
 │   ├── test_gateway_selection.py  # Gateway list parsing, matching, browser resolution (#7)
 │   ├── test_select_pty.py         # Full output pipeline over a real PTY
 │   ├── test_openssl_retry.py      # Legacy TLS renegotiation retry (#2)
+│   ├── test_tunnel_detection.py   # Finding our own tunnel among foreign ones (#13)
 │   └── test_auth_dialog.py        # Auth dialog protocol, incl. the SAML case (#8)
 ├── helpers/
 │   ├── __init__.py