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/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.