Snap for 4448085 from a237dedb06fa6dba167841e494e3fc0e77f21941 to oc-m3-releaseandroid-8.1.0_r9android-8.1.0_r7android-8.1.0_r22android-8.1.0_r21android-8.1.0_r18android-8.1.0_r17android-8.1.0_r14android-8.1.0_r13oreo-m5-releaseoreo-m3-release

Change-Id: I817cb0c5885ddfd34edf2ff74df3484fb2d6dd84

10 files changed, 120 insertions, 25 deletions

diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 903390bd..4962b7ca 100644
--- a/server/BandwidthController.cpp
+++ b/server/BandwidthController.cpp
@@ -52,6 +52,7 @@
 
 #include <netdutils/Syscalls.h>
 #include "BandwidthController.h"
+#include "FirewallController.h" /* For makeCriticalCommands */
 #include "NatController.h" /* For LOCAL_TETHER_COUNTERS_CHAIN */
 #include "NetdConstants.h"
 #include "ResponseCode.h"
@@ -248,12 +249,25 @@ int BandwidthController::disableBandwidthControl() {
     return 0;
 }
 
-int BandwidthController::enableDataSaver(bool enable) {
-    std::string cmd = StringPrintf(
+std::string BandwidthController::makeDataSaverCommand(IptablesTarget target, bool enable) {
+    std::string cmd;
+    const char *chainName = "bw_data_saver";
+    const char *op = jumpToString(enable ? IptJumpReject : IptJumpReturn);
+    std::string criticalCommands = enable ?
+            FirewallController::makeCriticalCommands(target, chainName) : "";
+    StringAppendF(&cmd,
         "*filter\n"
-        "-R bw_data_saver 1%s\n"
-        "COMMIT\n", jumpToString(enable ? IptJumpReject : IptJumpReturn));
-    return iptablesRestoreFunction(V4V6, cmd, nullptr);
+        ":%s -\n"
+        "%s"
+        "-A %s%s\n"
+        "COMMIT\n", chainName, criticalCommands.c_str(), chainName, op);
+    return cmd;
+}
+
+int BandwidthController::enableDataSaver(bool enable) {
+    int ret = iptablesRestoreFunction(V4, makeDataSaverCommand(V4, enable), nullptr);
+    ret |= iptablesRestoreFunction(V6, makeDataSaverCommand(V6, enable), nullptr);
+    return ret;
 }
 
 int BandwidthController::addNaughtyApps(int numUids, char *appUids[]) {
diff --git a/server/BandwidthController.h b/server/BandwidthController.h
index 1522a567..0eef5811 100644
--- a/server/BandwidthController.h
+++ b/server/BandwidthController.h
@@ -133,6 +133,8 @@ public:
     enum IptFailureLog { IptFailShow, IptFailHide = IptFailShow };
 #endif
 
+    std::string makeDataSaverCommand(IptablesTarget target, bool enable);
+
     int manipulateSpecialApps(const std::vector<std::string>& appStrUids, const std::string& chain,
                               IptJumpOp jumpHandling, IptOp appOp);
 
diff --git a/server/BandwidthControllerTest.cpp b/server/BandwidthControllerTest.cpp
index a0a57da7..8681be43 100644
--- a/server/BandwidthControllerTest.cpp
+++ b/server/BandwidthControllerTest.cpp
@@ -221,20 +221,38 @@ TEST_F(BandwidthControllerTest, TestDisableBandwidthControl) {
 
 TEST_F(BandwidthControllerTest, TestEnableDataSaver) {
     mBw.enableDataSaver(true);
-    std::vector<std::string> expected = {
+    std::string expected4 =
         "*filter\n"
-        "-R bw_data_saver 1 --jump REJECT\n"
-        "COMMIT\n"
-    };
-    expectIptablesRestoreCommands(expected);
+        ":bw_data_saver -\n"
+        "-A bw_data_saver --jump REJECT\n"
+        "COMMIT\n";
+    std::string expected6 =
+        "*filter\n"
+        ":bw_data_saver -\n"
+        "-A bw_data_saver -p icmpv6 --icmpv6-type packet-too-big -j RETURN\n"
+        "-A bw_data_saver -p icmpv6 --icmpv6-type router-solicitation -j RETURN\n"
+        "-A bw_data_saver -p icmpv6 --icmpv6-type router-advertisement -j RETURN\n"
+        "-A bw_data_saver -p icmpv6 --icmpv6-type neighbour-solicitation -j RETURN\n"
+        "-A bw_data_saver -p icmpv6 --icmpv6-type neighbour-advertisement -j RETURN\n"
+        "-A bw_data_saver -p icmpv6 --icmpv6-type redirect -j RETURN\n"
+        "-A bw_data_saver --jump REJECT\n"
+        "COMMIT\n";
+    expectIptablesRestoreCommands({
+        {V4, expected4},
+        {V6, expected6},
+    });
 
     mBw.enableDataSaver(false);
-    expected = {
+    std::string expected = {
         "*filter\n"
-        "-R bw_data_saver 1 --jump RETURN\n"
+        ":bw_data_saver -\n"
+        "-A bw_data_saver --jump RETURN\n"
         "COMMIT\n"
     };
-    expectIptablesRestoreCommands(expected);
+    expectIptablesRestoreCommands({
+        {V4, expected},
+        {V6, expected},
+    });
 }
 
 std::string kIPv4TetherCounters = Join(std::vector<std::string> {
diff --git a/server/FirewallController.cpp b/server/FirewallController.cpp
index 8e32bc94..f5da0691 100644
--- a/server/FirewallController.cpp
+++ b/server/FirewallController.cpp
@@ -239,6 +239,19 @@ int FirewallController::createChain(const char* chain, FirewallType type) {
     return replaceUidChain(chain, type == WHITELIST, NO_UIDS);
 }
 
+/* static */
+std::string FirewallController::makeCriticalCommands(IptablesTarget target, const char* chainName) {
+    // Allow ICMPv6 packets necessary to make IPv6 connectivity work. http://b/23158230 .
+    std::string commands;
+    if (target == V6) {
+        for (size_t i = 0; i < ARRAY_SIZE(ICMPV6_TYPES); i++) {
+            StringAppendF(&commands, "-A %s -p icmpv6 --icmpv6-type %s -j RETURN\n",
+                   chainName, ICMPV6_TYPES[i]);
+        }
+    }
+    return commands;
+}
+
 std::string FirewallController::makeUidRules(IptablesTarget target, const char *name,
         bool isWhitelist, const std::vector<int32_t>& uids) {
     std::string commands;
@@ -264,13 +277,7 @@ std::string FirewallController::makeUidRules(IptablesTarget target, const char *
     StringAppendF(&commands, "-A %s -p tcp --tcp-flags RST RST -j RETURN\n", name);
 
     if (isWhitelist) {
-        // Allow ICMPv6 packets necessary to make IPv6 connectivity work. http://b/23158230 .
-        if (target == V6) {
-            for (size_t i = 0; i < ARRAY_SIZE(ICMPV6_TYPES); i++) {
-                StringAppendF(&commands, "-A %s -p icmpv6 --icmpv6-type %s -j RETURN\n",
-                       name, ICMPV6_TYPES[i]);
-            }
-        }
+        commands.append(makeCriticalCommands(target, name));
     }
 
     // Blacklist chains have UIDs at the end, and new UIDs are added with '-A'.
diff --git a/server/FirewallController.h b/server/FirewallController.h
index 041aa40c..1da9e70c 100644
--- a/server/FirewallController.h
+++ b/server/FirewallController.h
@@ -64,6 +64,8 @@ public:
 
     int replaceUidChain(const char*, bool, const std::vector<int32_t>&);
 
+    static std::string makeCriticalCommands(IptablesTarget target, const char* chainName);
+
     static const char* TABLE;
 
     static const char* LOCAL_INPUT;
diff --git a/server/PhysicalNetwork.cpp b/server/PhysicalNetwork.cpp
index ccac3233..579d0bdd 100644
--- a/server/PhysicalNetwork.cpp
+++ b/server/PhysicalNetwork.cpp
@@ -86,6 +86,18 @@ int PhysicalNetwork::destroySocketsLackingPermission(Permission permission) {
     return 0;
 }
 
+void PhysicalNetwork::invalidateRouteCache(const std::string& interface) {
+    for (const auto& dst : { "0.0.0.0/0", "::/0" }) {
+        // If any of these operations fail, there's no point in logging because RouteController will
+        // have already logged a message. There's also no point returning an error since there's
+        // nothing we can do.
+        (void) RouteController::addRoute(interface.c_str(), dst, "throw",
+                                         RouteController::INTERFACE);
+        (void) RouteController::removeRoute(interface.c_str(), dst, "throw",
+                                         RouteController::INTERFACE);
+    }
+}
+
 int PhysicalNetwork::setPermission(Permission permission) {
     if (permission == mPermission) {
         return 0;
@@ -103,6 +115,7 @@ int PhysicalNetwork::setPermission(Permission permission) {
                   interface.c_str(), mNetId, mPermission, permission);
             return ret;
         }
+        invalidateRouteCache(interface);
     }
     if (mIsDefault) {
         for (const std::string& interface : mInterfaces) {
diff --git a/server/PhysicalNetwork.h b/server/PhysicalNetwork.h
index 9200955f..89c9443b 100644
--- a/server/PhysicalNetwork.h
+++ b/server/PhysicalNetwork.h
@@ -50,6 +50,7 @@ private:
     int addInterface(const std::string& interface) override WARN_UNUSED_RESULT;
     int removeInterface(const std::string& interface) override WARN_UNUSED_RESULT;
     int destroySocketsLackingPermission(Permission permission);
+    void invalidateRouteCache(const std::string& interface);
 
     Delegate* const mDelegate;
     Permission mPermission;
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 87a56982..26e8407b 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -81,6 +81,14 @@ const char* const ROUTE_TABLE_NAME_LEGACY_SYSTEM  = "legacy_system";
 const char* const ROUTE_TABLE_NAME_LOCAL = "local";
 const char* const ROUTE_TABLE_NAME_MAIN  = "main";
 
+// None of our routes specify priority, which causes them to have the default
+// priority. For throw routes, we use a fixed priority of 100000. This is
+// because we use throw routes either for maximum-length routes (/32 for IPv4,
+// /128 for IPv6), which we never create with any other priority, or for
+// purposely-low-priority default routes that should never match if there is
+// any other route in the table.
+uint32_t PRIO_THROW = 100000;
+
 const char* const RouteController::LOCAL_MANGLE_INPUT = "routectrl_mangle_INPUT";
 
 // These values are upstream, but not yet in our headers.
@@ -120,6 +128,7 @@ rtattr FRATTR_UID_RANGE = { U16_RTA_LENGTH(sizeof(fib_rule_uid_range)), FRA_UID_
 
 rtattr RTATTR_TABLE     = { U16_RTA_LENGTH(sizeof(uint32_t)),           RTA_TABLE };
 rtattr RTATTR_OIF       = { U16_RTA_LENGTH(sizeof(uint32_t)),           RTA_OIF };
+rtattr RTATTR_PRIO      = { U16_RTA_LENGTH(sizeof(uint32_t)),           RTA_PRIORITY };
 
 uint8_t PADDING_BUFFER[RTA_ALIGNTO] = {0, 0, 0, 0};
 
@@ -373,6 +382,8 @@ WARN_UNUSED_RESULT int modifyIpRoute(uint16_t action, uint32_t table, const char
         }
     }
 
+    bool isDefaultThrowRoute = (type == RTN_THROW && prefixLength == 0);
+
     // Assemble a rtmsg and put it in an array of iovec structures.
     rtmsg route = {
         .rtm_protocol = RTPROT_STATIC,
@@ -396,6 +407,8 @@ WARN_UNUSED_RESULT int modifyIpRoute(uint16_t action, uint32_t table, const char
         { &ifindex,      interface != OIF_NONE ? sizeof(ifindex) : 0 },
         { &rtaGateway,   nexthop ? sizeof(rtaGateway) : 0 },
         { rawNexthop,    nexthop ? static_cast<size_t>(rawLength) : 0 },
+        { &RTATTR_PRIO,  isDefaultThrowRoute ? sizeof(RTATTR_PRIO) : 0 },
+        { &PRIO_THROW,   isDefaultThrowRoute ? sizeof(PRIO_THROW) : 0 },
     };
 
     uint16_t flags = (action == RTM_NEWROUTE) ? NETLINK_CREATE_REQUEST_FLAGS :
diff --git a/server/TetherController.cpp b/server/TetherController.cpp
index 1785ec71..43a20486 100644
--- a/server/TetherController.cpp
+++ b/server/TetherController.cpp
@@ -45,6 +45,7 @@ const char BP_TOOLS_MODE[] = "bp-tools";
 const char IPV4_FORWARDING_PROC_FILE[] = "/proc/sys/net/ipv4/ip_forward";
 const char IPV6_FORWARDING_PROC_FILE[] = "/proc/sys/net/ipv6/conf/all/forwarding";
 const char SEPARATOR[] = "|";
+constexpr const char kTcpBeLiberal[] = "/proc/sys/net/netfilter/nf_conntrack_tcp_be_liberal";
 
 bool writeToFile(const char* filename, const char* value) {
     int fd = open(filename, O_WRONLY | O_CLOEXEC);
@@ -63,6 +64,11 @@ bool writeToFile(const char* filename, const char* value) {
     return true;
 }
 
+// TODO: Consider altering TCP and UDP timeouts as well.
+void configureForTethering(bool enabled) {
+    writeToFile(kTcpBeLiberal, enabled ? "1" : "0");
+}
+
 bool configureForIPv6Router(const char *interface) {
     return (InterfaceController::setEnableIPv6(interface, 0) == 0)
             && (InterfaceController::setAcceptIPv6Ra(interface, 0) == 0)
@@ -211,6 +217,7 @@ int TetherController::startTethering(int num_addrs, char **dhcp_ranges) {
         close(pipefd[0]);
         mDaemonPid = pid;
         mDaemonFd = pipefd[1];
+        configureForTethering(true);
         applyDnsInterfaces();
         ALOGD("Tethering services running");
     }
@@ -219,6 +226,7 @@ int TetherController::startTethering(int num_addrs, char **dhcp_ranges) {
 }
 
 int TetherController::stopTethering() {
+    configureForTethering(false);
 
     if (mDaemonPid == 0) {
         ALOGE("Tethering already stopped");
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index b2f362ee..02ec1ffc 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -57,6 +57,7 @@
 using namespace android;
 using namespace android::base;
 using namespace android::binder;
+using android::base::StartsWith;
 using android::net::INetd;
 using android::net::TunInterface;
 using android::net::UidRange;
@@ -212,13 +213,29 @@ static int bandwidthDataSaverEnabled(const char *binary) {
     // Chain bw_data_saver (1 references)
     // target     prot opt source               destination
     // RETURN     all  --  0.0.0.0/0            0.0.0.0/0
-    EXPECT_EQ(3U, lines.size());
-    if (lines.size() != 3) return -1;
+    //
+    // or:
+    //
+    // Chain bw_data_saver (1 references)
+    // target     prot opt source               destination
+    // ... possibly connectivity critical packet rules here ...
+    // REJECT     all  --  ::/0            ::/0
+
+    EXPECT_GE(lines.size(), 3U);
 
-    EXPECT_TRUE(android::base::StartsWith(lines[2], "RETURN ") ||
-                android::base::StartsWith(lines[2], "REJECT "));
+    if (lines.size() == 3 && StartsWith(lines[2], "RETURN ")) {
+        // Data saver disabled.
+        return 0;
+    }
+
+    size_t minSize = (std::string(binary) == IPTABLES_PATH) ? 3 : 9;
+
+    if (lines.size() >= minSize && StartsWith(lines[lines.size() -1], "REJECT ")) {
+        // Data saver enabled.
+        return 1;
+    }
 
-    return android::base::StartsWith(lines[2], "REJECT");
+    return -1;
 }
 
 bool enableDataSaver(sp<INetd>& netd, bool enable) {