Add app default local rule am: 776b68cecb am: f693a01e8f

Original change: https://googleplex-android-review.googlesource.com/c/platform/system/netd/+/18714757

Change-Id: Ia4fc5b8fe2df8e83257198a942b12ea63157fa84
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

3 files changed, 54 insertions, 9 deletions

diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index d63dbd2e..ea8baea8 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -859,6 +859,13 @@ int RouteController::modifyPhysicalNetwork(unsigned netId, const char* interface
                                                           subPriority, add)) {
                     return ret;
                 }
+
+                // Per-UID local network rules must always match per-app default network rules,
+                // because their purpose is to allow the UIDs to use the default network for
+                // local destinations within it.
+                if (int ret = modifyUidLocalNetworkRule(interface, range.start, range.stop, add)) {
+                    return ret;
+                }
             }
         }
     }
@@ -906,6 +913,32 @@ int RouteController::modifyPhysicalNetwork(unsigned netId, const char* interface
     return 0;
 }
 
+int RouteController::modifyUidLocalNetworkRule(const char* interface, uid_t uidStart, uid_t uidEnd,
+                                               bool add) {
+    uint32_t table = getRouteTableForInterface(interface, true /* local */);
+    if (table == RT_TABLE_UNSPEC) {
+        return -ESRCH;
+    }
+
+    if ((uidStart == INVALID_UID) || (uidEnd == INVALID_UID)) {
+        ALOGE("modifyUidLocalNetworkRule, invalid UIDs (%u, %u)", uidStart, uidEnd);
+        return -EUSERS;
+    }
+
+    Fwmark fwmark;
+    Fwmark mask;
+
+    fwmark.explicitlySelected = false;
+    mask.explicitlySelected = true;
+
+    // Access to this network is controlled by UID rules, not permission bits.
+    fwmark.permission = PERMISSION_NONE;
+    mask.permission = PERMISSION_NONE;
+
+    return modifyIpRule(add ? RTM_NEWRULE : RTM_DELRULE, RULE_PRIORITY_UID_LOCAL_ROUTES, table,
+                        fwmark.intValue, mask.intValue, IIF_LOOPBACK, OIF_NONE, uidStart, uidEnd);
+}
+
 [[nodiscard]] static int modifyUidUnreachableRule(unsigned netId, uid_t uidStart, uid_t uidEnd,
                                                   int32_t subPriority, bool add,
                                                   bool explicitSelect) {
diff --git a/server/RouteController.h b/server/RouteController.h
index 9b04cfd2..887187c8 100644
--- a/server/RouteController.h
+++ b/server/RouteController.h
@@ -55,11 +55,17 @@ constexpr int32_t RULE_PRIORITY_TETHERING                         = 21000;
 constexpr int32_t RULE_PRIORITY_UID_IMPLICIT_NETWORK              = 22000;
 constexpr int32_t RULE_PRIORITY_IMPLICIT_NETWORK                  = 23000;
 constexpr int32_t RULE_PRIORITY_BYPASSABLE_VPN_NO_LOCAL_EXCLUSION = 24000;
-// Rules used for excluding local route in the VPN network.
-constexpr int32_t RULE_PRIORITY_LOCAL_ROUTES                      = 25000;
-constexpr int32_t RULE_PRIORITY_BYPASSABLE_VPN_LOCAL_EXCLUSION    = 26000;
-constexpr int32_t RULE_PRIORITY_VPN_FALLTHROUGH                   = 27000;
-constexpr int32_t RULE_PRIORITY_UID_DEFAULT_NETWORK               = 28000;
+// Sets of rules used for excluding local routes from the VPN. Look up tables
+// that contain directly-connected local routes taken from the default network.
+// The first set is used for apps that have a per-UID default network. The rule
+// UID ranges match those of the per-UID default network rule for that network.
+// The second set has no UID ranges and is used for apps whose default network
+// is the system default network network.
+constexpr int32_t RULE_PRIORITY_UID_LOCAL_ROUTES                  = 25000;
+constexpr int32_t RULE_PRIORITY_LOCAL_ROUTES                      = 26000;
+constexpr int32_t RULE_PRIORITY_BYPASSABLE_VPN_LOCAL_EXCLUSION    = 27000;
+constexpr int32_t RULE_PRIORITY_VPN_FALLTHROUGH                   = 28000;
+constexpr int32_t RULE_PRIORITY_UID_DEFAULT_NETWORK               = 29000;
 // Rule used when framework wants to disable default network from specified applications. There will
 // be a small interval the same uid range exists in both UID_DEFAULT_UNREACHABLE and
 // UID_DEFAULT_NETWORK when framework is switching user preferences.
@@ -74,8 +80,8 @@ constexpr int32_t RULE_PRIORITY_UID_DEFAULT_NETWORK               = 28000;
 // The priority is lower than UID_DEFAULT_NETWORK. Otherwise, the app will be told by
 // ConnectivityService that it has a network in step 1 of the second case. But if it tries to use
 // the network, it will not work. That will potentially cause a user-visible error.
-constexpr int32_t RULE_PRIORITY_UID_DEFAULT_UNREACHABLE           = 29000;
-constexpr int32_t RULE_PRIORITY_DEFAULT_NETWORK                   = 30000;
+constexpr int32_t RULE_PRIORITY_UID_DEFAULT_UNREACHABLE           = 30000;
+constexpr int32_t RULE_PRIORITY_DEFAULT_NETWORK                   = 31000;
 constexpr int32_t RULE_PRIORITY_UNREACHABLE                       = 32000;
 // clang-format on
 
@@ -221,6 +227,8 @@ public:
                                     bool modifyNonUidBasedRules, bool excludeLocalRoutes);
     static void updateTableNamesFile() EXCLUDES(sInterfaceToTableLock);
     static int modifyVpnLocalExclusionRule(bool add, const char* physicalInterface);
+    static int modifyUidLocalNetworkRule(const char* interface, uid_t uidStart, uid_t uidEnd,
+                                         bool add);
 };
 
 // Public because they are called by by RouteControllerTest.cpp.
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index c01d78e9..5f5dff11 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -129,6 +129,7 @@ using android::net::RULE_PRIORITY_UID_DEFAULT_NETWORK;
 using android::net::RULE_PRIORITY_UID_DEFAULT_UNREACHABLE;
 using android::net::RULE_PRIORITY_UID_EXPLICIT_NETWORK;
 using android::net::RULE_PRIORITY_UID_IMPLICIT_NETWORK;
+using android::net::RULE_PRIORITY_UID_LOCAL_ROUTES;
 using android::net::RULE_PRIORITY_VPN_FALLTHROUGH;
 using android::net::SockDiag;
 using android::net::TetherOffloadRuleParcel;
@@ -3287,8 +3288,8 @@ void NetdBinderTest::createVpnNetworkWithUid(bool secure, uid_t uid, int vpnNetI
                                           INetd::PERMISSION_NONE, false, false);
     EXPECT_TRUE(mNetd->networkCreate(config).isOk());
     EXPECT_TRUE(mNetd->networkAddInterface(fallthroughNetId, sTun.name()).isOk());
-    // Create a physical network to test that local network access does not include the non-default
-    // networks.
+    // Create another physical network in order to test VPN behaviour with multiple networks
+    // connected, of which one may be the default.
     auto nonDefaultNetworkConfig = makeNativeNetworkConfig(
             nonDefaultNetId, NativeNetworkType::PHYSICAL, INetd::PERMISSION_NONE, false, false);
     EXPECT_TRUE(mNetd->networkCreate(nonDefaultNetworkConfig).isOk());
@@ -4025,6 +4026,7 @@ void verifyAppUidRules(std::vector<bool>&& expectedResults, std::vector<UidRange
     ASSERT_EQ(expectedResults.size(), uidRanges.size());
     if (iface.size()) {
         std::string action = StringPrintf("lookup %s ", iface.c_str());
+        std::string action_local = StringPrintf("lookup %s_local ", iface.c_str());
         for (unsigned long i = 0; i < uidRanges.size(); i++) {
             EXPECT_EQ(expectedResults[i],
                       ipRuleExistsForRange(RULE_PRIORITY_UID_EXPLICIT_NETWORK + subPriority,
@@ -4035,6 +4037,8 @@ void verifyAppUidRules(std::vector<bool>&& expectedResults, std::vector<UidRange
             EXPECT_EQ(expectedResults[i],
                       ipRuleExistsForRange(RULE_PRIORITY_UID_DEFAULT_NETWORK + subPriority,
                                            uidRanges[i], action));
+            EXPECT_EQ(expectedResults[i], ipRuleExistsForRange(RULE_PRIORITY_UID_LOCAL_ROUTES,
+                                                               uidRanges[i], action_local));
         }
     } else {
         std::string action = "unreachable";