shill: vpn: Spawn the OpenVPN process when tunnel index becomes available.
BUG=chromium-os:27373
TEST=unit tests
Change-Id: Ie946e4275beff4d5ac176413cc219dbe52356b28
Reviewed-on: https://gerrit.chromium.org/gerrit/17407
Commit-Ready: Darin Petkov <petkov@chromium.org>
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>
diff --git a/Makefile b/Makefile
index e3448f4..2714839 100644
--- a/Makefile
+++ b/Makefile
@@ -262,6 +262,7 @@
mock_supplicant_interface_proxy.o \
mock_supplicant_process_proxy.o \
mock_time.o \
+ mock_vpn.o \
mock_vpn_driver.o \
mock_wifi.o \
mock_wifi_service.o \
diff --git a/device.h b/device.h
index 130eabe..c27681a 100644
--- a/device.h
+++ b/device.h
@@ -65,8 +65,6 @@
virtual void LinkEvent(unsigned flags, unsigned change);
- virtual void ConfigIP() {}
-
// The default implementation sets |error| to kNotSupported.
virtual void Scan(Error *error);
virtual void RegisterOnNetwork(const std::string &network_id,
diff --git a/glib.cc b/glib.cc
index 7717adc..a2e9e07 100644
--- a/glib.cc
+++ b/glib.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -197,6 +197,29 @@
error);
}
+gboolean GLib::SpawnAsyncWithPipesCWD(gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ gint *standard_input,
+ gint *standard_output,
+ gint *standard_error,
+ GError **error) {
+ return g_spawn_async_with_pipes(NULL,
+ argv,
+ envp,
+ flags,
+ child_setup,
+ user_data,
+ child_pid,
+ standard_input,
+ standard_output,
+ standard_error,
+ error);
+}
+
void GLib::SpawnClosePID(GPid pid) {
g_spawn_close_pid(pid);
}
diff --git a/glib.h b/glib.h
index a3d321b..6f75048 100644
--- a/glib.h
+++ b/glib.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -132,6 +132,17 @@
gpointer user_data,
GPid *child_pid,
GError **error);
+ // g_spawn_async_with_pipes(NULL, ...
+ virtual gboolean SpawnAsyncWithPipesCWD(gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ gint *standard_input,
+ gint *standard_output,
+ gint *standard_error,
+ GError **error);
// g_spawn_close_pid
virtual void SpawnClosePID(GPid pid);
// g_strfreev
diff --git a/manager.h b/manager.h
index faf055d..80a021e 100644
--- a/manager.h
+++ b/manager.h
@@ -139,6 +139,7 @@
VPNProvider *vpn_provider() { return &vpn_provider_; }
PropertyStore *mutable_store() { return &store_; }
virtual const PropertyStore &store() const { return store_; }
+ GLib *glib() const { return glib_; }
std::vector<DeviceRefPtr>::iterator devices_begin() {
return devices_.begin();
diff --git a/mock_glib.h b/mock_glib.h
index 2820b12..60e3ff5 100644
--- a/mock_glib.h
+++ b/mock_glib.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -104,6 +104,17 @@
gpointer user_data,
GPid *child_pid,
GError **error));
+ MOCK_METHOD10(SpawnAsyncWithPipesCWD,
+ gboolean(gchar **argv,
+ gchar **envp,
+ GSpawnFlags flags,
+ GSpawnChildSetupFunc child_setup,
+ gpointer user_data,
+ GPid *child_pid,
+ gint *standard_input,
+ gint *standard_output,
+ gint *standard_error,
+ GError **error));
MOCK_METHOD1(SpawnClosePID, void(GPid pid));
MOCK_METHOD1(Strfreev, void(gchar **str_array));
MOCK_METHOD0(TypeInit, void());
diff --git a/mock_vpn.cc b/mock_vpn.cc
new file mode 100644
index 0000000..74d538a
--- /dev/null
+++ b/mock_vpn.cc
@@ -0,0 +1,19 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "shill/mock_vpn.h"
+
+namespace shill {
+
+MockVPN::MockVPN(ControlInterface *control,
+ EventDispatcher *dispatcher,
+ Metrics *metrics,
+ Manager *manager,
+ const std::string &link_name,
+ int interface_index)
+ : VPN(control, dispatcher, metrics, manager, link_name, interface_index) {}
+
+MockVPN::~MockVPN() {}
+
+} // namespace shill
diff --git a/mock_vpn.h b/mock_vpn.h
new file mode 100644
index 0000000..37ab936
--- /dev/null
+++ b/mock_vpn.h
@@ -0,0 +1,32 @@
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef SHILL_MOCK_VPN_
+#define SHILL_MOCK_VPN_
+
+#include <gmock/gmock.h>
+
+#include "shill/vpn.h"
+
+namespace shill {
+
+class MockVPN : public VPN {
+ public:
+ MockVPN(ControlInterface *control,
+ EventDispatcher *dispatcher,
+ Metrics *metrics,
+ Manager *manager,
+ const std::string &link_name,
+ int interface_index);
+ virtual ~MockVPN();
+
+ MOCK_METHOD0(Stop, void());
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockVPN);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_VPN_
diff --git a/openvpn_driver.cc b/openvpn_driver.cc
index de28daf..1ee4e5f 100644
--- a/openvpn_driver.cc
+++ b/openvpn_driver.cc
@@ -24,6 +24,7 @@
namespace shill {
namespace {
+const char kOpenVPNPath[] = "/usr/sbin/openvpn";
const char kOpenVPNScript[] = "/usr/lib/flimflam/scripts/openvpn-script";
const char kOpenVPNForeignOptionPrefix[] = "foreign_option_";
const char kOpenVPNIfconfigBroadcast[] = "ifconfig_broadcast";
@@ -41,15 +42,95 @@
Metrics *metrics,
Manager *manager,
DeviceInfo *device_info,
+ GLib *glib,
const KeyValueStore &args)
: control_(control),
dispatcher_(dispatcher),
metrics_(metrics),
manager_(manager),
device_info_(device_info),
- args_(args) {}
+ glib_(glib),
+ args_(args),
+ pid_(0),
+ child_watch_tag_(0) {}
-OpenVPNDriver::~OpenVPNDriver() {}
+OpenVPNDriver::~OpenVPNDriver() {
+ Cleanup();
+}
+
+void OpenVPNDriver::Cleanup() {
+ if (child_watch_tag_) {
+ glib_->SourceRemove(child_watch_tag_);
+ child_watch_tag_ = 0;
+ CHECK(pid_);
+ kill(pid_, SIGTERM);
+ }
+ if (pid_) {
+ glib_->SpawnClosePID(pid_);
+ pid_ = 0;
+ }
+ rpc_task_.reset();
+ if (device_) {
+ int interface_index = device_->interface_index();
+ device_->Stop();
+ device_ = NULL;
+ device_info_->DeleteInterface(interface_index);
+ }
+ tunnel_interface_.clear();
+}
+
+bool OpenVPNDriver::SpawnOpenVPN() {
+ VLOG(2) << __func__ << "(" << tunnel_interface_ << ")";
+
+ vector<string> options;
+ Error error;
+ InitOptions(&options, &error);
+ if (error.IsFailure()) {
+ return false;
+ }
+ VLOG(2) << "OpenVPN process options: " << JoinString(options, ' ');
+
+ // TODO(petkov): This code needs to be abstracted away in a separate external
+ // process module (crosbug.com/27131).
+ 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()));
+ }
+ process_args.push_back(NULL);
+ char *envp[1] = { NULL };
+
+ CHECK(!pid_);
+ // Redirect all openvpn output to stderr.
+ int stderr_fd = fileno(stderr);
+ if (!glib_->SpawnAsyncWithPipesCWD(process_args.data(),
+ envp,
+ G_SPAWN_DO_NOT_REAP_CHILD,
+ NULL,
+ NULL,
+ &pid_,
+ NULL,
+ &stderr_fd,
+ &stderr_fd,
+ NULL)) {
+ LOG(ERROR) << "Unable to spawn: " << kOpenVPNPath;
+ return false;
+ }
+ CHECK(!child_watch_tag_);
+ child_watch_tag_ = glib_->ChildWatchAdd(pid_, OnOpenVPNDied, this);
+ return true;
+}
+
+// static
+void OpenVPNDriver::OnOpenVPNDied(GPid pid, gint status, gpointer data) {
+ VLOG(2) << __func__ << "(" << pid << ", " << status << ")";
+ OpenVPNDriver *me = reinterpret_cast<OpenVPNDriver *>(data);
+ me->child_watch_tag_ = 0;
+ CHECK_EQ(pid, me->pid_);
+ me->Cleanup();
+ // TODO(petkov): Figure if we need to restart the connection.
+}
bool OpenVPNDriver::ClaimInterface(const string &link_name,
int interface_index) {
@@ -62,23 +143,23 @@
CHECK(!device_);
device_ = new VPN(control_, dispatcher_, metrics_, manager_,
link_name, interface_index);
-
- // TODO(petkov): Allocate rpc_task_.
-
- // TODO(petkov): Initialize options and spawn openvpn.
+ device_->Start();
+ rpc_task_.reset(new RPCTask(control_, this));
+ if (!SpawnOpenVPN()) {
+ Cleanup();
+ }
return true;
}
-bool OpenVPNDriver::Notify(const string &reason,
+void OpenVPNDriver::Notify(const string &reason,
const map<string, string> &dict) {
VLOG(2) << __func__ << "(" << reason << ")";
if (reason != "up") {
- return false;
+ return;
}
IPConfig::Properties properties;
ParseIPConfiguration(dict, &properties);
// TODO(petkov): Apply the properties to a VPNDevice's IPConfig.
- return true;
}
// static
diff --git a/openvpn_driver.h b/openvpn_driver.h
index 6201e01..256ef62 100644
--- a/openvpn_driver.h
+++ b/openvpn_driver.h
@@ -12,9 +12,11 @@
#include <base/memory/scoped_ptr.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
+#include "shill/glib.h"
#include "shill/ipconfig.h"
#include "shill/key_value_store.h"
#include "shill/refptr_types.h"
+#include "shill/rpc_task.h"
#include "shill/vpn_driver.h"
namespace shill {
@@ -25,22 +27,19 @@
class EventDispatcher;
class Manager;
class Metrics;
-class RPCTask;
-class DeviceStub;
-class OpenVPNDriver : public VPNDriver {
+class OpenVPNDriver : public VPNDriver,
+ public RPCTaskDelegate {
public:
OpenVPNDriver(ControlInterface *control,
EventDispatcher *dispatcher,
Metrics *metrics,
Manager *manager,
DeviceInfo *device_info,
+ GLib *glib,
const KeyValueStore &args);
virtual ~OpenVPNDriver();
- bool Notify(const std::string &reason,
- const std::map<std::string, std::string> &dict);
-
// Inherited from VPNDriver. |Connect| initiates the VPN connection by
// creating a tunnel device. When the device index becomes available, this
// instance is notified through |ClaimInterface| and resumes the connection
@@ -56,15 +55,19 @@
FRIEND_TEST(OpenVPNDriverTest, AppendFlag);
FRIEND_TEST(OpenVPNDriverTest, AppendValueOption);
FRIEND_TEST(OpenVPNDriverTest, ClaimInterface);
+ FRIEND_TEST(OpenVPNDriverTest, Cleanup);
FRIEND_TEST(OpenVPNDriverTest, Connect);
FRIEND_TEST(OpenVPNDriverTest, GetRouteOptionEntry);
FRIEND_TEST(OpenVPNDriverTest, InitOptions);
FRIEND_TEST(OpenVPNDriverTest, InitOptionsNoHost);
+ FRIEND_TEST(OpenVPNDriverTest, Notify);
+ FRIEND_TEST(OpenVPNDriverTest, OnOpenVPNDied);
FRIEND_TEST(OpenVPNDriverTest, ParseForeignOption);
FRIEND_TEST(OpenVPNDriverTest, ParseForeignOptions);
FRIEND_TEST(OpenVPNDriverTest, ParseIPConfiguration);
FRIEND_TEST(OpenVPNDriverTest, ParseRouteOption);
FRIEND_TEST(OpenVPNDriverTest, SetRoutes);
+ FRIEND_TEST(OpenVPNDriverTest, SpawnOpenVPN);
// The map is a sorted container that allows us to iterate through the options
// in order.
@@ -96,16 +99,34 @@
const std::string &option,
std::vector<std::string> *options);
+ bool SpawnOpenVPN();
+ void Cleanup();
+
+ // Called when the openpvn process exits.
+ static void OnOpenVPNDied(GPid pid, gint status, gpointer data);
+
+ // Implements RPCTaskDelegate.
+ virtual void Notify(const std::string &reason,
+ const std::map<std::string, std::string> &dict);
+
ControlInterface *control_;
EventDispatcher *dispatcher_;
Metrics *metrics_;
Manager *manager_;
DeviceInfo *device_info_;
+ GLib *glib_;
KeyValueStore args_;
scoped_ptr<RPCTask> rpc_task_;
std::string tunnel_interface_;
VPNRefPtr device_;
+ // The PID of the spawned openvpn process. May be 0 if no process has been
+ // spawned yet or the process has died.
+ int pid_;
+
+ // Child exit watch callback source tag.
+ unsigned int child_watch_tag_;
+
DISALLOW_COPY_AND_ASSIGN(OpenVPNDriver);
};
diff --git a/openvpn_driver_unittest.cc b/openvpn_driver_unittest.cc
index e4fc075..4c73b11 100644
--- a/openvpn_driver_unittest.cc
+++ b/openvpn_driver_unittest.cc
@@ -16,6 +16,7 @@
#include "shill/mock_glib.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
+#include "shill/mock_vpn.h"
#include "shill/nice_mock_control.h"
#include "shill/rpc_task.h"
#include "shill/vpn.h"
@@ -37,12 +38,15 @@
: device_info_(&control_, NULL, NULL, NULL),
manager_(&control_, &dispatcher_, &metrics_, &glib_),
driver_(&control_, &dispatcher_, &metrics_, &manager_, &device_info_,
- args_) {}
+ &glib_, args_) {}
virtual ~OpenVPNDriverTest() {}
- // Inherited from RPCTaskDelegate.
- virtual void Notify(const string &reason, const map<string, string> &dict);
+ virtual void TearDown() {
+ driver_.child_watch_tag_ = 0;
+ driver_.pid_ = 0;
+ driver_.device_ = NULL;
+ }
protected:
static const char kOption[];
@@ -66,6 +70,8 @@
void ExpectInFlags(const vector<string> &options, const string &flag,
const string &value);
+ // Inherited from RPCTaskDelegate.
+ virtual void Notify(const string &reason, const map<string, string> &dict);
NiceMockControl control_;
MockDeviceInfo device_info_;
@@ -75,6 +81,7 @@
MockManager manager_;
KeyValueStore args_;
OpenVPNDriver driver_;
+ scoped_refptr<MockVPN> mock_vpn_;
};
const char OpenVPNDriverTest::kOption[] = "--openvpn-option";
@@ -101,11 +108,11 @@
EXPECT_TRUE(it != options.end());
if (it != options.end())
- return; // Don't crash below.
+ return; // Don't crash below.
it++;
EXPECT_TRUE(it != options.end());
if (it != options.end())
- return; // Don't crash below.
+ return; // Don't crash below.
EXPECT_EQ(value, *it);
}
@@ -131,8 +138,8 @@
TEST_F(OpenVPNDriverTest, Notify) {
map<string, string> dict;
- EXPECT_FALSE(driver_.Notify("down", dict));
- EXPECT_TRUE(driver_.Notify("up", dict));
+ driver_.Notify("up", dict);
+ // TODO(petkov): Expect an IPConfig update.
}
TEST_F(OpenVPNDriverTest, GetRouteOptionEntry) {
@@ -339,9 +346,70 @@
EXPECT_FALSE(driver_.ClaimInterface(kInterfaceName + "XXX", kInterfaceIndex));
EXPECT_FALSE(driver_.device_);
+ static const char kHost[] = "192.168.2.254";
+ args_.SetString(flimflam::kProviderHostProperty, kHost);
+ SetArgs();
+ EXPECT_CALL(glib_, SpawnAsyncWithPipesCWD(_, _, _, _, _, _, _, _, _, _))
+ .WillOnce(Return(true));
+ EXPECT_CALL(glib_, ChildWatchAdd(_, _, _)).WillOnce(Return(1));
EXPECT_TRUE(driver_.ClaimInterface(kInterfaceName, kInterfaceIndex));
ASSERT_TRUE(driver_.device_);
EXPECT_EQ(kInterfaceIndex, driver_.device_->interface_index());
}
+TEST_F(OpenVPNDriverTest, Cleanup) {
+ const unsigned int kTag = 123;
+ const int kPID = 123456;
+ static const char kInterfaceName[] = "tun0";
+ const int kInterfaceIndex = 5;
+ driver_.child_watch_tag_ = kTag;
+ driver_.pid_ = kPID;
+ driver_.rpc_task_.reset(new RPCTask(&control_, this));
+ driver_.tunnel_interface_ = kInterfaceName;
+ mock_vpn_ = new MockVPN(&control_, &dispatcher_, &metrics_, &manager_,
+ kInterfaceName, kInterfaceIndex);
+ driver_.device_ = mock_vpn_;
+ EXPECT_CALL(glib_, SourceRemove(kTag));
+ EXPECT_CALL(glib_, SpawnClosePID(kPID));
+ EXPECT_CALL(*mock_vpn_, Stop());
+ driver_.Cleanup();
+ EXPECT_EQ(0, driver_.child_watch_tag_);
+ EXPECT_EQ(0, driver_.pid_);
+ EXPECT_FALSE(driver_.rpc_task_.get());
+ EXPECT_TRUE(driver_.tunnel_interface_.empty());
+ EXPECT_FALSE(driver_.device_);
+}
+
+TEST_F(OpenVPNDriverTest, SpawnOpenVPN) {
+ EXPECT_FALSE(driver_.SpawnOpenVPN());
+
+ static const char kHost[] = "192.168.2.254";
+ args_.SetString(flimflam::kProviderHostProperty, kHost);
+ SetArgs();
+ driver_.tunnel_interface_ = "tun0";
+ driver_.rpc_task_.reset(new RPCTask(&control_, this));
+
+ const int kPID = 234678;
+ EXPECT_CALL(glib_, SpawnAsyncWithPipesCWD(_, _, _, _, _, _, _, _, _, _))
+ .WillOnce(Return(false))
+ .WillOnce(DoAll(SetArgumentPointee<5>(kPID), Return(true)));
+ const int kTag = 6;
+ EXPECT_CALL(glib_, ChildWatchAdd(kPID, &driver_.OnOpenVPNDied, &driver_))
+ .WillOnce(Return(kTag));
+ EXPECT_FALSE(driver_.SpawnOpenVPN());
+ EXPECT_TRUE(driver_.SpawnOpenVPN());
+ EXPECT_EQ(kPID, driver_.pid_);
+ EXPECT_EQ(kTag, driver_.child_watch_tag_);
+}
+
+TEST_F(OpenVPNDriverTest, OnOpenVPNDied) {
+ const int kPID = 99999;
+ driver_.child_watch_tag_ = 333;
+ driver_.pid_ = kPID;
+ EXPECT_CALL(glib_, SpawnClosePID(kPID));
+ OpenVPNDriver::OnOpenVPNDied(kPID, 2, &driver_);
+ EXPECT_EQ(0, driver_.child_watch_tag_);
+ EXPECT_EQ(0, driver_.pid_);
+}
+
} // namespace shill
diff --git a/vpn_provider.cc b/vpn_provider.cc
index 24ac690..643b212 100644
--- a/vpn_provider.cc
+++ b/vpn_provider.cc
@@ -46,7 +46,7 @@
if (type == flimflam::kProviderOpenVpn) {
driver.reset(new OpenVPNDriver(
control_interface_, dispatcher_, metrics_, manager_,
- manager_->device_info(), args));
+ manager_->device_info(), manager_->glib(), args));
} else {
Error::PopulateAndLog(
error, Error::kNotSupported, "Unsupported VPN type: " + type);