From ed1848c6e32299d987d9dbd5b15c7bf0fb22d6ad Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Tue, 28 Mar 2023 18:08:12 +0900 Subject: [PATCH] Close sockets from ConnectivityService#setFirewallChainEnabled And replace netd.socketDestroy by Java implementation Bug: 270298713 Test: atest FrameworksNetTests CtsNetTestCases (cherry picked from https://android-review.googlesource.com/q/commit:d44a33adb958d973bb15c74635be6c15dbf8af88) Merged-In: I0e200247ca010f9649254eeaac02740bd2bfdb21 Change-Id: I0e200247ca010f9649254eeaac02740bd2bfdb21 --- .../src/com/android/server/BpfNetMaps.java | 60 ++++++++++++++++++- .../android/server/ConnectivityService.java | 35 +++++++++++ .../com/android/server/BpfNetMapsTest.java | 30 ++++++++++ .../server/ConnectivityServiceTest.java | 49 +++++++++++++++ 4 files changed, 173 insertions(+), 1 deletion(-) diff --git a/service/src/com/android/server/BpfNetMaps.java b/service/src/com/android/server/BpfNetMaps.java index 84e581edfe..ec168dd0ab 100644 --- a/service/src/com/android/server/BpfNetMaps.java +++ b/service/src/com/android/server/BpfNetMaps.java @@ -384,7 +384,6 @@ public class BpfNetMaps { * ALLOWLIST means the firewall denies all by default, uids must be explicitly allowed * DENYLIST means the firewall allows all by default, uids must be explicitly denyed */ - @VisibleForTesting public boolean isFirewallAllowList(final int chain) { switch (chain) { case FIREWALL_CHAIN_DOZABLE: @@ -745,6 +744,65 @@ public class BpfNetMaps { } } + private Set getUidsMatchEnabled(final int childChain) throws ErrnoException { + final long match = getMatchByFirewallChain(childChain); + Set uids = new ArraySet<>(); + synchronized (sUidOwnerMap) { + sUidOwnerMap.forEach((uid, val) -> { + if (val == null) { + Log.wtf(TAG, "sUidOwnerMap entry was deleted while holding a lock"); + } else { + if ((val.rule & match) != 0) { + uids.add(uid.val); + } + } + }); + } + return uids; + } + + /** + * Get uids that has FIREWALL_RULE_ALLOW on allowlist chain. + * Allowlist means the firewall denies all by default, uids must be explicitly allowed. + * + * Note that uids that has FIREWALL_RULE_DENY on allowlist chain can not be computed from the + * bpf map, since all the uids that does not have explicit FIREWALL_RULE_ALLOW rule in bpf map + * are determined to have FIREWALL_RULE_DENY. + * + * @param childChain target chain + * @return Set of uids + */ + public Set getUidsWithAllowRuleOnAllowListChain(final int childChain) + throws ErrnoException { + if (!isFirewallAllowList(childChain)) { + throw new IllegalArgumentException("getUidsWithAllowRuleOnAllowListChain is called with" + + " denylist chain:" + childChain); + } + // Corresponding match is enabled for uids that has FIREWALL_RULE_ALLOW on allowlist chain. + return getUidsMatchEnabled(childChain); + } + + /** + * Get uids that has FIREWALL_RULE_DENY on denylist chain. + * Denylist means the firewall allows all by default, uids must be explicitly denyed + * + * Note that uids that has FIREWALL_RULE_ALLOW on denylist chain can not be computed from the + * bpf map, since all the uids that does not have explicit FIREWALL_RULE_DENY rule in bpf map + * are determined to have the FIREWALL_RULE_ALLOW. + * + * @param childChain target chain + * @return Set of uids + */ + public Set getUidsWithDenyRuleOnDenyListChain(final int childChain) + throws ErrnoException { + if (isFirewallAllowList(childChain)) { + throw new IllegalArgumentException("getUidsWithDenyRuleOnDenyListChain is called with" + + " allowlist chain:" + childChain); + } + // Corresponding match is enabled for uids that has FIREWALL_RULE_DENY on denylist chain. + return getUidsMatchEnabled(childChain); + } + /** * Add ingress interface filtering rules to a list of UIDs * diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 5bab8e358d..8ec6eb9052 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1509,6 +1509,16 @@ public class ConnectivityService extends IConnectivityManager.Stub throws SocketException, InterruptedIOException, ErrnoException { InetDiagMessage.destroyLiveTcpSockets(ranges, exemptUids); } + + /** + * Call {@link InetDiagMessage#destroyLiveTcpSocketsByOwnerUids(Set)} + * + * @param ownerUids target uids to close sockets + */ + public void destroyLiveTcpSocketsByOwnerUids(final Set ownerUids) + throws SocketException, InterruptedIOException, ErrnoException { + InetDiagMessage.destroyLiveTcpSocketsByOwnerUids(ownerUids); + } } public ConnectivityService(Context context) { @@ -12002,6 +12012,23 @@ public class ConnectivityService extends IConnectivityManager.Stub return rule; } + private void closeSocketsForFirewallChainLocked(final int chain) + throws ErrnoException, SocketException, InterruptedIOException { + if (mBpfNetMaps.isFirewallAllowList(chain)) { + // Allowlist means the firewall denies all by default, uids must be explicitly allowed + // So, close all non-system socket owned by uids that are not explicitly allowed + Set> ranges = new ArraySet<>(); + ranges.add(new Range<>(Process.FIRST_APPLICATION_UID, Integer.MAX_VALUE)); + final Set exemptUids = mBpfNetMaps.getUidsWithAllowRuleOnAllowListChain(chain); + mDeps.destroyLiveTcpSockets(ranges, exemptUids); + } else { + // Denylist means the firewall allows all by default, uids must be explicitly denied + // So, close socket owned by uids that are explicitly denied + final Set ownerUids = mBpfNetMaps.getUidsWithDenyRuleOnDenyListChain(chain); + mDeps.destroyLiveTcpSocketsByOwnerUids(ownerUids); + } + } + @Override public void setFirewallChainEnabled(final int chain, final boolean enable) { enforceNetworkStackOrSettingsPermission(); @@ -12011,6 +12038,14 @@ public class ConnectivityService extends IConnectivityManager.Stub } catch (ServiceSpecificException e) { throw new IllegalStateException(e); } + + if (SdkLevel.isAtLeastU() && enable) { + try { + closeSocketsForFirewallChainLocked(chain); + } catch (ErrnoException | SocketException | InterruptedIOException e) { + Log.e(TAG, "Failed to close sockets after enabling chain (" + chain + "): " + e); + } + } } @Override diff --git a/tests/unit/java/com/android/server/BpfNetMapsTest.java b/tests/unit/java/com/android/server/BpfNetMapsTest.java index d189848bd0..19fa41df32 100644 --- a/tests/unit/java/com/android/server/BpfNetMapsTest.java +++ b/tests/unit/java/com/android/server/BpfNetMapsTest.java @@ -66,6 +66,7 @@ import android.net.INetd; import android.os.Build; import android.os.ServiceSpecificException; import android.system.ErrnoException; +import android.util.ArraySet; import android.util.IndentingPrintWriter; import androidx.test.filters.SmallTest; @@ -1151,4 +1152,33 @@ public final class BpfNetMapsTest { mCookieTagMap.updateEntry(new CookieTagMapKey(123), new CookieTagMapValue(456, 0x789)); assertDumpContains(getDump(), "cookie=123 tag=0x789 uid=456"); } + + @Test + public void testGetUids() throws ErrnoException { + final int uid0 = TEST_UIDS[0]; + final int uid1 = TEST_UIDS[1]; + final long match0 = DOZABLE_MATCH | POWERSAVE_MATCH; + final long match1 = DOZABLE_MATCH | STANDBY_MATCH; + mUidOwnerMap.updateEntry(new S32(uid0), new UidOwnerValue(NULL_IIF, match0)); + mUidOwnerMap.updateEntry(new S32(uid1), new UidOwnerValue(NULL_IIF, match1)); + + assertEquals(new ArraySet<>(List.of(uid0, uid1)), + mBpfNetMaps.getUidsWithAllowRuleOnAllowListChain(FIREWALL_CHAIN_DOZABLE)); + assertEquals(new ArraySet<>(List.of(uid0)), + mBpfNetMaps.getUidsWithAllowRuleOnAllowListChain(FIREWALL_CHAIN_POWERSAVE)); + + assertEquals(new ArraySet<>(List.of(uid1)), + mBpfNetMaps.getUidsWithDenyRuleOnDenyListChain(FIREWALL_CHAIN_STANDBY)); + assertEquals(new ArraySet<>(), + mBpfNetMaps.getUidsWithDenyRuleOnDenyListChain(FIREWALL_CHAIN_OEM_DENY_1)); + } + + @Test + public void testGetUidsIllegalArgument() { + final Class expected = IllegalArgumentException.class; + assertThrows(expected, + () -> mBpfNetMaps.getUidsWithDenyRuleOnDenyListChain(FIREWALL_CHAIN_DOZABLE)); + assertThrows(expected, + () -> mBpfNetMaps.getUidsWithAllowRuleOnAllowListChain(FIREWALL_CHAIN_OEM_DENY_1)); + } } diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index a90aa0d4d3..839625280a 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -2173,6 +2173,11 @@ public class ConnectivityServiceTest { final Set exemptUids) { // This function is empty since the invocation of this method is verified by mocks } + + @Override + public void destroyLiveTcpSocketsByOwnerUids(final Set ownerUids) { + // This function is empty since the invocation of this method is verified by mocks + } } private class AutomaticOnOffKeepaliveTrackerDependencies @@ -10244,6 +10249,50 @@ public class ConnectivityServiceTest { } } + private void doTestSetFirewallChainEnabledCloseSocket(final int chain, + final boolean isAllowList) throws Exception { + reset(mDeps); + + mCm.setFirewallChainEnabled(chain, true /* enabled */); + final Set uids = + new ArraySet<>(List.of(TEST_PACKAGE_UID, TEST_PACKAGE_UID2)); + if (isAllowList) { + final Set> range = new ArraySet<>( + List.of(new Range<>(Process.FIRST_APPLICATION_UID, Integer.MAX_VALUE))); + verify(mDeps).destroyLiveTcpSockets(range, uids); + } else { + verify(mDeps).destroyLiveTcpSocketsByOwnerUids(uids); + } + + mCm.setFirewallChainEnabled(chain, false /* enabled */); + verifyNoMoreInteractions(mDeps); + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testSetFirewallChainEnabledCloseSocket() throws Exception { + doReturn(new ArraySet<>(Arrays.asList(TEST_PACKAGE_UID, TEST_PACKAGE_UID2))) + .when(mBpfNetMaps) + .getUidsWithDenyRuleOnDenyListChain(anyInt()); + doReturn(new ArraySet<>(Arrays.asList(TEST_PACKAGE_UID, TEST_PACKAGE_UID2))) + .when(mBpfNetMaps) + .getUidsWithAllowRuleOnAllowListChain(anyInt()); + + final boolean allowlist = true; + final boolean denylist = false; + + doReturn(true).when(mBpfNetMaps).isFirewallAllowList(anyInt()); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_DOZABLE, allowlist); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_POWERSAVE, allowlist); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_RESTRICTED, allowlist); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_LOW_POWER_STANDBY, allowlist); + + doReturn(false).when(mBpfNetMaps).isFirewallAllowList(anyInt()); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_STANDBY, denylist); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_OEM_DENY_1, denylist); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_OEM_DENY_2, denylist); + doTestSetFirewallChainEnabledCloseSocket(FIREWALL_CHAIN_OEM_DENY_3, denylist); + } + private void doTestReplaceFirewallChain(final int chain) { final int[] uids = new int[] {1001, 1002}; mCm.replaceFirewallChain(chain, uids);