openvpn-devel — OpenVPN developers list

[Openvpn-devel] [M] Change in openvpn[master]: tapctl: make output of 'list' and 'create' commands more verbose

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: cron2, plaisthos.

stipa has posted comments on this change by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email )

Change subject: tapctl: make output of 'list' and 'create' commands more verbose
......................................................................


Patch Set 2:

(2 comments)

Patchset:

PS2: 
Fixed


File src/tapctl/main.c:

http://gerrit.openvpn.net/c/openvpn/+/1372/comment/6ccb1870_75142321?usp=email :
PS1, Line 292:         const WCHAR *name_to_print = final_name ? final_name : L"";
> this looks a bit superfluous?  This branch is only entered `if (final_name)`
Done



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iac6bcf8b5434aa414e86cb4b9742e7dd591dc796
Gerrit-Change-Number: 1372
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Comment-Date: Sat, 15 Nov 2025 18:57:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>

[Openvpn-devel] [L] Change in openvpn[master]: tapctl: refactor 'create' command

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1374?usp=email

to look at the new patch set (#2).


Change subject: tapctl: refactor 'create' command
......................................................................

tapctl: refactor 'create' command

Make default adapter name selection logic more robust -
sometimes renaming fails because the deleted adapter name
might present in the registry even though adapter is not shown
anymore in enumeration.

Ensure that adapter name doesn't contain disallowed symbols.

Use --hwid ovpn-dco by default and update documentation.

Change-Id: I270f679505c90ef78a5afbad1e05219f166be089
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/tapctl/main.c
1 file changed, 280 insertions(+), 82 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/74/1374/2

diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 15c25ae..c92c122 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -71,12 +71,12 @@
     L"\n"
     L"--name <name>  Set VPN network adapter name. Should the adapter with given     \n"
     L"               name already exist, an error is returned. If this option is not \n"
-    L"               specified, a default adapter name is chosen by Windows.         \n"
+    L"               specified, an OpenVPN-specific default name is chosen.          \n"
     L"               Note: This name can also be specified as OpenVPN's --dev-node   \n"
     L"               option.                                                         \n"
-    L"--hwid <hwid>  Adapter hardware ID. Default value is root\\tap0901, which      \n"
-    L"               describes tap-windows6 driver. To work with ovpn-dco driver,    \n"
-    L"               driver, specify 'ovpn-dco'.                                     \n"
+    L"--hwid <hwid>  Adapter hardware ID. Default value is ovpn-dco, which uses      \n"
+    L"               the OpenVPN Data Channel Offload driver. To work with          \n"
+    L"               tap-windows6 driver, specify root\\tap0901 or tap0901.         \n"
     L"\n"
     L"Output:\n"
     L"\n"
@@ -125,26 +125,163 @@
 }
 
 /**
- * Checks if adapter with given name doesn't already exist
+ * Locate an adapter node by its friendly name within the enumerated list.
+ *
+ * @param name          Friendly name to search for. Comparison is case-insensitive.
+ * @param adapter_list  Head of the adapter list returned by tap_list_adapters().
+ *
+ * @return Pointer to the matching node, or NULL when not found.
  */
-static BOOL
-is_adapter_name_available(LPCWSTR name, struct tap_adapter_node *adapter_list, BOOL log)
+static struct tap_adapter_node *
+find_adapter_by_name(LPCWSTR name, struct tap_adapter_node *adapter_list)
 {
     for (struct tap_adapter_node *a = adapter_list; a; a = a->pNext)
     {
-        if (wcsicmp(name, a->szName) == 0)
+        if (_wcsicmp(name, a->szName) == 0)
         {
-            if (log)
-            {
-                LPOLESTR adapter_id = NULL;
-                StringFromIID((REFIID)&a->guid, &adapter_id);
-                fwprintf(stderr,
-                         L"Adapter \"%ls\" already exists (GUID %"
-                         L"ls).\n",
-                         a->szName, adapter_id);
-                CoTaskMemFree(adapter_id);
-            }
+            return a;
+        }
+    }
 
+    return NULL;
+}
+
+/**
+ * Check whether the registry still reserves a given network-connection name.
+ *
+ * Windows keeps friendly names under
+ * \\HKLM\\SYSTEM\\CurrentControlSet\\Control\\Network\\{NETCLASS}\\{GUID}\\Connection\\Name,
+ * even after an adapter is removed. netsh refuses to rename to any reserved name.
+ *
+ * @param name  Friendly name to test.
+ *
+ * @return TRUE if the name exists in the registry, FALSE otherwise.
+ */
+static BOOL
+registry_name_exists(LPCWSTR name)
+{
+    static const WCHAR class_key[] =
+        L"SYSTEM\\CurrentControlSet\\Control\\Network\\{4d36e972-e325-11ce-bfc1-08002be10318}";
+
+    HKEY hClassKey = NULL;
+    if (RegOpenKeyEx(HKEY_LOCAL_MACHINE, class_key, 0, KEY_READ, &hClassKey) != ERROR_SUCCESS)
+    {
+        return FALSE;
+    }
+
+    BOOL found = FALSE;
+
+    for (DWORD index = 0;; ++index)
+    {
+        WCHAR adapter_id[64];
+        DWORD adapter_id_len = _countof(adapter_id);
+        LONG result = RegEnumKeyEx(hClassKey, index, adapter_id, &adapter_id_len, NULL, NULL, NULL,
+                                   NULL);
+        if (result == ERROR_NO_MORE_ITEMS)
+        {
+            break;
+        }
+        else if (result != ERROR_SUCCESS)
+        {
+            continue;
+        }
+
+        WCHAR connection_key[512];
+        swprintf_s(connection_key, _countof(connection_key), L"%ls\\%ls\\Connection", class_key,
+                   adapter_id);
+
+        DWORD value_size = 0;
+        LONG query = RegGetValueW(HKEY_LOCAL_MACHINE, connection_key, L"Name",
+                                  RRF_RT_REG_SZ | RRF_NOEXPAND, NULL, NULL, &value_size);
+        if (query != ERROR_SUCCESS || value_size < sizeof(WCHAR))
+        {
+            continue;
+        }
+
+        LPWSTR value = (LPWSTR)malloc(value_size);
+        if (!value)
+        {
+            continue;
+        }
+
+        query = RegGetValueW(HKEY_LOCAL_MACHINE, connection_key, L"Name",
+                             RRF_RT_REG_SZ | RRF_NOEXPAND, NULL, value, &value_size);
+        if (query == ERROR_SUCCESS && _wcsicmp(name, value) == 0)
+        {
+            found = TRUE;
+            free(value);
+            break;
+        }
+
+        free(value);
+
+        if (found)
+        {
+            break;
+        }
+    }
+
+    RegCloseKey(hClassKey);
+    return found;
+}
+
+/**
+ * Determine whether a friendly name is currently in use by an adapter or reserved
+ * in the registry.
+ *
+ * @param name          Friendly name to test.
+ * @param adapter_list  Head of the adapter list returned by tap_list_adapters().
+ *
+ * @return TRUE when the name is taken/reserved, FALSE when available.
+ */
+static BOOL
+tap_name_in_use(LPCWSTR name, struct tap_adapter_node *adapter_list)
+{
+    if (name == NULL)
+    {
+        return FALSE;
+    }
+
+    if (find_adapter_by_name(name, adapter_list))
+    {
+        return TRUE;
+    }
+
+    return registry_name_exists(name);
+}
+
+/**
+ * Check whether a proposed adapter name satisfies Windows connection-name rules.
+ *
+ * Tabs, control characters (except space), and the following characters are disallowed:
+ * \ / : * ? " < > |
+ * Names must also be non-empty and no longer than 255 characters.
+ */
+BOOL
+tap_is_valid_adapter_name(LPCWSTR name)
+{
+    if (name == NULL)
+    {
+        return FALSE;
+    }
+
+    size_t length = wcslen(name);
+    if (length == 0 || length > 255)
+    {
+        return FALSE;
+    }
+
+    static const WCHAR invalid_chars[] = L"\\/:*?\"<>|";
+
+    for (const WCHAR *p = name; *p; ++p)
+    {
+        WCHAR ch = *p;
+        if (ch < L' ')
+        {
+            return FALSE;
+        }
+        if (wcschr(invalid_chars, ch))
+        {
             return FALSE;
         }
     }
@@ -153,81 +290,152 @@
 }
 
 /**
- * Returns unique adapter name based on hwid or NULL if name cannot be generated.
- * Caller is responsible for freeing it.
+ * Resolve the adapter name we should apply:
+ *   - For user-specified names, ensure they are unique both in the adapter list and
+ *     in the registry. On conflict, an explanatory message is printed and NULL is returned.
+ *   - For automatic naming, derive the base string from HWID and append the first available
+ *     suffix recognised by Windows.
+ *
+ * @param requested_name  Name provided via CLI or configuration (may be NULL/empty).
+ * @param hwid            Hardware identifier of the adapter being created.
+ * @param adapter_list    Existing adapters enumerated via tap_list_adapters().
+ *
+ * @return Newly allocated wide string containing the final name, or NULL on failure.
  */
 static LPWSTR
-get_unique_adapter_name(LPCWSTR hwid, struct tap_adapter_node *adapter_list)
+tap_resolve_adapter_name(LPCWSTR requested_name, LPCWSTR hwid,
+                         struct tap_adapter_node *adapter_list)
 {
+    if (requested_name && requested_name[0])
+    {
+        if (!tap_is_valid_adapter_name(requested_name))
+        {
+            fwprintf(stderr,
+                     L"Adapter name \"%ls\" contains invalid characters. Avoid tabs or the "
+                     L"characters \\ / : * ? \" < > | and keep the length within 255 characters.\n",
+                     requested_name);
+            return NULL;
+        }
+
+        struct tap_adapter_node *conflict = find_adapter_by_name(requested_name, adapter_list);
+        if (conflict)
+        {
+            LPOLESTR adapter_id = NULL;
+            StringFromIID((REFIID)&conflict->guid, &adapter_id);
+            fwprintf(stderr,
+                     L"Adapter \"%ls\" already exists (GUID %"
+                     L"ls).\n",
+                     conflict->szName, adapter_id);
+            CoTaskMemFree(adapter_id);
+            return NULL;
+        }
+
+        if (registry_name_exists(requested_name))
+        {
+            fwprintf(stderr, L"Adapter name \"%ls\" is already in use.\n", requested_name);
+            return NULL;
+        }
+
+        return wcsdup(requested_name);
+    }
+
     if (hwid == NULL)
     {
         return NULL;
     }
 
-    LPCWSTR base_name;
-    if (wcsicmp(hwid, L"ovpn-dco") == 0)
+    LPCWSTR base_name = NULL;
+    if (_wcsicmp(hwid, L"ovpn-dco") == 0)
     {
         base_name = L"OpenVPN Data Channel Offload";
     }
-    else if (wcsicmp(hwid, L"root\\" _L(TAP_WIN_COMPONENT_ID)) == 0)
+    else if (_wcsicmp(hwid, L"root\\" _L(TAP_WIN_COMPONENT_ID)) == 0
+             || _wcsicmp(hwid, _L(TAP_WIN_COMPONENT_ID)) == 0)
     {
         base_name = L"OpenVPN TAP-Windows6";
     }
     else
     {
+        fwprintf(stderr,
+                 L"Cannot auto-generate adapter name for hardware ID \"%ls\".\n", hwid);
         return NULL;
     }
 
-    if (is_adapter_name_available(base_name, adapter_list, FALSE))
+    if (!tap_name_in_use(base_name, adapter_list))
     {
         return wcsdup(base_name);
     }
 
     size_t name_len = wcslen(base_name) + 10;
-    LPWSTR name = malloc(name_len * sizeof(WCHAR));
+    LPWSTR name = (LPWSTR)malloc(name_len * sizeof(WCHAR));
     if (name == NULL)
     {
         return NULL;
     }
-    for (int i = 1; i < 100; ++i)
+
+    /* Windows never assigns the "#1" suffix, so skip it to avoid netsh failures. */
+    for (int i = 2; i < 100; ++i)
     {
         swprintf_s(name, name_len, L"%ls #%d", base_name, i);
 
-        if (is_adapter_name_available(name, adapter_list, FALSE))
+        if (!tap_name_in_use(name, adapter_list))
         {
             return name;
         }
     }
 
+    free(name);
+    fwprintf(stderr, L"Unable to find available adapter name based on \"%ls\".\n", base_name);
     return NULL;
 }
 
-static int command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
-static int command_list(int argc, LPCWSTR argv[]);
-static int command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
-
 static int
 command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
 {
     LPCWSTR szName = NULL;
-    LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
-    struct tap_adapter_node *adapter_list = NULL;
-    LPWSTR rename_name = NULL;
-    LPWSTR final_name = NULL;
-    LPOLESTR adapter_id = NULL;
+    LPCWSTR defaultHwId = L"ovpn-dco";
+    LPCWSTR szHwId = defaultHwId;
+    LPWSTR adapter_name = NULL;
+    struct tap_adapter_node *pAdapterList = NULL;
     GUID guidAdapter;
-    int result = 1;
-    BOOL delete_created_adapter = FALSE;
+    LPOLESTR szAdapterId = NULL;
+    DWORD dwResult;
+    int iResult = 1;
+    BOOL adapter_created = FALSE;
 
     for (int i = 2; i < argc; i++)
     {
         if (wcsicmp(argv[i], L"--name") == 0)
         {
-            szName = argv[++i];
+            if (++i >= argc)
+            {
+                fwprintf(stderr, L"--name option requires a value. Ignored.\n");
+                break;
+            }
+            szName = argv[i];
+            if (szName[0] == L'\0')
+            {
+                fwprintf(stderr, L"--name option cannot be empty. Ignored.\n");
+                szName = NULL;
+            }
         }
         else if (wcsicmp(argv[i], L"--hwid") == 0)
         {
-            szHwId = argv[++i];
+            if (++i >= argc)
+            {
+                fwprintf(stderr,
+                         L"--hwid option requires a value. Using default \"%ls\".\n",
+                         defaultHwId);
+                break;
+            }
+            szHwId = argv[i];
+            if (szHwId[0] == L'\0')
+            {
+                fwprintf(stderr,
+                         L"--hwid option cannot be empty. Using default \"%ls\".\n",
+                         defaultHwId);
+                szHwId = defaultHwId;
+            }
         }
         else
         {
@@ -238,69 +446,59 @@
         }
     }
 
-    DWORD dwResult =
-        tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, bRebootRequired, &guidAdapter);
+    dwResult = tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, bRebootRequired,
+                                  &guidAdapter);
     if (dwResult != ERROR_SUCCESS)
     {
-        fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
-        return result;
+        fwprintf(stderr, L"Creating network adapter failed (error 0x%x).\n", dwResult);
+        goto cleanup;
     }
+    adapter_created = TRUE;
 
-    dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+    dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
     if (dwResult != ERROR_SUCCESS)
     {
         fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
-        delete_created_adapter = TRUE;
         goto cleanup;
     }
 
-    rename_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
-    if (rename_name)
+    adapter_name = tap_resolve_adapter_name(szName, szHwId, pAdapterList);
+    if (adapter_name == NULL)
     {
-        if (szName && !is_adapter_name_available(rename_name, adapter_list, TRUE))
-        {
-            delete_created_adapter = TRUE;
-            goto cleanup;
-        }
-
-        dwResult = tap_set_adapter_name(&guidAdapter, rename_name, FALSE);
-        if (dwResult == ERROR_SUCCESS)
-        {
-            final_name = rename_name;
-            rename_name = NULL;
-        }
-        else
-        {
-            StringFromIID((REFIID)&guidAdapter, &adapter_id);
-            fwprintf(stderr,
-                     L"Renaming TUN/TAP adapter %ls"
-                     L" to \"%ls\" failed (error 0x%x).\n",
-                     adapter_id, rename_name, dwResult);
-            CoTaskMemFree(adapter_id);
-        }
+        goto cleanup;
     }
 
-    result = 0;
+    dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        StringFromIID((REFIID)&guidAdapter, &szAdapterId);
+        fwprintf(stderr,
+                 L"Renaming network adapter %ls to \"%ls\" failed (error 0x%x).\n", szAdapterId,
+                 adapter_name, dwResult);
+        CoTaskMemFree(szAdapterId);
+        goto cleanup;
+    }
+
+    iResult = 0;
+    StringFromIID((REFIID)&guidAdapter, &szAdapterId);
+    const WCHAR *name_to_print = (adapter_name && adapter_name[0]) ? adapter_name : L"(unnamed)";
+    const WCHAR *hwid_to_print = (szHwId && szHwId[0]) ? szHwId : L"(unknown hwid)";
+    fwprintf(stdout, L"%ls\t%ls\t%ls\n", szAdapterId, name_to_print, hwid_to_print);
+    CoTaskMemFree(szAdapterId);
 
 cleanup:
-    tap_free_adapter_list(adapter_list);
-    free(rename_name);
-
-    if (result == 0 && final_name)
+    if (pAdapterList)
     {
-        StringFromIID((REFIID)&guidAdapter, &adapter_id);
-        fwprintf(stdout, L"%ls\t%ls\t%ls\n", adapter_id, final_name, szHwId ? szHwId : L"");
-        CoTaskMemFree(adapter_id);
+        tap_free_adapter_list(pAdapterList);
     }
+    free(adapter_name);
 
-    free(final_name);
-
-    if (result != 0 && delete_created_adapter)
+    if (adapter_created && iResult != 0)
     {
         tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
     }
 
-    return result;
+    return iResult;
 }
 
 static int

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1374?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I270f679505c90ef78a5afbad1e05219f166be089
Gerrit-Change-Number: 1374
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>

[Openvpn-devel] [M] Change in openvpn[master]: tapctl: make output of 'list' and 'create' commands more verbose

[stipa (C. Review) <ge...@op...>]

Attention is currently required from: cron2, plaisthos, stipa.

Hello cron2, plaisthos, 

I'd like you to reexamine a change. Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email

to look at the new patch set (#2).

The following approvals got outdated and were removed:
Code-Review-1 by cron2


Change subject: tapctl: make output of 'list' and 'create' commands more verbose
......................................................................

tapctl: make output of 'list' and 'create' commands more verbose

Print adapter GUID, name and hwid.

Change-Id: Iac6bcf8b5434aa414e86cb4b9742e7dd591dc796
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/tapctl/main.c
1 file changed, 35 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/72/1372/2

diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 4d25998..15c25ae 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -80,7 +80,8 @@
     L"\n"
     L"Output:\n"
     L"\n"
-    L"This command prints newly created VPN network adapter's GUID to stdout.        \n"
+    L"This command prints newly created VPN network adapter's GUID, name and         \n"
+    L"hardware ID to stdout.                                                         \n"
 ;
 
 static const WCHAR usage_message_list[] =
@@ -99,7 +100,7 @@
     L"\n"
     L"Output:\n"
     L"\n"
-    L"This command prints all VPN network adapters to stdout.                        \n"
+    L"This command prints VPN network adapter GUID, name and hardware ID to stdout.  \n"
 ;
 
 static const WCHAR usage_message_delete[] =
@@ -211,10 +212,12 @@
     LPCWSTR szName = NULL;
     LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
     struct tap_adapter_node *adapter_list = NULL;
-    LPWSTR adapter_name = NULL;
+    LPWSTR rename_name = NULL;
+    LPWSTR final_name = NULL;
     LPOLESTR adapter_id = NULL;
     GUID guidAdapter;
     int result = 1;
+    BOOL delete_created_adapter = FALSE;
 
     for (int i = 2; i < argc; i++)
     {
@@ -247,45 +250,56 @@
     if (dwResult != ERROR_SUCCESS)
     {
         fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
-        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
-        return result;
+        delete_created_adapter = TRUE;
+        goto cleanup;
     }
 
-    adapter_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
-    if (adapter_name)
+    rename_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
+    if (rename_name)
     {
-        if (szName && !is_adapter_name_available(adapter_name, adapter_list, TRUE))
+        if (szName && !is_adapter_name_available(rename_name, adapter_list, TRUE))
         {
-            tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+            delete_created_adapter = TRUE;
             goto cleanup;
         }
 
-        dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
-        if (dwResult != ERROR_SUCCESS)
+        dwResult = tap_set_adapter_name(&guidAdapter, rename_name, FALSE);
+        if (dwResult == ERROR_SUCCESS)
+        {
+            final_name = rename_name;
+            rename_name = NULL;
+        }
+        else
         {
             StringFromIID((REFIID)&guidAdapter, &adapter_id);
             fwprintf(stderr,
                      L"Renaming TUN/TAP adapter %ls"
                      L" to \"%ls\" failed (error 0x%x).\n",
-                     adapter_id, adapter_name, dwResult);
+                     adapter_id, rename_name, dwResult);
             CoTaskMemFree(adapter_id);
-            goto cleanup;
         }
     }
 
     result = 0;
 
 cleanup:
-    free(adapter_name);
     tap_free_adapter_list(adapter_list);
+    free(rename_name);
 
-    if (result == 0)
+    if (result == 0 && final_name)
     {
         StringFromIID((REFIID)&guidAdapter, &adapter_id);
-        fwprintf(stdout, L"%ls\n", adapter_id);
+        fwprintf(stdout, L"%ls\t%ls\t%ls\n", adapter_id, final_name, szHwId ? szHwId : L"");
         CoTaskMemFree(adapter_id);
     }
 
+    free(final_name);
+
+    if (result != 0 && delete_created_adapter)
+    {
+        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+    }
+
     return result;
 }
 
@@ -327,10 +341,11 @@
     {
         LPOLESTR adapter_id = NULL;
         StringFromIID((REFIID)&adapter->guid, &adapter_id);
-        fwprintf(stdout,
-                 L"%ls\t%"
-                 L"ls\n",
-                 adapter_id, adapter->szName);
+        const WCHAR *name = adapter->szName ? adapter->szName : L"";
+        const WCHAR *hwid = (adapter->szzHardwareIDs && adapter->szzHardwareIDs[0])
+                                ? adapter->szzHardwareIDs
+                                : L"";
+        fwprintf(stdout, L"%ls\t%ls\t%ls\n", adapter_id, name, hwid);
         CoTaskMemFree(adapter_id);
     }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iac6bcf8b5434aa414e86cb4b9742e7dd591dc796
Gerrit-Change-Number: 1372
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: stipa <lst...@gm...>

[Openvpn-devel] [S] Change in openvpn[master]: win32: ensure plugin dir has the trailing slash

[selvanair (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos, stipa.

selvanair has posted comments on this change by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1332?usp=email )

Change subject: win32: ensure plugin dir has the trailing slash
......................................................................


Patch Set 2:

(2 comments)

Patchset:

PS2: 
> This is a bit annoying, with the back and forth of `PATHCCH_ENSURE_TRAILING_SLASH` in the iservice,  […]
A work around is to use PathCchAddBackslash() followed by PathCchCanonicalize() avoiding the dependence on the `PATHCCH_ENSURE_TRAILING_SLASH` flag.


File src/openvpn/win32.c:

http://gerrit.openvpn.net/c/openvpn/+/1332/comment/3384bb13_c17698f4?usp=email :
PS2, Line 1607:         && (wcsnicmp(plugin_dir, plugin_path, wcslen(plugin_dir)) == 0))
Is the logic right here? We are canonicalizing plugin_dir, but not plugin_path. Are we sure plugin_path cannot not contain "../"? The latter being user supplied, its more important to canonicalize it than the path read from HKLM.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1332?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I5ea90594493ab5cb858f7495f773e080b379e8e8
Gerrit-Change-Number: 1332
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-CC: selvanair <sel...@gm...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Sat, 15 Nov 2025 17:16:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: cron2 <ge...@gr...>

[Openvpn-devel] [S] Change in openvpn[master]: route: handle default gateway (net_gateway) and nexthop towards VPN s...

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, mrbff, plaisthos.

cron2 has posted comments on this change by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/1222?usp=email )

Change subject: route: handle default gateway (net_gateway) and nexthop towards VPN server separately
......................................................................


Patch Set 2: Code-Review-1

(1 comment)

Patchset:

PS2: 
This is only half-working for me.  Triggered by https://github.com/OpenVPN/openvpn/issues/890 I have built a testbed to reproduce this.

- ssh -R *:12345:conn-test-server.openvpn.org:51194 $linuxserver
- on the linux server, run `openvpn --client --remote 127.0.0.5 51194 tcp ... --route 1.1.1.1 255.255.255.255 net_gateway`

it will do both gateway lookups, and setenv `net_gateway` accordingly

```
2025-11-15 17:57:26 net_route_v4_best_gw query: dst 0.0.0.0
2025-11-15 17:57:26 net_route_v4_best_gw result: via 194.97.140.30 dev enp0s18
2025-11-15 17:57:26 net_route_v4_best_gw query: dst 127.0.0.5
2025-11-15 17:57:26 net_route_v4_best_gw result: via 0.0.0.0 dev lo
2025-11-15 17:57:26 GDG6: remote_host_ipv6=n/a
2025-11-15 17:57:26 net_route_v6_best_gw query: dst ::
2025-11-15 17:57:26 net_route_v6_best_gw result: via 2001:608:0:814::ffff dev enp0s18
2025-11-15 17:57:26 net_route_v6_best_gw query: dst ::
2025-11-15 17:57:26 net_route_v6_best_gw result: via 2001:608:0:814::ffff dev enp0s18
```

but the gateway address used for `net_gateway` is the one from the second lookup

```
2025-11-15 18:03:45 net_route_v4_add: 1.1.1.1/32 via 0.0.0.0 dev [NULL] table 0 metric -1
2025-11-15 18:03:45 sitnl_send: rtnl: generic error (-19): No such device
2025-11-15 18:03:45 ERROR: Linux route add command failed
```

so we might need to store the `ngi` in the `struct route-list` as well, and depending on use case use `rgi` or `ngi` - for the bypass-routes, we want `rgi`, for `get_special_addr()` use case, we want `ngi`...

right?



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1222?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I16d90221d0a75193035253817ff195f6da9dc0b3
Gerrit-Change-Number: 1222
Gerrit-PatchSet: 2
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-Comment-Date: Sat, 15 Nov 2025 17:07:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

[Openvpn-devel] [S] Change in openvpn[master]: win32: ensure plugin dir has the trailing slash

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos, stipa.

cron2 has posted comments on this change by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1332?usp=email )

Change subject: win32: ensure plugin dir has the trailing slash
......................................................................


Patch Set 2: Code-Review-2

(1 comment)

Patchset:

PS2: 
This is a bit annoying, with the back and forth of `PATHCCH_ENSURE_TRAILING_SLASH` in the iservice, we actually lost the configure.ac block that tests for it again, in commit 763160a16a.  So this can't go in as it is.

So I do wonder if it makes more sense to have a manual check + `strcat()` on `normalized_plugin_dir` instead?  Since this is already normalized via `GetFullPathNameW()`, bringing in the new API just for the last character really is a bit heavy...



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1332?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I5ea90594493ab5cb858f7495f773e080b379e8e8
Gerrit-Change-Number: 1332
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Sat, 15 Nov 2025 16:49:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

[Openvpn-devel] [L] Change in openvpn[master]: tapctl: factor out command handlers

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1370?usp=email )

Change subject: tapctl: factor out command handlers
......................................................................

tapctl: factor out command handlers

Change-Id: I432e07216f9adb8f767af172fa37b626b350f994
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1370
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34432.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/tapctl/main.c
1 file changed, 196 insertions(+), 202 deletions(-)




diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 84da161..4d25998 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -201,6 +201,198 @@
     return NULL;
 }
 
+static int command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
+static int command_list(int argc, LPCWSTR argv[]);
+static int command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
+
+static int
+command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
+{
+    LPCWSTR szName = NULL;
+    LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
+    struct tap_adapter_node *adapter_list = NULL;
+    LPWSTR adapter_name = NULL;
+    LPOLESTR adapter_id = NULL;
+    GUID guidAdapter;
+    int result = 1;
+
+    for (int i = 2; i < argc; i++)
+    {
+        if (wcsicmp(argv[i], L"--name") == 0)
+        {
+            szName = argv[++i];
+        }
+        else if (wcsicmp(argv[i], L"--hwid") == 0)
+        {
+            szHwId = argv[++i];
+        }
+        else
+        {
+            fwprintf(stderr,
+                     L"Unknown option \"%ls"
+                     L"\". Please, use \"tapctl help create\" to list supported options. Ignored.\n",
+                     argv[i]);
+        }
+    }
+
+    DWORD dwResult =
+        tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, bRebootRequired, &guidAdapter);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
+        return result;
+    }
+
+    dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
+        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+        return result;
+    }
+
+    adapter_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
+    if (adapter_name)
+    {
+        if (szName && !is_adapter_name_available(adapter_name, adapter_list, TRUE))
+        {
+            tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+            goto cleanup;
+        }
+
+        dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            StringFromIID((REFIID)&guidAdapter, &adapter_id);
+            fwprintf(stderr,
+                     L"Renaming TUN/TAP adapter %ls"
+                     L" to \"%ls\" failed (error 0x%x).\n",
+                     adapter_id, adapter_name, dwResult);
+            CoTaskMemFree(adapter_id);
+            goto cleanup;
+        }
+    }
+
+    result = 0;
+
+cleanup:
+    free(adapter_name);
+    tap_free_adapter_list(adapter_list);
+
+    if (result == 0)
+    {
+        StringFromIID((REFIID)&guidAdapter, &adapter_id);
+        fwprintf(stdout, L"%ls\n", adapter_id);
+        CoTaskMemFree(adapter_id);
+    }
+
+    return result;
+}
+
+static int
+command_list(int argc, LPCWSTR argv[])
+{
+    WCHAR szzHwId[0x100] =
+        L"root\\" _L(TAP_WIN_COMPONENT_ID) L"\0" _L(TAP_WIN_COMPONENT_ID) L"\0"
+                                                                          L"ovpn-dco\0";
+
+    for (int i = 2; i < argc; i++)
+    {
+        if (wcsicmp(argv[i], L"--hwid") == 0)
+        {
+            memset(szzHwId, 0, sizeof(szzHwId));
+            ++i;
+            memcpy_s(szzHwId,
+                     sizeof(szzHwId) - 2 * sizeof(WCHAR),
+                     argv[i], wcslen(argv[i]) * sizeof(WCHAR));
+        }
+        else
+        {
+            fwprintf(stderr,
+                     L"Unknown option \"%ls"
+                     L"\". Please, use \"tapctl help list\" to list supported options. Ignored.\n",
+                     argv[i]);
+        }
+    }
+
+    struct tap_adapter_node *adapter_list = NULL;
+    DWORD dwResult = tap_list_adapters(NULL, szzHwId, &adapter_list);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
+        return 1;
+    }
+
+    for (struct tap_adapter_node *adapter = adapter_list; adapter; adapter = adapter->pNext)
+    {
+        LPOLESTR adapter_id = NULL;
+        StringFromIID((REFIID)&adapter->guid, &adapter_id);
+        fwprintf(stdout,
+                 L"%ls\t%"
+                 L"ls\n",
+                 adapter_id, adapter->szName);
+        CoTaskMemFree(adapter_id);
+    }
+
+    tap_free_adapter_list(adapter_list);
+
+    return 0;
+}
+
+static int
+command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
+{
+    if (argc < 3)
+    {
+        fwprintf(stderr,
+                 L"Missing adapter GUID or name. Please, use \"tapctl help delete\" for usage info.\n");
+        return 1;
+    }
+
+    GUID guidAdapter;
+    if (FAILED(IIDFromString(argv[2], (LPIID)&guidAdapter)))
+    {
+        struct tap_adapter_node *adapter_list = NULL;
+        DWORD dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
+            return 1;
+        }
+
+        BOOL found = FALSE;
+        for (struct tap_adapter_node *adapter = adapter_list; adapter; adapter = adapter->pNext)
+        {
+            if (wcsicmp(argv[2], adapter->szName) == 0)
+            {
+                memcpy(&guidAdapter, &adapter->guid, sizeof(GUID));
+                found = TRUE;
+                break;
+            }
+        }
+
+        tap_free_adapter_list(adapter_list);
+
+        if (!found)
+        {
+            fwprintf(stderr, L"\"%ls\" adapter not found.\n", argv[2]);
+            return 1;
+        }
+    }
+
+    DWORD dwResult = tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr,
+                 L"Deleting adapter \"%ls"
+                 L"\" failed (error 0x%x).\n",
+                 argv[2], dwResult);
+        return 1;
+    }
+
+    return 0;
+}
+
 /**
  * Program entry point
  */
@@ -248,215 +440,17 @@
     }
     else if (wcsicmp(argv[1], L"create") == 0)
     {
-        LPCWSTR szName = NULL;
-        LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
-
-        /* Parse options. */
-        for (int i = 2; i < argc; i++)
-        {
-            if (wcsicmp(argv[i], L"--name") == 0)
-            {
-                szName = argv[++i];
-            }
-            else if (wcsicmp(argv[i], L"--hwid") == 0)
-            {
-                szHwId = argv[++i];
-            }
-            else
-            {
-                fwprintf(
-                    stderr,
-                    L"Unknown option \"%ls"
-                    L"\". Please, use \"tapctl help create\" to list supported options. Ignored.\n",
-                    argv[i]);
-            }
-        }
-
-        /* Create TUN/TAP adapter. */
-        GUID guidAdapter;
-        LPOLESTR szAdapterId = NULL;
-        DWORD dwResult =
-            tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, &bRebootRequired, &guidAdapter);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        /* Get existing network adapters. */
-        struct tap_adapter_node *pAdapterList = NULL;
-        dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto create_delete_adapter;
-        }
-
-        LPWSTR adapter_name =
-            szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, pAdapterList);
-        if (adapter_name)
-        {
-            /* Check for duplicates when name was specified,
-             * otherwise get_adapter_default_name() takes care of it */
-            if (szName && !is_adapter_name_available(adapter_name, pAdapterList, TRUE))
-            {
-                iResult = 1;
-                goto create_cleanup_pAdapterList;
-            }
-
-            /* Rename the adapter. */
-            dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
-            if (dwResult != ERROR_SUCCESS)
-            {
-                StringFromIID((REFIID)&guidAdapter, &szAdapterId);
-                fwprintf(stderr,
-                         L"Renaming TUN/TAP adapter %ls"
-                         L" to \"%ls\" failed (error 0x%x).\n",
-                         szAdapterId, adapter_name, dwResult);
-                CoTaskMemFree(szAdapterId);
-                iResult = 1;
-                goto quit;
-            }
-        }
-
-        iResult = 0;
-
-create_cleanup_pAdapterList:
-        free(adapter_name);
-
-        tap_free_adapter_list(pAdapterList);
-        if (iResult)
-        {
-            goto create_delete_adapter;
-        }
-
-        /* Output adapter GUID. */
-        StringFromIID((REFIID)&guidAdapter, &szAdapterId);
-        fwprintf(stdout, L"%ls\n", szAdapterId);
-        CoTaskMemFree(szAdapterId);
-
-        iResult = 0;
-        goto quit;
-
-create_delete_adapter:
-        tap_delete_adapter(NULL, &guidAdapter, &bRebootRequired);
-        iResult = 1;
+        iResult = command_create(argc, argv, &bRebootRequired);
         goto quit;
     }
     else if (wcsicmp(argv[1], L"list") == 0)
     {
-        WCHAR szzHwId[0x100] =
-            L"root\\" _L(TAP_WIN_COMPONENT_ID) L"\0" _L(TAP_WIN_COMPONENT_ID) L"\0"
-                                                                              L"ovpn-dco\0";
-
-        /* Parse options. */
-        for (int i = 2; i < argc; i++)
-        {
-            if (wcsicmp(argv[i], L"--hwid") == 0)
-            {
-                memset(szzHwId, 0, sizeof(szzHwId));
-                ++i;
-                memcpy_s(szzHwId,
-                         sizeof(szzHwId) - 2 * sizeof(WCHAR) /*requires double zero termination*/,
-                         argv[i], wcslen(argv[i]) * sizeof(WCHAR));
-            }
-            else
-            {
-                fwprintf(
-                    stderr,
-                    L"Unknown option \"%ls"
-                    L"\". Please, use \"tapctl help list\" to list supported options. Ignored.\n",
-                    argv[i]);
-            }
-        }
-
-        /* Output list of adapters with given hardware ID. */
-        struct tap_adapter_node *pAdapterList = NULL;
-        DWORD dwResult = tap_list_adapters(NULL, szzHwId, &pAdapterList);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        for (struct tap_adapter_node *pAdapter = pAdapterList; pAdapter; pAdapter = pAdapter->pNext)
-        {
-            LPOLESTR szAdapterId = NULL;
-            StringFromIID((REFIID)&pAdapter->guid, &szAdapterId);
-            fwprintf(stdout,
-                     L"%ls\t%"
-                     L"ls\n",
-                     szAdapterId, pAdapter->szName);
-            CoTaskMemFree(szAdapterId);
-        }
-
-        iResult = 0;
-        tap_free_adapter_list(pAdapterList);
+        iResult = command_list(argc, argv);
+        goto quit;
     }
     else if (wcsicmp(argv[1], L"delete") == 0)
     {
-        if (argc < 3)
-        {
-            fwprintf(
-                stderr,
-                L"Missing adapter GUID or name. Please, use \"tapctl help delete\" for usage info.\n");
-            return 1;
-        }
-
-        GUID guidAdapter;
-        if (FAILED(IIDFromString(argv[2], (LPIID)&guidAdapter)))
-        {
-            /* The argument failed to covert to GUID. Treat it as the adapter name. */
-            struct tap_adapter_node *pAdapterList = NULL;
-            DWORD dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
-            if (dwResult != ERROR_SUCCESS)
-            {
-                fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
-                iResult = 1;
-                goto quit;
-            }
-
-            for (struct tap_adapter_node *pAdapter = pAdapterList;; pAdapter = pAdapter->pNext)
-            {
-                if (pAdapter == NULL)
-                {
-                    fwprintf(stderr, L"\"%ls\" adapter not found.\n", argv[2]);
-                    iResult = 1;
-                    goto delete_cleanup_pAdapterList;
-                }
-                else if (wcsicmp(argv[2], pAdapter->szName) == 0)
-                {
-                    memcpy(&guidAdapter, &pAdapter->guid, sizeof(GUID));
-                    break;
-                }
-            }
-
-            iResult = 0;
-
-delete_cleanup_pAdapterList:
-            tap_free_adapter_list(pAdapterList);
-            if (iResult)
-            {
-                goto quit;
-            }
-        }
-
-        /* Delete the network adapter. */
-        DWORD dwResult = tap_delete_adapter(NULL, &guidAdapter, &bRebootRequired);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr,
-                     L"Deleting adapter \"%ls"
-                     L"\" failed (error 0x%x).\n",
-                     argv[2], dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        iResult = 0;
+        iResult = command_delete(argc, argv, &bRebootRequired);
         goto quit;
     }
     else

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1370?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I432e07216f9adb8f767af172fa37b626b350f994
Gerrit-Change-Number: 1370
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [L] Change in openvpn[master]: tapctl: factor out command handlers

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#2) to the change originally created by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1370?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: tapctl: factor out command handlers
......................................................................

tapctl: factor out command handlers

Change-Id: I432e07216f9adb8f767af172fa37b626b350f994
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1370
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34432.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/tapctl/main.c
1 file changed, 196 insertions(+), 202 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/70/1370/2

diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 84da161..4d25998 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -201,6 +201,198 @@
     return NULL;
 }
 
+static int command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
+static int command_list(int argc, LPCWSTR argv[]);
+static int command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
+
+static int
+command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
+{
+    LPCWSTR szName = NULL;
+    LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
+    struct tap_adapter_node *adapter_list = NULL;
+    LPWSTR adapter_name = NULL;
+    LPOLESTR adapter_id = NULL;
+    GUID guidAdapter;
+    int result = 1;
+
+    for (int i = 2; i < argc; i++)
+    {
+        if (wcsicmp(argv[i], L"--name") == 0)
+        {
+            szName = argv[++i];
+        }
+        else if (wcsicmp(argv[i], L"--hwid") == 0)
+        {
+            szHwId = argv[++i];
+        }
+        else
+        {
+            fwprintf(stderr,
+                     L"Unknown option \"%ls"
+                     L"\". Please, use \"tapctl help create\" to list supported options. Ignored.\n",
+                     argv[i]);
+        }
+    }
+
+    DWORD dwResult =
+        tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, bRebootRequired, &guidAdapter);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
+        return result;
+    }
+
+    dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
+        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+        return result;
+    }
+
+    adapter_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
+    if (adapter_name)
+    {
+        if (szName && !is_adapter_name_available(adapter_name, adapter_list, TRUE))
+        {
+            tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+            goto cleanup;
+        }
+
+        dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            StringFromIID((REFIID)&guidAdapter, &adapter_id);
+            fwprintf(stderr,
+                     L"Renaming TUN/TAP adapter %ls"
+                     L" to \"%ls\" failed (error 0x%x).\n",
+                     adapter_id, adapter_name, dwResult);
+            CoTaskMemFree(adapter_id);
+            goto cleanup;
+        }
+    }
+
+    result = 0;
+
+cleanup:
+    free(adapter_name);
+    tap_free_adapter_list(adapter_list);
+
+    if (result == 0)
+    {
+        StringFromIID((REFIID)&guidAdapter, &adapter_id);
+        fwprintf(stdout, L"%ls\n", adapter_id);
+        CoTaskMemFree(adapter_id);
+    }
+
+    return result;
+}
+
+static int
+command_list(int argc, LPCWSTR argv[])
+{
+    WCHAR szzHwId[0x100] =
+        L"root\\" _L(TAP_WIN_COMPONENT_ID) L"\0" _L(TAP_WIN_COMPONENT_ID) L"\0"
+                                                                          L"ovpn-dco\0";
+
+    for (int i = 2; i < argc; i++)
+    {
+        if (wcsicmp(argv[i], L"--hwid") == 0)
+        {
+            memset(szzHwId, 0, sizeof(szzHwId));
+            ++i;
+            memcpy_s(szzHwId,
+                     sizeof(szzHwId) - 2 * sizeof(WCHAR),
+                     argv[i], wcslen(argv[i]) * sizeof(WCHAR));
+        }
+        else
+        {
+            fwprintf(stderr,
+                     L"Unknown option \"%ls"
+                     L"\". Please, use \"tapctl help list\" to list supported options. Ignored.\n",
+                     argv[i]);
+        }
+    }
+
+    struct tap_adapter_node *adapter_list = NULL;
+    DWORD dwResult = tap_list_adapters(NULL, szzHwId, &adapter_list);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
+        return 1;
+    }
+
+    for (struct tap_adapter_node *adapter = adapter_list; adapter; adapter = adapter->pNext)
+    {
+        LPOLESTR adapter_id = NULL;
+        StringFromIID((REFIID)&adapter->guid, &adapter_id);
+        fwprintf(stdout,
+                 L"%ls\t%"
+                 L"ls\n",
+                 adapter_id, adapter->szName);
+        CoTaskMemFree(adapter_id);
+    }
+
+    tap_free_adapter_list(adapter_list);
+
+    return 0;
+}
+
+static int
+command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
+{
+    if (argc < 3)
+    {
+        fwprintf(stderr,
+                 L"Missing adapter GUID or name. Please, use \"tapctl help delete\" for usage info.\n");
+        return 1;
+    }
+
+    GUID guidAdapter;
+    if (FAILED(IIDFromString(argv[2], (LPIID)&guidAdapter)))
+    {
+        struct tap_adapter_node *adapter_list = NULL;
+        DWORD dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
+            return 1;
+        }
+
+        BOOL found = FALSE;
+        for (struct tap_adapter_node *adapter = adapter_list; adapter; adapter = adapter->pNext)
+        {
+            if (wcsicmp(argv[2], adapter->szName) == 0)
+            {
+                memcpy(&guidAdapter, &adapter->guid, sizeof(GUID));
+                found = TRUE;
+                break;
+            }
+        }
+
+        tap_free_adapter_list(adapter_list);
+
+        if (!found)
+        {
+            fwprintf(stderr, L"\"%ls\" adapter not found.\n", argv[2]);
+            return 1;
+        }
+    }
+
+    DWORD dwResult = tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr,
+                 L"Deleting adapter \"%ls"
+                 L"\" failed (error 0x%x).\n",
+                 argv[2], dwResult);
+        return 1;
+    }
+
+    return 0;
+}
+
 /**
  * Program entry point
  */
@@ -248,215 +440,17 @@
     }
     else if (wcsicmp(argv[1], L"create") == 0)
     {
-        LPCWSTR szName = NULL;
-        LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
-
-        /* Parse options. */
-        for (int i = 2; i < argc; i++)
-        {
-            if (wcsicmp(argv[i], L"--name") == 0)
-            {
-                szName = argv[++i];
-            }
-            else if (wcsicmp(argv[i], L"--hwid") == 0)
-            {
-                szHwId = argv[++i];
-            }
-            else
-            {
-                fwprintf(
-                    stderr,
-                    L"Unknown option \"%ls"
-                    L"\". Please, use \"tapctl help create\" to list supported options. Ignored.\n",
-                    argv[i]);
-            }
-        }
-
-        /* Create TUN/TAP adapter. */
-        GUID guidAdapter;
-        LPOLESTR szAdapterId = NULL;
-        DWORD dwResult =
-            tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, &bRebootRequired, &guidAdapter);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        /* Get existing network adapters. */
-        struct tap_adapter_node *pAdapterList = NULL;
-        dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto create_delete_adapter;
-        }
-
-        LPWSTR adapter_name =
-            szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, pAdapterList);
-        if (adapter_name)
-        {
-            /* Check for duplicates when name was specified,
-             * otherwise get_adapter_default_name() takes care of it */
-            if (szName && !is_adapter_name_available(adapter_name, pAdapterList, TRUE))
-            {
-                iResult = 1;
-                goto create_cleanup_pAdapterList;
-            }
-
-            /* Rename the adapter. */
-            dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
-            if (dwResult != ERROR_SUCCESS)
-            {
-                StringFromIID((REFIID)&guidAdapter, &szAdapterId);
-                fwprintf(stderr,
-                         L"Renaming TUN/TAP adapter %ls"
-                         L" to \"%ls\" failed (error 0x%x).\n",
-                         szAdapterId, adapter_name, dwResult);
-                CoTaskMemFree(szAdapterId);
-                iResult = 1;
-                goto quit;
-            }
-        }
-
-        iResult = 0;
-
-create_cleanup_pAdapterList:
-        free(adapter_name);
-
-        tap_free_adapter_list(pAdapterList);
-        if (iResult)
-        {
-            goto create_delete_adapter;
-        }
-
-        /* Output adapter GUID. */
-        StringFromIID((REFIID)&guidAdapter, &szAdapterId);
-        fwprintf(stdout, L"%ls\n", szAdapterId);
-        CoTaskMemFree(szAdapterId);
-
-        iResult = 0;
-        goto quit;
-
-create_delete_adapter:
-        tap_delete_adapter(NULL, &guidAdapter, &bRebootRequired);
-        iResult = 1;
+        iResult = command_create(argc, argv, &bRebootRequired);
         goto quit;
     }
     else if (wcsicmp(argv[1], L"list") == 0)
     {
-        WCHAR szzHwId[0x100] =
-            L"root\\" _L(TAP_WIN_COMPONENT_ID) L"\0" _L(TAP_WIN_COMPONENT_ID) L"\0"
-                                                                              L"ovpn-dco\0";
-
-        /* Parse options. */
-        for (int i = 2; i < argc; i++)
-        {
-            if (wcsicmp(argv[i], L"--hwid") == 0)
-            {
-                memset(szzHwId, 0, sizeof(szzHwId));
-                ++i;
-                memcpy_s(szzHwId,
-                         sizeof(szzHwId) - 2 * sizeof(WCHAR) /*requires double zero termination*/,
-                         argv[i], wcslen(argv[i]) * sizeof(WCHAR));
-            }
-            else
-            {
-                fwprintf(
-                    stderr,
-                    L"Unknown option \"%ls"
-                    L"\". Please, use \"tapctl help list\" to list supported options. Ignored.\n",
-                    argv[i]);
-            }
-        }
-
-        /* Output list of adapters with given hardware ID. */
-        struct tap_adapter_node *pAdapterList = NULL;
-        DWORD dwResult = tap_list_adapters(NULL, szzHwId, &pAdapterList);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        for (struct tap_adapter_node *pAdapter = pAdapterList; pAdapter; pAdapter = pAdapter->pNext)
-        {
-            LPOLESTR szAdapterId = NULL;
-            StringFromIID((REFIID)&pAdapter->guid, &szAdapterId);
-            fwprintf(stdout,
-                     L"%ls\t%"
-                     L"ls\n",
-                     szAdapterId, pAdapter->szName);
-            CoTaskMemFree(szAdapterId);
-        }
-
-        iResult = 0;
-        tap_free_adapter_list(pAdapterList);
+        iResult = command_list(argc, argv);
+        goto quit;
     }
     else if (wcsicmp(argv[1], L"delete") == 0)
     {
-        if (argc < 3)
-        {
-            fwprintf(
-                stderr,
-                L"Missing adapter GUID or name. Please, use \"tapctl help delete\" for usage info.\n");
-            return 1;
-        }
-
-        GUID guidAdapter;
-        if (FAILED(IIDFromString(argv[2], (LPIID)&guidAdapter)))
-        {
-            /* The argument failed to covert to GUID. Treat it as the adapter name. */
-            struct tap_adapter_node *pAdapterList = NULL;
-            DWORD dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
-            if (dwResult != ERROR_SUCCESS)
-            {
-                fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
-                iResult = 1;
-                goto quit;
-            }
-
-            for (struct tap_adapter_node *pAdapter = pAdapterList;; pAdapter = pAdapter->pNext)
-            {
-                if (pAdapter == NULL)
-                {
-                    fwprintf(stderr, L"\"%ls\" adapter not found.\n", argv[2]);
-                    iResult = 1;
-                    goto delete_cleanup_pAdapterList;
-                }
-                else if (wcsicmp(argv[2], pAdapter->szName) == 0)
-                {
-                    memcpy(&guidAdapter, &pAdapter->guid, sizeof(GUID));
-                    break;
-                }
-            }
-
-            iResult = 0;
-
-delete_cleanup_pAdapterList:
-            tap_free_adapter_list(pAdapterList);
-            if (iResult)
-            {
-                goto quit;
-            }
-        }
-
-        /* Delete the network adapter. */
-        DWORD dwResult = tap_delete_adapter(NULL, &guidAdapter, &bRebootRequired);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr,
-                     L"Deleting adapter \"%ls"
-                     L"\" failed (error 0x%x).\n",
-                     argv[2], dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        iResult = 0;
+        iResult = command_delete(argc, argv, &bRebootRequired);
         goto quit;
     }
     else

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1370?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I432e07216f9adb8f767af172fa37b626b350f994
Gerrit-Change-Number: 1370
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [PATCH applied] Re: tapctl: factor out command handlers

[Gert D. <ge...@gr...>]

Reviewed ("this is all code that is the same as before, just moved into
dedicated functions, pointers renamed/adjusted, status signalling
adjusted...", so a bit less trivial than "git show --color-moved -w").

Reasonable change, much simpler "units of comprehension" now.

BB is happy, and so is my local Ubuntu + MinGW compile.

Your patch has been applied to the master branch.

commit ef4937f87280f68fb14459ca53d955c8d5d771e5
Author: Lev Stipakov
Date:   Fri Nov 14 22:21:07 2025 +0100

     tapctl: factor out command handlers

     Signed-off-by: Lev Stipakov <le...@op...>
     Acked-by: Gert Doering <ge...@gr...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1370
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg34432.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

Re: [Openvpn-devel] [PATCH ovpn v3] ovpn: consolidate crypto allocations in one chunk

[Sabrina D. <sd...@qu...>]

2025-11-14, 11:40:29 +0100, Ralf Lici wrote:
> Currently ovpn uses three separate dynamically allocated structures to
> set up cryptographic operations for both encryption and decryption. This
> adds overhead to performance-critical paths and contribute to memory
> fragmentation.
> 
> This commit consolidates those allocations into a single temporary blob,
> similar to what esp_alloc_tmp() does.
> 
> The resulting performance gain is +7.7% and +4.3% for UDP when using AES
> and ChaChaPoly respectively, and +4.3% for TCP.
> 
> Signed-off-by: Ralf Lici <ra...@ma...>
> Signed-off-by: Antonio Quartulli <an...@op...>
> ---
> Changes since v2:
> - Replaced manual multiplication with array_size() helper
> - Made ovpn_aead_crypto_tmp_size() less generic by asserting that IV
>   size equals OVPN_NONCE_SIZE
> 
> Changes since v1:
> - Fixed typo in commit message
> - Adjusted ovpn_aead_crypto_tmp_size comment to follow kdoc style
> - Stored allocated blob in the skb control block immediately after
>   allocation to prevent leakage on failure
> - Removed 'inline' from function declarations in crypto_aead.c
> 
>  drivers/net/ovpn/crypto_aead.c | 160 +++++++++++++++++++++++++--------
>  drivers/net/ovpn/io.c          |   8 +-
>  drivers/net/ovpn/skb.h         |  13 ++-
>  3 files changed, 135 insertions(+), 46 deletions(-)

Reviewed-by: Sabrina Dubroca <sd...@qu...>

Thanks Ralf.

-- 
Sabrina

Re: [Openvpn-devel] [PATCH ovpn v2] ovpn: notify userspace on client float event

[Sabrina D. <sd...@qu...>]

2025-11-14, 11:39:40 +0100, Ralf Lici wrote:
> Send a netlink notification when a client updates its remote UDP
> endpoint. The notification includes the new IP address, port, and scope
> ID (for IPv6).
> 
> Signed-off-by: Ralf Lici <ra...@ma...>
> Signed-off-by: Antonio Quartulli <an...@op...>
> ---
> Changes since v1:
> - correctly set return value for unsupported AF in
>   ovpn_nl_peer_float_notify
> 
>  Documentation/netlink/specs/ovpn.yaml       |  6 ++
>  drivers/net/ovpn/netlink.c                  | 82 +++++++++++++++++++++
>  drivers/net/ovpn/netlink.h                  |  2 +
>  drivers/net/ovpn/peer.c                     |  2 +
>  include/uapi/linux/ovpn.h                   |  1 +
>  tools/testing/selftests/net/ovpn/ovpn-cli.c |  3 +
>  6 files changed, 96 insertions(+)

Reviewed-by: Sabrina Dubroca <sd...@qu...>

-- 
Sabrina

[Openvpn-devel] [PATCH v2] win32: ensure plugin dir has the trailing slash

[Gert D. <ge...@gr...>]

From: Lev Stipakov <le...@op...>

This prevents loading plugins from the directories
which share initial characters with the trusted path.

Reported-by: Joshua Rogers <co...@jo...>
Found-by: ZeroPath (https://zeropath.com/)

Change-Id: I5ea90594493ab5cb858f7495f773e080b379e8e8
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1332
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1332
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/CMakeLists.txt b/CMakeLists.txt
index c9301e6..6888de3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -245,6 +245,9 @@
 check_symbol_exists(chsize io.h HAVE_CHSIZE)
 check_symbol_exists(getrlimit sys/resource.h HAVE_GETRLIMIT)
 
+# Some old versions of mingw does not have PATHCCH_OPTIONS enums -- add a check
+check_symbol_exists(PATHCCH_ENSURE_TRAILING_SLASH pathcch.h HAVE_PATHCCH_ENSURE_TRAILING_SLASH)
+
 # Some OS (e.g. FreeBSD) need some basic headers to allow
 # including network headers
 set(NETEXTRA sys/types.h)
@@ -338,7 +341,7 @@
         if (WIN32)
             target_link_libraries(${target} PUBLIC
                 ws2_32.lib crypt32.lib fwpuclnt.lib iphlpapi.lib
-                wininet.lib setupapi.lib rpcrt4.lib wtsapi32.lib ncrypt.lib bcrypt.lib)
+                wininet.lib setupapi.lib rpcrt4.lib wtsapi32.lib ncrypt.lib bcrypt.lib pathcch.lib)
         endif ()
 
     endif ()
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index db87dfc..a2b5e92 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -166,5 +166,5 @@
 	$(OPTIONAL_INOTIFY_LIBS)
 if WIN32
 openvpn_SOURCES += openvpn_win32_resources.rc wfp_block.c wfp_block.h
-openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt
+openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi -lbcrypt -lpathcch
 endif
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index df29dd7..3ed28f6 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -51,6 +51,12 @@
 
 #include "wfp_block.h"
 
+#include <pathcch.h>
+
+#ifndef HAVE_PATHCCH_ENSURE_TRAILING_SLASH
+#define PATHCCH_ENSURE_TRAILING_SLASH 0x20
+#endif
+
 /*
  * WFP handle
  */
@@ -1553,12 +1559,12 @@
         return false;
     }
 
-    WCHAR plugin_dir[MAX_PATH] = { 0 };
+    WCHAR plugin_dir_reg[MAX_PATH] = { 0 };
 
     /* Attempt to retrieve the trusted plugin directory path from the registry,
      * using installation path as a fallback */
-    if (!get_openvpn_reg_value(L"plugin_dir", plugin_dir, _countof(plugin_dir))
-        && !get_openvpn_reg_value(NULL, plugin_dir, _countof(plugin_dir)))
+    if (!get_openvpn_reg_value(L"plugin_dir", plugin_dir_reg, _countof(plugin_dir_reg))
+        && !get_openvpn_reg_value(NULL, plugin_dir_reg, _countof(plugin_dir_reg)))
     {
         msg(M_WARN, "Installation path could not be determined.");
     }
@@ -1570,26 +1576,35 @@
         msg(M_NONFATAL | M_ERRNO, "Failed to get system directory.");
     }
 
-    if ((wcslen(plugin_dir) == 0) && (wcslen(system_dir) == 0))
+    if ((wcslen(plugin_dir_reg) == 0) && (wcslen(system_dir) == 0))
     {
         return false;
     }
 
-    WCHAR normalized_plugin_dir[MAX_PATH] = { 0 };
+    WCHAR plugin_dir[MAX_PATH] = { 0 };
 
-    /* Normalize the plugin dir */
-    if (wcslen(plugin_dir) > 0)
+    /* normalize and canonicalize the plugin dir and add trailing slash */
+    if (wcslen(plugin_dir_reg) > 0)
     {
-        if (!GetFullPathNameW(plugin_dir, MAX_PATH, normalized_plugin_dir, NULL))
+        WCHAR normalized_plugin_dir[MAX_PATH] = { 0 };
+        if (!GetFullPathNameW(plugin_dir_reg, MAX_PATH, normalized_plugin_dir, NULL))
         {
             msg(M_NONFATAL | M_ERRNO, "Failed to normalize plugin dir.");
+        }
+
+        HRESULT res = PathCchCanonicalizeEx(plugin_dir, _countof(plugin_dir), normalized_plugin_dir,
+                                            PATHCCH_ENSURE_TRAILING_SLASH);
+        if (res != S_OK)
+        {
+            /* doc says we cannot rely on GetLastError() */
+            msg(M_NONFATAL, "Failed to canonicalize plugin dir.");
             return false;
         }
     }
 
     /* Check if the plugin path resides within the plugin/install directory */
-    if ((wcslen(normalized_plugin_dir) > 0)
-        && (wcsnicmp(normalized_plugin_dir, plugin_path, wcslen(normalized_plugin_dir)) == 0))
+    if ((wcslen(plugin_dir) > 0)
+        && (wcsnicmp(plugin_dir, plugin_path, wcslen(plugin_dir)) == 0))
     {
         return true;
     }

[Openvpn-devel] [S] Change in openvpn[master]: win32: ensure plugin dir has the trailing slash

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos, stipa.

cron2 has posted comments on this change by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1332?usp=email )

Change subject: win32: ensure plugin dir has the trailing slash
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1332?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I5ea90594493ab5cb858f7495f773e080b379e8e8
Gerrit-Change-Number: 1332
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Fri, 14 Nov 2025 21:29:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [M] Change in openvpn[master]: tapctl: make output of 'list' and 'create' commands more verbose

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos, stipa.

cron2 has posted comments on this change by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email )

Change subject: tapctl: make output of 'list' and 'create' commands more verbose
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

File src/tapctl/main.c:

http://gerrit.openvpn.net/c/openvpn/+/1372/comment/e8c04fb5_f9261de4?usp=email :
PS1, Line 292:         const WCHAR *name_to_print = final_name ? final_name : L"";
this looks a bit superfluous?  This branch is only entered `if (final_name)`



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1372?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iac6bcf8b5434aa414e86cb4b9742e7dd591dc796
Gerrit-Change-Number: 1372
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Fri, 14 Nov 2025 21:25:38 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

[Openvpn-devel] [PATCH v1] tapctl: factor out command handlers

[Gert D. <ge...@gr...>]

From: Lev Stipakov <le...@op...>

Change-Id: I432e07216f9adb8f767af172fa37b626b350f994
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1370
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1370
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>

        
diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 031e262..3caffd8 100644
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -201,6 +201,198 @@
     return NULL;
 }
 
+static int command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
+static int command_list(int argc, LPCWSTR argv[]);
+static int command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired);
+
+static int
+command_create(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
+{
+    LPCWSTR szName = NULL;
+    LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
+    struct tap_adapter_node *adapter_list = NULL;
+    LPWSTR adapter_name = NULL;
+    LPOLESTR adapter_id = NULL;
+    GUID guidAdapter;
+    int result = 1;
+
+    for (int i = 2; i < argc; i++)
+    {
+        if (wcsicmp(argv[i], L"--name") == 0)
+        {
+            szName = argv[++i];
+        }
+        else if (wcsicmp(argv[i], L"--hwid") == 0)
+        {
+            szHwId = argv[++i];
+        }
+        else
+        {
+            fwprintf(stderr,
+                     L"Unknown option \"%ls"
+                     L"\". Please, use \"tapctl help create\" to list supported options. Ignored.\n",
+                     argv[i]);
+        }
+    }
+
+    DWORD dwResult =
+        tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, bRebootRequired, &guidAdapter);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
+        return result;
+    }
+
+    dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
+        tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+        return result;
+    }
+
+    adapter_name = szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, adapter_list);
+    if (adapter_name)
+    {
+        if (szName && !is_adapter_name_available(adapter_name, adapter_list, TRUE))
+        {
+            tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+            goto cleanup;
+        }
+
+        dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            StringFromIID((REFIID)&guidAdapter, &adapter_id);
+            fwprintf(stderr,
+                     L"Renaming TUN/TAP adapter %ls"
+                     L" to \"%ls\" failed (error 0x%x).\n",
+                     adapter_id, adapter_name, dwResult);
+            CoTaskMemFree(adapter_id);
+            goto cleanup;
+        }
+    }
+
+    result = 0;
+
+cleanup:
+    free(adapter_name);
+    tap_free_adapter_list(adapter_list);
+
+    if (result == 0)
+    {
+        StringFromIID((REFIID)&guidAdapter, &adapter_id);
+        fwprintf(stdout, L"%ls\n", adapter_id);
+        CoTaskMemFree(adapter_id);
+    }
+
+    return result;
+}
+
+static int
+command_list(int argc, LPCWSTR argv[])
+{
+    WCHAR szzHwId[0x100] =
+        L"root\\" _L(TAP_WIN_COMPONENT_ID) L"\0" _L(TAP_WIN_COMPONENT_ID) L"\0"
+                                                                          L"ovpn-dco\0";
+
+    for (int i = 2; i < argc; i++)
+    {
+        if (wcsicmp(argv[i], L"--hwid") == 0)
+        {
+            memset(szzHwId, 0, sizeof(szzHwId));
+            ++i;
+            memcpy_s(szzHwId,
+                     sizeof(szzHwId) - 2 * sizeof(WCHAR),
+                     argv[i], wcslen(argv[i]) * sizeof(WCHAR));
+        }
+        else
+        {
+            fwprintf(stderr,
+                     L"Unknown option \"%ls"
+                     L"\". Please, use \"tapctl help list\" to list supported options. Ignored.\n",
+                     argv[i]);
+        }
+    }
+
+    struct tap_adapter_node *adapter_list = NULL;
+    DWORD dwResult = tap_list_adapters(NULL, szzHwId, &adapter_list);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
+        return 1;
+    }
+
+    for (struct tap_adapter_node *adapter = adapter_list; adapter; adapter = adapter->pNext)
+    {
+        LPOLESTR adapter_id = NULL;
+        StringFromIID((REFIID)&adapter->guid, &adapter_id);
+        fwprintf(stdout,
+                 L"%ls\t%"
+                 L"ls\n",
+                 adapter_id, adapter->szName);
+        CoTaskMemFree(adapter_id);
+    }
+
+    tap_free_adapter_list(adapter_list);
+
+    return 0;
+}
+
+static int
+command_delete(int argc, LPCWSTR argv[], BOOL *bRebootRequired)
+{
+    if (argc < 3)
+    {
+        fwprintf(stderr,
+                 L"Missing adapter GUID or name. Please, use \"tapctl help delete\" for usage info.\n");
+        return 1;
+    }
+
+    GUID guidAdapter;
+    if (FAILED(IIDFromString(argv[2], (LPIID)&guidAdapter)))
+    {
+        struct tap_adapter_node *adapter_list = NULL;
+        DWORD dwResult = tap_list_adapters(NULL, NULL, &adapter_list);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
+            return 1;
+        }
+
+        BOOL found = FALSE;
+        for (struct tap_adapter_node *adapter = adapter_list; adapter; adapter = adapter->pNext)
+        {
+            if (wcsicmp(argv[2], adapter->szName) == 0)
+            {
+                memcpy(&guidAdapter, &adapter->guid, sizeof(GUID));
+                found = TRUE;
+                break;
+            }
+        }
+
+        tap_free_adapter_list(adapter_list);
+
+        if (!found)
+        {
+            fwprintf(stderr, L"\"%ls\" adapter not found.\n", argv[2]);
+            return 1;
+        }
+    }
+
+    DWORD dwResult = tap_delete_adapter(NULL, &guidAdapter, bRebootRequired);
+    if (dwResult != ERROR_SUCCESS)
+    {
+        fwprintf(stderr,
+                 L"Deleting adapter \"%ls"
+                 L"\" failed (error 0x%x).\n",
+                 argv[2], dwResult);
+        return 1;
+    }
+
+    return 0;
+}
+
 /**
  * Program entry point
  */
@@ -248,215 +440,17 @@
     }
     else if (wcsicmp(argv[1], L"create") == 0)
     {
-        LPCWSTR szName = NULL;
-        LPCWSTR szHwId = L"root\\" _L(TAP_WIN_COMPONENT_ID);
-
-        /* Parse options. */
-        for (int i = 2; i < argc; i++)
-        {
-            if (wcsicmp(argv[i], L"--name") == 0)
-            {
-                szName = argv[++i];
-            }
-            else if (wcsicmp(argv[i], L"--hwid") == 0)
-            {
-                szHwId = argv[++i];
-            }
-            else
-            {
-                fwprintf(
-                    stderr,
-                    L"Unknown option \"%ls"
-                    L"\". Please, use \"tapctl help create\" to list supported options. Ignored.\n",
-                    argv[i]);
-            }
-        }
-
-        /* Create TUN/TAP adapter. */
-        GUID guidAdapter;
-        LPOLESTR szAdapterId = NULL;
-        DWORD dwResult =
-            tap_create_adapter(NULL, L"Virtual Ethernet", szHwId, &bRebootRequired, &guidAdapter);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Creating TUN/TAP adapter failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        /* Get existing network adapters. */
-        struct tap_adapter_node *pAdapterList = NULL;
-        dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Enumerating adapters failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto create_delete_adapter;
-        }
-
-        LPWSTR adapter_name =
-            szName ? wcsdup(szName) : get_unique_adapter_name(szHwId, pAdapterList);
-        if (adapter_name)
-        {
-            /* Check for duplicates when name was specified,
-             * otherwise get_adapter_default_name() takes care of it */
-            if (szName && !is_adapter_name_available(adapter_name, pAdapterList, TRUE))
-            {
-                iResult = 1;
-                goto create_cleanup_pAdapterList;
-            }
-
-            /* Rename the adapter. */
-            dwResult = tap_set_adapter_name(&guidAdapter, adapter_name, FALSE);
-            if (dwResult != ERROR_SUCCESS)
-            {
-                StringFromIID((REFIID)&guidAdapter, &szAdapterId);
-                fwprintf(stderr,
-                         L"Renaming TUN/TAP adapter %ls"
-                         L" to \"%ls\" failed (error 0x%x).\n",
-                         szAdapterId, adapter_name, dwResult);
-                CoTaskMemFree(szAdapterId);
-                iResult = 1;
-                goto quit;
-            }
-        }
-
-        iResult = 0;
-
-create_cleanup_pAdapterList:
-        free(adapter_name);
-
-        tap_free_adapter_list(pAdapterList);
-        if (iResult)
-        {
-            goto create_delete_adapter;
-        }
-
-        /* Output adapter GUID. */
-        StringFromIID((REFIID)&guidAdapter, &szAdapterId);
-        fwprintf(stdout, L"%ls\n", szAdapterId);
-        CoTaskMemFree(szAdapterId);
-
-        iResult = 0;
-        goto quit;
-
-create_delete_adapter:
-        tap_delete_adapter(NULL, &guidAdapter, &bRebootRequired);
-        iResult = 1;
+        iResult = command_create(argc, argv, &bRebootRequired);
         goto quit;
     }
     else if (wcsicmp(argv[1], L"list") == 0)
     {
-        WCHAR szzHwId[0x100] =
-            L"root\\" _L(TAP_WIN_COMPONENT_ID) L"\0" _L(TAP_WIN_COMPONENT_ID) L"\0"
-                                                                              L"ovpn-dco\0";
-
-        /* Parse options. */
-        for (int i = 2; i < argc; i++)
-        {
-            if (wcsicmp(argv[i], L"--hwid") == 0)
-            {
-                memset(szzHwId, 0, sizeof(szzHwId));
-                ++i;
-                memcpy_s(szzHwId,
-                         sizeof(szzHwId) - 2 * sizeof(WCHAR) /*requires double zero termination*/,
-                         argv[i], wcslen(argv[i]) * sizeof(WCHAR));
-            }
-            else
-            {
-                fwprintf(
-                    stderr,
-                    L"Unknown option \"%ls"
-                    L"\". Please, use \"tapctl help list\" to list supported options. Ignored.\n",
-                    argv[i]);
-            }
-        }
-
-        /* Output list of adapters with given hardware ID. */
-        struct tap_adapter_node *pAdapterList = NULL;
-        DWORD dwResult = tap_list_adapters(NULL, szzHwId, &pAdapterList);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        for (struct tap_adapter_node *pAdapter = pAdapterList; pAdapter; pAdapter = pAdapter->pNext)
-        {
-            LPOLESTR szAdapterId = NULL;
-            StringFromIID((REFIID)&pAdapter->guid, &szAdapterId);
-            fwprintf(stdout,
-                     L"%ls\t%"
-                     L"ls\n",
-                     szAdapterId, pAdapter->szName);
-            CoTaskMemFree(szAdapterId);
-        }
-
-        iResult = 0;
-        tap_free_adapter_list(pAdapterList);
+        iResult = command_list(argc, argv);
+        goto quit;
     }
     else if (wcsicmp(argv[1], L"delete") == 0)
     {
-        if (argc < 3)
-        {
-            fwprintf(
-                stderr,
-                L"Missing adapter GUID or name. Please, use \"tapctl help delete\" for usage info.\n");
-            return 1;
-        }
-
-        GUID guidAdapter;
-        if (FAILED(IIDFromString(argv[2], (LPIID)&guidAdapter)))
-        {
-            /* The argument failed to covert to GUID. Treat it as the adapter name. */
-            struct tap_adapter_node *pAdapterList = NULL;
-            DWORD dwResult = tap_list_adapters(NULL, NULL, &pAdapterList);
-            if (dwResult != ERROR_SUCCESS)
-            {
-                fwprintf(stderr, L"Enumerating TUN/TAP adapters failed (error 0x%x).\n", dwResult);
-                iResult = 1;
-                goto quit;
-            }
-
-            for (struct tap_adapter_node *pAdapter = pAdapterList;; pAdapter = pAdapter->pNext)
-            {
-                if (pAdapter == NULL)
-                {
-                    fwprintf(stderr, L"\"%ls\" adapter not found.\n", argv[2]);
-                    iResult = 1;
-                    goto delete_cleanup_pAdapterList;
-                }
-                else if (wcsicmp(argv[2], pAdapter->szName) == 0)
-                {
-                    memcpy(&guidAdapter, &pAdapter->guid, sizeof(GUID));
-                    break;
-                }
-            }
-
-            iResult = 0;
-
-delete_cleanup_pAdapterList:
-            tap_free_adapter_list(pAdapterList);
-            if (iResult)
-            {
-                goto quit;
-            }
-        }
-
-        /* Delete the network adapter. */
-        DWORD dwResult = tap_delete_adapter(NULL, &guidAdapter, &bRebootRequired);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            fwprintf(stderr,
-                     L"Deleting adapter \"%ls"
-                     L"\" failed (error 0x%x).\n",
-                     argv[2], dwResult);
-            iResult = 1;
-            goto quit;
-        }
-
-        iResult = 0;
+        iResult = command_delete(argc, argv, &bRebootRequired);
         goto quit;
     }
     else

[Openvpn-devel] [L] Change in openvpn[master]: tapctl: factor out command handlers

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos, stipa.

cron2 has posted comments on this change by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1370?usp=email )

Change subject: tapctl: factor out command handlers
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1370?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I432e07216f9adb8f767af172fa37b626b350f994
Gerrit-Change-Number: 1370
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-Comment-Date: Fri, 14 Nov 2025 21:20:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [S] Change in openvpn[master]: iservice: use saved iface index to restore metric

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#3) to the change originally created by d12fk. ( http://gerrit.openvpn.net/c/openvpn/+/1363?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by stipa


Change subject: iservice: use saved iface index to restore metric
......................................................................

iservice: use saved iface index to restore metric

When adding block rules, the interface metric of the VPN adapter is
temporarily modified so that an old version of Windows 10 would pick
it up first when looking up stuff via DNS. These metrics are reverted to
the old value when the block is removed.

When reverting them, instead of using the stored interface index where
the original values were read from, we were using the interface index
passed to the service with the wfp block message. That index could
theoretically be different from the one stored, which would result in
the metric being set to the wrong interface.

Reported-by: st...@sr...
Change-Id: Ia74a931c703d594bdf8ccada9b783b94608de278
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Lev Stipakov <lst...@gm...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1363
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34400.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpnserv/interactive.c
1 file changed, 5 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/63/1363/3

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 0712986..33282c63 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -752,7 +752,7 @@
 }
 
 static DWORD
-DeleteWfpBlock(const wfp_block_message_t *msg, undo_lists_t *lists)
+DeleteWfpBlock(undo_lists_t *lists)
 {
     DWORD err = 0;
     wfp_block_data_t *block_data = RemoveListItem(&(*lists)[wfp_block], CmpAny, NULL);
@@ -762,11 +762,11 @@
         err = delete_wfp_block_filters(block_data->engine);
         if (block_data->metric_v4 >= 0)
         {
-            set_interface_metric(msg->iface.index, AF_INET, block_data->metric_v4);
+            set_interface_metric(block_data->index, AF_INET, block_data->metric_v4);
         }
         if (block_data->metric_v6 >= 0)
         {
-            set_interface_metric(msg->iface.index, AF_INET6, block_data->metric_v6);
+            set_interface_metric(block_data->index, AF_INET6, block_data->metric_v6);
         }
         free(block_data);
     }
@@ -829,7 +829,7 @@
             if (err)
             {
                 /* delete the filters, remove undo item and free interface data */
-                DeleteWfpBlock(msg, lists);
+                DeleteWfpBlock(lists);
                 engine = NULL;
             }
         }
@@ -854,7 +854,7 @@
     }
     else
     {
-        return DeleteWfpBlock(msg, lists);
+        return DeleteWfpBlock(lists);
     }
 }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1363?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia74a931c703d594bdf8ccada9b783b94608de278
Gerrit-Change-Number: 1363
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: iservice: use saved iface index to restore metric

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1363?usp=email )

Change subject: iservice: use saved iface index to restore metric
......................................................................

iservice: use saved iface index to restore metric

When adding block rules, the interface metric of the VPN adapter is
temporarily modified so that an old version of Windows 10 would pick
it up first when looking up stuff via DNS. These metrics are reverted to
the old value when the block is removed.

When reverting them, instead of using the stored interface index where
the original values were read from, we were using the interface index
passed to the service with the wfp block message. That index could
theoretically be different from the one stored, which would result in
the metric being set to the wrong interface.

Reported-by: st...@sr...
Change-Id: Ia74a931c703d594bdf8ccada9b783b94608de278
Signed-off-by: Heiko Hund <he...@is...>
Acked-by: Lev Stipakov <lst...@gm...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1363
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg34400.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpnserv/interactive.c
1 file changed, 5 insertions(+), 5 deletions(-)




diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 0712986..33282c63 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -752,7 +752,7 @@
 }
 
 static DWORD
-DeleteWfpBlock(const wfp_block_message_t *msg, undo_lists_t *lists)
+DeleteWfpBlock(undo_lists_t *lists)
 {
     DWORD err = 0;
     wfp_block_data_t *block_data = RemoveListItem(&(*lists)[wfp_block], CmpAny, NULL);
@@ -762,11 +762,11 @@
         err = delete_wfp_block_filters(block_data->engine);
         if (block_data->metric_v4 >= 0)
         {
-            set_interface_metric(msg->iface.index, AF_INET, block_data->metric_v4);
+            set_interface_metric(block_data->index, AF_INET, block_data->metric_v4);
         }
         if (block_data->metric_v6 >= 0)
         {
-            set_interface_metric(msg->iface.index, AF_INET6, block_data->metric_v6);
+            set_interface_metric(block_data->index, AF_INET6, block_data->metric_v6);
         }
         free(block_data);
     }
@@ -829,7 +829,7 @@
             if (err)
             {
                 /* delete the filters, remove undo item and free interface data */
-                DeleteWfpBlock(msg, lists);
+                DeleteWfpBlock(lists);
                 engine = NULL;
             }
         }
@@ -854,7 +854,7 @@
     }
     else
     {
-        return DeleteWfpBlock(msg, lists);
+        return DeleteWfpBlock(lists);
     }
 }
 

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1363?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia74a931c703d594bdf8ccada9b783b94608de278
Gerrit-Change-Number: 1363
Gerrit-PatchSet: 3
Gerrit-Owner: d12fk <he...@op...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [PATCH applied] Re: iservice: use saved iface index to restore metric

[Gert D. <ge...@gr...>]

Makes sense.  Lev happy, BB happy, test compile happy.

release/2.6 might benefit from this as well, but the code is different
enough that this is not an "automatic" backport

  DeleteBlockDNS(const block_dns_message_t *msg, undo_lists_t *lists)
  =======
  DeleteWfpBlock(undo_lists_t *lists)

(it's fairly straightforward, but still "more code change than I should do",
and it needs explicit testing on the 2.6 branch)

Your patch has been applied to the master branch.

commit a1d4a79487ec0089ee01ab635921761933e4e1d6
Author: Heiko Hund
Date:   Wed Nov 12 22:51:00 2025 +0100

     iservice: use saved iface index to restore metric

     Signed-off-by: Heiko Hund <he...@is...>
     Acked-by: Lev Stipakov <lst...@gm...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1363
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg34400.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: mudp: fix unaligned 32-bit read when parsing peer ID

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: its_Giaan, plaisthos.

cron2 has posted comments on this change by its_Giaan. ( http://gerrit.openvpn.net/c/openvpn/+/1348?usp=email )

Change subject: mudp: fix unaligned 32-bit read when parsing peer ID
......................................................................


Patch Set 1:

(1 comment)

File src/openvpn/mudp.c:

http://gerrit.openvpn.net/c/openvpn/+/1348/comment/3f419bdf_37dbf5a2?usp=email :
PS1, Line 213:             memcpy(&tmp, ptr, sizeof(tmp));
> This reimplements the same logic as buf_read_u32() except that it doesn't advance the buffer. […]
I am thinking of something more straightforward...

```
/* peer_id is a 24 byte integer in network byte order */
peer_id = (ptr[1]<<16) + (ptr[2]<<8) + ptr[3]
```

(or the other way round, didn't do the byte order calculations yet)...



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1348?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id0bb4c45d373437ab8dbaff7a311745f9b538cbf
Gerrit-Change-Number: 1348
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: cron2 <ge...@gr...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: its_Giaan <gia...@ma...>
Gerrit-Comment-Date: Fri, 14 Nov 2025 18:39:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld <fr...@li...>

[Openvpn-devel] [S] Change in openvpn[master]: tls_crypt: Avoid some conversion warnings

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: plaisthos.

Hello plaisthos,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1379?usp=email

to review the following change.


Change subject: tls_crypt: Avoid some conversion warnings
......................................................................

tls_crypt: Avoid some conversion warnings

The casts should be safe, since one is a constant
(but got type from sizeof()) and the other is
limited by the buffer length.

While here make the code in tls_crypt_v2_wrap_client_key
as little easier to follow.

Change-Id: I3f11423834814bab5d653f160fc2326dae4c0e8e
Signed-off-by: Frank Lichtenheld <fr...@li...>
---
M src/openvpn/tls_crypt.c
1 file changed, 6 insertions(+), 14 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/79/1379/1

diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index a808de3..ab719b3 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -205,11 +205,6 @@
     return false;
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
 bool
 tls_crypt_unwrap(const struct buffer *src, struct buffer *dst, struct crypto_options *opt)
 {
@@ -246,7 +241,7 @@
             CRYPT_ERROR("cipher reset failed");
         }
         if (!cipher_ctx_update(ctx->cipher, BPTR(dst), &outlen, BPTR(src) + TLS_CRYPT_OFF_CT,
-                               BLEN(src) - TLS_CRYPT_OFF_CT))
+                               BLEN(src) - (int)TLS_CRYPT_OFF_CT))
         {
             CRYPT_ERROR("cipher update failed");
         }
@@ -381,8 +376,9 @@
         msg(M_WARN, "ERROR: could not write tag");
         return false;
     }
-    uint16_t net_len = htons(sizeof(src_key->keys) + BLEN(src_metadata) + TLS_CRYPT_V2_TAG_SIZE
-                             + sizeof(uint16_t));
+    const int data_len = BLEN(src_metadata) + sizeof(src_key->keys) + sizeof(uint16_t);
+    const int tagged_len = data_len + TLS_CRYPT_TAG_SIZE;
+    const uint16_t net_len = htons((uint16_t)tagged_len);
     hmac_ctx_t *hmac_ctx = server_key->hmac;
     hmac_ctx_reset(hmac_ctx);
     hmac_ctx_update(hmac_ctx, (void *)&net_len, sizeof(net_len));
@@ -396,8 +392,8 @@
     ASSERT(cipher_ctx_reset(cipher_ctx, tag));
 
     /* Overflow check (OpenSSL requires an extra block in the dst buffer) */
-    if (buf_forward_capacity(&work) < (sizeof(src_key->keys) + BLEN(src_metadata) + sizeof(net_len)
-                                       + cipher_ctx_block_size(cipher_ctx)))
+    const int padded_len = data_len + cipher_ctx_block_size(cipher_ctx);
+    if (buf_forward_capacity(&work) < padded_len)
     {
         msg(M_WARN, "ERROR: could not crypt: insufficient space in dst");
         return false;
@@ -418,10 +414,6 @@
     return buf_copy(wkc, &work);
 }
 
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
 static bool
 tls_crypt_v2_unwrap_client_key(struct key2 *client_key, struct buffer *metadata,
                                struct buffer wrapped_client_key, struct key_ctx *server_key)

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1379?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I3f11423834814bab5d653f160fc2326dae4c0e8e
Gerrit-Change-Number: 1379
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>

[Openvpn-devel] [XS] Change in openvpn[master]: mudp: fix unaligned 32-bit read when parsing peer ID

[flichtenheld (C. Review) <ge...@op...>]

Attention is currently required from: its_Giaan, plaisthos.

flichtenheld has posted comments on this change by its_Giaan. ( http://gerrit.openvpn.net/c/openvpn/+/1348?usp=email )

Change subject: mudp: fix unaligned 32-bit read when parsing peer ID
......................................................................


Patch Set 1:

(1 comment)

File src/openvpn/mudp.c:

http://gerrit.openvpn.net/c/openvpn/+/1348/comment/b1c152bd_e837d78c?usp=email :
PS1, Line 213:             memcpy(&tmp, ptr, sizeof(tmp));
This reimplements the same logic as buf_read_u32() except that it doesn't advance the buffer. Would it be possible to use that function instead?



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1348?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id0bb4c45d373437ab8dbaff7a311745f9b538cbf
Gerrit-Change-Number: 1348
Gerrit-PatchSet: 1
Gerrit-Owner: its_Giaan <gia...@ma...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: its_Giaan <gia...@ma...>
Gerrit-Comment-Date: Fri, 14 Nov 2025 16:07:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

[Openvpn-devel] [PATCH v1] mudp: fix unaligned 32-bit read when parsing peer ID

[Frank L. <fr...@li...>]

From: Gianmarco De Gregori <gia...@ma...>

The code previously read a 32-bit value from a uint8_t
buffer using a direct cast and dereference.
This can cause unaligned memory access and undefined
behavior on architectures that do not support unaligned
reads, potentially leading to a one-packet crash.

This patch replaces the unsafe cast with a safe
memcpy-based read.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: ZeroPath (https://zeropath.com)

Change-Id: Id0bb4c45d373437ab8dbaff7a311745f9b538cbf
Signed-off-by: Gianmarco De Gregori <gia...@ma...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1348
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1348
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <fr...@li...>

        
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 31134be..0653b219 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -209,7 +209,9 @@
         /* make sure buffer has enough length to read opcode (1 byte) and peer-id (3 bytes) */
         if (v2)
         {
-            uint32_t peer_id = ntohl(*(uint32_t *)ptr) & 0xFFFFFF;
+            uint32_t tmp;
+            memcpy(&tmp, ptr, sizeof(tmp));
+            uint32_t peer_id = ntohl(tmp) & 0xFFFFFF;
             peer_id_disabled = (peer_id == MAX_PEER_ID);
 
             if (!peer_id_disabled && (peer_id < m->max_clients) && (m->instances[peer_id]))

Re: [Openvpn-devel] [PATCH v2] recursive routing: fixes and clean-ups

[Frank L. <fr...@li...>]

On Fri, Nov 14, 2025 at 04:35:02PM +0100, Frank Lichtenheld wrote:
> From: Lev Stipakov <le...@op...>

Sorry for the redundant mail.

Regards,
-- 
  Frank Lichtenheld

[Openvpn-devel] [PATCH v2] recursive routing: fixes and clean-ups

[Frank L. <fr...@li...>]

From: Lev Stipakov <le...@op...>

 - get rid of atoi() for getting the remote transport port.
 It doesn't change, so no point to do in on every packet.
 In addition, atoi() breaks when we use service names as ports.

 - ensure we correctly handle IPv4 headers with options

 - directly use buf parameter in place of c->c2.buf

GitHub: #902

Change-Id: I8a0a8029da02fc63adc918e8d98e9f676ff4ea0d
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Frank Lichtenheld <fr...@li...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1377
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1377
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <fr...@li...>

        
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index aa1f858..90e52d2 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1382,15 +1382,24 @@
 
     struct openvpn_sockaddr *link_addr = &c->c2.to_link_addr->dest;
     struct link_socket_info *lsi = get_link_socket_info(c);
-    uint16_t link_port = atoi(c->c2.link_sockets[0]->remote_port);
 
     int ip_hdr_offset = 0;
-    int tun_ip_ver = get_tun_ip_ver(TUNNEL_TYPE(c->c1.tuntap), &c->c2.buf, &ip_hdr_offset);
+    int tun_ip_ver = get_tun_ip_ver(TUNNEL_TYPE(c->c1.tuntap), buf, &ip_hdr_offset);
 
     if (tun_ip_ver == 4)
     {
-        /* make sure we got whole IP header and TCP/UDP src/dst ports */
-        if (BLEN(buf) < ((int)sizeof(struct openvpn_iphdr) + ip_hdr_offset + sizeof(uint16_t) * 2))
+        /* Ensure we can safely read the IPv4 header */
+        const int min_ip_header = ip_hdr_offset + sizeof(struct openvpn_iphdr);
+        if (BLEN(buf) < min_ip_header)
+        {
+            return;
+        }
+
+        struct openvpn_iphdr *pip = (struct openvpn_iphdr *)(BPTR(buf) + ip_hdr_offset);
+        const int ip_hlen = OPENVPN_IPH_GET_LEN(pip->version_len);
+        /* Reject malformed or truncated headers */
+        if (ip_hlen < sizeof(struct openvpn_iphdr)
+            || BLEN(buf) < (int)(ip_hdr_offset + ip_hlen + sizeof(uint16_t) * 2))
         {
             return;
         }
@@ -1401,8 +1410,6 @@
             return;
         }
 
-        struct openvpn_iphdr *pip = (struct openvpn_iphdr *)(BPTR(buf) + ip_hdr_offset);
-
         /* skip if tun protocol doesn't match link protocol */
         if ((lsi->proto == PROTO_TCP && pip->protocol != OPENVPN_IPPROTO_TCP)
             || (lsi->proto == PROTO_UDP && pip->protocol != OPENVPN_IPPROTO_UDP))
@@ -1410,9 +1417,10 @@
             return;
         }
 
-
         /* drop packets with same dest addr and port as remote */
-        uint8_t *l4_hdr = (uint8_t *)pip + sizeof(struct openvpn_iphdr);
+        uint8_t *l4_hdr = (uint8_t *)pip + ip_hlen;
+
+        uint16_t link_port = ntohs(link_addr->addr.in4.sin_port);
 
         /* TCP and UDP ports are at the same place in the header, and other protocols
          * can not happen here due to the lsi->proto check above */
@@ -1420,7 +1428,7 @@
         uint16_t dst_port = ntohs(*(uint16_t *)(l4_hdr + sizeof(uint16_t)));
         if ((memcmp(&link_addr->addr.in4.sin_addr.s_addr, &pip->daddr, sizeof(pip->daddr)) == 0) && (link_port == dst_port))
         {
-            c->c2.buf.len = 0;
+            buf->len = 0;
 
             struct gc_arena gc = gc_new();
             msg(D_LOW, "Recursive routing detected, packet dropped %s:%" PRIu16 " -> %s",
@@ -1433,7 +1441,8 @@
     else if (tun_ip_ver == 6)
     {
         /* make sure we got whole IPv6 header and TCP/UDP src/dst ports */
-        if (BLEN(buf) < ((int)sizeof(struct openvpn_ipv6hdr) + ip_hdr_offset + sizeof(uint16_t) * 2))
+        const int min_ipv6 = ip_hdr_offset + sizeof(struct openvpn_ipv6hdr) + sizeof(uint16_t) * 2;
+        if (BLEN(buf) < min_ipv6)
         {
             return;
         }
@@ -1453,13 +1462,15 @@
             return;
         }
 
+        uint16_t link_port = ntohs(link_addr->addr.in6.sin6_port);
+
         /* drop packets with same dest addr and port as remote */
         uint8_t *l4_hdr = (uint8_t *)pip6 + sizeof(struct openvpn_ipv6hdr);
         uint16_t src_port = ntohs(*(uint16_t *)l4_hdr);
         uint16_t dst_port = ntohs(*(uint16_t *)(l4_hdr + sizeof(uint16_t)));
         if ((OPENVPN_IN6_ARE_ADDR_EQUAL(&link_addr->addr.in6.sin6_addr, &pip6->daddr)) && (link_port == dst_port))
         {
-            c->c2.buf.len = 0;
+            buf->len = 0;
 
             struct gc_arena gc = gc_new();
             msg(D_LOW, "Recursive routing detected, packet dropped %s:%" PRIu16 " -> %s",