Launch dhcpcd using Minijail.
dhcpcd runs as root and listens on the network. Launch it using Minijail
so that we can run it as a regular user, mitigating the risk of an eventual
compromise.
Add a mock Minijail wrapper for unittesting.
BUG=chromium-os:28336
TEST=dhcp_config_unittest
TEST=network_netperf2
TEST=Manual connection to ethernet, GoogleGuest, Google-A.
CQ-DEPEND=I243e02c82f70c6a3469ca712e539ec9fb6e3e4d4
Change-Id: I14c4e843eba478ed39b10fa4fcb0e25eb3186c1a
Reviewed-on: https://gerrit.chromium.org/gerrit/20414
Commit-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
diff --git a/Makefile b/Makefile
index 6e16a83..7821d3b 100644
--- a/Makefile
+++ b/Makefile
@@ -17,7 +17,7 @@
LIBDIR = /usr/lib
SCRIPTDIR = $(LIBDIR)/flimflam/scripts
CPPFLAGS += -DSCRIPTDIR=\"$(SCRIPTDIR)\"
-BASE_LIBS = -lcares -lmobile-provider -lmetrics
+BASE_LIBS = -lcares -lmobile-provider -lmetrics -lminijail
BASE_INCLUDE_DIRS = -iquote.. -iquote $(BUILDDIR)
BASE_LIB_DIRS =
@@ -152,6 +152,7 @@
manager.o \
manager_dbus_adaptor.o \
metrics.o \
+ minijail.o \
mm1_modem_modem3gpp_proxy.o \
mm1_modem_modemcdma_proxy.o \
mm1_modem_proxy.o \
@@ -278,6 +279,7 @@
mock_log_unittest.o \
mock_manager.o \
mock_metrics.o \
+ mock_minijail.o \
mock_mm1_modem_modemcdma_proxy.o \
mock_mm1_modem_modem3gpp_proxy.o \
mock_mm1_modem_proxy.o \
diff --git a/device_unittest.cc b/device_unittest.cc
index fb24466..cb769ad 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -25,6 +25,8 @@
#include "shill/mock_connection.h"
#include "shill/mock_device.h"
#include "shill/mock_device_info.h"
+#include "shill/mock_dhcp_config.h"
+#include "shill/mock_dhcp_provider.h"
#include "shill/mock_glib.h"
#include "shill/mock_ipconfig.h"
#include "shill/mock_manager.h"
@@ -190,12 +192,20 @@
TEST_F(DeviceTest, AcquireIPConfig) {
device_->ipconfig_ = new IPConfig(control_interface(), "randomname");
- EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
+ scoped_ptr<MockDHCPProvider> dhcp_provider(new MockDHCPProvider());
+ device_->dhcp_provider_ = dhcp_provider.get();
+ scoped_refptr<MockDHCPConfig> dhcp_config(new MockDHCPConfig(
+ control_interface(),
+ kDeviceName));
+ EXPECT_CALL(*dhcp_provider, CreateConfig(_, _, _, _))
+ .WillOnce(Return(dhcp_config));
+ EXPECT_CALL(*dhcp_config, RequestIP())
.WillOnce(Return(false));
EXPECT_FALSE(device_->AcquireIPConfig());
ASSERT_TRUE(device_->ipconfig_.get());
EXPECT_EQ(kDeviceName, device_->ipconfig_->device_name());
EXPECT_FALSE(device_->ipconfig_->update_callback_.is_null());
+ device_->dhcp_provider_ = NULL;
}
TEST_F(DeviceTest, Load) {
diff --git a/dhcp_config.cc b/dhcp_config.cc
index e1f610d..ab58d1a 100644
--- a/dhcp_config.cc
+++ b/dhcp_config.cc
@@ -17,6 +17,7 @@
#include "shill/event_dispatcher.h"
#include "shill/glib.h"
#include "shill/ip_address.h"
+#include "shill/minijail.h"
#include "shill/proxy_factory.h"
#include "shill/scope_logger.h"
@@ -42,6 +43,7 @@
const char DHCPConfig::kDHCPCDPathFormatPID[] =
"var/run/dhcpcd/dhcpcd-%s.pid";
const int DHCPConfig::kDHCPTimeoutSeconds = 30;
+const char DHCPConfig::kDHCPCDUser[] = "dhcp";
const int DHCPConfig::kMinMTU = 576;
const char DHCPConfig::kReasonBound[] = "BOUND";
const char DHCPConfig::kReasonFail[] = "FAIL";
@@ -72,7 +74,8 @@
root_("/"),
weak_ptr_factory_(this),
dispatcher_(dispatcher),
- glib_(glib) {
+ glib_(glib),
+ minijail_(Minijail::GetInstance()) {
SLOG(DHCP, 2) << __func__ << ": " << device_name;
if (lease_file_suffix_.empty()) {
lease_file_suffix_ = device_name;
@@ -175,18 +178,18 @@
}
args.push_back(const_cast<char *>(interface_arg.c_str()));
args.push_back(NULL);
- char *envp[1] = { NULL };
+
+ struct minijail *jail = minijail_->New();
+ minijail_->DropRoot(jail, kDHCPCDUser);
+ minijail_->UseCapabilities(jail,
+ CAP_TO_MASK(CAP_NET_BIND_SERVICE) |
+ CAP_TO_MASK(CAP_NET_BROADCAST) |
+ CAP_TO_MASK(CAP_NET_ADMIN) |
+ CAP_TO_MASK(CAP_NET_RAW));
CHECK(!pid_);
- if (!glib_->SpawnAsync(NULL,
- args.data(),
- envp,
- G_SPAWN_DO_NOT_REAP_CHILD,
- NULL,
- NULL,
- &pid_,
- NULL)) {
- LOG(ERROR) << "Unable to spawn " << kDHCPCDPath;
+ if (!minijail_->RunAndDestroy(jail, args, &pid_)) {
+ LOG(ERROR) << "Unable to spawn " << kDHCPCDPath << " in a jail.";
return false;
}
LOG(INFO) << "Spawned " << kDHCPCDPath << " with pid: " << pid_;
@@ -320,10 +323,7 @@
glib_->SourceRemove(child_watch_tag_);
child_watch_tag_ = 0;
}
- if (pid_) {
- glib_->SpawnClosePID(pid_);
- pid_ = 0;
- }
+ pid_ = 0;
proxy_.reset();
if (lease_file_suffix_ == device_name()) {
// If the lease file suffix was left as default, clean it up at exit.
diff --git a/dhcp_config.h b/dhcp_config.h
index e66ee98..c850a9f 100644
--- a/dhcp_config.h
+++ b/dhcp_config.h
@@ -14,6 +14,7 @@
#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/ipconfig.h"
+#include "shill/minijail.h"
namespace shill {
@@ -106,6 +107,7 @@
static const char kDHCPCDPathFormatLease[];
static const char kDHCPCDPathFormatPID[];
static const int kDHCPTimeoutSeconds;
+ static const char kDHCPCDUser[];
static const char kReasonBound[];
static const char kReasonFail[];
@@ -192,6 +194,8 @@
EventDispatcher *dispatcher_;
GLib *glib_;
+ Minijail *minijail_;
+
DISALLOW_COPY_AND_ASSIGN(DHCPConfig);
};
diff --git a/dhcp_config_unittest.cc b/dhcp_config_unittest.cc
index 702f67c..434b57c 100644
--- a/dhcp_config_unittest.cc
+++ b/dhcp_config_unittest.cc
@@ -17,6 +17,7 @@
#include "shill/mock_control.h"
#include "shill/mock_dhcp_proxy.h"
#include "shill/mock_glib.h"
+#include "shill/mock_minijail.h"
#include "shill/property_store_unittest.h"
#include "shill/proxy_factory.h"
@@ -43,6 +44,7 @@
DHCPConfigTest()
: proxy_(new MockDHCPProxy()),
proxy_factory_(this),
+ minijail_(new MockMinijail()),
config_(new DHCPConfig(&control_,
dispatcher(),
DHCPProvider::GetInstance(),
@@ -54,12 +56,17 @@
virtual void SetUp() {
config_->proxy_factory_ = &proxy_factory_;
+ config_->minijail_ = minijail_.get();
}
virtual void TearDown() {
config_->proxy_factory_ = NULL;
+ config_->minijail_ = NULL;
}
+ DHCPConfigRefPtr CreateMockMinijailConfig(const string &hostname,
+ const string &lease_suffix,
+ bool arp_gateway);
DHCPConfigRefPtr CreateRunningConfig(const string &hostname,
const string &lease_suffix,
bool arp_gateway);
@@ -88,12 +95,31 @@
scoped_ptr<MockDHCPProxy> proxy_;
TestProxyFactory proxy_factory_;
MockControl control_;
+ scoped_ptr<MockMinijail> minijail_;
DHCPConfigRefPtr config_;
};
const int DHCPConfigTest::kPID = 123456;
const unsigned int DHCPConfigTest::kTag = 77;
+DHCPConfigRefPtr DHCPConfigTest::CreateMockMinijailConfig(
+ const string &hostname,
+ const string &lease_suffix,
+ bool arp_gateway) {
+ DHCPConfigRefPtr config(new DHCPConfig(&control_,
+ dispatcher(),
+ DHCPProvider::GetInstance(),
+ kDeviceName,
+ hostname,
+ lease_suffix,
+ arp_gateway,
+ glib()));
+ config->minijail_ = minijail_.get();
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _)).WillOnce(Return(false));
+
+ return config;
+}
+
DHCPConfigRefPtr DHCPConfigTest::CreateRunningConfig(const string &hostname,
const string &lease_suffix,
bool arp_gateway) {
@@ -105,8 +131,9 @@
lease_suffix,
arp_gateway,
glib()));
- EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
- .WillOnce(DoAll(SetArgumentPointee<6>(kPID), Return(true)));
+ config->minijail_ = minijail_.get();
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(kPID), Return(true)));
EXPECT_CALL(*glib(), ChildWatchAdd(kPID, _, _)).WillOnce(Return(kTag));
EXPECT_TRUE(config->Start());
EXPECT_EQ(kPID, config->pid_);
@@ -132,7 +159,6 @@
void DHCPConfigTest::StopRunningConfigAndExpect(DHCPConfigRefPtr config,
bool lease_file_exists) {
- EXPECT_CALL(*glib(), SpawnClosePID(kPID)).Times(1);
DHCPConfig::ChildWatchCallback(kPID, 0, config.get());
EXPECT_EQ(NULL, DHCPProvider::GetInstance()->GetConfig(kPID).get());
@@ -210,8 +236,7 @@
}
TEST_F(DHCPConfigTest, StartFail) {
- EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
- .WillOnce(Return(false));
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _)).WillOnce(Return(false));
EXPECT_CALL(*glib(), ChildWatchAdd(_, _, _)).Times(0);
EXPECT_FALSE(config_->Start());
EXPECT_EQ(0, config_->pid_);
@@ -244,57 +269,28 @@
}
TEST_F(DHCPConfigTest, StartWithHostname) {
- EXPECT_CALL(*glib(),
- SpawnAsync(_, IsDHCPCDArgs(true, true, true), _, _, _, _, _, _))
- .WillOnce(Return(false));
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _)).WillOnce(Return(false));
EXPECT_FALSE(config_->Start());
}
TEST_F(DHCPConfigTest, StartWithoutHostname) {
- DHCPConfigRefPtr config(new DHCPConfig(&control_,
- dispatcher(),
- DHCPProvider::GetInstance(),
- kDeviceName,
- "",
- kLeaseFileSuffix,
- kArpGateway,
- glib()));
-
- EXPECT_CALL(*glib(),
- SpawnAsync(_, IsDHCPCDArgs(false, true, true), _, _, _, _, _, _))
- .WillOnce(Return(false));
+ DHCPConfigRefPtr config = CreateMockMinijailConfig("",
+ kLeaseFileSuffix,
+ kArpGateway);
EXPECT_FALSE(config->Start());
}
TEST_F(DHCPConfigTest, StartWithoutArpGateway) {
- DHCPConfigRefPtr config(new DHCPConfig(&control_,
- dispatcher(),
- DHCPProvider::GetInstance(),
- kDeviceName,
- kHostName,
- kLeaseFileSuffix,
- false,
- glib()));
-
- EXPECT_CALL(*glib(),
- SpawnAsync(_, IsDHCPCDArgs(true, false, true), _, _, _, _, _, _))
- .WillOnce(Return(false));
+ DHCPConfigRefPtr config = CreateMockMinijailConfig(kHostName,
+ kLeaseFileSuffix,
+ false);
EXPECT_FALSE(config->Start());
}
TEST_F(DHCPConfigTest, StartWithoutLeaseSuffix) {
- DHCPConfigRefPtr config(new DHCPConfig(&control_,
- dispatcher(),
- DHCPProvider::GetInstance(),
- kDeviceName,
- kHostName,
- kDeviceName,
- kArpGateway,
- glib()));
-
- EXPECT_CALL(*glib(),
- SpawnAsync(_, IsDHCPCDArgs(true, true, false), _, _, _, _, _, _))
- .WillOnce(Return(false));
+ DHCPConfigRefPtr config = CreateMockMinijailConfig(kHostName,
+ kDeviceName,
+ kArpGateway);
EXPECT_FALSE(config->Start());
}
@@ -433,9 +429,8 @@
config_->child_watch_tag_ = kTag1;
DHCPProvider::GetInstance()->BindPID(kPID1, config_);
EXPECT_CALL(*glib(), SourceRemove(kTag1)).WillOnce(Return(true));
- EXPECT_CALL(*glib(), SpawnClosePID(kPID1)).Times(1);
- EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
- .WillOnce(DoAll(SetArgumentPointee<6>(kPID2), Return(true)));
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _)).WillOnce(
+ DoAll(SetArgumentPointee<2>(kPID2), Return(true)));
EXPECT_CALL(*glib(), ChildWatchAdd(kPID2, _, _)).WillOnce(Return(kTag2));
EXPECT_TRUE(config_->Restart());
EXPECT_EQ(kPID2, config_->pid_);
@@ -450,9 +445,8 @@
const int kPID = 777;
const unsigned int kTag = 66;
EXPECT_CALL(*glib(), SourceRemove(_)).Times(0);
- EXPECT_CALL(*glib(), SpawnClosePID(_)).Times(0);
- EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
- .WillOnce(DoAll(SetArgumentPointee<6>(kPID), Return(true)));
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _)).WillOnce(
+ DoAll(SetArgumentPointee<2>(kPID), Return(true)));
EXPECT_CALL(*glib(), ChildWatchAdd(kPID, _, _)).WillOnce(Return(kTag));
EXPECT_TRUE(config_->Restart());
EXPECT_EQ(kPID, config_->pid_);
@@ -481,9 +475,7 @@
Bind(&UpdateCallbackTest::Callback, Unretained(&callback_test)));
config_->lease_acquisition_timeout_seconds_ = 0;
config_->proxy_.reset(proxy_.release());
- EXPECT_CALL(*glib(),
- SpawnAsync(_, IsDHCPCDArgs(true, true, true), _, _, _, _, _, _))
- .WillOnce(Return(true));
+ EXPECT_CALL(*minijail_, RunAndDestroy(_, _, _)).WillOnce(Return(true));
config_->Start();
config_->dispatcher_->DispatchPendingEvents();
EXPECT_TRUE(callback_test.called());
@@ -492,10 +484,8 @@
TEST_F(DHCPConfigTest, Stop) {
// Ensure no crashes.
const int kPID = 1 << 17; // Ensure unknown positive PID.
- config_->Stop();
config_->pid_ = kPID;
config_->Stop();
- EXPECT_CALL(*glib(), SpawnClosePID(kPID)).Times(1); // Invoked by destructor.
EXPECT_TRUE(config_->lease_acquisition_timeout_callback_.IsCancelled());
}
diff --git a/minijail.cc b/minijail.cc
new file mode 100644
index 0000000..3cb7a4d
--- /dev/null
+++ b/minijail.cc
@@ -0,0 +1,53 @@
+// 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/minijail.h"
+
+using std::vector;
+
+namespace shill {
+
+static base::LazyInstance<Minijail> g_minijail = LAZY_INSTANCE_INITIALIZER;
+
+Minijail::Minijail() {}
+
+Minijail::~Minijail() {}
+
+// static
+Minijail *Minijail::GetInstance() {
+ return g_minijail.Pointer();
+}
+
+struct minijail *Minijail::New() {
+ return minijail_new();
+}
+
+void Minijail::Destroy(struct minijail *jail) {
+ minijail_destroy(jail);
+}
+
+bool Minijail::DropRoot(struct minijail *jail, const char *user) {
+ // |user| is copied so the only reason either of these calls can fail
+ // is ENOMEM.
+ return !minijail_change_user(jail, user) &&
+ !minijail_change_group(jail, user);
+}
+
+void Minijail::UseCapabilities(struct minijail *jail, uint64_t capmask) {
+ minijail_use_caps(jail, capmask);
+}
+
+bool Minijail::Run(struct minijail *jail,
+ vector<char *> args, pid_t *pid) {
+ return minijail_run_pid(jail, args[0], args.data(), pid) == 0;
+}
+
+bool Minijail::RunAndDestroy(struct minijail *jail,
+ vector<char *> args, pid_t *pid) {
+ bool res = Run(jail, args, pid);
+ Destroy(jail);
+ return res;
+}
+
+} // namespace shill
diff --git a/minijail.h b/minijail.h
new file mode 100644
index 0000000..42f4580
--- /dev/null
+++ b/minijail.h
@@ -0,0 +1,56 @@
+// 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_MINIJAIL_H_
+#define SHILL_MINIJAIL_H_
+
+#include <vector>
+
+extern "C" {
+#include <linux/capability.h>
+}
+
+#include <base/lazy_instance.h>
+#include <chromeos/libminijail.h>
+
+namespace shill {
+
+// A Minijail abstraction allowing Minijail mocking in tests.
+class Minijail {
+ public:
+ virtual ~Minijail();
+
+ // This is a singleton -- use Minijail::GetInstance()->Foo()
+ static Minijail *GetInstance();
+
+ // minijail_new
+ virtual struct minijail *New();
+ // minijail_destroy
+ virtual void Destroy(struct minijail *jail);
+
+ // minijail_change_user/minijail_change_group
+ virtual bool DropRoot(struct minijail *jail, const char *user);
+ // minijail_use_caps
+ virtual void UseCapabilities(struct minijail *jail, uint64_t capmask);
+
+ // minijail_run_pid
+ virtual bool Run(struct minijail *jail, std::vector<char *> args, pid_t *pid);
+
+ // Run() and Destroy()
+ virtual bool RunAndDestroy(struct minijail *jail,
+ std::vector<char *> args,
+ pid_t *pid);
+
+ protected:
+ Minijail();
+
+ private:
+ friend struct base::DefaultLazyInstanceTraits<Minijail>;
+
+ DISALLOW_COPY_AND_ASSIGN(Minijail);
+};
+
+} // namespace shill
+
+#endif // SHILL_MINIJAIL_H_
diff --git a/mock_minijail.cc b/mock_minijail.cc
new file mode 100644
index 0000000..890a9c4
--- /dev/null
+++ b/mock_minijail.cc
@@ -0,0 +1,13 @@
+// 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_minijail.h"
+
+namespace shill {
+
+MockMinijail::MockMinijail() {}
+
+MockMinijail::~MockMinijail() {}
+
+} // namespace shill
diff --git a/mock_minijail.h b/mock_minijail.h
new file mode 100644
index 0000000..fe4169c
--- /dev/null
+++ b/mock_minijail.h
@@ -0,0 +1,36 @@
+// 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_MINIJAIL_H_
+#define SHILL_MOCK_MINIJAIL_H_
+
+#include <base/basictypes.h>
+#include <gmock/gmock.h>
+
+#include "shill/minijail.h"
+
+namespace shill {
+
+class MockMinijail : public Minijail {
+ public:
+ MockMinijail();
+ virtual ~MockMinijail();
+
+ MOCK_METHOD0(New, struct minijail *());
+ MOCK_METHOD1(Destroy, void (struct minijail *));
+
+ MOCK_METHOD2(DropRoot, bool(struct minijail *jail, const char *user));
+ MOCK_METHOD2(UseCapabilities, void(struct minijail *jail, uint64_t capmask));
+ MOCK_METHOD3(Run, bool(struct minijail *jail,
+ std::vector<char *> args, pid_t *pid));
+ MOCK_METHOD3(RunAndDestroy, bool(struct minijail *jail,
+ std::vector<char *> args, pid_t *pid));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockMinijail);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_MINIJAIL_H_