From bb92f28088b2416577575e0b9f9dbfbfdcc729a6 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 25 Nov 2022 12:54:23 +0900 Subject: [PATCH 1/3] Improve vpn connection logic on Windows fix https://github.com/brave/brave-browser/issues/26980 Prevent issuing same commands when it's already in-progress. --- .../brave_vpn_os_connection_api_win.cc | 37 ++++---- .../brave_vpn_os_connection_api_win.h | 6 +- components/brave_vpn/utils_win.cc | 88 ++++++++++++++----- 3 files changed, 84 insertions(+), 47 deletions(-) diff --git a/components/brave_vpn/brave_vpn_os_connection_api_win.cc b/components/brave_vpn/brave_vpn_os_connection_api_win.cc index 6157498b14a5..b5daf7ea250d 100644 --- a/components/brave_vpn/brave_vpn_os_connection_api_win.cc +++ b/components/brave_vpn/brave_vpn_os_connection_api_win.cc @@ -55,8 +55,7 @@ BraveVPNOSConnectionAPIWin::BraveVPNOSConnectionAPIWin() { } BraveVPNOSConnectionAPIWin::~BraveVPNOSConnectionAPIWin() { - CloseHandle(event_handle_for_connected_); - CloseHandle(event_handle_for_disconnected_); + CloseHandle(event_handle_for_connected_disconnected_); CloseEventHandleForConnecting(); CloseEventHandleForDisconnecting(); CloseEventHandleForConnectFailed(); @@ -108,6 +107,14 @@ void BraveVPNOSConnectionAPIWin::CheckConnectionImpl(const std::string& name) { void BraveVPNOSConnectionAPIWin::OnObjectSignaled(HANDLE object) { DCHECK(!target_vpn_entry_name().empty()); + // Check connection state for BraveVPN entry again when connected or + // disconnected events are arrived because we can get both event from any os + // vpn entry. All other events are sent by our code at utils_win.cc. + if (object == event_handle_for_connected_disconnected_) { + CheckConnectionImpl(target_vpn_entry_name()); + return; + } + CheckConnectionResult result = CheckConnectionResult::DISCONNECTING; if (object == GetEventHandleForConnecting()) { result = CheckConnectionResult::CONNECTING; @@ -115,10 +122,6 @@ void BraveVPNOSConnectionAPIWin::OnObjectSignaled(HANDLE object) { result = CheckConnectionResult::CONNECT_FAILED; } else if (object == GetEventHandleForDisconnecting()) { result = CheckConnectionResult::DISCONNECTING; - } else if (object == event_handle_for_connected_) { - result = CheckConnectionResult::CONNECTED; - } else if (object == event_handle_for_disconnected_) { - result = CheckConnectionResult::DISCONNECTED; } else { NOTREACHED(); } @@ -164,23 +167,19 @@ void BraveVPNOSConnectionAPIWin::OnRemoved(const std::string& name, bool success) {} void BraveVPNOSConnectionAPIWin::StartVPNConnectionChangeMonitoring() { - DCHECK(!event_handle_for_connected_ && !event_handle_for_disconnected_); + DCHECK(!event_handle_for_connected_disconnected_); - event_handle_for_connected_ = CreateEvent(NULL, false, false, NULL); - event_handle_for_disconnected_ = CreateEvent(NULL, false, false, NULL); + event_handle_for_connected_disconnected_ = + CreateEvent(NULL, false, false, NULL); - // We don't need to check current connection state again if monitor each event - // separately. - RasConnectionNotificationW(static_cast(INVALID_HANDLE_VALUE), - event_handle_for_connected_, RASCN_Connection); + // Ase we pass INVALID_HANDLE_VALUE, we can get connected or disconnected + // event from any os vpn entry. It's filtered by OnObjectSignaled(). RasConnectionNotificationW(static_cast(INVALID_HANDLE_VALUE), - event_handle_for_disconnected_, - RASCN_Disconnection); + event_handle_for_connected_disconnected_, + RASCN_Connection | RASCN_Disconnection); - connected_event_watcher_.StartWatchingMultipleTimes( - event_handle_for_connected_, this); - disconnected_event_watcher_.StartWatchingMultipleTimes( - event_handle_for_disconnected_, this); + connected_disconnected_event_watcher_.StartWatchingMultipleTimes( + event_handle_for_connected_disconnected_, this); connecting_event_watcher_.StartWatchingMultipleTimes( GetEventHandleForConnecting(), this); disconnecting_event_watcher_.StartWatchingMultipleTimes( diff --git a/components/brave_vpn/brave_vpn_os_connection_api_win.h b/components/brave_vpn/brave_vpn_os_connection_api_win.h index 485e4e9bfad0..72eb243e79d1 100644 --- a/components/brave_vpn/brave_vpn_os_connection_api_win.h +++ b/components/brave_vpn/brave_vpn_os_connection_api_win.h @@ -50,13 +50,11 @@ class BraveVPNOSConnectionAPIWin : public BraveVPNOSConnectionAPI, void StartVPNConnectionChangeMonitoring(); - HANDLE event_handle_for_connected_ = NULL; - HANDLE event_handle_for_disconnected_ = NULL; - base::win::ObjectWatcher connected_event_watcher_; + HANDLE event_handle_for_connected_disconnected_ = NULL; + base::win::ObjectWatcher connected_disconnected_event_watcher_; base::win::ObjectWatcher connect_failed_event_watcher_; base::win::ObjectWatcher connecting_event_watcher_; base::win::ObjectWatcher disconnecting_event_watcher_; - base::win::ObjectWatcher disconnected_event_watcher_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/brave_vpn/utils_win.cc b/components/brave_vpn/utils_win.cc index 257b2cf46fd5..810e2abab4a4 100644 --- a/components/brave_vpn/utils_win.cc +++ b/components/brave_vpn/utils_win.cc @@ -24,6 +24,24 @@ HANDLE g_connecting_event_handle = NULL; HANDLE g_connect_failed_event_handle = NULL; HANDLE g_disconnecting_event_handle = NULL; +class ScopedHeapAlloc { + public: + explicit ScopedHeapAlloc(SIZE_T dw_bytes) { + lp_alloc_mem_ = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dw_bytes); + } + ~ScopedHeapAlloc() { + if (lp_alloc_mem_) + HeapFree(GetProcessHeap(), 0, lp_alloc_mem_); + } + ScopedHeapAlloc(const ScopedHeapAlloc&) = delete; + ScopedHeapAlloc& operator=(const ScopedHeapAlloc&) = delete; + + LPVOID lp_alloc_mem() { return lp_alloc_mem_; } + + private: + LPVOID lp_alloc_mem_ = NULL; +}; + void WINAPI RasDialFunc(UINT, RASCONNSTATE rasconnstate, DWORD error) { if (error) { SetEvent(g_connect_failed_event_handle); @@ -151,8 +169,8 @@ std::wstring GetPhonebookPath() { } // Allocate required buf. - app_data_path = reinterpret_cast(HeapAlloc( - GetProcessHeap(), HEAP_ZERO_MEMORY, dw_buf_size * sizeof(TCHAR))); + ScopedHeapAlloc app_data_path_mem(dw_buf_size * sizeof(TCHAR)); + app_data_path = reinterpret_cast(app_data_path_mem.lp_alloc_mem()); if (app_data_path == NULL) { LOG(ERROR) << "HeapAlloc failed!"; return app_data_path; @@ -164,15 +182,19 @@ std::wstring GetPhonebookPath() { L"%ls\\Microsoft\\Network\\Connections\\Pbk\\rasphone.pbk", app_data_path); - // Deallocate memory for the connection buffer - HeapFree(GetProcessHeap(), 0, app_data_path); - app_data_path = NULL; - return phone_book_path; } // https://docs.microsoft.com/en-us/windows/win32/api/ras/nf-ras-rasenumconnectionsa bool DisconnectEntry(const std::wstring& entry_name) { + auto connection_result = CheckConnection(entry_name); + if (connection_result == CheckConnectionResult::DISCONNECTING) { + VLOG(2) << __func__ + << " Don't try to disconnect while brave vpn entry is already in " + "disconnecting state"; + return true; + } + DWORD dw_cb = 0; DWORD dw_ret = ERROR_SUCCESS; DWORD dw_connections = 0; @@ -184,8 +206,8 @@ bool DisconnectEntry(const std::wstring& entry_name) { if (dw_ret == ERROR_BUFFER_TOO_SMALL) { // Allocate the memory needed for the array of RAS structure(s). - lp_ras_conn = reinterpret_cast( - HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dw_cb)); + ScopedHeapAlloc ras_conn(dw_cb); + lp_ras_conn = reinterpret_cast(ras_conn.lp_alloc_mem()); if (lp_ras_conn == NULL) { LOG(ERROR) << "HeapAlloc failed!"; return false; @@ -214,9 +236,6 @@ bool DisconnectEntry(const std::wstring& entry_name) { } } } - // Deallocate memory for the connection buffer - HeapFree(GetProcessHeap(), 0, lp_ras_conn); - lp_ras_conn = NULL; return dw_ret == ERROR_SUCCESS; } @@ -233,11 +252,21 @@ bool DisconnectEntry(const std::wstring& entry_name) { // https://docs.microsoft.com/en-us/windows/win32/api/ras/nf-ras-rasdiala bool ConnectEntry(const std::wstring& entry_name) { + auto connection_result = CheckConnection(entry_name); + if (connection_result == CheckConnectionResult::CONNECTING || + connection_result == CheckConnectionResult::CONNECTED) { + VLOG(2) + << __func__ + << " Don't try to connect when it's in-progress or already connected."; + return true; + } + LPRASDIALPARAMS lp_ras_dial_params = NULL; DWORD cb = sizeof(RASDIALPARAMS); - lp_ras_dial_params = reinterpret_cast( - HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, cb)); + ScopedHeapAlloc ras_dial_params(cb); + lp_ras_dial_params = + reinterpret_cast(ras_dial_params.lp_alloc_mem()); if (lp_ras_dial_params == NULL) { LOG(ERROR) << "HeapAlloc failed!"; SetEvent(g_connect_failed_event_handle); @@ -256,7 +285,6 @@ bool ConnectEntry(const std::wstring& entry_name) { DWORD dw_ret = RasGetCredentials(DEFAULT_PHONE_BOOK, entry_name.c_str(), &credentials); if (dw_ret != ERROR_SUCCESS) { - HeapFree(GetProcessHeap(), 0, (LPVOID)lp_ras_dial_params); PrintRasError(dw_ret); SetEvent(g_connect_failed_event_handle); return false; @@ -268,15 +296,19 @@ bool ConnectEntry(const std::wstring& entry_name) { HRASCONN h_ras_conn = NULL; dw_ret = RasDial(NULL, DEFAULT_PHONE_BOOK, lp_ras_dial_params, 0, (LPVOID)(&RasDialFunc), &h_ras_conn); + + if (dw_ret == ERROR_DIAL_ALREADY_IN_PROGRESS) { + // We should not treat this as failure state. + // Just return when already dialed. + PrintRasError(dw_ret); + return true; + } + if (dw_ret != ERROR_SUCCESS) { - HeapFree(GetProcessHeap(), 0, (LPVOID)lp_ras_dial_params); PrintRasError(dw_ret); SetEvent(g_connect_failed_event_handle); return false; } - VLOG(2) << "SUCCESS!"; - - HeapFree(GetProcessHeap(), 0, (LPVOID)lp_ras_dial_params); return true; } @@ -295,6 +327,15 @@ bool CreateEntry(const std::wstring& entry_name, const std::wstring& hostname, const std::wstring& username, const std::wstring& password) { + auto connection_result = CheckConnection(entry_name); + if (connection_result == CheckConnectionResult::CONNECTING || + connection_result == CheckConnectionResult::CONNECTED) { + VLOG(2) << __func__ + << " Don't try to create entry when brave vpn entry is in " + "connecting or connected state"; + return true; + } + RASENTRY entry; ZeroMemory(&entry, sizeof(RASENTRY)); // For descriptions of each field (including valid values) see: @@ -412,7 +453,7 @@ CheckConnectionResult GetConnectionState(HRASCONN h_ras_conn) { VLOG(2) << "Connecting device..."; return CheckConnectionResult::CONNECTING; case RASCS_Connected: - VLOG(2) << "Connection completed"; + VLOG(2) << "Connected"; return CheckConnectionResult::CONNECTED; case RASCS_Disconnected: VLOG(2) << "Disconnected"; @@ -425,6 +466,7 @@ CheckConnectionResult GetConnectionState(HRASCONN h_ras_conn) { } CheckConnectionResult CheckConnection(const std::wstring& entry_name) { + VLOG(2) << "Check connection state for " << entry_name; if (entry_name.empty()) return CheckConnectionResult::DISCONNECTED; @@ -439,6 +481,7 @@ CheckConnectionResult CheckConnection(const std::wstring& entry_name) { // If got success here, it means there is no connected vpn entry. if (dw_ret == ERROR_SUCCESS) { + VLOG(2) << " There is no active connections."; return CheckConnectionResult::DISCONNECTED; } @@ -447,7 +490,8 @@ CheckConnectionResult CheckConnection(const std::wstring& entry_name) { return CheckConnectionResult::DISCONNECTED; // Allocate the memory needed for the array of RAS structure(s). - lp_ras_conn = (LPRASCONN)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dw_cb); + ScopedHeapAlloc ras_conn(dw_cb); + lp_ras_conn = reinterpret_cast(ras_conn.lp_alloc_mem()); if (lp_ras_conn == NULL) { LOG(ERROR) << "HeapAlloc failed!"; return CheckConnectionResult::DISCONNECTED; @@ -461,7 +505,6 @@ CheckConnectionResult CheckConnection(const std::wstring& entry_name) { dw_ret = RasEnumConnections(lp_ras_conn, &dw_cb, &dw_connections); if (ERROR_SUCCESS != dw_ret) { - HeapFree(GetProcessHeap(), 0, lp_ras_conn); lp_ras_conn = NULL; return CheckConnectionResult::DISCONNECTED; } @@ -474,9 +517,6 @@ CheckConnectionResult CheckConnection(const std::wstring& entry_name) { break; } } - - HeapFree(GetProcessHeap(), 0, lp_ras_conn); - lp_ras_conn = NULL; return result; } From 184210e460eb2142308e57ab424809a6cb1a1ba9 Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 25 Nov 2022 17:33:19 +0900 Subject: [PATCH 2/3] Use OnConnected() reply on Windows --- .../brave_vpn_os_connection_api_win.cc | 18 +++++++++++++----- .../brave_vpn_os_connection_api_win.h | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/components/brave_vpn/brave_vpn_os_connection_api_win.cc b/components/brave_vpn/brave_vpn_os_connection_api_win.cc index b5daf7ea250d..e585d7bf5e79 100644 --- a/components/brave_vpn/brave_vpn_os_connection_api_win.cc +++ b/components/brave_vpn/brave_vpn_os_connection_api_win.cc @@ -34,8 +34,8 @@ namespace brave_vpn { namespace { -void ConnectEntry(const std::wstring& name) { - brave_vpn::internal::ConnectEntry(name); +bool ConnectEntry(const std::wstring& name) { + return brave_vpn::internal::ConnectEntry(name); } void DisconnectEntry(const std::wstring& name) { @@ -75,9 +75,11 @@ void BraveVPNOSConnectionAPIWin::CreateVPNConnectionImpl( void BraveVPNOSConnectionAPIWin::ConnectImpl(const std::string& name) { // Connection state update from this call will be done by monitoring. - base::ThreadPool::PostTask( + base::ThreadPool::PostTaskAndReplyWithResult( FROM_HERE, {base::MayBlock()}, - base::BindOnce(&ConnectEntry, base::UTF8ToWide(name))); + base::BindOnce(&ConnectEntry, base::UTF8ToWide(name)), + base::BindOnce(&BraveVPNOSConnectionAPIWin::OnConnected, + weak_factory_.GetWeakPtr())); } void BraveVPNOSConnectionAPIWin::DisconnectImpl(const std::string& name) { @@ -134,7 +136,7 @@ void BraveVPNOSConnectionAPIWin::OnCheckConnection( CheckConnectionResult result) { switch (result) { case CheckConnectionResult::CONNECTED: - OnConnected(); + BraveVPNOSConnectionAPI::OnConnected(); break; case CheckConnectionResult::CONNECTING: OnIsConnecting(); @@ -163,6 +165,12 @@ void BraveVPNOSConnectionAPIWin::OnCreated(const std::string& name, BraveVPNOSConnectionAPI::OnCreated(); } +void BraveVPNOSConnectionAPIWin::OnConnected(bool success) { + if (success) { + BraveVPNOSConnectionAPI::OnConnected(); + } +} + void BraveVPNOSConnectionAPIWin::OnRemoved(const std::string& name, bool success) {} diff --git a/components/brave_vpn/brave_vpn_os_connection_api_win.h b/components/brave_vpn/brave_vpn_os_connection_api_win.h index 72eb243e79d1..359c36781e7c 100644 --- a/components/brave_vpn/brave_vpn_os_connection_api_win.h +++ b/components/brave_vpn/brave_vpn_os_connection_api_win.h @@ -44,6 +44,7 @@ class BraveVPNOSConnectionAPIWin : public BraveVPNOSConnectionAPI, void OnObjectSignaled(HANDLE object) override; void OnCreated(const std::string& name, bool success); + void OnConnected(bool success); void OnRemoved(const std::string& name, bool success); void OnCheckConnection(const std::string& name, internal::CheckConnectionResult result); From 9ff1d7eb16f7605060cfdc8a78adaeb64dd4c9fe Mon Sep 17 00:00:00 2001 From: Simon Hong Date: Fri, 25 Nov 2022 18:34:06 +0900 Subject: [PATCH 3/3] Removed all event handle except connected/disconnected --- .../brave_vpn_os_connection_api_win.cc | 34 +-------- .../brave_vpn_os_connection_api_win.h | 3 - components/brave_vpn/utils_win.cc | 69 +------------------ components/brave_vpn/utils_win.h | 8 --- 4 files changed, 4 insertions(+), 110 deletions(-) diff --git a/components/brave_vpn/brave_vpn_os_connection_api_win.cc b/components/brave_vpn/brave_vpn_os_connection_api_win.cc index e585d7bf5e79..333939599073 100644 --- a/components/brave_vpn/brave_vpn_os_connection_api_win.cc +++ b/components/brave_vpn/brave_vpn_os_connection_api_win.cc @@ -19,13 +19,7 @@ // (brian@clifton.me)'s work (https://github.com/bsclifton/winvpntool). using brave_vpn::internal::CheckConnectionResult; -using brave_vpn::internal::CloseEventHandleForConnectFailed; -using brave_vpn::internal::CloseEventHandleForConnecting; -using brave_vpn::internal::CloseEventHandleForDisconnecting; using brave_vpn::internal::CreateEntry; -using brave_vpn::internal::GetEventHandleForConnectFailed; -using brave_vpn::internal::GetEventHandleForConnecting; -using brave_vpn::internal::GetEventHandleForDisconnecting; using brave_vpn::internal::GetPhonebookPath; using brave_vpn::internal::PrintRasError; using brave_vpn::internal::RemoveEntry; @@ -56,9 +50,6 @@ BraveVPNOSConnectionAPIWin::BraveVPNOSConnectionAPIWin() { BraveVPNOSConnectionAPIWin::~BraveVPNOSConnectionAPIWin() { CloseHandle(event_handle_for_connected_disconnected_); - CloseEventHandleForConnecting(); - CloseEventHandleForDisconnecting(); - CloseEventHandleForConnectFailed(); } void BraveVPNOSConnectionAPIWin::CreateVPNConnectionImpl( @@ -116,19 +107,6 @@ void BraveVPNOSConnectionAPIWin::OnObjectSignaled(HANDLE object) { CheckConnectionImpl(target_vpn_entry_name()); return; } - - CheckConnectionResult result = CheckConnectionResult::DISCONNECTING; - if (object == GetEventHandleForConnecting()) { - result = CheckConnectionResult::CONNECTING; - } else if (object == GetEventHandleForConnectFailed()) { - result = CheckConnectionResult::CONNECT_FAILED; - } else if (object == GetEventHandleForDisconnecting()) { - result = CheckConnectionResult::DISCONNECTING; - } else { - NOTREACHED(); - } - - OnCheckConnection(target_vpn_entry_name(), result); } void BraveVPNOSConnectionAPIWin::OnCheckConnection( @@ -166,9 +144,8 @@ void BraveVPNOSConnectionAPIWin::OnCreated(const std::string& name, } void BraveVPNOSConnectionAPIWin::OnConnected(bool success) { - if (success) { - BraveVPNOSConnectionAPI::OnConnected(); - } + success ? BraveVPNOSConnectionAPI::OnConnected() + : BraveVPNOSConnectionAPI::OnConnectFailed(); } void BraveVPNOSConnectionAPIWin::OnRemoved(const std::string& name, @@ -185,15 +162,8 @@ void BraveVPNOSConnectionAPIWin::StartVPNConnectionChangeMonitoring() { RasConnectionNotificationW(static_cast(INVALID_HANDLE_VALUE), event_handle_for_connected_disconnected_, RASCN_Connection | RASCN_Disconnection); - connected_disconnected_event_watcher_.StartWatchingMultipleTimes( event_handle_for_connected_disconnected_, this); - connecting_event_watcher_.StartWatchingMultipleTimes( - GetEventHandleForConnecting(), this); - disconnecting_event_watcher_.StartWatchingMultipleTimes( - GetEventHandleForDisconnecting(), this); - connect_failed_event_watcher_.StartWatchingMultipleTimes( - GetEventHandleForConnectFailed(), this); } } // namespace brave_vpn diff --git a/components/brave_vpn/brave_vpn_os_connection_api_win.h b/components/brave_vpn/brave_vpn_os_connection_api_win.h index 359c36781e7c..57fec84864a1 100644 --- a/components/brave_vpn/brave_vpn_os_connection_api_win.h +++ b/components/brave_vpn/brave_vpn_os_connection_api_win.h @@ -53,9 +53,6 @@ class BraveVPNOSConnectionAPIWin : public BraveVPNOSConnectionAPI, HANDLE event_handle_for_connected_disconnected_ = NULL; base::win::ObjectWatcher connected_disconnected_event_watcher_; - base::win::ObjectWatcher connect_failed_event_watcher_; - base::win::ObjectWatcher connecting_event_watcher_; - base::win::ObjectWatcher disconnecting_event_watcher_; base::WeakPtrFactory weak_factory_{this}; }; diff --git a/components/brave_vpn/utils_win.cc b/components/brave_vpn/utils_win.cc index 810e2abab4a4..42294554966c 100644 --- a/components/brave_vpn/utils_win.cc +++ b/components/brave_vpn/utils_win.cc @@ -20,10 +20,6 @@ namespace brave_vpn { namespace { -HANDLE g_connecting_event_handle = NULL; -HANDLE g_connect_failed_event_handle = NULL; -HANDLE g_disconnecting_event_handle = NULL; - class ScopedHeapAlloc { public: explicit ScopedHeapAlloc(SIZE_T dw_bytes) { @@ -42,24 +38,6 @@ class ScopedHeapAlloc { LPVOID lp_alloc_mem_ = NULL; }; -void WINAPI RasDialFunc(UINT, RASCONNSTATE rasconnstate, DWORD error) { - if (error) { - SetEvent(g_connect_failed_event_handle); - internal::PrintRasError(error); - return; - } - - // Only interested in connecting event. - switch (rasconnstate) { - case RASCS_ConnectDevice: - SetEvent(g_connecting_event_handle); - break; - default: - // Ignore all other states. - break; - } -} - // https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-formatmessage void PrintSystemError(DWORD error) { constexpr DWORD kBufSize = 512; @@ -99,45 +77,6 @@ DWORD SetCredentials(LPCTSTR entry_name, LPCTSTR username, LPCTSTR password) { namespace internal { -HANDLE GetEventHandleForConnecting() { - if (!g_connecting_event_handle) - g_connecting_event_handle = CreateEvent(NULL, false, false, NULL); - return g_connecting_event_handle; -} - -void CloseEventHandleForConnecting() { - if (g_connecting_event_handle) { - CloseHandle(g_connecting_event_handle); - g_connecting_event_handle = NULL; - } -} - -HANDLE GetEventHandleForConnectFailed() { - if (!g_connect_failed_event_handle) - g_connect_failed_event_handle = CreateEvent(NULL, false, false, NULL); - return g_connect_failed_event_handle; -} - -void CloseEventHandleForConnectFailed() { - if (g_connect_failed_event_handle) { - CloseHandle(g_connect_failed_event_handle); - g_connect_failed_event_handle = NULL; - } -} - -HANDLE GetEventHandleForDisconnecting() { - if (!g_disconnecting_event_handle) - g_disconnecting_event_handle = CreateEvent(NULL, false, false, NULL); - return g_disconnecting_event_handle; -} - -void CloseEventHandleForDisconnecting() { - if (g_disconnecting_event_handle) { - CloseHandle(g_disconnecting_event_handle); - g_disconnecting_event_handle = NULL; - } -} - // https://docs.microsoft.com/en-us/windows/win32/api/ras/nf-ras-rasgeterrorstringa void PrintRasError(DWORD error) { constexpr DWORD kBufSize = 512; @@ -230,7 +169,6 @@ bool DisconnectEntry(const std::wstring& entry_name) { VLOG(2) << __func__ << " : " << name << ", " << type; if (name.compare(entry_name) == 0 && type.compare(L"VPN") == 0) { VLOG(2) << __func__ << " : Disconnect... " << entry_name; - SetEvent(g_disconnecting_event_handle); dw_ret = RasHangUpA(lp_ras_conn[i].hrasconn); break; } @@ -269,7 +207,6 @@ bool ConnectEntry(const std::wstring& entry_name) { reinterpret_cast(ras_dial_params.lp_alloc_mem()); if (lp_ras_dial_params == NULL) { LOG(ERROR) << "HeapAlloc failed!"; - SetEvent(g_connect_failed_event_handle); return false; } lp_ras_dial_params->dwSize = sizeof(RASDIALPARAMS); @@ -286,7 +223,6 @@ bool ConnectEntry(const std::wstring& entry_name) { RasGetCredentials(DEFAULT_PHONE_BOOK, entry_name.c_str(), &credentials); if (dw_ret != ERROR_SUCCESS) { PrintRasError(dw_ret); - SetEvent(g_connect_failed_event_handle); return false; } wcscpy_s(lp_ras_dial_params->szUserName, UNLEN + 1, credentials.szUserName); @@ -294,8 +230,8 @@ bool ConnectEntry(const std::wstring& entry_name) { VLOG(2) << __func__ << " : Connecting to " << entry_name; HRASCONN h_ras_conn = NULL; - dw_ret = RasDial(NULL, DEFAULT_PHONE_BOOK, lp_ras_dial_params, 0, - (LPVOID)(&RasDialFunc), &h_ras_conn); + dw_ret = RasDial(NULL, DEFAULT_PHONE_BOOK, lp_ras_dial_params, 0, NULL, + &h_ras_conn); if (dw_ret == ERROR_DIAL_ALREADY_IN_PROGRESS) { // We should not treat this as failure state. @@ -306,7 +242,6 @@ bool ConnectEntry(const std::wstring& entry_name) { if (dw_ret != ERROR_SUCCESS) { PrintRasError(dw_ret); - SetEvent(g_connect_failed_event_handle); return false; } diff --git a/components/brave_vpn/utils_win.h b/components/brave_vpn/utils_win.h index 548c1c5983f7..dde63a302cdd 100644 --- a/components/brave_vpn/utils_win.h +++ b/components/brave_vpn/utils_win.h @@ -32,14 +32,6 @@ bool CreateEntry(const std::wstring& entry_name, bool RemoveEntry(const std::wstring& entry_name); bool DisconnectEntry(const std::wstring& entry_name); bool ConnectEntry(const std::wstring& entry_name); -// Don't cache returned HANDLE. It could be invalidated. -HANDLE GetEventHandleForConnecting(); -void CloseEventHandleForConnecting(); -HANDLE GetEventHandleForConnectFailed(); -void CloseEventHandleForConnectFailed(); -HANDLE GetEventHandleForDisconnecting(); -void CloseEventHandleForDisconnecting(); - CheckConnectionResult CheckConnection(const std::wstring& entry_name); } // namespace internal