From d22c7588d7242604091a8efee39f79bb4b2bef70 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 18 Mar 2021 20:57:11 +0900 Subject: Test a bug with NETWORK_SETTINGS+registerDefaultNetworkCallback. Currently, if a process with NETWORK_SETTINGS registers a default network callback, its uid will be ignored and replaced with an empty list of UIDs. This means it will incorrectly match VPNs with any UID range. Add a test for this bug to make it easier to review the upcoming change that fixes it. Bug: 165835257 Test: test-only change Change-Id: If58524b01fdd60045fb7236d17dedf31fb563f99 --- .../com/android/server/ConnectivityServiceTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'tests/net/java/com/android/server/ConnectivityServiceTest.java') diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index cc1bee5a5104..2ec8707000b6 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -7487,6 +7487,9 @@ public class ConnectivityServiceTest { final NetworkRequest vpnUidRequest = new NetworkRequest.Builder().build(); registerNetworkCallbackAsUid(vpnUidRequest, vpnUidCallback, VPN_UID); + final TestNetworkCallback vpnUidDefaultCallback = new TestNetworkCallback(); + registerDefaultNetworkCallbackAsUid(vpnUidDefaultCallback, VPN_UID); + final int uid = Process.myUid(); final int userId = UserHandle.getUserId(uid); final ArrayList allowList = new ArrayList<>(); @@ -7505,6 +7508,7 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); vpnUidCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + vpnUidDefaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); @@ -7517,6 +7521,7 @@ public class ConnectivityServiceTest { callback.expectBlockedStatusCallback(false, mWiFiNetworkAgent); defaultCallback.expectBlockedStatusCallback(false, mWiFiNetworkAgent); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); expectNetworkRejectNonSecureVpn(inOrder, false, firstHalf, secondHalf); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); @@ -7531,6 +7536,7 @@ public class ConnectivityServiceTest { callback.assertNoCallback(); defaultCallback.assertNoCallback(); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); // The following requires that the UID of this test package is greater than VPN_UID. This // is always true in practice because a plain AOSP build with no apps installed has almost @@ -7551,6 +7557,7 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); defaultCallback.assertNoCallback(); vpnUidCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); + vpnUidDefaultCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -7571,6 +7578,7 @@ public class ConnectivityServiceTest { defaultCallback.expectBlockedStatusCallback(true, mWiFiNetworkAgent); assertBlockedCallbackInAnyOrder(callback, true, mWiFiNetworkAgent, mCellNetworkAgent); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); @@ -7582,6 +7590,7 @@ public class ConnectivityServiceTest { defaultCallback.expectBlockedStatusCallback(false, mWiFiNetworkAgent); assertBlockedCallbackInAnyOrder(callback, false, mWiFiNetworkAgent, mCellNetworkAgent); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -7596,6 +7605,7 @@ public class ConnectivityServiceTest { callback.assertNoCallback(); defaultCallback.assertNoCallback(); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -7607,6 +7617,7 @@ public class ConnectivityServiceTest { callback.assertNoCallback(); defaultCallback.assertNoCallback(); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -7619,6 +7630,7 @@ public class ConnectivityServiceTest { defaultCallback.expectBlockedStatusCallback(true, mWiFiNetworkAgent); assertBlockedCallbackInAnyOrder(callback, true, mWiFiNetworkAgent, mCellNetworkAgent); vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.assertNoCallback(); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertNull(mCm.getActiveNetwork()); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.BLOCKED); @@ -7629,6 +7641,8 @@ public class ConnectivityServiceTest { assertUidRangesUpdatedForMyUid(true); defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); vpnUidCallback.assertNoCallback(); // vpnUidCallback has NOT_VPN capability. + // TODO: this is a bug. The VPN does not apply to VPN_UID. + vpnUidDefaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -7639,11 +7653,14 @@ public class ConnectivityServiceTest { mMockVpn.disconnect(); defaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn); defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); + vpnUidCallback.assertNoCallback(); + vpnUidDefaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn); // BUG! assertNull(mCm.getActiveNetwork()); mCm.unregisterNetworkCallback(callback); mCm.unregisterNetworkCallback(defaultCallback); mCm.unregisterNetworkCallback(vpnUidCallback); + mCm.unregisterNetworkCallback(vpnUidDefaultCallback); } private void setupLegacyLockdownVpn() { -- cgit v1.2.3 From ae67988721788b99b6150aeb8904c1c9d432fb0e Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 22 Mar 2021 02:12:04 +0900 Subject: Fix privileged apps calling registerDefaultNetworkCallback. When registerDefaultNetworkCallback is called by an app that has NETWORK_SETTINGS, the UID of the app is forgotten and the request that is filed has an empty UID set. This results in that request matching networks that have UID ranges that do not include it, e.g., VPNs. Fix this by ensuring that the UID ranges are properly set. Bug: 165835257 Test: updated specific tests for this bug Change-Id: I90bf79573342c144d1cfbc2f61a3155fdd5b1fa7 --- tests/net/java/com/android/server/ConnectivityServiceTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'tests/net/java/com/android/server/ConnectivityServiceTest.java') diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 2ec8707000b6..ed9a44b614d5 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -7641,8 +7641,7 @@ public class ConnectivityServiceTest { assertUidRangesUpdatedForMyUid(true); defaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); vpnUidCallback.assertNoCallback(); // vpnUidCallback has NOT_VPN capability. - // TODO: this is a bug. The VPN does not apply to VPN_UID. - vpnUidDefaultCallback.expectAvailableThenValidatedCallbacks(mMockVpn); + vpnUidDefaultCallback.assertNoCallback(); // VPN does not apply to VPN_UID assertEquals(mMockVpn.getNetwork(), mCm.getActiveNetwork()); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetworkForUid(VPN_UID)); assertActiveNetworkInfo(TYPE_WIFI, DetailedState.CONNECTED); @@ -7654,7 +7653,7 @@ public class ConnectivityServiceTest { defaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn); defaultCallback.expectAvailableCallbacksUnvalidatedAndBlocked(mWiFiNetworkAgent); vpnUidCallback.assertNoCallback(); - vpnUidDefaultCallback.expectCallback(CallbackEntry.LOST, mMockVpn); // BUG! + vpnUidDefaultCallback.assertNoCallback(); assertNull(mCm.getActiveNetwork()); mCm.unregisterNetworkCallback(callback); -- cgit v1.2.3