diff --git a/CMakeLists.txt b/CMakeLists.txt index 92d8942..bcffb5d 100644 --- a/CMakeLists.txt +++ b/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}) diff --git a/Makefile.am b/Makefile.am index 5c3432e..f5c5e1b 100644 --- a/Makefile.am +++ b/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 diff --git a/main.c b/main.c index 28662c2..5660c2a 100644 --- a/main.c +++ b/main.c @@ -184,13 +184,21 @@ _tWinMain(HINSTANCE hThisInstance, } /* Initialize handlers for manangement interface notifications */ - mgmt_rtmsg_handler handler[] = { { ready_, OnReady }, { hold_, OnHold }, - { log_, OnLogLine }, { state_, OnStateChange }, - { password_, OnPassword }, { proxy_, OnProxy }, - { stop_, OnStop }, { needok_, OnNeedOk }, - { needstr_, OnNeedStr }, { echo_, OnEcho }, - { bytecount_, OnByteCount }, { infomsg_, OnInfoMsg }, - { timeout_, OnTimeout }, { 0, NULL } }; + mgmt_rtmsg_handler handler[] = { { ready_, OnReady }, + { hold_, OnHold }, + { log_, OnLogLine }, + { state_, OnStateChange }, + { password_, OnPassword }, + { proxy_, OnProxy }, + { 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 */ diff --git a/manage.c b/manage.c index 1b8f656..ff322ee 100644 --- a/manage.c +++ b/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; diff --git a/manage.h b/manage.h index 9f222c7..4b331b7 100644 --- a/manage.h +++ b/manage.h @@ -40,6 +40,7 @@ typedef enum pkcs11_id_count_, infomsg_, timeout_, + validate_, mgmt_rtmsg_type_max } mgmt_rtmsg_type; diff --git a/misc.c b/misc.c index 4f9d613..2959060 100644 --- a/misc.c +++ b/misc.c @@ -32,6 +32,8 @@ #include #include #include +#include +#include #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; +} diff --git a/misc.h b/misc.h index b475f11..e0df33d 100644 --- a/misc.h +++ b/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 */ diff --git a/openvpn.c b/openvpn.c index 1eebf6a..ca3e8fe 100644 --- a/openvpn.c +++ b/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 */ diff --git a/openvpn.h b/openvpn.h index a1d7df5..ba5b57e 100644 --- a/openvpn.h +++ b/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; diff --git a/plap/Makefile.am b/plap/Makefile.am index 75716d2..af9f6ab 100644 --- a/plap/Makefile.am +++ b/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 diff --git a/plap/ui_glue.c b/plap/ui_glue.c index 3abec3c..1c91656 100644 --- a/plap/ui_glue.c +++ b/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 }, - { log_, OnLogLine }, { state_, OnStateChange_ }, - { password_, OnPassword_ }, { proxy_, OnProxy_ }, - { stop_, OnStop_ }, { needok_, OnNeedOk_ }, - { needstr_, OnNeedStr_ }, { echo_, OnEcho }, - { bytecount_, OnByteCount }, { infomsg_, OnInfoMsg_ }, - { timeout_, OnTimeout }, { 0, NULL } }; + mgmt_rtmsg_handler handler[] = { { ready_, OnReady }, + { hold_, OnHold }, + { log_, OnLogLine }, + { state_, OnStateChange_ }, + { password_, OnPassword_ }, + { proxy_, OnProxy_ }, + { 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");