patchpanel: Support open a new netns via ConnectNamespace
Currently the ConnectNamespace API exposed by patchpanel via d-bus only
supports passing in a pid of a process and doing "ConnectNamespace" for
the netns associated with this process. While in the tast tests,
sometimes we need to open a new netns directly, and execute processes in
the created netns.
For this usage, this patch modifies the ConnectNamespace API so that
patchpanel accepts passing a special pid (i.e., pid==-1) to indicates
the client wants a new netns, invokes `ip netns add` for this case, and
returns the name of the created netns.
BUG=b:185210339
TEST=unit_tests;
TEST=Used this API in test VPN tast test, verified it worked;
TEST=Checked playstore still worked.
Change-Id: I3bbfab89df24899127e6087b0c0533e2c96037dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2896672
Reviewed-by: Garrick Evans <garrick@chromium.org>
Reviewed-by: Hugo Benichi <hugobenichi@google.com>
Tested-by: Jie Jiang <jiejiang@chromium.org>
Commit-Queue: Jie Jiang <jiejiang@chromium.org>
diff --git a/patchpanel/arc_service.cc b/patchpanel/arc_service.cc
index 7abaf68..ecfd012 100644
--- a/patchpanel/arc_service.cc
+++ b/patchpanel/arc_service.cc
@@ -62,8 +62,8 @@
// Makes Android root the owner of /sys/class/ + |path|. |pid| is the ARC
// container pid.
void SetSysfsOwnerToAndroidRoot(uint32_t pid, const std::string& path) {
- ScopedNS ns(pid, ScopedNS::Type::Mount);
- if (!ns.IsValid()) {
+ auto ns = ScopedNS::EnterMountNS(pid);
+ if (!ns) {
LOG(ERROR) << "Cannot enter mnt namespace for pid " << pid;
return;
}
diff --git a/patchpanel/datapath.cc b/patchpanel/datapath.cc
index 7335a25..caf6d9f 100644
--- a/patchpanel/datapath.cc
+++ b/patchpanel/datapath.cc
@@ -340,7 +340,11 @@
// did not exit cleanly.
if (process_runner_->ip_netns_delete(netns_name, false /*log_failures*/) == 0)
LOG(INFO) << "Deleted left over network namespace name " << netns_name;
- return process_runner_->ip_netns_attach(netns_name, netns_pid) == 0;
+
+ if (netns_pid == ConnectedNamespace::kNewNetnsPid)
+ return process_runner_->ip_netns_add(netns_name) == 0;
+ else
+ return process_runner_->ip_netns_attach(netns_name, netns_pid) == 0;
}
bool Datapath::NetnsDeleteName(const std::string& netns_name) {
@@ -520,8 +524,8 @@
// Configure the remote veth in namespace |netns_name|.
{
- ScopedNS ns(netns_pid, ScopedNS::Type::Network);
- if (!ns.IsValid() && netns_pid != kTestPID) {
+ auto ns = ScopedNS::EnterNetworkNS(netns_name);
+ if (!ns && netns_pid != kTestPID) {
LOG(ERROR)
<< "Cannot create virtual link -- invalid container namespace?";
return false;
@@ -597,7 +601,8 @@
bool Datapath::StartRoutingNamespace(const ConnectedNamespace& nsinfo) {
// Veth interface configuration and client routing configuration:
- // - attach a name to the client namespace.
+ // - attach a name to the client namespace (or create a new named namespace
+ // if no client is specified).
// - create veth pair across the current namespace and the client namespace.
// - configure IPv4 address on remote veth inside client namespace.
// - configure IPv4 address on local veth inside host namespace.
@@ -630,8 +635,8 @@
}
{
- ScopedNS ns(nsinfo.pid, ScopedNS::Type::Network);
- if (!ns.IsValid() && nsinfo.pid != kTestPID) {
+ auto ns = ScopedNS::EnterNetworkNS(nsinfo.netns_name);
+ if (!ns && nsinfo.pid != kTestPID) {
LOG(ERROR) << "Invalid namespace pid " << nsinfo.pid;
RemoveInterface(nsinfo.host_ifname);
NetnsDeleteName(nsinfo.netns_name);
diff --git a/patchpanel/datapath.h b/patchpanel/datapath.h
index 37f4de3..7babb95 100644
--- a/patchpanel/datapath.h
+++ b/patchpanel/datapath.h
@@ -28,6 +28,10 @@
// Struct holding parameters for Datapath::StartRoutingNamespace requests.
struct ConnectedNamespace {
+ // The special pid which indicates this namespace is not attached to an
+ // associated process but should be/was created by `ip netns add`.
+ static constexpr pid_t kNewNetnsPid = -1;
+
// The pid of the client network namespace.
pid_t pid;
// The name attached to the client network namespace.
@@ -100,8 +104,9 @@
virtual void Stop();
// Attaches the name |netns_name| to a network namespace identified by
- // |netns_pid|. If |netns_name| had already been created, it will be deleted
- // first.
+ // |netns_pid|. If |netns_pid| is -1, a new namespace with name |netns_name|
+ // will be created instead. If |netns_name| had already been created, it will
+ // be deleted first.
virtual bool NetnsAttachName(const std::string& netns_name, pid_t netns_pid);
// Deletes the name |netns_name| of a network namespace.
diff --git a/patchpanel/datapath_test.cc b/patchpanel/datapath_test.cc
index ae76b37..19cf8d7 100644
--- a/patchpanel/datapath_test.cc
+++ b/patchpanel/datapath_test.cc
@@ -118,6 +118,8 @@
int(const std::string& key,
const std::string& value,
bool log_failures));
+ MOCK_METHOD2(ip_netns_add,
+ int(const std::string& netns_name, bool log_failures));
MOCK_METHOD3(ip_netns_attach,
int(const std::string& netns_name,
pid_t netns_pid,
@@ -171,6 +173,11 @@
EXPECT_CALL(runner, sysctl_w(StrEq(key), StrEq(value), _));
}
+void Verify_ip_netns_add(MockProcessRunner& runner,
+ const std::string& netns_name) {
+ EXPECT_CALL(runner, ip_netns_add(StrEq(netns_name), _));
+}
+
void Verify_ip_netns_attach(MockProcessRunner& runner,
const std::string& netns_name,
pid_t pid) {
@@ -645,6 +652,33 @@
datapath.StopRoutingNamespace(nsinfo);
}
+TEST(DatapathTest, StartRoutingNewNamespace) {
+ MockProcessRunner runner;
+ MockFirewall firewall;
+ MacAddress mac = {1, 2, 3, 4, 5, 6};
+
+ // The running may fail at checking ScopedNS.IsValid() in
+ // Datapath::ConnectVethPair(), so we only check if `ip netns add` is invoked
+ // correctly here.
+ Verify_ip_netns_add(runner, "netns_foo");
+
+ ConnectedNamespace nsinfo = {};
+ nsinfo.pid = ConnectedNamespace::kNewNetnsPid;
+ nsinfo.netns_name = "netns_foo";
+ nsinfo.source = TrafficSource::USER;
+ nsinfo.outbound_ifname = "";
+ nsinfo.route_on_vpn = true;
+ nsinfo.host_ifname = "arc_ns0";
+ nsinfo.peer_ifname = "veth0";
+ nsinfo.peer_subnet = std::make_unique<Subnet>(Ipv4Addr(100, 115, 92, 128), 30,
+ base::DoNothing());
+ nsinfo.peer_mac_addr = mac;
+ Datapath datapath(&runner, &firewall, (ioctl_t)ioctl_rtentry_cap);
+ datapath.StartRoutingNamespace(nsinfo);
+ ioctl_reqs.clear();
+ ioctl_rtentry_args.clear();
+}
+
TEST(DatapathTest, StartRoutingDevice_Arc) {
MockProcessRunner runner;
MockFirewall firewall;
diff --git a/patchpanel/manager.cc b/patchpanel/manager.cc
index be254b9..854d76d 100644
--- a/patchpanel/manager.cc
+++ b/patchpanel/manager.cc
@@ -889,9 +889,9 @@
writer.AppendProtoAsArrayOfBytes(patchpanel::ConnectNamespaceResponse());
return dbus_response;
}
- {
- ScopedNS ns(pid, ScopedNS::Type::Network);
- if (!ns.IsValid()) {
+ if (pid != ConnectedNamespace::kNewNetnsPid) {
+ auto ns = ScopedNS::EnterNetworkNS(pid);
+ if (!ns) {
LOG(ERROR) << "ConnectNamespaceRequest: invalid namespace pid " << pid;
writer.AppendProtoAsArrayOfBytes(patchpanel::ConnectNamespaceResponse());
return dbus_response;
@@ -1067,6 +1067,7 @@
response->set_peer_ipv4_address(nsinfo.peer_subnet->AddressAtOffset(1));
response->set_host_ifname(nsinfo.host_ifname);
response->set_host_ipv4_address(nsinfo.peer_subnet->AddressAtOffset(0));
+ response->set_netns_name(nsinfo.netns_name);
auto* response_subnet = response->mutable_ipv4_subnet();
response_subnet->set_base_addr(nsinfo.peer_subnet->BaseAddress());
response_subnet->set_prefix_len(nsinfo.peer_subnet->PrefixLength());
diff --git a/patchpanel/minijailed_process_runner.cc b/patchpanel/minijailed_process_runner.cc
index a19b695..92f8508 100644
--- a/patchpanel/minijailed_process_runner.cc
+++ b/patchpanel/minijailed_process_runner.cc
@@ -201,6 +201,12 @@
return RunSync(args, log_failures, nullptr);
}
+int MinijailedProcessRunner::ip_netns_add(const std::string& netns_name,
+ bool log_failures) {
+ std::vector<std::string> args = {kIpPath, "netns", "add", netns_name};
+ return RunSync(args, log_failures, nullptr);
+}
+
int MinijailedProcessRunner::ip_netns_attach(const std::string& netns_name,
pid_t netns_pid,
bool log_failures) {
diff --git a/patchpanel/minijailed_process_runner.h b/patchpanel/minijailed_process_runner.h
index a140436..77c6c62 100644
--- a/patchpanel/minijailed_process_runner.h
+++ b/patchpanel/minijailed_process_runner.h
@@ -72,6 +72,10 @@
const std::string& value,
bool log_failures = true);
+ // Creates a new named network namespace with name |netns_name|.
+ virtual int ip_netns_add(const std::string& netns_name,
+ bool log_failures = true);
+
// Attaches a name to the network namespace of the given pid
// TODO(hugobenichi) How can patchpanel create a |netns_name| file in
// /run/netns without running ip as root ?
diff --git a/patchpanel/scoped_ns.cc b/patchpanel/scoped_ns.cc
index 431deaf..3e4585e 100644
--- a/patchpanel/scoped_ns.cc
+++ b/patchpanel/scoped_ns.cc
@@ -6,32 +6,45 @@
#include <fcntl.h>
#include <sched.h>
-#include <sys/types.h>
#include <sys/stat.h>
+#include <sys/types.h>
#include <string>
+#include <utility>
+
+#include <base/memory/ptr_util.h>
namespace patchpanel {
-ScopedNS::ScopedNS(pid_t pid, Type type) : valid_(false) {
- std::string current_ns_path;
- std::string target_ns_path;
- switch (type) {
- case Mount:
- nstype_ = CLONE_NEWNS;
- current_ns_path = "/proc/self/ns/mnt";
- target_ns_path = "/proc/" + std::to_string(pid) + "/ns/mnt";
- break;
- case Network:
- nstype_ = CLONE_NEWNET;
- current_ns_path = "/proc/self/ns/net";
- target_ns_path = "/proc/" + std::to_string(pid) + "/ns/net";
- break;
- default:
- LOG(ERROR) << "Unsupported namespace type " << type;
- return;
- }
+std::unique_ptr<ScopedNS> ScopedNS::EnterMountNS(pid_t pid) {
+ int nstype = CLONE_NEWNS;
+ const std::string current_path = "/proc/self/ns/mnt";
+ const std::string target_path = "/proc/" + std::to_string(pid) + "/ns/mnt";
+ auto ns = base::WrapUnique(new ScopedNS(nstype, current_path, target_path));
+ return ns->valid_ ? std::move(ns) : nullptr;
+}
+std::unique_ptr<ScopedNS> ScopedNS::EnterNetworkNS(pid_t pid) {
+ int nstype = CLONE_NEWNET;
+ const std::string current_path = "/proc/self/ns/net";
+ const std::string target_path = "/proc/" + std::to_string(pid) + "/ns/net";
+ auto ns = base::WrapUnique(new ScopedNS(nstype, current_path, target_path));
+ return ns->valid_ ? std::move(ns) : nullptr;
+}
+
+std::unique_ptr<ScopedNS> ScopedNS::EnterNetworkNS(
+ const std::string& netns_name) {
+ int nstype = CLONE_NEWNET;
+ const std::string current_path = "/proc/self/ns/net";
+ const std::string target_path = "/run/netns/" + netns_name;
+ auto ns = base::WrapUnique(new ScopedNS(nstype, current_path, target_path));
+ return ns->valid_ ? std::move(ns) : nullptr;
+}
+
+ScopedNS::ScopedNS(int nstype,
+ const std::string& current_ns_path,
+ const std::string& target_ns_path)
+ : nstype_(nstype), valid_(false) {
ns_fd_.reset(open(target_ns_path.c_str(), O_RDONLY | O_CLOEXEC));
if (!ns_fd_.is_valid()) {
PLOG(ERROR) << "Could not open namespace " << target_ns_path;
diff --git a/patchpanel/scoped_ns.h b/patchpanel/scoped_ns.h
index a77a8f1..ab25180 100644
--- a/patchpanel/scoped_ns.h
+++ b/patchpanel/scoped_ns.h
@@ -5,6 +5,9 @@
#ifndef PATCHPANEL_SCOPED_NS_H_
#define PATCHPANEL_SCOPED_NS_H_
+#include <memory>
+#include <string>
+
#include <base/files/scoped_file.h>
#include <base/macros.h>
@@ -14,21 +17,24 @@
// namespace.
class ScopedNS {
public:
- enum Type {
- Network = 1,
- Mount,
- };
+ // Records the current mount (network) namespace and enters another namespace
+ // identified by the input argument. Will go back to the current namespace if
+ // the returned object goes out of scope. Returns nullptr on failure.
+ static std::unique_ptr<ScopedNS> EnterMountNS(pid_t pid);
+ static std::unique_ptr<ScopedNS> EnterNetworkNS(pid_t pid);
+ static std::unique_ptr<ScopedNS> EnterNetworkNS(
+ const std::string& netns_name);
- explicit ScopedNS(pid_t pid, Type type);
ScopedNS(const ScopedNS&) = delete;
ScopedNS& operator=(const ScopedNS&) = delete;
~ScopedNS();
- // Returns whether or not the object was able to enter the target namespace.
- bool IsValid() const { return valid_; }
-
private:
+ ScopedNS(int nstype,
+ const std::string& current_ns_path,
+ const std::string& target_ns_path);
+
int nstype_;
bool valid_;
base::ScopedFD ns_fd_;