Limit tapctl.exe and openvpnmsica.dll to TAP-Windows6 adapters only

Note: Hardware ID check is used selectively. When naming the adapter, we
still need to check all existing adapters to prevent duplicate names.
When listing or removing adapters by name, the operation is limited to
TUN-Windows6 adapters only.

This patch follows Gert's recommendations from [openvpn-devel].

Signed-off-by: Simon Rozman <simon@rozman.si>
Message-ID: <20190120130813.GY962@greenie.muc.de>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20190224181621.27020-1-simon@rozman.si>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18234.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>

diff --git a/src/openvpnmsica/msica_op.c b/src/openvpnmsica/msica_op.c
index 9704189bb6079d8546964578abf06b7afcde266f..3b9878dc4346b71148859d7d2d193b9309ce531a 100644 (file)
--- a/src/openvpnmsica/msica_op.c
+++ b/src/openvpnmsica/msica_op.c
@@ -444,9 +444,9 @@ msica_op_tap_interface_create_exec(
         }
     }
 
-    /* Get available network interfaces. */
+    /* Get all available network interfaces. */
     struct tap_interface_node *pInterfaceList = NULL;
-    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, TRUE);
     if (dwResult == ERROR_SUCCESS)
     {
         /* Does interface exist? */
@@ -599,9 +599,9 @@ msica_op_tap_interface_delete_by_name_exec(
         }
     }
 
-    /* Get available network interfaces. */
+    /* Get available TUN/TAP interfaces. */
     struct tap_interface_node *pInterfaceList = NULL;
-    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, FALSE);
     if (dwResult == ERROR_SUCCESS)
     {
         /* Does interface exist? */
@@ -657,9 +657,9 @@ msica_op_tap_interface_delete_by_guid_exec(
         }
     }
 
-    /* Get available network interfaces. */
+    /* Get all available interfaces. */
     struct tap_interface_node *pInterfaceList = NULL;
-    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, TRUE);
     if (dwResult == ERROR_SUCCESS)
     {
         /* Does interface exist? */
@@ -716,9 +716,9 @@ msica_op_tap_interface_set_name_exec(
         }
     }
 
-    /* Get available network interfaces. */
+    /* Get all available network interfaces. */
     struct tap_interface_node *pInterfaceList = NULL;
-    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+    DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, TRUE);
     if (dwResult == ERROR_SUCCESS)
     {
         /* Does interface exist? */

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 6e2c806cce35a5bb4fd6af479144ddada225b22c..f5ad2295ac74ebde3787a27a3d5628c7a45e1533 100644 (file)
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -476,9 +476,9 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
 
     OPENVPNMSICA_SAVE_MSI_SESSION(hInstall);
 
-    /* Get available network interfaces. */
+    /* Get all TUN/TAP network interfaces. */
     struct tap_interface_node *pInterfaceList = NULL;
-    uiResult = tap_list_interfaces(NULL, &pInterfaceList);
+    uiResult = tap_list_interfaces(NULL, &pInterfaceList, FALSE);
     if (uiResult != ERROR_SUCCESS)
     {
         goto cleanup_CoInitialize;
@@ -517,58 +517,15 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
         }
     }
 
-    /* Enumerate interfaces. */
-    struct interface_node
+    if (pInterfaceList != NULL)
     {
-        const struct tap_interface_node *iface;
-        struct interface_node *next;
-    } *interfaces_head = NULL, *interfaces_tail = NULL;
-    size_t interface_count = 0;
-    MSIHANDLE hRecord = MsiCreateRecord(1);
-    for (struct tap_interface_node *pInterface = pInterfaceList; pInterface; pInterface = pInterface->pNext)
-    {
-        for (LPCTSTR hwid = pInterface->szzHardwareIDs; hwid[0]; hwid += _tcslen(hwid) + 1)
+        /* Count interfaces. */
+        size_t interface_count = 0;
+        for (struct tap_interface_node *pInterface = pInterfaceList; pInterface; pInterface = pInterface->pNext)
         {
-            if (_tcsicmp(hwid, TEXT(TAP_WIN_COMPONENT_ID)) == 0
-                || _tcsicmp(hwid, TEXT("root\\") TEXT(TAP_WIN_COMPONENT_ID)) == 0)
-            {
-                /* TAP interface found. */
-
-                /* Report the GUID of the interface to installer. */
-                LPOLESTR szInterfaceId = NULL;
-                StringFromIID((REFIID)&pInterface->guid, &szInterfaceId);
-                MsiRecordSetString(hRecord, 1, szInterfaceId);
-                MsiProcessMessage(hInstall, INSTALLMESSAGE_ACTIONDATA, hRecord);
-                CoTaskMemFree(szInterfaceId);
-
-                /* Append interface to the list. */
-                struct interface_node *node = (struct interface_node *)malloc(sizeof(struct interface_node));
-                if (node == NULL)
-                {
-                    MsiCloseHandle(hRecord);
-                    msg(M_FATAL, "%s: malloc(%u) failed", __FUNCTION__, sizeof(struct interface_node));
-                    uiResult = ERROR_OUTOFMEMORY; goto cleanup_pAdapterAdresses;
-                }
-
-                node->iface = pInterface;
-                node->next = NULL;
-                if (interfaces_head)
-                {
-                    interfaces_tail = interfaces_tail->next = node;
-                }
-                else
-                {
-                    interfaces_head = interfaces_tail = node;
-                }
-                interface_count++;
-                break;
-            }
+            interface_count++;
         }
-    }
-    MsiCloseHandle(hRecord);
 
-    if (interface_count)
-    {
         /* Prepare semicolon delimited list of TAP interface ID(s) and active TAP interface ID(s). */
         LPTSTR
             szTAPInterfaces     = (LPTSTR)malloc(interface_count * (38 /*GUID*/ + 1 /*separator/terminator*/) * sizeof(TCHAR)),
@@ -588,11 +545,11 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
             uiResult = ERROR_OUTOFMEMORY; goto cleanup_szTAPInterfaces;
         }
 
-        while (interfaces_head)
+        for (struct tap_interface_node *pInterface = pInterfaceList; pInterface; pInterface = pInterface->pNext)
         {
             /* Convert interface GUID to UTF-16 string. (LPOLESTR defaults to LPWSTR) */
             LPOLESTR szInterfaceId = NULL;
-            StringFromIID((REFIID)&interfaces_head->iface->guid, &szInterfaceId);
+            StringFromIID((REFIID)&pInterface->guid, &szInterfaceId);
 
             /* Append to the list of TAP interface ID(s). */
             if (szTAPInterfaces < szTAPInterfacesTail)
@@ -605,11 +562,11 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
             /* If this interface is active (connected), add it to the list of active TAP interface ID(s). */
             for (PIP_ADAPTER_ADDRESSES p = pAdapterAdresses; p; p = p->Next)
             {
-                OLECHAR szId[38 + 1];
+                OLECHAR szId[38 /*GUID*/ + 1 /*terminator*/];
                 GUID guid;
                 if (MultiByteToWideChar(CP_ACP, MB_PRECOMPOSED, p->AdapterName, -1, szId, _countof(szId)) > 0
                     && SUCCEEDED(IIDFromString(szId, &guid))
-                    && memcmp(&guid, &interfaces_head->iface->guid, sizeof(GUID)) == 0)
+                    && memcmp(&guid, &pInterface->guid, sizeof(GUID)) == 0)
                 {
                     if (p->OperStatus == IfOperStatusUp)
                     {
@@ -625,10 +582,6 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
                 }
             }
             CoTaskMemFree(szInterfaceId);
-
-            struct interface_node *p = interfaces_head;
-            interfaces_head = interfaces_head->next;
-            free(p);
         }
         szTAPInterfacesTail      [0] = 0;
         szTAPInterfacesActiveTail[0] = 0;

diff --git a/src/tapctl/main.c b/src/tapctl/main.c
index 295366b49fa1db55d8c0030d66f3cb7410797fcb..04c03ddc542f8c1e03ad127e05a041cf3ff65189 100644 (file)
--- a/src/tapctl/main.c
+++ b/src/tapctl/main.c
@@ -58,7 +58,7 @@ static const TCHAR usage_message[] =
     TEXT("Commands:\n")
     TEXT("\n")
     TEXT("create     Create a new TUN/TAP interface\n")
-    TEXT("list       List network interfaces\n")
+    TEXT("list       List TUN/TAP interfaces\n")
     TEXT("delete     Delete specified network interface\n")
     TEXT("help       Display this text\n")
     TEXT("\n")
@@ -90,7 +90,7 @@ static const TCHAR usage_message_create[] =
 static const TCHAR usage_message_list[] =
     TEXT("%s\n")
     TEXT("\n")
-    TEXT("Lists network interfaces\n")
+    TEXT("Lists TUN/TAP interfaces\n")
     TEXT("\n")
     TEXT("Usage:\n")
     TEXT("\n")
@@ -98,7 +98,7 @@ static const TCHAR usage_message_list[] =
     TEXT("\n")
     TEXT("Output:\n")
     TEXT("\n")
-    TEXT("This command prints all network interfaces to stdout.                          \n")
+    TEXT("This command prints all TUN/TAP interfaces to stdout.                          \n")
 ;
 
 static const TCHAR usage_message_delete[] =
@@ -200,9 +200,9 @@ _tmain(int argc, LPCTSTR argv[])
 
         if (szName)
         {
-            /* Get the list of available interfaces. */
+            /* Get the list of all available interfaces. */
             struct tap_interface_node *pInterfaceList = NULL;
-            dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+            dwResult = tap_list_interfaces(NULL, &pInterfaceList, TRUE);
             if (dwResult != ERROR_SUCCESS)
             {
                 _ftprintf(stderr, TEXT("Enumerating interfaces failed (error 0x%x).\n"), dwResult);
@@ -257,12 +257,12 @@ create_delete_interface:
     }
     else if (_tcsicmp(argv[1], TEXT("list")) == 0)
     {
-        /* Output list of network interfaces. */
+        /* Output list of TUN/TAP interfaces. */
         struct tap_interface_node *pInterfaceList = NULL;
-        DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+        DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, FALSE);
         if (dwResult != ERROR_SUCCESS)
         {
-            _ftprintf(stderr, TEXT("Enumerating interfaces failed (error 0x%x).\n"), dwResult);
+            _ftprintf(stderr, TEXT("Enumerating TUN/TAP interfaces failed (error 0x%x).\n"), dwResult);
             iResult = 1; goto quit;
         }
 
@@ -290,10 +290,10 @@ create_delete_interface:
         {
             /* The argument failed to covert to GUID. Treat it as the interface name. */
             struct tap_interface_node *pInterfaceList = NULL;
-            DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList);
+            DWORD dwResult = tap_list_interfaces(NULL, &pInterfaceList, FALSE);
             if (dwResult != ERROR_SUCCESS)
             {
-                _ftprintf(stderr, TEXT("Enumerating interfaces failed (error 0x%x).\n"), dwResult);
+                _ftprintf(stderr, TEXT("Enumerating TUN/TAP interfaces failed (error 0x%x).\n"), dwResult);
                 iResult = 1; goto quit;
             }
 

diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index 7f1cde46560b69686fa41817003b9113730d703f..e75db354cbaefb169773bde94875003c066d5fa5 100644 (file)
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -953,7 +953,8 @@ cleanup_szInterfaceId:
 DWORD
 tap_list_interfaces(
     _In_opt_ HWND hwndParent,
-    _Out_ struct tap_interface_node **ppInterface)
+    _Out_ struct tap_interface_node **ppInterface,
+    _In_ BOOL bAll)
 {
     DWORD dwResult;
 
@@ -1015,19 +1016,6 @@ tap_list_interfaces(
             }
         }
 
-        /* Get interface GUID. */
-        GUID guidInterface;
-        dwResult = get_net_interface_guid(hDevInfoList, &devinfo_data, 1, &guidInterface);
-        if (dwResult != ERROR_SUCCESS)
-        {
-            /* Something is wrong with this device. Skip it. */
-            continue;
-        }
-
-        /* Get the interface GUID as string. */
-        LPOLESTR szInterfaceId = NULL;
-        StringFromIID((REFIID)&guidInterface, &szInterfaceId);
-
         /* Get device hardware ID(s). */
         DWORD dwDataType = REG_NONE;
         LPTSTR szzDeviceHardwareIDs = NULL;
@@ -1039,9 +1027,57 @@ tap_list_interfaces(
             (LPVOID)&szzDeviceHardwareIDs);
         if (dwResult != ERROR_SUCCESS)
         {
-            goto cleanup_szInterfaceId;
+            /* Something is wrong with this device. Skip it. */
+            continue;
         }
 
+        /* Check that hardware ID is REG_SZ/REG_MULTI_SZ, and optionally if it matches ours. */
+        if (dwDataType == REG_SZ)
+        {
+            if (!bAll && _tcsicmp(szzDeviceHardwareIDs, szzHardwareIDs) != 0)
+            {
+                /* This is not our device. Skip it. */
+                goto cleanup_szzDeviceHardwareIDs;
+            }
+        }
+        else if (dwDataType == REG_MULTI_SZ)
+        {
+            if (!bAll)
+            {
+                for (LPTSTR szHwdID = szzDeviceHardwareIDs;; szHwdID += _tcslen(szHwdID) + 1)
+                {
+                    if (szHwdID[0] == 0)
+                    {
+                        /* This is not our device. Skip it. */
+                        goto cleanup_szzDeviceHardwareIDs;
+                    }
+                    else if (_tcsicmp(szHwdID, szzHardwareIDs) == 0)
+                    {
+                        /* This is our device. */
+                        break;
+                    }
+                }
+            }
+        }
+        else
+        {
+            /* Unexpected hardware ID format. Skip device. */
+            goto cleanup_szzDeviceHardwareIDs;
+        }
+
+        /* Get interface GUID. */
+        GUID guidInterface;
+        dwResult = get_net_interface_guid(hDevInfoList, &devinfo_data, 1, &guidInterface);
+        if (dwResult != ERROR_SUCCESS)
+        {
+            /* Something is wrong with this device. Skip it. */
+            goto cleanup_szzDeviceHardwareIDs;
+        }
+
+        /* Get the interface GUID as string. */
+        LPOLESTR szInterfaceId = NULL;
+        StringFromIID((REFIID)&guidInterface, &szInterfaceId);
+
         /* Render registry key path. */
         TCHAR szRegKey[INTERFACE_REGKEY_PATH_MAX];
         _stprintf_s(
@@ -1062,7 +1098,7 @@ tap_list_interfaces(
         {
             SetLastError(dwResult); /* MSDN does not mention RegOpenKeyEx() to set GetLastError(). But we do have an error code. Set last error manually. */
             msg(M_WARN | M_ERRNO, "%s: RegOpenKeyEx(HKLM, \"%" PRIsLPTSTR "\") failed", __FUNCTION__, szRegKey);
-            goto cleanup_szzDeviceHardwareIDs;
+            goto cleanup_szInterfaceId;
         }
 
         /* Read interface name. */
@@ -1108,10 +1144,10 @@ cleanup_szName:
         free(szName);
 cleanup_hKey:
         RegCloseKey(hKey);
-cleanup_szzDeviceHardwareIDs:
-        free(szzDeviceHardwareIDs);
 cleanup_szInterfaceId:
         CoTaskMemFree(szInterfaceId);
+cleanup_szzDeviceHardwareIDs:
+        free(szzDeviceHardwareIDs);
     }
 
     dwResult = ERROR_SUCCESS;

diff --git a/src/tapctl/tap.h b/src/tapctl/tap.h
index c09c476620f678460f02eaecfea267d29320c68e..2437d05695ba4b6e9cdce6697bb41bc9e9cfe9db 100644 (file)
--- a/src/tapctl/tap.h
+++ b/src/tapctl/tap.h
@@ -55,7 +55,7 @@ tap_create_interface(
 
 
 /**
- * Deletes a TUN/TAP interface.
+ * Deletes an interface.
  *
  * @param hwndParent    A handle to the top-level window to use for any user interface that is
  *                      related to non-device-specific actions (such as a select-device dialog
@@ -120,12 +120,16 @@ struct tap_interface_node
  *                      the list. After the list is no longer required, free it using
  *                      tap_free_interface_list().
  *
+ * @param bAll          When TRUE, all network interfaces found are added to the list. When
+ *                      FALSE, only TUN/TAP interfaces found are added.
+ *
  * @return ERROR_SUCCESS on success; Win32 error code otherwise
  */
 DWORD
 tap_list_interfaces(
     _In_opt_ HWND hwndParent,
-    _Out_ struct tap_interface_node **ppInterfaceList);
+    _Out_ struct tap_interface_node **ppInterfaceList,
+    _In_ BOOL bAll);
 
 
 /**