shill: OpenVPNDriver: Use double vector of strings for arguments
Restructure the code that creates command-line arguments for the
openvpn process. Instead of building a flat vector of strings,
each logical option supplied to openvpn is itself a vector of
strings. Thus, it is possible to output either a command line
argument list for OpenVPN (as is still done here) or a configuration
file which will contain each multi-word option on a separate line.
This structure also allows testing to be more rigorous since
expectations won't conflate the flag with its arguments.
BUG=chromium:217624
TEST=Unit tests, network_VPNConnect.openvpn_user_pass
Change-Id: I7d2973792372d43df6c3a3ebe3728debd09e1e68
Reviewed-on: https://gerrit.chromium.org/gerrit/64292
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/mock_openvpn_management_server.h b/mock_openvpn_management_server.h
index 16e3053..33966f4 100644
--- a/mock_openvpn_management_server.h
+++ b/mock_openvpn_management_server.h
@@ -18,7 +18,7 @@
MOCK_METHOD3(Start, bool(EventDispatcher *dispatcher,
Sockets *sockets,
- std::vector<std::string> *options));
+ std::vector<std::vector<std::string>> *options));
MOCK_METHOD0(Stop, void());
MOCK_METHOD0(ReleaseHold, void());
MOCK_METHOD0(Hold, void());
diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index bc5aa80..baa7415 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -215,24 +215,34 @@
ip_properties_ = IPConfig::Properties();
}
+// static
+string OpenVPNDriver::JoinOptions(const vector<vector<string>> &options) {
+ vector<string> option_strings;
+ for (const auto &option : options) {
+ option_strings.push_back(JoinString(option, ' '));
+ }
+ return JoinString(option_strings, ',');
+}
+
bool OpenVPNDriver::SpawnOpenVPN() {
SLOG(VPN, 2) << __func__ << "(" << tunnel_interface_ << ")";
- vector<string> options;
+ vector<vector<string>> options;
Error error;
InitOptions(&options, &error);
if (error.IsFailure()) {
return false;
}
- LOG(INFO) << "OpenVPN process options: " << JoinString(options, ' ');
+ LOG(INFO) << "OpenVPN process options: " << JoinOptions(options);
// TODO(quiche): This should be migrated to use ExternalTask.
// (crbug.com/246263).
vector<char *> process_args;
process_args.push_back(const_cast<char *>(kOpenVPNPath));
- for (vector<string>::const_iterator it = options.begin();
- it != options.end(); ++it) {
- process_args.push_back(const_cast<char *>(it->c_str()));
+ for (const auto &option : options) {
+ for (const auto &argument : option) {
+ process_args.push_back(const_cast<char *>(argument.c_str()));
+ }
}
process_args.push_back(NULL);
@@ -240,9 +250,8 @@
InitEnvironment(&environment);
vector<char *> process_env;
- for (vector<string>::const_iterator it = environment.begin();
- it != environment.end(); ++it) {
- process_env.push_back(const_cast<char *>(it->c_str()));
+ for (const auto &environment_variable : environment) {
+ process_env.push_back(const_cast<char *>(environment_variable.c_str()));
}
process_env.push_back(NULL);
@@ -337,10 +346,9 @@
properties->subnet_prefix =
IPAddress::GetMaxPrefixLength(properties->address_family);
}
- for (map<string, string>::const_iterator it = configuration.begin();
- it != configuration.end(); ++it) {
- const string &key = it->first;
- const string &value = it->second;
+ for (const auto &configuration_map : configuration) {
+ const string &key = configuration_map.first;
+ const string &value = configuration_map.second;
SLOG(VPN, 2) << "Processing: " << key << " -> " << value;
if (LowerCaseEqualsASCII(key, kOpenVPNIfconfigLocal)) {
properties->address = value;
@@ -400,9 +408,8 @@
IPConfig::Properties *properties) {
vector<string> domain_search;
vector<string> dns_servers;
- for (ForeignOptions::const_iterator it = options.begin();
- it != options.end(); ++it) {
- ParseForeignOption(it->second, &domain_search, &dns_servers);
+ for (const auto &option_map : options) {
+ ParseForeignOption(option_map.second, &domain_search, &dns_servers);
}
if (!domain_search.empty()) {
properties->domain_search.swap(domain_search);
@@ -469,11 +476,10 @@
void OpenVPNDriver::SetRoutes(const RouteOptions &routes,
IPConfig::Properties *properties) {
vector<IPConfig::Route> new_routes;
- for (RouteOptions::const_iterator it = routes.begin();
- it != routes.end(); ++it) {
- const IPConfig::Route &route = it->second;
+ for (const auto &route_map : routes) {
+ const IPConfig::Route &route = route_map.second;
if (route.host.empty() || route.netmask.empty() || route.gateway.empty()) {
- LOG(WARNING) << "Ignoring incomplete route: " << it->first;
+ LOG(WARNING) << "Ignoring incomplete route: " << route_map.first;
continue;
}
new_routes.push_back(route);
@@ -512,36 +518,32 @@
// Wait for the ClaimInterface callback to continue the connection process.
}
-void OpenVPNDriver::InitOptions(vector<string> *options, Error *error) {
+void OpenVPNDriver::InitOptions(vector<vector<string>> *options, Error *error) {
string vpnhost = args()->LookupString(flimflam::kProviderHostProperty, "");
if (vpnhost.empty()) {
Error::PopulateAndLog(
error, Error::kInvalidArguments, "VPN host not specified.");
return;
}
- options->push_back("--client");
- options->push_back("--tls-client");
+ AppendOption("--client", options);
+ AppendOption("--tls-client", options);
- options->push_back("--remote");
string host_name, host_port;
if (SplitPortFromHost(vpnhost, &host_name, &host_port)) {
DCHECK(!host_name.empty());
DCHECK(!host_port.empty());
- options->push_back(host_name);
- options->push_back(host_port);
+ AppendOption("--remote", host_name, host_port, options);
} else {
- options->push_back(vpnhost);
+ AppendOption("--remote", vpnhost, options);
}
- options->push_back("--nobind");
- options->push_back("--persist-key");
- options->push_back("--persist-tun");
+ AppendOption("--nobind", options);
+ AppendOption("--persist-key", options);
+ AppendOption("--persist-tun", options);
CHECK(!tunnel_interface_.empty());
- options->push_back("--dev");
- options->push_back(tunnel_interface_);
- options->push_back("--dev-type");
- options->push_back("tun");
+ AppendOption("--dev", tunnel_interface_, options);
+ AppendOption("--dev-type", "tun", options);
InitLoggingOptions(options);
@@ -561,8 +563,7 @@
error, Error::kInternalError, "Unable to setup tls-auth file.");
return;
}
- options->push_back("--tls-auth");
- options->push_back(tls_auth_file_.value());
+ AppendOption("--tls-auth", tls_auth_file_.value(), options);
}
}
AppendValueOption(
@@ -603,8 +604,7 @@
remote_cert_tls = "server";
}
if (remote_cert_tls != "none") {
- options->push_back("--remote-cert-tls");
- options->push_back(remote_cert_tls);
+ AppendOption("--remote-cert-tls", remote_cert_tls, options);
}
// This is an undocumented command line argument that works like a .cfg file
@@ -623,32 +623,28 @@
// Setup openvpn-script options and RPC information required to send back
// Layer 3 configuration.
- options->push_back("--setenv");
- options->push_back(kRPCTaskServiceVariable);
- options->push_back(rpc_task_->GetRpcConnectionIdentifier());
- options->push_back("--setenv");
- options->push_back(kRPCTaskPathVariable);
- options->push_back(rpc_task_->GetRpcIdentifier());
- options->push_back("--script-security");
- options->push_back("2");
- options->push_back("--up");
- options->push_back(kOpenVPNScript);
- options->push_back("--up-restart");
+ AppendOption("--setenv", kRPCTaskServiceVariable,
+ rpc_task_->GetRpcConnectionIdentifier(), options);
+ AppendOption("--setenv", kRPCTaskServiceVariable,
+ rpc_task_->GetRpcConnectionIdentifier(), options);
+ AppendOption("--setenv", kRPCTaskPathVariable, rpc_task_->GetRpcIdentifier(),
+ options);
+ AppendOption("--script-security", "2", options);
+ AppendOption("--up", kOpenVPNScript, options);
+ AppendOption("--up-restart", options);
// Disable openvpn handling since we do route+ifconfig work.
- options->push_back("--route-noexec");
- options->push_back("--ifconfig-noexec");
+ AppendOption("--route-noexec", options);
+ AppendOption("--ifconfig-noexec", options);
// Drop root privileges on connection and enable callback scripts to send
// notify messages.
- options->push_back("--user");
- options->push_back("openvpn");
- options->push_back("--group");
- options->push_back("openvpn");
+ AppendOption("--user", "openvpn", options);
+ AppendOption("--group", "openvpn", options);
}
-bool OpenVPNDriver::InitCAOptions(vector<string> *options, Error *error) {
- options->push_back("--ca");
+bool OpenVPNDriver::InitCAOptions(
+ vector<vector<string>> *options, Error *error) {
string ca_cert =
args()->LookupString(flimflam::kOpenVPNCaCertProperty, "");
string ca_cert_nss =
@@ -667,7 +663,7 @@
num_ca_cert_types++;
if (num_ca_cert_types == 0) {
// Use default CAs if no CA certificate is provided.
- options->push_back(kDefaultCACertificates);
+ AppendOption("--ca", kDefaultCACertificates, options);
return true;
} else if (num_ca_cert_types > 1) {
Error::PopulateAndLog(
@@ -688,7 +684,7 @@
"Unable to extract NSS CA certificate: " + ca_cert_nss);
return false;
}
- options->push_back(certfile.value());
+ AppendOption("--ca", certfile.value(), options);
return true;
} else if (!ca_cert_pem.empty()) {
DCHECK(ca_cert.empty() && ca_cert_nss.empty());
@@ -700,15 +696,15 @@
"Unable to extract PEM CA certificates.");
return false;
}
- options->push_back(certfile.value());
+ AppendOption("--ca", certfile.value(), options);
return true;
}
DCHECK(!ca_cert.empty() && ca_cert_nss.empty() && ca_cert_pem.empty());
- options->push_back(ca_cert);
+ AppendOption("--ca", ca_cert, options);
return true;
}
-void OpenVPNDriver::InitPKCS11Options(vector<string> *options) {
+void OpenVPNDriver::InitPKCS11Options(vector<vector<string>> *options) {
string id = args()->LookupString(flimflam::kOpenVPNClientCertIdProperty, "");
if (!id.empty()) {
string provider =
@@ -716,14 +712,12 @@
if (provider.empty()) {
provider = kDefaultPKCS11Provider;
}
- options->push_back("--pkcs11-providers");
- options->push_back(provider);
- options->push_back("--pkcs11-id");
- options->push_back(id);
+ AppendOption("--pkcs11-providers", provider, options);
+ AppendOption("--pkcs11-id", id, options);
}
}
-void OpenVPNDriver::InitClientAuthOptions(vector<string> *options) {
+void OpenVPNDriver::InitClientAuthOptions(vector<vector<string>> *options) {
bool has_cert = AppendValueOption(kOpenVPNCertProperty, "--cert", options) ||
!args()->LookupString(flimflam::kOpenVPNClientCertIdProperty, "").empty();
bool has_key = AppendValueOption(kOpenVPNKeyProperty, "--key", options);
@@ -733,12 +727,12 @@
if (args()->ContainsString(flimflam::kOpenVPNAuthUserPassProperty) ||
!args()->LookupString(flimflam::kOpenVPNUserProperty, "").empty() ||
(!has_cert && !has_key)) {
- options->push_back("--auth-user-pass");
+ AppendOption("--auth-user-pass", options);
}
}
bool OpenVPNDriver::InitManagementChannelOptions(
- vector<string> *options, Error *error) {
+ vector<vector<string>> *options, Error *error) {
if (!management_server_->Start(dispatcher(), &sockets_, options)) {
Error::PopulateAndLog(
error, Error::kInternalError, "Unable to setup management channel.");
@@ -754,34 +748,56 @@
return true;
}
-void OpenVPNDriver::InitLoggingOptions(vector<string> *options) {
- options->push_back("--syslog");
+void OpenVPNDriver::InitLoggingOptions(vector<vector<string>> *options) {
+ AppendOption("--syslog", options);
string verb = args()->LookupString(kOpenVPNVerbProperty, "");
if (verb.empty() && SLOG_IS_ON(VPN, 0)) {
verb = "3";
}
if (!verb.empty()) {
- options->push_back("--verb");
- options->push_back(verb);
+ AppendOption("--verb", verb, options);
}
}
+void OpenVPNDriver::AppendOption(
+ const string &option, vector<vector<string>> *options) {
+ options->push_back(vector<string>{ option });
+}
+
+void OpenVPNDriver::AppendOption(
+ const string &option,
+ const string &value,
+ vector<vector<string>> *options) {
+ options->push_back(vector<string>{ option, value });
+}
+
+void OpenVPNDriver::AppendOption(
+ const string &option,
+ const string &value0,
+ const string &value1,
+ vector<vector<string>> *options) {
+ options->push_back(vector<string>{ option, value0, value1 });
+}
+
bool OpenVPNDriver::AppendValueOption(
- const string &property, const string &option, vector<string> *options) {
+ const string &property,
+ const string &option,
+ vector<vector<string>> *options) {
string value = args()->LookupString(property, "");
if (!value.empty()) {
- options->push_back(option);
- options->push_back(value);
+ AppendOption(option, value, options);
return true;
}
return false;
}
bool OpenVPNDriver::AppendFlag(
- const string &property, const string &option, vector<string> *options) {
+ const string &property,
+ const string &option,
+ vector<vector<string>> *options) {
if (args()->ContainsString(property)) {
- options->push_back(option);
+ AppendOption(option, options);
return true;
}
return false;
@@ -873,13 +889,12 @@
}
vector<string> lines;
SplitString(contents, '\n', &lines);
- for (vector<string>::const_iterator it = lines.begin();
- it != lines.end(); ++it) {
- size_t assign = it->find('=');
+ for (const auto &line : lines) {
+ size_t assign = line.find('=');
if (assign == string::npos) {
continue;
}
- (*lsb_release)[it->substr(0, assign)] = it->substr(assign + 1);
+ (*lsb_release)[line.substr(0, assign)] = line.substr(assign + 1);
}
return true;
}
diff --git a/openvpn_driver.h b/openvpn_driver.h
index 50c7034..727df96 100644
--- a/openvpn_driver.h
+++ b/openvpn_driver.h
@@ -70,15 +70,30 @@
virtual void FailService(Service::ConnectFailure failure,
const std::string &error_details);
+ // Append zero-valued, single-valued and double-valued options to the
+ // |options| array.
+ static void AppendOption(
+ const std::string &option,
+ std::vector<std::vector<std::string>> *options);
+ static void AppendOption(
+ const std::string &option,
+ const std::string &value,
+ std::vector<std::vector<std::string>> *options);
+ static void AppendOption(
+ const std::string &option,
+ const std::string &value0,
+ const std::string &value1,
+ std::vector<std::vector<std::string>> *options);
+
// Returns true if an option was appended.
bool AppendValueOption(const std::string &property,
const std::string &option,
- std::vector<std::string> *options);
+ std::vector<std::vector<std::string>> *options);
// Returns true if a flag was appended.
bool AppendFlag(const std::string &property,
const std::string &option,
- std::vector<std::string> *options);
+ std::vector<std::vector<std::string>> *options);
protected:
// Inherited from VPNDriver. |Connect| initiates the VPN connection by
@@ -168,13 +183,15 @@
std::string *name,
std::string *port);
- void InitOptions(std::vector<std::string> *options, Error *error);
- bool InitCAOptions(std::vector<std::string> *options, Error *error);
- void InitClientAuthOptions(std::vector<std::string> *options);
- void InitPKCS11Options(std::vector<std::string> *options);
+ void InitOptions(
+ std::vector<std::vector<std::string>> *options, Error *error);
+ bool InitCAOptions(
+ std::vector<std::vector<std::string>> *options, Error *error);
+ void InitClientAuthOptions(std::vector<std::vector<std::string>> *options);
+ void InitPKCS11Options(std::vector<std::vector<std::string>> *options);
bool InitManagementChannelOptions(
- std::vector<std::string> *options, Error *error);
- void InitLoggingOptions(std::vector<std::string> *options);
+ std::vector<std::vector<std::string>> *options, Error *error);
+ void InitLoggingOptions(std::vector<std::vector<std::string>> *options);
void InitEnvironment(std::vector<std::string> *environment);
bool ParseLSBRelease(std::map<std::string, std::string> *lsb_release);
@@ -192,6 +209,10 @@
static int GetReconnectTimeoutSeconds(ReconnectReason reason);
+ // Join a list of options into a single string.
+ static std::string JoinOptions(
+ const std::vector<std::vector<std::string>> &options);
+
// Called when the openpvn process exits.
static void OnOpenVPNDied(GPid pid, gint status, gpointer data);
diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index 4a356e9..1931950 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -126,7 +126,8 @@
return device_->selected_service();
}
- bool InitManagementChannelOptions(vector<string> *options, Error *error) {
+ bool InitManagementChannelOptions(
+ vector<vector<string>> *options, Error *error) {
return driver_->InitManagementChannelOptions(options, error);
}
@@ -183,10 +184,13 @@
}
// Used to assert that a flag appears in the options.
- void ExpectInFlags(const vector<string> &options, const string &flag,
+ void ExpectInFlags(const vector<vector<string>> &options, const string &flag);
+ void ExpectInFlags(const vector<vector<string>> &options, const string &flag,
const string &value);
- void ExpectInFlags(const vector<string> &options, const string &flag);
- void ExpectNotInFlags(const vector<string> &options, const string &flag);
+ void ExpectInFlags(const vector<vector<string>> &options,
+ const vector<string> &arguments);
+ void ExpectNotInFlags(const vector<vector<string>> &options,
+ const string &flag);
void SetupLSBRelease();
@@ -233,32 +237,28 @@
void OpenVPNDriverTest::Notify(const string &/*reason*/,
const map<string, string> &/*dict*/) {}
-void OpenVPNDriverTest::ExpectInFlags(const vector<string> &options,
+void OpenVPNDriverTest::ExpectInFlags(const vector<vector<string>> &options,
+ const string &flag) {
+ ExpectInFlags(options, vector<string> { flag });
+}
+
+void OpenVPNDriverTest::ExpectInFlags(const vector<vector<string>> &options,
const string &flag,
const string &value) {
- vector<string>::const_iterator it =
- std::find(options.begin(), options.end(), flag);
-
- EXPECT_TRUE(it != options.end());
- if (it != options.end())
- return; // Don't crash below.
- it++;
- EXPECT_TRUE(it != options.end());
- if (it != options.end())
- return; // Don't crash below.
- EXPECT_EQ(value, *it);
+ ExpectInFlags(options, vector<string> { flag, value });
}
-void OpenVPNDriverTest::ExpectInFlags(const vector<string> &options,
- const string &flag) {
- EXPECT_TRUE(std::find(options.begin(), options.end(), flag) !=
+void OpenVPNDriverTest::ExpectInFlags(const vector<vector<string>> &options,
+ const vector<string> &arguments) {
+ EXPECT_TRUE(std::find(options.begin(), options.end(), arguments) !=
options.end());
}
-void OpenVPNDriverTest::ExpectNotInFlags(const vector<string> &options,
+void OpenVPNDriverTest::ExpectNotInFlags(const vector<vector<string>> &options,
const string &flag) {
- EXPECT_TRUE(std::find(options.begin(), options.end(), flag) ==
- options.end());
+ for (const auto &option : options) {
+ EXPECT_NE(flag, option[0]);
+ }
}
void OpenVPNDriverTest::SetupLSBRelease() {
@@ -564,7 +564,7 @@
TEST_F(OpenVPNDriverTest, InitOptionsNoHost) {
Error error;
- vector<string> options;
+ vector<vector<string>> options;
driver_->InitOptions(&options, &error);
EXPECT_EQ(Error::kInvalidArguments, error.type());
EXPECT_TRUE(options.empty());
@@ -584,12 +584,13 @@
EXPECT_CALL(manager_, IsOnline()).WillOnce(Return(false));
Error error;
- vector<string> options;
+ vector<vector<string>> options;
driver_->InitOptions(&options, &error);
EXPECT_TRUE(error.IsSuccess());
- EXPECT_EQ("--client", options[0]);
+ EXPECT_EQ(vector<string> { "--client" }, options[0]);
ExpectInFlags(options, "--remote", kHost);
- ExpectInFlags(options, kRPCTaskPathVariable, RPCTaskMockAdaptor::kRpcId);
+ ExpectInFlags(options, vector<string> { "--setenv", kRPCTaskPathVariable,
+ RPCTaskMockAdaptor::kRpcId });
ExpectInFlags(options, "--dev", kInterfaceName);
ExpectInFlags(options, "--group", "openvpn");
EXPECT_EQ(kInterfaceName, driver_->tunnel_interface_);
@@ -613,16 +614,10 @@
EXPECT_CALL(manager_, IsOnline()).WillOnce(Return(false));
Error error;
- vector<string> options;
+ vector<vector<string>> options;
driver_->InitOptions(&options, &error);
EXPECT_TRUE(error.IsSuccess());
- vector<string>::const_iterator it =
- std::find(options.begin(), options.end(), "--remote");
- ASSERT_TRUE(it != options.end());
- ASSERT_TRUE(++it != options.end());
- EXPECT_EQ("v.com", *it);
- ASSERT_TRUE(++it != options.end());
- EXPECT_EQ("1234", *it);
+ ExpectInFlags(options, vector<string> { "--remote", "v.com", "1234" });
}
TEST_F(OpenVPNDriverTest, InitCAOptions) {
@@ -632,7 +627,7 @@
static const char kNSSCertfile[] = "/tmp/nss-cert";
Error error;
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_TRUE(driver_->InitCAOptions(&options, &error));
EXPECT_TRUE(error.IsSuccess());
ExpectInFlags(options, "--ca", OpenVPNDriver::kDefaultCACertificates);
@@ -700,7 +695,7 @@
TEST_F(OpenVPNDriverTest, InitClientAuthOptions) {
static const char kTestValue[] = "foo";
- vector<string> options;
+ vector<vector<string>> options;
// No key or cert, assume user/password authentication.
driver_->InitClientAuthOptions(&options);
@@ -779,7 +774,7 @@
}
TEST_F(OpenVPNDriverTest, InitPKCS11Options) {
- vector<string> options;
+ vector<vector<string>> options;
driver_->InitPKCS11Options(&options);
EXPECT_TRUE(options.empty());
@@ -798,7 +793,7 @@
}
TEST_F(OpenVPNDriverTest, InitManagementChannelOptionsServerFail) {
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_CALL(*management_server_, Start(&dispatcher_, GetSockets(), &options))
.WillOnce(Return(false));
Error error;
@@ -808,7 +803,7 @@
}
TEST_F(OpenVPNDriverTest, InitManagementChannelOptionsOnline) {
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_CALL(*management_server_, Start(&dispatcher_, GetSockets(), &options))
.WillOnce(Return(true));
EXPECT_CALL(manager_, IsOnline()).WillOnce(Return(true));
@@ -819,7 +814,7 @@
}
TEST_F(OpenVPNDriverTest, InitManagementChannelOptionsOffline) {
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_CALL(*management_server_, Start(&dispatcher_, GetSockets(), &options))
.WillOnce(Return(true));
EXPECT_CALL(manager_, IsOnline()).WillOnce(Return(false));
@@ -830,12 +825,12 @@
}
TEST_F(OpenVPNDriverTest, InitLoggingOptions) {
- vector<string> options;
+ vector<vector<string>> options;
bool vpn_logging = SLOG_IS_ON(VPN, 0);
ScopeLogger::GetInstance()->EnableScopesByName("-vpn");
driver_->InitLoggingOptions(&options);
ASSERT_EQ(1, options.size());
- EXPECT_EQ("--syslog", options[0]);
+ EXPECT_EQ(vector<string> { "--syslog" }, options[0]);
ScopeLogger::GetInstance()->EnableScopesByName("+vpn");
options.clear();
driver_->InitLoggingOptions(&options);
@@ -856,7 +851,7 @@
}
TEST_F(OpenVPNDriverTest, AppendValueOption) {
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_FALSE(
driver_->AppendValueOption("OpenVPN.UnknownProperty", kOption, &options));
EXPECT_TRUE(options.empty());
@@ -869,15 +864,15 @@
SetArg(kProperty2, kValue2);
EXPECT_TRUE(driver_->AppendValueOption(kProperty, kOption, &options));
EXPECT_TRUE(driver_->AppendValueOption(kProperty2, kOption2, &options));
- EXPECT_EQ(4, options.size());
- EXPECT_EQ(kOption, options[0]);
- EXPECT_EQ(kValue, options[1]);
- EXPECT_EQ(kOption2, options[2]);
- EXPECT_EQ(kValue2, options[3]);
+ EXPECT_EQ(2, options.size());
+ vector<string> expected_value { kOption, kValue };
+ EXPECT_EQ(expected_value, options[0]);
+ vector<string> expected_value2 { kOption2, kValue2 };
+ EXPECT_EQ(expected_value2, options[1]);
}
TEST_F(OpenVPNDriverTest, AppendFlag) {
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_FALSE(
driver_->AppendFlag("OpenVPN.UnknownProperty", kOption, &options));
EXPECT_TRUE(options.empty());
@@ -887,8 +882,8 @@
EXPECT_TRUE(driver_->AppendFlag(kProperty, kOption, &options));
EXPECT_TRUE(driver_->AppendFlag(kProperty2, kOption2, &options));
EXPECT_EQ(2, options.size());
- EXPECT_EQ(kOption, options[0]);
- EXPECT_EQ(kOption2, options[1]);
+ EXPECT_EQ(vector<string> { kOption }, options[0]);
+ EXPECT_EQ(vector<string> { kOption2 }, options[1]);
}
TEST_F(OpenVPNDriverTest, ClaimInterface) {
diff --git a/openvpn_management_server.cc b/openvpn_management_server.cc
index c170f21..507423e 100644
--- a/openvpn_management_server.cc
+++ b/openvpn_management_server.cc
@@ -55,7 +55,7 @@
bool OpenVPNManagementServer::Start(EventDispatcher *dispatcher,
Sockets *sockets,
- vector<string> *options) {
+ vector<vector<string>> *options) {
SLOG(VPN, 2) << __func__;
if (IsStarted()) {
return true;
@@ -92,20 +92,18 @@
dispatcher_ = dispatcher;
// Append openvpn management API options.
- options->push_back("--management");
- options->push_back(inet_ntoa(addr.sin_addr));
- options->push_back(IntToString(ntohs(addr.sin_port)));
- options->push_back("--management-client");
-
- options->push_back("--management-hold");
+ driver_->AppendOption("--management", inet_ntoa(addr.sin_addr),
+ IntToString(ntohs(addr.sin_port)), options);
+ driver_->AppendOption("--management-client", options);
+ driver_->AppendOption("--management-hold", options);
hold_release_ = false;
hold_waiting_ = false;
- options->push_back("--management-query-passwords");
+ driver_->AppendOption("--management-query-passwords", options);
if (driver_->AppendValueOption(flimflam::kOpenVPNStaticChallengeProperty,
"--static-challenge",
options)) {
- options->push_back("1"); // Force echo.
+ options->back().push_back("1"); // Force echo.
}
return true;
}
diff --git a/openvpn_management_server.h b/openvpn_management_server.h
index b61ec19..3ac15d8 100644
--- a/openvpn_management_server.h
+++ b/openvpn_management_server.h
@@ -34,7 +34,7 @@
// interface openvpn options to |options|.
virtual bool Start(EventDispatcher *dispatcher,
Sockets *sockets,
- std::vector<std::string> *options);
+ std::vector<std::vector<std::string>> *options);
virtual void Stop();
diff --git a/openvpn_management_server_unittest.cc b/openvpn_management_server_unittest.cc
index 5aae991..124e405 100644
--- a/openvpn_management_server_unittest.cc
+++ b/openvpn_management_server_unittest.cc
@@ -176,6 +176,9 @@
}
TEST_F(OpenVPNManagementServerTest, Start) {
+ const string kStaticChallenge = "static-challenge";
+ driver_.args()->SetString(
+ flimflam::kOpenVPNStaticChallengeProperty, kStaticChallenge);
const int kSocket = 123;
EXPECT_CALL(sockets_, Socket(AF_INET, SOCK_STREAM, IPPROTO_TCP))
.WillOnce(Return(kSocket));
@@ -185,13 +188,20 @@
EXPECT_CALL(dispatcher_,
CreateReadyHandler(kSocket, IOHandler::kModeInput, _))
.WillOnce(ReturnNew<IOHandler>());
- vector<string> options;
+ vector<vector<string>> options;
EXPECT_TRUE(server_.Start(&dispatcher_, &sockets_, &options));
EXPECT_EQ(&sockets_, server_.sockets_);
EXPECT_EQ(kSocket, server_.socket_);
EXPECT_TRUE(server_.ready_handler_.get());
EXPECT_EQ(&dispatcher_, server_.dispatcher_);
- EXPECT_FALSE(options.empty());
+ vector<vector<string>> expected_options {
+ { "--management", "127.0.0.1", "0" },
+ { "--management-client" },
+ { "--management-hold" },
+ { "--management-query-passwords" },
+ { "--static-challenge", kStaticChallenge, "1" }
+ };
+ EXPECT_EQ(expected_options, options);
}
TEST_F(OpenVPNManagementServerTest, Stop) {