Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added more detailed log to VPN api on Windows #17946

Merged
merged 2 commits into from
Apr 6, 2023
Merged

Added more detailed log to VPN api on Windows #17946

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c42cb3
Added more detailed log to VPN api on Windows
simonhong Apr 6, 2023
d10fdc1
Added validation with `RasValidateEntryName`
bsclifton Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 52 additions & 13 deletions components/brave_vpn/browser/connection/win/utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/files/file_util.h"
#include "base/logging.h"
#include "base/path_service.h"
#include "base/strings/strcat.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "brave/components/brave_vpn/browser/connection/brave_vpn_connection_info.h"
Expand Down Expand Up @@ -75,7 +76,8 @@ absl::optional<std::string> SetCredentials(LPCTSTR entry_name,
DWORD dw_ret =
RasSetCredentials(DEFAULT_PHONE_BOOK, entry_name, &credentials, FALSE);
if (dw_ret != ERROR_SUCCESS) {
return internal::GetRasErrorMessage(dw_ret);
return base::StrCat(
{"RasSetCredential() - ", internal::GetRasErrorMessage(dw_ret)});
}

return absl::nullopt;
Expand Down Expand Up @@ -132,10 +134,12 @@ internal::RasOperationResult GetRasSuccessResult() {
return result;
}

internal::RasOperationResult GetRasErrorResult(const std::string& error) {
internal::RasOperationResult GetRasErrorResult(const std::string& error,
const std::string& caller = {}) {
internal::RasOperationResult result;
result.success = false;
result.error_description = error;
result.error_description =
caller.empty() ? error : base::StrCat({caller, " - ", error});
return result;
}

Expand Down Expand Up @@ -216,6 +220,7 @@ RasOperationResult DisconnectEntry(const std::wstring& entry_name) {

// Call RasEnumConnections to enumerate active connections
dw_ret = RasEnumConnections(lp_ras_conn, &dw_cb, &dw_connections);
std::string caller = "RasEnumConnection()";

// If successful, print the names of the active connections.
if (ERROR_SUCCESS == dw_ret) {
Expand All @@ -228,7 +233,10 @@ RasOperationResult 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;
dw_ret = RasHangUpA(lp_ras_conn[i].hrasconn);
dw_ret = RasHangUp(lp_ras_conn[i].hrasconn);
if (dw_ret != ERROR_SUCCESS) {
caller = "RasHangUp()";
}
break;
}
}
Expand All @@ -237,7 +245,7 @@ RasOperationResult DisconnectEntry(const std::wstring& entry_name) {
if (dw_ret == ERROR_SUCCESS)
return GetRasSuccessResult();

return GetRasErrorResult(GetRasErrorMessage(dw_ret));
return GetRasErrorResult(GetRasErrorMessage(dw_ret), caller);
}

// There was either a problem with RAS or there are no connections to
Expand Down Expand Up @@ -285,7 +293,7 @@ RasOperationResult ConnectEntry(const std::wstring& entry_name) {
DWORD dw_ret =
RasGetCredentials(DEFAULT_PHONE_BOOK, entry_name.c_str(), &credentials);
if (dw_ret != ERROR_SUCCESS) {
return GetRasErrorResult(GetRasErrorMessage(dw_ret));
return GetRasErrorResult(GetRasErrorMessage(dw_ret), "RasGetCredentials()");
}
wcscpy_s(lp_ras_dial_params->szUserName, UNLEN + 1, credentials.szUserName);
wcscpy_s(lp_ras_dial_params->szPassword, PWLEN + 1, credentials.szPassword);
Expand All @@ -303,12 +311,15 @@ RasOperationResult ConnectEntry(const std::wstring& entry_name) {
}

if (dw_ret != ERROR_SUCCESS) {
auto result = GetRasErrorResult(GetRasErrorMessage(dw_ret));
auto result = GetRasErrorResult(GetRasErrorMessage(dw_ret), "RasDial()");

// To clear state.
VLOG(2) << __func__ << ": RasDial() failed. Try RasHangUp() to clear state";
if (dw_ret = RasHangUp(h_ras_conn); dw_ret != ERROR_SUCCESS)
result.error_description = GetRasErrorMessage(dw_ret);
if (dw_ret = RasHangUp(h_ras_conn); dw_ret != ERROR_SUCCESS) {
result.error_description =
base::StrCat({result.error_description, ", ", "RasHangUp() - ",
GetRasErrorMessage(dw_ret)});
}

return result;
}
Expand All @@ -319,7 +330,7 @@ RasOperationResult ConnectEntry(const std::wstring& entry_name) {
RasOperationResult RemoveEntry(const std::wstring& entry_name) {
DWORD dw_ret = RasDeleteEntry(DEFAULT_PHONE_BOOK, entry_name.c_str());
if (dw_ret != ERROR_SUCCESS) {
return GetRasErrorResult(GetRasErrorMessage(dw_ret));
return GetRasErrorResult(GetRasErrorMessage(dw_ret), "RasDeleteEntry()");
}

return GetRasSuccessResult();
Expand All @@ -340,6 +351,30 @@ RasOperationResult CreateEntry(const BraveVPNConnectionInfo& info) {
return GetRasErrorResult("`hostname` is empty");
}

// Simple validation on the entry name.
DWORD dw_ret = RasValidateEntryName(DEFAULT_PHONE_BOOK, entry_name.c_str());
switch (dw_ret) {
// New or existing is the happy path.
// `RasSetEntryProperties` will add new entry or update an existing entry.
case ERROR_SUCCESS:
VLOG(2) << __func__ << " Entry name is valid";
break;
case ERROR_ALREADY_EXISTS:
VLOG(2) << __func__
<< " The entry name already exists in the specified phonebook.";
break;
// Possible error conditions.
case ERROR_INVALID_NAME:
VLOG(2) << __func__ << " Entry name: `" << entry_name.c_str()
<< "` is invalid";
return GetRasErrorResult(
"The format of the specified entry name is invalid.");
default:
VLOG(2) << __func__ << " RasValidateEntryName failed: Error=\""
<< GetRasErrorMessage(dw_ret) << "\" (" << dw_ret << ")";
break;
}

auto connection_result = CheckConnection(entry_name);
if (connection_result == CheckConnectionResult::CONNECTING ||
connection_result == CheckConnectionResult::CONNECTED) {
Expand All @@ -349,6 +384,9 @@ RasOperationResult CreateEntry(const BraveVPNConnectionInfo& info) {
return GetRasSuccessResult();
}

VLOG(2) << __func__ << " Create Entry(" << entry_name << ") with "
<< hostname;

RASENTRY entry;
ZeroMemory(&entry, sizeof(RASENTRY));
// For descriptions of each field (including valid values) see:
Expand All @@ -374,10 +412,11 @@ RasOperationResult CreateEntry(const BraveVPNConnectionInfo& info) {
// this maps to "Type of sign-in info" => "User name and password"
entry.dwCustomAuthKey = 26;

DWORD dw_ret = RasSetEntryProperties(DEFAULT_PHONE_BOOK, entry_name.c_str(),
&entry, entry.dwSize, NULL, NULL);
dw_ret = RasSetEntryProperties(DEFAULT_PHONE_BOOK, entry_name.c_str(), &entry,
entry.dwSize, NULL, NULL);
if (dw_ret != ERROR_SUCCESS) {
return GetRasErrorResult(GetRasErrorMessage(dw_ret));
return GetRasErrorResult(GetRasErrorMessage(dw_ret),
"RasSetEntryProperties()");
}

if (const auto error = SetCredentials(entry_name.c_str(), username.c_str(),
Expand Down