patchpanel: mark connections with fwmark routing tags
This patch enables per-network tracking of connections using
CONNMARK conntrack for storing the fwmark routing tag associated with
the output interface selected by routing. This is achieved with a mangle
POSTROUTING rule using the CONNMARK target.
A follow-up patch in shill will add the final piece and add ip rules
using these fwmark routing tags to complete the per-network connection
routing and fwmark routing system.
BUG=b:161507671
BUG=b:161508179
TEST=Unit tests. Manual tests: verified with /proc/net/ip_conntrack that
conntrack entries get marked with the fwmark routing tag of the network
through which they are routed.
Change-Id: I9f147179283246c1aafda87ad716e209ce387736
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2377337
Tested-by: Hugo Benichi <hugobenichi@google.com>
Tested-by: Mitu Aktar <mitu64649@gmail.com>
Commit-Queue: Hugo Benichi <hugobenichi@google.com>
Reviewed-by: Taoyu Li <taoyl@chromium.org>
diff --git a/patchpanel/datapath.cc b/patchpanel/datapath.cc
index 73075bd..6155749 100644
--- a/patchpanel/datapath.cc
+++ b/patchpanel/datapath.cc
@@ -571,6 +571,56 @@
process_runner_->ip6("addr", "del", {ipv6_addr, "dev", ifname});
}
+void Datapath::StartConnectionPinning(const std::string& ext_ifname) {
+ if (!ModifyConnmarkSetPostrouting(IpFamily::Dual, "-A", ext_ifname))
+ LOG(ERROR) << "Could not start connection pinning on " << ext_ifname;
+}
+
+void Datapath::StopConnectionPinning(const std::string& ext_ifname) {
+ if (!ModifyConnmarkSetPostrouting(IpFamily::Dual, "-D", ext_ifname))
+ LOG(ERROR) << "Could not stop connection pinning on " << ext_ifname;
+}
+
+bool Datapath::ModifyConnmarkSetPostrouting(IpFamily family,
+ const std::string& op,
+ const std::string& oif) {
+ if (oif.empty()) {
+ LOG(ERROR) << "Cannot change POSTROUTING CONNMARK set-mark with no "
+ "interface specified";
+ return false;
+ }
+
+ if (!IsValidIpFamily(family)) {
+ LOG(ERROR) << "Cannot change POSTROUTING CONNMARK set-mark for " << oif
+ << ": incorrect IP family " << family;
+ return false;
+ }
+
+ int ifindex = FindIfIndex(oif);
+ if (ifindex == 0) {
+ PLOG(ERROR) << "if_nametoindex(" << oif << ") failed";
+ return false;
+ }
+
+ std::vector<std::string> args = {op,
+ "POSTROUTING",
+ "-o",
+ oif,
+ "-j",
+ "CONNMARK",
+ "--set-mark",
+ Fwmark::FromIfIndex(ifindex).ToString() +
+ "/" + kFwmarkRoutingMask.ToString(),
+ "-w"};
+
+ bool success = true;
+ if (family & IPv4)
+ success &= process_runner_->iptables("mangle", args) == 0;
+ if (family & IPv6)
+ success &= process_runner_->ip6tables("mangle", args) == 0;
+ return false;
+}
+
bool Datapath::ModifyConnmarkRestore(IpFamily family,
const std::string& chain,
const std::string& op,
diff --git a/patchpanel/datapath.h b/patchpanel/datapath.h
index c2dcbfd..ea7ae28 100644
--- a/patchpanel/datapath.h
+++ b/patchpanel/datapath.h
@@ -144,6 +144,13 @@
uint32_t int_ipv4_addr,
TrafficSource source);
+ // Starts or stops marking conntrack entries routed to |ext_ifname| with its
+ // associated fwmark routing tag. Once a conntrack entry is marked with the
+ // fwmark routing tag of a external device, the connection will be pinned
+ // to that deviced if conntrack fwmark restore is set for the source.
+ virtual void StartConnectionPinning(const std::string& ext_ifname);
+ virtual void StopConnectionPinning(const std::string& ext_ifname);
+
// Create (or delete) pre-routing rules allowing direct ingress on |ifname|
// to guest desintation |ipv4_addr|.
virtual bool AddInboundIPv4DNAT(const std::string& ifname,
@@ -242,6 +249,9 @@
MinijailedProcessRunner& runner() const;
private:
+ bool ModifyConnmarkSetPostrouting(IpFamily family,
+ const std::string& op,
+ const std::string& oif);
bool ModifyConnmarkRestore(IpFamily family,
const std::string& chain,
const std::string& op,
diff --git a/patchpanel/datapath_test.cc b/patchpanel/datapath_test.cc
index 4df4342..f69e7d6 100644
--- a/patchpanel/datapath_test.cc
+++ b/patchpanel/datapath_test.cc
@@ -591,6 +591,35 @@
}
}
+TEST(DatapathTest, StartStopConnectionPinning) {
+ MockProcessRunner runner;
+ MockFirewall firewall;
+ EXPECT_CALL(runner, iptables(StrEq("mangle"),
+ ElementsAre("-A", "POSTROUTING", "-o", "eth0",
+ "-j", "CONNMARK", "--set-mark",
+ "0x03eb0000/0xffff0000", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner, iptables(StrEq("mangle"),
+ ElementsAre("-D", "POSTROUTING", "-o", "eth0",
+ "-j", "CONNMARK", "--set-mark",
+ "0x03eb0000/0xffff0000", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner, ip6tables(StrEq("mangle"),
+ ElementsAre("-A", "POSTROUTING", "-o", "eth0",
+ "-j", "CONNMARK", "--set-mark",
+ "0x03eb0000/0xffff0000", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner, ip6tables(StrEq("mangle"),
+ ElementsAre("-D", "POSTROUTING", "-o", "eth0",
+ "-j", "CONNMARK", "--set-mark",
+ "0x03eb0000/0xffff0000", "-w"),
+ true, nullptr));
+ Datapath datapath(&runner, &firewall);
+ datapath.SetIfnameIndex("eth0", 3);
+ datapath.StartConnectionPinning("eth0");
+ datapath.StopConnectionPinning("eth0");
+}
+
TEST(DatapathTest, AddInboundIPv4DNAT) {
MockProcessRunner runner;
MockFirewall firewall;
diff --git a/patchpanel/manager.cc b/patchpanel/manager.cc
index 370df2e..841526c 100644
--- a/patchpanel/manager.cc
+++ b/patchpanel/manager.cc
@@ -185,6 +185,8 @@
<< " object";
}
+ shill_client_ = std::make_unique<ShillClient>(bus_);
+
using ServiceMethod =
std::unique_ptr<dbus::Response> (Manager::*)(dbus::MethodCall*);
const std::map<const char*, ServiceMethod> kServiceMethods = {
@@ -225,7 +227,6 @@
nd_proxy_->RegisterNDProxyMessageHandler(
base::Bind(&Manager::OnNDProxyMessage, weak_factory_.GetWeakPtr()));
- shill_client_ = std::make_unique<ShillClient>(bus_);
auto* const forwarder = static_cast<TrafficForwarder*>(this);
GuestMessage::GuestType arc_guest =
@@ -319,7 +320,7 @@
LOG(ERROR) << "Failed to limit local port range. Some Android features or"
<< " apps may not work correctly.";
}
- // Enable IPv6 packet forarding
+ // Enable IPv6 packet forwarding
if (runner.sysctl_w("net.ipv6.conf.all.forwarding", "1") != 0) {
LOG(ERROR) << "Failed to update net.ipv6.conf.all.forwarding."
<< " IPv6 functionality may be broken.";
@@ -349,6 +350,9 @@
LOG(ERROR) << "Failed to set up NAT for TAP devices."
<< " Guest connectivity may be broken.";
}
+
+ shill_client_->RegisterDevicesChangedHandler(base::BindRepeating(
+ &Manager::OnDevicesChanged, weak_factory_.GetWeakPtr()));
}
void Manager::StopDatapath() {
@@ -377,6 +381,15 @@
}
}
+void Manager::OnDevicesChanged(const std::set<std::string>& added,
+ const std::set<std::string>& removed) {
+ for (const std::string& ifname : removed)
+ datapath_->StopConnectionPinning(ifname);
+
+ for (const std::string& ifname : added)
+ datapath_->StartConnectionPinning(ifname);
+}
+
bool Manager::StartArc(pid_t pid) {
if (!arc_svc_->Start(pid))
return false;
diff --git a/patchpanel/manager.h b/patchpanel/manager.h
index d3977c9..3f9d5e7 100644
--- a/patchpanel/manager.h
+++ b/patchpanel/manager.h
@@ -85,6 +85,8 @@
int OnInit() override;
private:
+ void OnDevicesChanged(const std::set<std::string>& added,
+ const std::set<std::string>& removed);
void InitialSetup();
// Start and stop the Datapath, creating or destroying the initial iptables
