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.

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

Passed phonebook path for ras api instead of NULL #17964

Merged
merged 1 commit into from
Apr 7, 2023
Merged
Changes from all commits
Commits
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
82 changes: 38 additions & 44 deletions components/brave_vpn/browser/connection/win/utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@
#include "brave/components/brave_vpn/browser/connection/brave_vpn_connection_info.h"
#include "brave/components/brave_vpn/common/brave_vpn_constants.h"

#define DEFAULT_PHONE_BOOK NULL

namespace brave_vpn {

namespace {
Expand Down Expand Up @@ -61,7 +59,8 @@ std::string GetSystemError(DWORD error) {
}

// https://docs.microsoft.com/en-us/windows/win32/api/ras/nf-ras-rassetcredentialsa
absl::optional<std::string> SetCredentials(LPCTSTR entry_name,
absl::optional<std::string> SetCredentials(LPCTSTR phone_book_path,
LPCTSTR entry_name,
LPCTSTR username,
LPCTSTR password) {
RASCREDENTIALS credentials;
Expand All @@ -74,7 +73,7 @@ absl::optional<std::string> SetCredentials(LPCTSTR entry_name,
wcscpy_s(credentials.szPassword, PWLEN + 1, password);

DWORD dw_ret =
RasSetCredentials(DEFAULT_PHONE_BOOK, entry_name, &credentials, FALSE);
RasSetCredentials(phone_book_path, entry_name, &credentials, FALSE);
if (dw_ret != ERROR_SUCCESS) {
return base::StrCat(
{"RasSetCredential() - ", internal::GetRasErrorMessage(dw_ret)});
Expand Down Expand Up @@ -180,7 +179,7 @@ std::wstring GetPhonebookPath(const std::wstring& entry_name,
return path;
}

*error = "failed to get phonebook path from ALLUSERSPROFILE";
*error = "failed to get phonebook path from ALLUSERSPROFILE and APPDATA";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix here 😄

VLOG(2) << __func__
<< " : did not find phone book file. This is required to add the VPN "
"entry.";
Expand Down Expand Up @@ -271,6 +270,14 @@ RasOperationResult ConnectEntry(const std::wstring& entry_name) {
return GetRasSuccessResult();
}

std::string error_get_phone_book_path;
const std::wstring phone_book_path =
GetPhonebookPath(entry_name, &error_get_phone_book_path);
if (phone_book_path.empty()) {
return GetRasErrorResult(error_get_phone_book_path,
"GetPhonebookPath() from ConnectEntry()");
}

LPRASDIALPARAMS lp_ras_dial_params = NULL;
DWORD cb = sizeof(RASDIALPARAMS);

Expand All @@ -290,8 +297,8 @@ RasOperationResult ConnectEntry(const std::wstring& entry_name) {
ZeroMemory(&credentials, sizeof(RASCREDENTIALS));
credentials.dwSize = sizeof(RASCREDENTIALS);
credentials.dwMask = RASCM_UserName | RASCM_Password;
DWORD dw_ret =
RasGetCredentials(DEFAULT_PHONE_BOOK, entry_name.c_str(), &credentials);
DWORD dw_ret = RasGetCredentials(phone_book_path.c_str(), entry_name.c_str(),
&credentials);
if (dw_ret != ERROR_SUCCESS) {
return GetRasErrorResult(GetRasErrorMessage(dw_ret), "RasGetCredentials()");
}
Expand All @@ -300,7 +307,7 @@ RasOperationResult 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, NULL,
dw_ret = RasDial(NULL, phone_book_path.c_str(), lp_ras_dial_params, 0, NULL,
&h_ras_conn);

if (dw_ret == ERROR_DIAL_ALREADY_IN_PROGRESS) {
Expand Down Expand Up @@ -328,7 +335,15 @@ 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());
std::string error_get_phone_book_path;
const std::wstring phone_book_path =
GetPhonebookPath(entry_name, &error_get_phone_book_path);
if (phone_book_path.empty()) {
return GetRasErrorResult(error_get_phone_book_path,
"GetPhonebookPath() from RemoveEntry()");
}

DWORD dw_ret = RasDeleteEntry(phone_book_path.c_str(), entry_name.c_str());
if (dw_ret != ERROR_SUCCESS) {
return GetRasErrorResult(GetRasErrorMessage(dw_ret), "RasDeleteEntry()");
}
Expand All @@ -351,28 +366,12 @@ 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;
std::string error_get_phone_book_path;
const std::wstring phone_book_path =
GetPhonebookPath(entry_name, &error_get_phone_book_path);
if (phone_book_path.empty()) {
return GetRasErrorResult(error_get_phone_book_path,
"GetPhonebookPath() from CreateEntry()");
}

auto connection_result = CheckConnection(entry_name);
Expand Down Expand Up @@ -412,15 +411,17 @@ RasOperationResult CreateEntry(const BraveVPNConnectionInfo& info) {
// this maps to "Type of sign-in info" => "User name and password"
entry.dwCustomAuthKey = 26;

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

if (const auto error = SetCredentials(entry_name.c_str(), username.c_str(),
password.c_str())) {
if (const auto error =
SetCredentials(phone_book_path.c_str(), entry_name.c_str(),
username.c_str(), password.c_str())) {
return GetRasErrorResult(*error);
}

Expand Down Expand Up @@ -457,28 +458,21 @@ RasOperationResult CreateEntry(const BraveVPNConnectionInfo& info) {
constexpr wchar_t kNumCustomPolicy[] = L"1";
constexpr wchar_t kCustomIPSecPolicies[] =
L"030000000400000002000000050000000200000000000000";
std::string error;
std::wstring phone_book_path = GetPhonebookPath(entry_name, &error);
if (phone_book_path.empty()) {
return GetRasErrorResult("GetPhonebookPath() failed.");
}

// https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-writeprivateprofilestringw
BOOL wrote_entry =
WritePrivateProfileString(entry_name.c_str(), L"NumCustomPolicy",
kNumCustomPolicy, phone_book_path.c_str());
if (!wrote_entry) {
return GetRasErrorResult(
"ERROR: failed to write \"NumCustomPolicy\" field to `rasphone.pbk`");
"failed to write \"NumCustomPolicy\" field to `rasphone.pbk`");
}

wrote_entry =
WritePrivateProfileString(entry_name.c_str(), L"CustomIPSecPolicies",
kCustomIPSecPolicies, phone_book_path.c_str());
if (!wrote_entry) {
return GetRasErrorResult(
"ERROR: failed to write \"CustomIPSecPolicies\" field to "
"`rasphone.pbk`");
"failed to write \"CustomIPSecPolicies\" field to `rasphone.pbk`");
}

return GetRasSuccessResult();
Expand Down