patchpanel: drop local traffic with src ip in 100.115.92.0/23
Some connectivity scenarios like webRTC can cause Chrome to send packets
to the physical network with incorrect source IPs. This happen when
Chrome incorrectly binds to one of the virtual interfaces used for ARC
or other VMs and ends up sending packets that get routed to the default
logical network through the catch-all routing rule set by shill.
On some networks like cellular networks, such traffic with incorrect
source IP addresses can cause the network to terminate the connection.
To avoid these disconnections this patch adds iptables DROP rules in
FILTER to drop any locally originated packet that would exit a physical
interface with an IPv4 source address in the subnet used for assigning
static IPv4 addresses to hosted VMs and containers.
BUG=chromium:898210
TEST=Deployed patchpanel, connected to remote Meet meeting from Chrome,
- observed no traffic outgoing eth or wifi with an incorrect src ip,
- observed that the iptables DROP rules in FILTER caught incorrect
packets,
- checked that Meet from Chrome works over eth, wifi, and an Android
VPN connection.
Change-Id: I80a07770412a0be36e4512f7db085d418e087315
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2428657
Reviewed-by: Taoyu Li <taoyl@chromium.org>
Reviewed-by: Garrick Evans <garrick@chromium.org>
Tested-by: Hugo Benichi <hugobenichi@google.com>
Commit-Queue: Hugo Benichi <hugobenichi@google.com>
diff --git a/patchpanel/datapath.cc b/patchpanel/datapath.cc
index ae36a70..09abc13 100644
--- a/patchpanel/datapath.cc
+++ b/patchpanel/datapath.cc
@@ -323,6 +323,18 @@
process_runner_->ip("link", "delete", {ifname}, false /*log_failures*/);
}
+bool Datapath::AddSourceIPv4DropRule(const std::string& oif,
+ const std::string& src_ip) {
+ return process_runner_->iptables("filter", {"-I", "OUTPUT", "-o", oif, "-s",
+ src_ip, "-j", "DROP", "-w"}) == 0;
+}
+
+bool Datapath::RemoveSourceIPv4DropRule(const std::string& oif,
+ const std::string& src_ip) {
+ return process_runner_->iptables("filter", {"-D", "OUTPUT", "-o", oif, "-s",
+ src_ip, "-j", "DROP", "-w"}) == 0;
+}
+
void Datapath::StartRoutingDevice(const std::string& ext_ifname,
const std::string& int_ifname,
uint32_t int_ipv4_addr,
diff --git a/patchpanel/datapath.h b/patchpanel/datapath.h
index 7856075..9b6b498 100644
--- a/patchpanel/datapath.h
+++ b/patchpanel/datapath.h
@@ -119,6 +119,15 @@
virtual void RemoveInterface(const std::string& ifname);
+ // Create (or delete) an OUTPUT DROP rule for any locally originated traffic
+ // whose src IPv4 matches |src_ip| and would exit |oif|. This is mainly used
+ // for dropping Chrome webRTC traffic incorrectly bound on ARC and other
+ // guests virtual interfaces (chromium:898210).
+ virtual bool AddSourceIPv4DropRule(const std::string& oif,
+ const std::string& src_ip);
+ virtual bool RemoveSourceIPv4DropRule(const std::string& oif,
+ const std::string& src_ip);
+
// Sets up IPv4 SNAT, IP forwarding, and traffic marking for the given
// virtual device |int_ifname| associated to |source|. if |ext_ifname| is
// empty, the device is implicitly routed through the highest priority
diff --git a/patchpanel/datapath_test.cc b/patchpanel/datapath_test.cc
index fa85c95..2ccf949 100644
--- a/patchpanel/datapath_test.cc
+++ b/patchpanel/datapath_test.cc
@@ -294,6 +294,24 @@
datapath.RemoveBridge("br");
}
+TEST(DatapathTest, AddRemoveSourceIPv4DropRule) {
+ MockProcessRunner runner;
+ MockFirewall firewall;
+ EXPECT_CALL(runner,
+ iptables(StrEq("filter"),
+ ElementsAre("-I", "OUTPUT", "-o", "eth+", "-s",
+ "100.115.92.0/24", "-j", "DROP", "-w"),
+ true, nullptr));
+ EXPECT_CALL(runner,
+ iptables(StrEq("filter"),
+ ElementsAre("-D", "OUTPUT", "-o", "eth+", "-s",
+ "100.115.92.0/24", "-j", "DROP", "-w"),
+ true, nullptr));
+ Datapath datapath(&runner, &firewall);
+ datapath.AddSourceIPv4DropRule("eth+", "100.115.92.0/24");
+ datapath.RemoveSourceIPv4DropRule("eth+", "100.115.92.0/24");
+}
+
TEST(DatapathTest, StartRoutingDevice_Arc) {
MockProcessRunner runner;
MockFirewall firewall;
diff --git a/patchpanel/manager.cc b/patchpanel/manager.cc
index 1abc7d8..7cee20b 100644
--- a/patchpanel/manager.cc
+++ b/patchpanel/manager.cc
@@ -36,6 +36,12 @@
namespace {
constexpr int kSubprocessRestartDelayMs = 900;
+// Constants used for dropping locally originated traffic bound to an incorrect
+// source IPv4 address.
+constexpr char kGuestIPv4Subnet[] = "100.115.92.0/23";
+constexpr std::array<const char*, 6> kPhysicalIfnamePrefixes{
+ {"eth+", "wlan+", "mlan+", "usb+", "wwan+", "rmnet+"}};
+
// Time interval between epoll checks on file descriptors committed by callers
// of ConnectNamespace DBus API.
constexpr const base::TimeDelta kConnectNamespaceCheckInterval =
@@ -325,11 +331,15 @@
<< " connections.";
}
- // TODO(chromium:898210): Move interface-specific masquerading setup to shill;
- // such that we can better set up the masquerade rules based on connection
- // type rather than interface names.
- if (!datapath_->AddInterfaceSNAT("wwan+")) {
- LOG(ERROR) << "Failed to set up wifi masquerade";
+ // chromium:898210: Drop any locally originated traffic that would exit a
+ // physical interface with a source IPv4 address from the subnet of IPs used
+ // for VMs, containers, and connected namespaces This is needed to prevent
+ // packets leaking with an incorrect src IP when a local process binds to the
+ // wrong interface.
+ for (const auto& oif : kPhysicalIfnamePrefixes) {
+ if (!datapath_->AddSourceIPv4DropRule(oif, kGuestIPv4Subnet))
+ LOG(WARNING) << "Failed to set up IPv4 drop rule for src ip "
+ << kGuestIPv4Subnet << " exiting " << oif;
}
if (!datapath_->AddOutboundIPv4SNATMark("vmtap+")) {
@@ -343,9 +353,10 @@
return;
datapath_->RemoveOutboundIPv4SNATMark("vmtap+");
- datapath_->RemoveInterfaceSNAT("wwan+");
datapath_->RemoveForwardEstablishedRule();
datapath_->RemoveSNATMarkRules();
+ for (const auto& oif : kPhysicalIfnamePrefixes)
+ datapath_->RemoveSourceIPv4DropRule(oif, kGuestIPv4Subnet);
auto& runner = datapath_->runner();
// Restore original local port range.