diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java index 5769b92642..d1c202cac3 100644 --- a/framework/src/android/net/ConnectivityManager.java +++ b/framework/src/android/net/ConnectivityManager.java @@ -982,16 +982,6 @@ public class ConnectivityManager { @SystemApi(client = MODULE_LIBRARIES) public static final int FIREWALL_CHAIN_LOW_POWER_STANDBY = 5; - /** - * Firewall chain used for lockdown VPN. - * Denylist of apps that cannot receive incoming packets except on loopback because they are - * subject to an always-on VPN which is not currently connected. - * - * @see #BLOCKED_REASON_LOCKDOWN_VPN - * @hide - */ - public static final int FIREWALL_CHAIN_LOCKDOWN_VPN = 6; - /** * Firewall chain used for OEM-specific application restrictions. * Denylist of apps that will not have network access due to OEM-specific restrictions. @@ -1021,7 +1011,6 @@ public class ConnectivityManager { FIREWALL_CHAIN_POWERSAVE, FIREWALL_CHAIN_RESTRICTED, FIREWALL_CHAIN_LOW_POWER_STANDBY, - FIREWALL_CHAIN_LOCKDOWN_VPN, FIREWALL_CHAIN_OEM_DENY_1, FIREWALL_CHAIN_OEM_DENY_2, FIREWALL_CHAIN_OEM_DENY_3 diff --git a/service/jni/com_android_server_BpfNetMaps.cpp b/service/jni/com_android_server_BpfNetMaps.cpp index bc70c93469..2780044ec6 100644 --- a/service/jni/com_android_server_BpfNetMaps.cpp +++ b/service/jni/com_android_server_BpfNetMaps.cpp @@ -151,6 +151,12 @@ static jint native_removeUidInterfaceRules(JNIEnv* env, jobject self, jintArray return (jint)status.code(); } +static jint native_updateUidLockdownRule(JNIEnv* env, jobject self, jint uid, jboolean add) { + Status status = mTc.updateUidLockdownRule(uid, add); + CHECK_LOG(status); + return (jint)status.code(); +} + static jint native_swapActiveStatsMap(JNIEnv* env, jobject self) { Status status = mTc.swapActiveStatsMap(); CHECK_LOG(status); @@ -203,6 +209,8 @@ static const JNINativeMethod gMethods[] = { (void*)native_addUidInterfaceRules}, {"native_removeUidInterfaceRules", "([I)I", (void*)native_removeUidInterfaceRules}, + {"native_updateUidLockdownRule", "(IZ)I", + (void*)native_updateUidLockdownRule}, {"native_swapActiveStatsMap", "()I", (void*)native_swapActiveStatsMap}, {"native_setPermissionForUids", "(I[I)V", diff --git a/service/native/TrafficController.cpp b/service/native/TrafficController.cpp index d05e6fa21b..8ad42c7c0b 100644 --- a/service/native/TrafficController.cpp +++ b/service/native/TrafficController.cpp @@ -340,8 +340,6 @@ FirewallType TrafficController::getFirewallType(ChildChain chain) { return ALLOWLIST; case LOW_POWER_STANDBY: return ALLOWLIST; - case LOCKDOWN: - return DENYLIST; case OEM_DENY_1: return DENYLIST; case OEM_DENY_2: @@ -373,9 +371,6 @@ int TrafficController::changeUidOwnerRule(ChildChain chain, uid_t uid, FirewallR case LOW_POWER_STANDBY: res = updateOwnerMapEntry(LOW_POWER_STANDBY_MATCH, uid, rule, type); break; - case LOCKDOWN: - res = updateOwnerMapEntry(LOCKDOWN_VPN_MATCH, uid, rule, type); - break; case OEM_DENY_1: res = updateOwnerMapEntry(OEM_DENY_1_MATCH, uid, rule, type); break; @@ -447,6 +442,18 @@ Status TrafficController::removeUidInterfaceRules(const std::vector& ui return netdutils::status::ok; } +Status TrafficController::updateUidLockdownRule(const uid_t uid, const bool add) { + std::lock_guard guard(mMutex); + + netdutils::Status result = add ? addRule(uid, LOCKDOWN_VPN_MATCH) + : removeRule(uid, LOCKDOWN_VPN_MATCH); + if (!isOk(result)) { + ALOGW("%s Lockdown rule failed(%d): uid=%d", + (add ? "add": "remove"), result.code(), uid); + } + return result; +} + int TrafficController::replaceUidOwnerMap(const std::string& name, bool isAllowlist __unused, const std::vector& uids) { // FirewallRule rule = isAllowlist ? ALLOW : DENY; diff --git a/service/native/TrafficControllerTest.cpp b/service/native/TrafficControllerTest.cpp index 1aca633644..0b4550e227 100644 --- a/service/native/TrafficControllerTest.cpp +++ b/service/native/TrafficControllerTest.cpp @@ -215,7 +215,7 @@ class TrafficControllerTest : public ::testing::Test { checkEachUidValue(uids, match); } - void expectUidOwnerMapValues(const std::vector& appUids, uint8_t expectedRule, + void expectUidOwnerMapValues(const std::vector& appUids, uint32_t expectedRule, uint32_t expectedIif) { for (uint32_t uid : appUids) { Result value = mFakeUidOwnerMap.readValue(uid); @@ -389,7 +389,6 @@ TEST_F(TrafficControllerTest, TestChangeUidOwnerRule) { checkUidOwnerRuleForChain(POWERSAVE, POWERSAVE_MATCH); checkUidOwnerRuleForChain(RESTRICTED, RESTRICTED_MATCH); checkUidOwnerRuleForChain(LOW_POWER_STANDBY, LOW_POWER_STANDBY_MATCH); - checkUidOwnerRuleForChain(LOCKDOWN, LOCKDOWN_VPN_MATCH); checkUidOwnerRuleForChain(OEM_DENY_1, OEM_DENY_1_MATCH); checkUidOwnerRuleForChain(OEM_DENY_2, OEM_DENY_2_MATCH); checkUidOwnerRuleForChain(OEM_DENY_3, OEM_DENY_3_MATCH); @@ -521,6 +520,21 @@ TEST_F(TrafficControllerTest, TestRemoveUidInterfaceFilteringRules) { expectMapEmpty(mFakeUidOwnerMap); } +TEST_F(TrafficControllerTest, TestUpdateUidLockdownRule) { + // Add Lockdown rules + ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1000, true /* add */))); + ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1001, true /* add */))); + expectUidOwnerMapValues({1000, 1001}, LOCKDOWN_VPN_MATCH, 0); + + // Remove one of Lockdown rules + ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1000, false /* add */))); + expectUidOwnerMapValues({1001}, LOCKDOWN_VPN_MATCH, 0); + + // Remove remaining Lockdown rule + ASSERT_TRUE(isOk(mTc.updateUidLockdownRule(1001, false /* add */))); + expectMapEmpty(mFakeUidOwnerMap); +} + TEST_F(TrafficControllerTest, TestUidInterfaceFilteringRulesCoexistWithExistingMatches) { // Set up existing PENALTY_BOX_MATCH rules ASSERT_TRUE(isOk(updateUidOwnerMaps({1000, 1001, 10012}, PENALTY_BOX_MATCH, @@ -802,7 +816,6 @@ TEST_F(TrafficControllerTest, getFirewallType) { {POWERSAVE, ALLOWLIST}, {RESTRICTED, ALLOWLIST}, {LOW_POWER_STANDBY, ALLOWLIST}, - {LOCKDOWN, DENYLIST}, {OEM_DENY_1, DENYLIST}, {OEM_DENY_2, DENYLIST}, {INVALID_CHAIN, DENYLIST}, diff --git a/service/native/include/Common.h b/service/native/include/Common.h index 2427aa906c..3f28991bea 100644 --- a/service/native/include/Common.h +++ b/service/native/include/Common.h @@ -35,7 +35,6 @@ enum ChildChain { POWERSAVE = 3, RESTRICTED = 4, LOW_POWER_STANDBY = 5, - LOCKDOWN = 6, OEM_DENY_1 = 7, OEM_DENY_2 = 8, OEM_DENY_3 = 9, diff --git a/service/native/include/TrafficController.h b/service/native/include/TrafficController.h index c019ce78b0..c921ff2df0 100644 --- a/service/native/include/TrafficController.h +++ b/service/native/include/TrafficController.h @@ -71,6 +71,8 @@ class TrafficController { EXCLUDES(mMutex); netdutils::Status removeUidInterfaceRules(const std::vector& uids) EXCLUDES(mMutex); + netdutils::Status updateUidLockdownRule(const uid_t uid, const bool add) EXCLUDES(mMutex); + netdutils::Status updateUidOwnerMap(const uint32_t uid, UidOwnerMatchType matchType, IptOp op) EXCLUDES(mMutex); diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java index c006bc605f..151d0e3da4 100644 --- a/service/src/com/android/server/BpfNetMaps.java +++ b/service/src/com/android/server/BpfNetMaps.java @@ -215,6 +215,19 @@ public class BpfNetMaps { maybeThrow(err, "Unable to remove uid interface rules"); } + /** + * Update lockdown rule for uid + * + * @param uid target uid to add/remove the rule + * @param add {@code true} to add the rule, {@code false} to remove the rule. + * @throws ServiceSpecificException in case of failure, with an error code indicating the + * cause of the failure. + */ + public void updateUidLockdownRule(final int uid, final boolean add) { + final int err = native_updateUidLockdownRule(uid, add); + maybeThrow(err, "Unable to update lockdown rule"); + } + /** * Request netd to change the current active network stats map. * @@ -271,6 +284,7 @@ public class BpfNetMaps { private native int native_setUidRule(int childChain, int uid, int firewallRule); private native int native_addUidInterfaceRules(String ifName, int[] uids); private native int native_removeUidInterfaceRules(int[] uids); + private native int native_updateUidLockdownRule(int uid, boolean add); private native int native_swapActiveStatsMap(); private native void native_setPermissionForUids(int permissions, int[] uids); private native void native_dump(FileDescriptor fd, boolean verbose); diff --git a/service/src/com/android/server/connectivity/PermissionMonitor.java b/service/src/com/android/server/connectivity/PermissionMonitor.java index e4a2c20bd5..10db088a53 100755 --- a/service/src/com/android/server/connectivity/PermissionMonitor.java +++ b/service/src/com/android/server/connectivity/PermissionMonitor.java @@ -23,9 +23,6 @@ import static android.Manifest.permission.NETWORK_STACK; import static android.Manifest.permission.UPDATE_DEVICE_STATS; import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; import static android.content.pm.PackageManager.GET_PERMISSIONS; -import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOCKDOWN_VPN; -import static android.net.ConnectivityManager.FIREWALL_RULE_ALLOW; -import static android.net.ConnectivityManager.FIREWALL_RULE_DENY; import static android.net.ConnectivitySettingsManager.UIDS_ALLOWED_ON_RESTRICTED_NETWORKS; import static android.net.INetd.PERMISSION_INTERNET; import static android.net.INetd.PERMISSION_NETWORK; @@ -1079,11 +1076,7 @@ public class PermissionMonitor { private void updateLockdownUidRule(int uid, boolean add) { try { - if (add) { - mBpfNetMaps.setUidRule(FIREWALL_CHAIN_LOCKDOWN_VPN, uid, FIREWALL_RULE_DENY); - } else { - mBpfNetMaps.setUidRule(FIREWALL_CHAIN_LOCKDOWN_VPN, uid, FIREWALL_RULE_ALLOW); - } + mBpfNetMaps.updateUidLockdownRule(uid, add); } catch (ServiceSpecificException e) { loge("Failed to " + (add ? "add" : "remove") + " Lockdown rule: " + e); } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index e84df162e1..b0b07505b1 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -52,7 +52,6 @@ import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.EXTRA_NETWORK_INFO; import static android.net.ConnectivityManager.EXTRA_NETWORK_TYPE; import static android.net.ConnectivityManager.FIREWALL_CHAIN_DOZABLE; -import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOCKDOWN_VPN; import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOW_POWER_STANDBY; import static android.net.ConnectivityManager.FIREWALL_CHAIN_OEM_DENY_1; import static android.net.ConnectivityManager.FIREWALL_CHAIN_OEM_DENY_2; @@ -9531,10 +9530,8 @@ public class ConnectivityServiceTest { waitForIdle(); // Lockdown rule is set to apps uids - verify(mBpfNetMaps).setUidRule( - eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP1_UID), eq(FIREWALL_RULE_DENY)); - verify(mBpfNetMaps).setUidRule( - eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP2_UID), eq(FIREWALL_RULE_DENY)); + verify(mBpfNetMaps).updateUidLockdownRule(APP1_UID, true /* add */); + verify(mBpfNetMaps).updateUidLockdownRule(APP2_UID, true /* add */); reset(mBpfNetMaps); @@ -9544,10 +9541,8 @@ public class ConnectivityServiceTest { waitForIdle(); // Lockdown rule is removed from apps uids - verify(mBpfNetMaps).setUidRule( - eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP1_UID), eq(FIREWALL_RULE_ALLOW)); - verify(mBpfNetMaps).setUidRule( - eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(APP2_UID), eq(FIREWALL_RULE_ALLOW)); + verify(mBpfNetMaps).updateUidLockdownRule(APP1_UID, false /* add */); + verify(mBpfNetMaps).updateUidLockdownRule(APP2_UID, false /* add */); // Interface rules are not changed by Lockdown mode enable/disable verify(mBpfNetMaps, never()).addUidInterfaceRules(any(), any()); diff --git a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java index ecd17bab69..7716522fea 100644 --- a/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/tests/unit/java/com/android/server/connectivity/PermissionMonitorTest.java @@ -30,9 +30,6 @@ import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_REQUIRED; import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; -import static android.net.ConnectivityManager.FIREWALL_CHAIN_LOCKDOWN_VPN; -import static android.net.ConnectivityManager.FIREWALL_RULE_ALLOW; -import static android.net.ConnectivityManager.FIREWALL_RULE_DENY; import static android.net.ConnectivitySettingsManager.UIDS_ALLOWED_ON_RESTRICTED_NETWORKS; import static android.net.INetd.PERMISSION_INTERNET; import static android.net.INetd.PERMISSION_NETWORK; @@ -872,19 +869,14 @@ public class PermissionMonitorTest { // Add Lockdown uid range, expect a rule to be set up for user app MOCK_UID11 mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange1); - verify(mBpfNetMaps) - .setUidRule( - eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_DENY)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */); assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange1)); reset(mBpfNetMaps); // Remove Lockdown uid range, expect rules to be torn down mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange1); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_ALLOW)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */); assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty()); } @@ -902,9 +894,7 @@ public class PermissionMonitorTest { // Add Lockdown uid range at 1st time, expect a rule to be set up mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_DENY)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */); assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange)); reset(mBpfNetMaps); @@ -912,7 +902,7 @@ public class PermissionMonitorTest { // Add Lockdown uid range at 2nd time, expect a rule not to be set up because the uid // already has the rule mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRange); - verify(mBpfNetMaps, never()).setUidRule(anyInt(), anyInt(), anyInt()); + verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean()); assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange)); reset(mBpfNetMaps); @@ -920,7 +910,7 @@ public class PermissionMonitorTest { // Remove Lockdown uid range at 1st time, expect a rule not to be torn down because we added // the range 2 times. mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange); - verify(mBpfNetMaps, never()).setUidRule(anyInt(), anyInt(), anyInt()); + verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean()); assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange)); reset(mBpfNetMaps); @@ -928,9 +918,7 @@ public class PermissionMonitorTest { // Remove Lockdown uid range at 2nd time, expect a rule to be torn down because we added // twice and we removed twice. mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_ALLOW)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */); assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty()); } @@ -949,9 +937,7 @@ public class PermissionMonitorTest { // Add Lockdown uid ranges which contains duplicated uid ranges mPermissionMonitor.updateVpnLockdownUidRanges(true /* add */, vpnRangeDuplicates); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_DENY)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */); assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange)); reset(mBpfNetMaps); @@ -959,16 +945,14 @@ public class PermissionMonitorTest { // Remove Lockdown uid range at 1st time, expect a rule not to be torn down because uid // ranges we added contains duplicated uid ranges. mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange); - verify(mBpfNetMaps, never()).setUidRule(anyInt(), anyInt(), anyInt()); + verify(mBpfNetMaps, never()).updateUidLockdownRule(anyInt(), anyBoolean()); assertEquals(mPermissionMonitor.getVpnLockdownUidRanges(), Set.of(vpnRange)); reset(mBpfNetMaps); // Remove Lockdown uid range at 2nd time, expect a rule to be torn down. mPermissionMonitor.updateVpnLockdownUidRanges(false /* false */, vpnRange); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_ALLOW)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */); assertTrue(mPermissionMonitor.getVpnLockdownUidRanges().isEmpty()); } @@ -989,23 +973,15 @@ public class PermissionMonitorTest { // Installing package should add Lockdown rules addPackageForUsers(new UserHandle[]{MOCK_USER1, MOCK_USER2}, MOCK_PACKAGE1, MOCK_APPID1); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_DENY)); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID21), - eq(FIREWALL_RULE_DENY)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, true /* add */); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID21, true /* add */); reset(mBpfNetMaps); // Uninstalling package should remove Lockdown rules mPermissionMonitor.onPackageRemoved(MOCK_PACKAGE1, MOCK_UID11); - verify(mBpfNetMaps) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID11), - eq(FIREWALL_RULE_ALLOW)); - verify(mBpfNetMaps, never()) - .setUidRule(eq(FIREWALL_CHAIN_LOCKDOWN_VPN), eq(MOCK_UID21), - eq(FIREWALL_RULE_ALLOW)); + verify(mBpfNetMaps).updateUidLockdownRule(MOCK_UID11, false /* add */); + verify(mBpfNetMaps, never()).updateUidLockdownRule(MOCK_UID21, false /* add */); } // Normal package add/remove operations will trigger multiple intent for uids corresponding to