Check the path of the process listening on management port

Check that the process listening on management port has image path
matching exe_path set in the registry. The check is done soon after
connecting to the port, but before sending any data to it.

Helps with:
  (i) not revealing management password to a malicious process
  (ii) passing user credentials etc. only to a known process
  (iii) ensuring PLAP interface is connecting to a known process

Note: This uses an undocumented API as alternatives like "QueryFullProcessImageNameW"
requires PROCESS_QUERY_INFORMATION rights which we normally do not have.

Motivated by some issues found by ZeroPath

Signed-off-by: Selva Nair <selva.nair@gmail.com>

CMakeLists.txt

@@ -70,7 +70,9 @@ target_link_libraries(${PROJECT_NAME} PRIVATE
     Comdlg32.lib
     Ole32.lib
     Cryptui.lib
-    Wininet.lib)
+    Wininet.lib
+    Iphlpapi.lib
+    Ntdll.lib)
 
 target_include_directories(${PROJECT_NAME} PUBLIC ${CMAKE_CURRENT_BINARY_DIR})
 
@@ -141,7 +143,9 @@ target_link_libraries(${PROJECT_NAME_PLAP} PRIVATE
     Secur32.lib
     Gdi32.lib
     Cryptui.lib
-    Rpcrt4.lib)
+    Rpcrt4.lib
+    Iphlpapi.lib
+    Ntdll.lib)
 
 set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} /MANIFEST:NO")
 target_include_directories(${PROJECT_NAME_PLAP} PRIVATE ${CMAKE_CURRENT_BINARY_DIR} ${CMAKE_SOURCE_DIR})

Makefile.am

@@ -127,6 +127,8 @@ openvpn_gui_LDADD = \
 	-lshlwapi \
 	-lsecur32 \
 	-lwininet \
+	-liphlpapi \
+	-lntdll \
 	$(JSON_LIBS)
 
 openvpn-gui-res.o: $(openvpn_gui_RESOURCES) $(srcdir)/openvpn-gui-res.h

main.c

@@ -184,13 +184,21 @@ _tWinMain(HINSTANCE hThisInstance,
     }
 
     /* Initialize handlers for manangement interface notifications */
-    mgmt_rtmsg_handler handler[] = { { ready_, OnReady },         { hold_, OnHold },
+    mgmt_rtmsg_handler handler[] = { { ready_, OnReady },
-                                     { log_, OnLogLine },         { state_, OnStateChange },
+                                     { hold_, OnHold },
-                                     { password_, OnPassword },   { proxy_, OnProxy },
+                                     { log_, OnLogLine },
-                                     { stop_, OnStop },           { needok_, OnNeedOk },
+                                     { state_, OnStateChange },
-                                     { needstr_, OnNeedStr },     { echo_, OnEcho },
+                                     { password_, OnPassword },
-                                     { bytecount_, OnByteCount }, { infomsg_, OnInfoMsg },
+                                     { proxy_, OnProxy },
-                                     { timeout_, OnTimeout },     { 0, NULL } };
+                                     { stop_, OnStop },
+                                     { needok_, OnNeedOk },
+                                     { needstr_, OnNeedStr },
+                                     { echo_, OnEcho },
+                                     { bytecount_, OnByteCount },
+                                     { infomsg_, OnInfoMsg },
+                                     { timeout_, OnTimeout },
+                                     { validate_, OnMgmtValidate },
+                                     { 0, NULL } };
     InitManagement(handler);
 
     /* initialize options to default state */

manage.c

@@ -250,6 +250,10 @@ OnManagement(SOCKET sk, LPARAM lParam)
             else
             {
                 c->manage.connected = 1;
+                if (rtmsg_handler[validate_])
+                {
+                    rtmsg_handler[validate_](c, "");
+                }
             }
             break;
 

manage.h

@@ -40,6 +40,7 @@ typedef enum
     pkcs11_id_count_,
     infomsg_,
     timeout_,
+    validate_,
     mgmt_rtmsg_type_max
 } mgmt_rtmsg_type;
 

misc.c

@@ -32,6 +32,8 @@
 #include <shellapi.h>
 #include <ws2tcpip.h>
 #include <shlwapi.h>
+#include <iphlpapi.h>
+#include <winternl.h>
 
 #include "localization.h"
 #include "options.h"
@@ -1336,3 +1338,131 @@ IsSamePath(const wchar_t *path1, const wchar_t *path2)
 
     return ret;
 }
+
+/* Find pid of process listening on a TCP port. Returns 0 on error */
+static DWORD
+find_listening_pid(DWORD port)
+{
+    PMIB_TCPTABLE_OWNER_PID pTable = NULL;
+    DWORD size = 0;
+    DWORD result =
+        GetExtendedTcpTable(NULL, &size, FALSE, AF_INET, TCP_TABLE_OWNER_PID_LISTENER, 0);
+
+    if (result != ERROR_INSUFFICIENT_BUFFER)
+    {
+        return 0;
+    }
+
+    pTable = (PMIB_TCPTABLE_OWNER_PID)malloc(size);
+    if (!pTable)
+    {
+        return 0;
+    }
+    if (GetExtendedTcpTable(pTable, &size, FALSE, AF_INET, TCP_TABLE_OWNER_PID_LISTENER, 0)
+        != NO_ERROR)
+    {
+        free(pTable);
+        return 0;
+    }
+
+    DWORD pid = 0;
+    for (DWORD i = 0; i < pTable->dwNumEntries; i++)
+    {
+        DWORD localPort = ntohs((u_short)pTable->table[i].dwLocalPort);
+        if (localPort == port)
+        {
+            pid = pTable->table[i].dwOwningPid;
+            break;
+        }
+    }
+
+    free(pTable);
+    return pid;
+}
+
+/* Find the process image name given pid. name should have space
+ * for max_chars wide characters.
+ */
+static bool
+imagename_from_pid(DWORD pid, wchar_t *name, DWORD max_chars)
+{
+    /* Using undocumented API (vista and later).
+     * See https://www.geoffchappell.com/studies/windows/km/ntoskrnl/api/ex/sysinfo/query.htm
+     */
+    struct SYSTEM_PROCESS_ID_INFORMATION
+    {
+        PVOID ProcessId;
+        UNICODE_STRING ImageName;
+    } pinfo;
+
+    SYSTEM_INFORMATION_CLASS pinfo_class = 0x58; /* for querying SYSTEM_PROCESS_ID_INFORMATION */
+
+    pinfo.ProcessId = (PVOID)(UINT_PTR)pid;
+    pinfo.ImageName.Length = 0;
+    pinfo.ImageName.MaximumLength = (USHORT)max_chars;
+    pinfo.ImageName.Buffer = name;
+
+    NTSTATUS res = NtQuerySystemInformation(pinfo_class, &pinfo, sizeof(pinfo), NULL);
+
+    if (!NT_SUCCESS(res))
+    {
+        MsgToEventLog(EVENTLOG_WARNING_TYPE,
+                      L"Error querying process information for pid = %lu (error = %lu)",
+                      pid,
+                      RtlNtStatusToDosError(res));
+        return false;
+    }
+    /* Null terminate the result */
+    if (pinfo.ImageName.Length < (USHORT)max_chars)
+    {
+        name[pinfo.ImageName.Length] = L'\0';
+    }
+    else if (max_chars > 0)
+    {
+        name[max_chars - 1] = L'\0';
+    }
+    return true;
+}
+
+/* check the process listening on management port is legit openvpn.exe */
+bool
+ValidateManagementDaemon(connection_t *c)
+{
+    wchar_t nt_path[MAX_PATH];
+
+    DWORD port = ntohs(c->manage.skaddr.sin_port);
+    DWORD pid = find_listening_pid(port);
+
+    if (pid == 0)
+    {
+        MsgToEventLog(EVENTLOG_ERROR_TYPE,
+                      L"Failed to get pid of process listening on management port <%lu>",
+                      port);
+        return false;
+    }
+
+    BOOL res = imagename_from_pid(pid, nt_path, _countof(nt_path));
+    if (!res)
+    {
+        MsgToEventLog(EVENTLOG_ERROR_TYPE,
+                      L"Failed to get management daemon image name (error = %lu)",
+                      GetLastError());
+        return res;
+    }
+
+    /* imagename_from_pid() uses NtQuery.. which returns an NT path of the form
+     * \Device\HardDisk\Volume2... instead of a drive letter or UNC path.
+     * Convert this nt_path to a form that could be passed to win32 API
+     */
+    wchar_t win32_path[MAX_PATH];
+    swprintf(win32_path, _countof(win32_path), L"%ls%ls", L"\\\\?\\GLOBALROOT", nt_path);
+
+    res = IsSamePath(win32_path, o.exe_path);
+    if (!res)
+    {
+        MsgToEventLog(EVENTLOG_ERROR_TYPE,
+                      L"Unknown process listening on management port (<%ls>)",
+                      win32_path);
+    }
+    return res;
+}

misc.h

@@ -192,4 +192,10 @@ void ChangePasswordVisibility(HWND edit, HWND btn, WPARAM wParam);
  */
 bool IsSamePath(const wchar_t *path1, const wchar_t *path2);
 
+/**
+ * Check that the process listening at management port is
+ * openvpn.exe as defined in o.exe_path. Returns true on success.
+ */
+bool ValidateManagementDaemon(connection_t *c);
+
 #endif /* ifndef MISC_H */

openvpn.c

@@ -1584,6 +1584,21 @@ OnTimeout(connection_t *c, UNUSED char *msg)
     return;
 }
 
+/* Handle daemon validation request from manage.c -- called after FD_CONNECT */
+void
+OnMgmtValidate(connection_t *c, UNUSED char *msg)
+{
+    /* check the process we have connected to via the management port*/
+    if (!ValidateManagementDaemon(c))
+    {
+        /* clear sensitive data and close the management port right away */
+        SecureZeroMemory(c->manage.password, sizeof(c->manage.password));
+        CloseManagement(c);
+        StopOpenVPN(c);
+    }
+}
+
+
 /*
  * Handle exit of the OpenVPN process
  */

openvpn.h

@@ -75,6 +75,8 @@ void OnInfoMsg(connection_t *, char *);
 
 void OnTimeout(connection_t *, char *);
 
+void OnMgmtValidate(connection_t *, char *);
+
 void ResetSavePasswords(connection_t *);
 
 extern const TCHAR *cfgProp;

plap/Makefile.am

@@ -116,7 +116,9 @@ libopenvpn_plap_la_LIBADD = \
 	-lole32 \
 	-lshlwapi \
 	-lgdi32 \
-	-lrpcrt4
+	-lrpcrt4 \
+	-liphlpapi \
+	-lntdll
 
 libopenvpn_plap_la_LDFLAGS = -no-undefined -avoid-version -static-libgcc
 

plap/ui_glue.c

@@ -194,13 +194,21 @@ InitializeUI(HINSTANCE hinstance)
     /* Initialize handlers for management interface notifications
      * Some handlers are replaced by local functions
      */
-    mgmt_rtmsg_handler handler[] = { { ready_, OnReady },         { hold_, OnHold },
+    mgmt_rtmsg_handler handler[] = { { ready_, OnReady },
-                                     { log_, OnLogLine },         { state_, OnStateChange_ },
+                                     { hold_, OnHold },
-                                     { password_, OnPassword_ },  { proxy_, OnProxy_ },
+                                     { log_, OnLogLine },
-                                     { stop_, OnStop_ },          { needok_, OnNeedOk_ },
+                                     { state_, OnStateChange_ },
-                                     { needstr_, OnNeedStr_ },    { echo_, OnEcho },
+                                     { password_, OnPassword_ },
-                                     { bytecount_, OnByteCount }, { infomsg_, OnInfoMsg_ },
+                                     { proxy_, OnProxy_ },
-                                     { timeout_, OnTimeout },     { 0, NULL } };
+                                     { stop_, OnStop_ },
+                                     { needok_, OnNeedOk_ },
+                                     { needstr_, OnNeedStr_ },
+                                     { echo_, OnEcho },
+                                     { bytecount_, OnByteCount },
+                                     { infomsg_, OnInfoMsg_ },
+                                     { timeout_, OnTimeout },
+                                     { validate_, OnMgmtValidate },
+                                     { 0, NULL } };
 
     InitManagement(handler);
     dmsg(L"Init Management done");