From bf91f5f182b9a3d54e40ff3b1848a40d2c1f257f Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 3 Dec 2019 22:16:26 +0900 Subject: [PATCH 1/8] [NS A37] Don't reassign requests multiple times This is an optimization that skips doing intermediate assignments of networks to requests that will undergo multiple changes during the recomputation. It happens to fix a bug where some of these intermediate states used to have a visible, transient side effect. Bug: 113554781 Test: ConnectivityServiceTest Change-Id: I7af3728152a1cf7571de67f394088a5970ee3c1e --- .../android/server/ConnectivityService.java | 32 +++++++++++++------ .../server/ConnectivityServiceTest.java | 8 ++--- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0e17b23249..25e34e2786 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6452,18 +6452,37 @@ public class ConnectivityService extends IConnectivityManager.Stub } @NonNull private final Set mRematchedNetworks = new ArraySet<>(); - @NonNull private final List mReassignments = new ArrayList<>(); + @NonNull private final Map mReassignments = + new ArrayMap<>(); @NonNull Iterable getRematchedNetworks() { return mRematchedNetworks; } @NonNull Iterable getRequestReassignments() { - return mReassignments; + return mReassignments.values(); } void addRequestReassignment(@NonNull final RequestReassignment reassignment) { - mReassignments.add(reassignment); + final RequestReassignment oldChange = mReassignments.get(reassignment.mRequest); + if (null == oldChange) { + mReassignments.put(reassignment.mRequest, reassignment); + return; + } + if (oldChange.mNewNetwork != reassignment.mOldNetwork) { + throw new IllegalArgumentException("Reassignment <" + reassignment.mRequest + "> [" + + reassignment.mOldNetwork + " -> " + reassignment.mNewNetwork + + "] conflicts with [" + + oldChange.mOldNetwork + " -> " + oldChange.mNewNetwork + "]"); + } + // There was already a note to reassign this request from a network A to a network B, + // and a reassignment is added from network B to some other network C. The following + // synthesizes the merged reassignment that goes A -> C. An interesting (but not + // special) case to think about is when B is null, which can happen when the rematch + // loop notices the current satisfier doesn't satisfy the request any more, but + // hasn't yet encountered another network that could. + mReassignments.put(reassignment.mRequest, new RequestReassignment(reassignment.mRequest, + oldChange.mOldNetwork, reassignment.mNewNetwork)); } void addRematchedNetwork(@NonNull final NetworkBgStatePair network) { @@ -6653,13 +6672,6 @@ public class ConnectivityService extends IConnectivityManager.Stub if (null != event.mNewNetwork) { notifyNetworkAvailable(event.mNewNetwork, event.mRequest); } else { - // TODO: Technically, sending CALLBACK_LOST here is - // incorrect if there is a replacement network currently - // connected that can satisfy nri, which is a request - // (not a listen). However, the only capability that can both - // a) be requested and b) change is NET_CAPABILITY_TRUSTED, - // so this code is only incorrect for a network that loses - // the TRUSTED capability, which is a rare case. callCallbackForRequest(event.mRequest, event.mOldNetwork, ConnectivityManager.CALLBACK_LOST, 0); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 1442b31aa8..072d336bc6 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -5753,20 +5753,18 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(true); trustedCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); verify(mNetworkManagementService).setDefaultNetId(eq(mCellNetworkAgent.getNetwork().netId)); + reset(mNetworkManagementService); mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(true); trustedCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); verify(mNetworkManagementService).setDefaultNetId(eq(mWiFiNetworkAgent.getNetwork().netId)); + reset(mNetworkManagementService); mWiFiNetworkAgent.removeCapability(NET_CAPABILITY_TRUSTED); - // There is currently a bug where losing the TRUSTED capability will send a LOST - // callback to requests before the available callback, in spite of the semantics - // of the requests dictating this should not happen. This is considered benign, but - // ideally should be fixed in the future. - trustedCallback.expectCallback(CallbackEntry.LOST, mWiFiNetworkAgent); trustedCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); verify(mNetworkManagementService).setDefaultNetId(eq(mCellNetworkAgent.getNetwork().netId)); + reset(mNetworkManagementService); mCellNetworkAgent.removeCapability(NET_CAPABILITY_TRUSTED); trustedCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent); From 84292aaa46b90465ebbcfd88e6569a4571b0744d Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 3 Dec 2019 23:02:47 +0900 Subject: [PATCH 2/8] [NS A38] Fill the initial reassignment This is more expensive for now but it will allow subsequent patches to read relevant state from the reassignment instead of from the global state, which will stop the computation of the reassignment from reading state that is mutated by the same loop. Eventually this lets us completely split the computation from the side effects. The ugly parts of this patch will be cleaned up later as a result, namely in patches [NS B04] and [NS B05]. Test: ConnectivityServiceTest Change-Id: I271e7f4d4bc81493c1ea212025b7130619592a8a --- .../android/server/ConnectivityService.java | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 25e34e2786..b567887b0e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6500,6 +6500,16 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // TODO : remove this when it's useless + @NonNull private NetworkReassignment computeInitialReassignment() { + final NetworkReassignment change = new NetworkReassignment(); + for (NetworkRequestInfo nri : mNetworkRequests.values()) { + change.addRequestReassignment(new NetworkReassignment.RequestReassignment(nri, + nri.mSatisfier, nri.mSatisfier)); + } + return change; + } + private ArrayMap computeRequestReassignmentForNetwork( @NonNull final NetworkAgentInfo newNetwork) { final int score = newNetwork.getCurrentScore(); @@ -6541,19 +6551,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // satisfied by newNetwork, and reassigns to newNetwork // any such requests for which newNetwork is the best. // - // - Lingers any validated Networks that as a result are no longer - // needed. A network is needed if it is the best network for - // one or more NetworkRequests, or if it is a VPN. - // // - Writes into the passed reassignment object all changes that should be done for // rematching this network with all requests, to be applied later. // - // NOTE: This function only adds NetworkRequests that "newNetwork" could satisfy, - // it does not remove NetworkRequests that other Networks could better satisfy. - // If you need to handle decreases in score, use {@link rematchAllNetworksAndRequests}. - // This function should be used when possible instead of {@code rematchAllNetworksAndRequests} - // as it performs better by a factor of the number of Networks. - // // TODO : stop writing to the passed reassignment. This is temporarily more useful, but // it's unidiomatic Java and it's hard to read. // @@ -6634,7 +6634,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // scoring network and then a higher scoring network, which could produce multiple // callbacks. Arrays.sort(nais); - final NetworkReassignment changes = new NetworkReassignment(); + final NetworkReassignment changes = computeInitialReassignment(); for (final NetworkAgentInfo nai : nais) { rematchNetworkAndRequests(changes, nai, now); } @@ -6663,6 +6663,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // before LegacyTypeTracker sends legacy broadcasts for (final NetworkReassignment.RequestReassignment event : changes.getRequestReassignments()) { + if (event.mOldNetwork == event.mNewNetwork) continue; + // Tell NetworkProviders about the new score, so they can stop // trying to connect if they know they cannot match it. // TODO - this could get expensive if there are a lot of outstanding requests for this From 933ebfa503b6d8bd891c9777931a95cc0620f47d Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 3 Dec 2019 23:31:17 +0900 Subject: [PATCH 3/8] [NS A39] Simplification If newNetwork is satisfying this request, it means it is the old satisfier. Plain and simple. Test: ConnectivityServiceTest Change-Id: Ic1a5d032801bac476b1c1f53da6f1c4c6056bff0 --- services/core/java/com/android/server/ConnectivityService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b567887b0e..24a92d07e5 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6538,7 +6538,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (currentNetwork == null || currentNetwork.getCurrentScore() < score) { reassignedRequests.put(nri, newNetwork); } - } else if (newNetwork.isSatisfyingRequest(nri.request.requestId)) { + } else if (newNetwork == currentNetwork) { reassignedRequests.put(nri, null); } } From a73199168b4fb3fbbd65c1f5f252f3b66c5c2fb0 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Wed, 6 Nov 2019 00:20:15 -0800 Subject: [PATCH 4/8] Add separate user consent for Platform VPNs This change adds a new VPN user consent flow (using the same text) for granting the lesser OP_ACTIVATE_PLATFORM_VPN. A new PlatformVpnConfirmDialog is created as a subclass to preserve all logic, but ensure the right appop is granted for the relevant dialog. Intent extras were considered, but are inherently unsafe, since the caller may add any extras that they would want. Bug: 144246835 Test: FrameworksNetTests passing Change-Id: Ia6f36207d43c3748f938430c2780dcf29e5623f3 Merged-In: Ia6f36207d43c3748f938430c2780dcf29e5623f3 --- .../android/net/IConnectivityManager.aidl | 2 +- .../android/server/ConnectivityService.java | 27 ++++++---- .../net/java/android/net/VpnManagerTest.java | 11 +++- .../android/server/connectivity/VpnTest.java | 50 ++++++++++++++++++- 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index b050e479d2..06b18a6467 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -116,7 +116,7 @@ interface IConnectivityManager boolean prepareVpn(String oldPackage, String newPackage, int userId); - void setVpnPackageAuthorization(String packageName, int userId, boolean authorized); + void setVpnPackageAuthorization(String packageName, int userId, int vpnType); ParcelFileDescriptor establishVpn(in VpnConfig config); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index caf185145f..44932fc01e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -112,6 +112,7 @@ import android.net.SocketKeepalive; import android.net.TetheringManager; import android.net.UidRange; import android.net.Uri; +import android.net.VpnManager; import android.net.VpnService; import android.net.metrics.IpConnectivityLog; import android.net.metrics.NetworkEvent; @@ -4293,7 +4294,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throwIfLockdownEnabled(); Vpn vpn = mVpns.get(userId); if (vpn != null) { - return vpn.prepare(oldPackage, newPackage, false); + return vpn.prepare(oldPackage, newPackage, VpnManager.TYPE_VPN_SERVICE); } else { return false; } @@ -4301,26 +4302,29 @@ public class ConnectivityService extends IConnectivityManager.Stub } /** - * Set whether the VPN package has the ability to launch VPNs without user intervention. - * This method is used by system-privileged apps. - * VPN permissions are checked in the {@link Vpn} class. If the caller is not {@code userId}, - * {@link android.Manifest.permission.INTERACT_ACROSS_USERS_FULL} permission is required. + * Set whether the VPN package has the ability to launch VPNs without user intervention. This + * method is used by system-privileged apps. VPN permissions are checked in the {@link Vpn} + * class. If the caller is not {@code userId}, {@link + * android.Manifest.permission.INTERACT_ACROSS_USERS_FULL} permission is required. * * @param packageName The package for which authorization state should change. * @param userId User for whom {@code packageName} is installed. * @param authorized {@code true} if this app should be able to start a VPN connection without - * explicit user approval, {@code false} if not. - * + * explicit user approval, {@code false} if not. + * @param vpnType The {@link VpnManager.VpnType} constant representing what class of VPN + * permissions should be granted. When unauthorizing an app, {@link + * VpnManager.TYPE_VPN_NONE} should be used. * @hide */ @Override - public void setVpnPackageAuthorization(String packageName, int userId, boolean authorized) { + public void setVpnPackageAuthorization( + String packageName, int userId, @VpnManager.VpnType int vpnType) { enforceCrossUserPermission(userId); synchronized (mVpns) { Vpn vpn = mVpns.get(userId); if (vpn != null) { - vpn.setPackageAuthorization(packageName, authorized); + vpn.setPackageAuthorization(packageName, vpnType); } } } @@ -7193,7 +7197,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final String alwaysOnPackage = getAlwaysOnVpnPackage(userId); if (alwaysOnPackage != null) { setAlwaysOnVpnPackage(userId, null, false, null); - setVpnPackageAuthorization(alwaysOnPackage, userId, false); + setVpnPackageAuthorization(alwaysOnPackage, userId, VpnManager.TYPE_VPN_NONE); } // Turn Always-on VPN off @@ -7216,7 +7220,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { // Prevent this app (packagename = vpnConfig.user) from initiating // VPN connections in the future without user intervention. - setVpnPackageAuthorization(vpnConfig.user, userId, false); + setVpnPackageAuthorization( + vpnConfig.user, userId, VpnManager.TYPE_VPN_NONE); prepareVpn(null, VpnConfig.LEGACY_VPN, userId); } diff --git a/tests/net/java/android/net/VpnManagerTest.java b/tests/net/java/android/net/VpnManagerTest.java index 97551c94e2..95a794235a 100644 --- a/tests/net/java/android/net/VpnManagerTest.java +++ b/tests/net/java/android/net/VpnManagerTest.java @@ -16,6 +16,7 @@ package android.net; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.mockito.Matchers.any; @@ -24,6 +25,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.content.ComponentName; +import android.content.Intent; import android.test.mock.MockContext; import androidx.test.filters.SmallTest; @@ -78,7 +81,13 @@ public class VpnManagerTest { when(mMockCs.provisionVpnProfile(any(VpnProfile.class), eq(PKG_NAME))).thenReturn(false); // Expect intent to be returned, as consent has not already been granted. - assertNotNull(mVpnManager.provisionVpnProfile(profile)); + final Intent intent = mVpnManager.provisionVpnProfile(profile); + assertNotNull(intent); + + final ComponentName expectedComponentName = + ComponentName.unflattenFromString( + "com.android.vpndialogs/com.android.vpndialogs.PlatformVpnConfirmDialog"); + assertEquals(expectedComponentName, intent.getComponent()); verify(mMockCs).provisionVpnProfile(eq(profile.toVpnProfile()), eq(PKG_NAME)); } diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 084ec73302..155c61f3f8 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -63,6 +63,7 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo.DetailedState; import android.net.UidRange; +import android.net.VpnManager; import android.net.VpnService; import android.os.Build.VERSION_CODES; import android.os.Bundle; @@ -471,12 +472,12 @@ public class VpnTest { order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); // When a new VPN package is set the rules should change to cover that package. - vpn.prepare(null, PKGS[0], false /* isPlatformVpn */); + vpn.prepare(null, PKGS[0], VpnManager.TYPE_VPN_SERVICE); order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(entireUser)); order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(exceptPkg0)); // When that VPN package is unset, everything should be undone again in reverse. - vpn.prepare(null, VpnConfig.LEGACY_VPN, false /* isPlatformVpn */); + vpn.prepare(null, VpnConfig.LEGACY_VPN, VpnManager.TYPE_VPN_SERVICE); order.verify(mNetService).setAllowOnlyVpnForUids(eq(false), aryEq(exceptPkg0)); order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); } @@ -817,6 +818,51 @@ public class VpnTest { eq(TEST_VPN_PKG)); } + @Test + public void testSetPackageAuthorizationVpnService() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + assertTrue(vpn.setPackageAuthorization(TEST_VPN_PKG, VpnManager.TYPE_VPN_SERVICE)); + verify(mAppOps) + .setMode( + eq(AppOpsManager.OP_ACTIVATE_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG), + eq(AppOpsManager.MODE_ALLOWED)); + } + + @Test + public void testSetPackageAuthorizationPlatformVpn() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + assertTrue(vpn.setPackageAuthorization(TEST_VPN_PKG, VpnManager.TYPE_VPN_PLATFORM)); + verify(mAppOps) + .setMode( + eq(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG), + eq(AppOpsManager.MODE_ALLOWED)); + } + + @Test + public void testSetPackageAuthorizationRevokeAuthorization() throws Exception { + final Vpn vpn = createVpnAndSetupUidChecks(); + + assertTrue(vpn.setPackageAuthorization(TEST_VPN_PKG, VpnManager.TYPE_VPN_NONE)); + verify(mAppOps) + .setMode( + eq(AppOpsManager.OP_ACTIVATE_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG), + eq(AppOpsManager.MODE_IGNORED)); + verify(mAppOps) + .setMode( + eq(AppOpsManager.OP_ACTIVATE_PLATFORM_VPN), + eq(Process.myUid()), + eq(TEST_VPN_PKG), + eq(AppOpsManager.MODE_IGNORED)); + } + /** * Mock some methods of vpn object. */ From c8e5bf6effd175c0e95e8fea5d31147a4d787cb0 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Sun, 5 Jan 2020 14:06:39 -0800 Subject: [PATCH 5/8] Implement INetworkMonitorCallbacks#notifyNetworkTestedWithExtras. INetworkMonitorCallbacks defines notifyNetworkTestedWithExtras() for notifying ConnectivityService of networks being tested along with a PersistableBundle of extras. A new event is introduced for NetworkStateTrackerHandler to notify the ConnectivityDiagnosticsHandler before continuing with the normal processing for "network tested" notifications. The event is also used in the ConnectivityDiagnosticsHandler. Bug: 143187964 Bug: 147391402 Test: compiles. Test: atest CtsNetTestCases FrameworksNetTests Change-Id: Iab29da790c0f5faae68227770bc3a84bbc94f124 (cherry picked from commit c5326407d592490783259a48a8ca653c4ff13122) --- .../net/ConnectivityDiagnosticsManager.java | 3 +- .../android/net/IConnectivityManager.aidl | 2 +- .../java/android/net/NetworkCapabilities.java | 4 +- .../android/server/ConnectivityService.java | 354 +++++++++++++----- .../ConnectivityDiagnosticsManagerTest.java | 15 +- .../server/ConnectivityServiceTest.java | 179 ++++++++- 6 files changed, 455 insertions(+), 102 deletions(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 140363c482..b128ea7f3e 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -676,7 +676,8 @@ public class ConnectivityDiagnosticsManager { } try { - mService.registerConnectivityDiagnosticsCallback(binder, request); + mService.registerConnectivityDiagnosticsCallback( + binder, request, mContext.getOpPackageName()); } catch (RemoteException exception) { exception.rethrowFromSystemServer(); } diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 1089a197ff..0fae607ca6 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -222,7 +222,7 @@ interface IConnectivityManager boolean isCallerCurrentAlwaysOnVpnLockdownApp(); void registerConnectivityDiagnosticsCallback(in IConnectivityDiagnosticsCallback callback, - in NetworkRequest request); + in NetworkRequest request, String callingPackageName); void unregisterConnectivityDiagnosticsCallback(in IConnectivityDiagnosticsCallback callback); IBinder startOrGetTestNetworkService(); diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 4f4e27b446..cf5f2259af 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -858,8 +858,8 @@ public final class NetworkCapabilities implements Parcelable { * *

In general, user-supplied networks (such as WiFi networks) do not have an administrator. * - *

An app is granted owner privileges over Networks that it supplies. Owner privileges - * implicitly include administrator privileges. + *

An app is granted owner privileges over Networks that it supplies. The owner UID MUST + * always be included in administratorUids. * * @param administratorUids the UIDs to be set as administrators of this Network. * @hide diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 2968a81683..9702c50279 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -48,8 +48,11 @@ import static android.os.Process.INVALID_UID; import static android.system.OsConstants.IPPROTO_TCP; import static android.system.OsConstants.IPPROTO_UDP; +import static java.util.Map.Entry; + import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.AppOpsManager; import android.app.BroadcastOptions; import android.app.NotificationManager; import android.app.PendingIntent; @@ -62,6 +65,7 @@ import android.content.res.Configuration; import android.database.ContentObserver; import android.net.CaptivePortal; import android.net.ConnectionInfo; +import android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import android.net.ConnectivityManager; import android.net.ICaptivePortal; import android.net.IConnectivityDiagnosticsCallback; @@ -130,6 +134,7 @@ import android.os.Message; import android.os.Messenger; import android.os.ParcelFileDescriptor; import android.os.Parcelable; +import android.os.PersistableBundle; import android.os.PowerManager; import android.os.Process; import android.os.RemoteException; @@ -170,6 +175,7 @@ import com.android.internal.util.ArrayUtils; import com.android.internal.util.AsyncChannel; import com.android.internal.util.DumpUtils; import com.android.internal.util.IndentingPrintWriter; +import com.android.internal.util.LocationPermissionChecker; import com.android.internal.util.MessageUtils; import com.android.internal.util.XmlUtils; import com.android.server.am.BatteryStatsService; @@ -492,9 +498,9 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * Event for NetworkMonitor/NetworkAgentInfo to inform ConnectivityService that the network has * been tested. - * obj = String representing URL that Internet probe was redirect to, if it was redirected. - * arg1 = One of the NETWORK_TESTED_RESULT_* constants. - * arg2 = NetID. + * obj = {@link NetworkTestedResults} representing information sent from NetworkMonitor. + * data = PersistableBundle of extras passed from NetworkMonitor. If {@link + * NetworkMonitorCallbacks#notifyNetworkTested} is called, this will be null. */ private static final int EVENT_NETWORK_TESTED = 41; @@ -596,6 +602,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set mWolSupportedInterfaces; private TelephonyManager mTelephonyManager; + private final AppOpsManager mAppOpsManager; + + private final LocationPermissionChecker mLocationPermissionChecker; private KeepaliveTracker mKeepaliveTracker; private NetworkNotificationManager mNotifier; @@ -992,6 +1001,8 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetd = netd; mKeyStore = KeyStore.getInstance(); mTelephonyManager = (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE); + mAppOpsManager = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE); + mLocationPermissionChecker = new LocationPermissionChecker(mContext); // To ensure uid rules are synchronized with Network Policy, register for // NetworkPolicyManagerService events must happen prior to NetworkPolicyManagerService @@ -2101,6 +2112,12 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } + private boolean checkNetworkStackPermission(int pid, int uid) { + return checkAnyPermissionOf(pid, uid, + android.Manifest.permission.NETWORK_STACK, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); + } + private boolean checkNetworkSignalStrengthWakeupPermission(int pid, int uid) { return checkAnyPermissionOf(pid, uid, android.Manifest.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP, @@ -2747,88 +2764,21 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case EVENT_NETWORK_TESTED: { - final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); + final NetworkTestedResults results = (NetworkTestedResults) msg.obj; + + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(results.mNetId); if (nai == null) break; - final boolean wasPartial = nai.partialConnectivity; - nai.partialConnectivity = ((msg.arg1 & NETWORK_VALIDATION_RESULT_PARTIAL) != 0); - final boolean partialConnectivityChanged = - (wasPartial != nai.partialConnectivity); + handleNetworkTested(nai, results.mTestResult, + (results.mRedirectUrl == null) ? "" : results.mRedirectUrl); - final boolean valid = ((msg.arg1 & NETWORK_VALIDATION_RESULT_VALID) != 0); - final boolean wasValidated = nai.lastValidated; - final boolean wasDefault = isDefaultNetwork(nai); - // Only show a connected notification if the network is pending validation - // after the captive portal app was open, and it has now validated. - if (nai.captivePortalValidationPending && valid) { - // User is now logged in, network validated. - nai.captivePortalValidationPending = false; - showNetworkNotification(nai, NotificationType.LOGGED_IN); - } - - final String redirectUrl = (msg.obj instanceof String) ? (String) msg.obj : ""; - - if (DBG) { - final String logMsg = !TextUtils.isEmpty(redirectUrl) - ? " with redirect to " + redirectUrl - : ""; - log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); - } - if (valid != nai.lastValidated) { - if (wasDefault) { - mDeps.getMetricsLogger() - .defaultNetworkMetrics().logDefaultNetworkValidity( - SystemClock.elapsedRealtime(), valid); - } - final int oldScore = nai.getCurrentScore(); - nai.lastValidated = valid; - nai.everValidated |= valid; - updateCapabilities(oldScore, nai, nai.networkCapabilities); - // If score has changed, rebroadcast to NetworkProviders. b/17726566 - if (oldScore != nai.getCurrentScore()) sendUpdatedScoreToFactories(nai); - if (valid) { - handleFreshlyValidatedNetwork(nai); - // Clear NO_INTERNET, PRIVATE_DNS_BROKEN, PARTIAL_CONNECTIVITY and - // LOST_INTERNET notifications if network becomes valid. - mNotifier.clearNotification(nai.network.netId, - NotificationType.NO_INTERNET); - mNotifier.clearNotification(nai.network.netId, - NotificationType.LOST_INTERNET); - mNotifier.clearNotification(nai.network.netId, - NotificationType.PARTIAL_CONNECTIVITY); - mNotifier.clearNotification(nai.network.netId, - NotificationType.PRIVATE_DNS_BROKEN); - // If network becomes valid, the hasShownBroken should be reset for - // that network so that the notification will be fired when the private - // DNS is broken again. - nai.networkAgentConfig.hasShownBroken = false; - } - } else if (partialConnectivityChanged) { - updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); - } - updateInetCondition(nai); - // Let the NetworkAgent know the state of its network - Bundle redirectUrlBundle = new Bundle(); - redirectUrlBundle.putString(NetworkAgent.REDIRECT_URL_KEY, redirectUrl); - // TODO: Evaluate to update partial connectivity to status to NetworkAgent. - nai.asyncChannel.sendMessage( - NetworkAgent.CMD_REPORT_NETWORK_STATUS, - (valid ? NetworkAgent.VALID_NETWORK : NetworkAgent.INVALID_NETWORK), - 0, redirectUrlBundle); - - // If NetworkMonitor detects partial connectivity before - // EVENT_PROMPT_UNVALIDATED arrives, show the partial connectivity notification - // immediately. Re-notify partial connectivity silently if no internet - // notification already there. - if (!wasPartial && nai.partialConnectivity) { - // Remove delayed message if there is a pending message. - mHandler.removeMessages(EVENT_PROMPT_UNVALIDATED, nai.network); - handlePromptUnvalidated(nai.network); - } - - if (wasValidated && !nai.lastValidated) { - handleNetworkUnvalidated(nai); - } + // Invoke ConnectivityReport generation for this Network test event. + final Message m = + mConnectivityDiagnosticsHandler.obtainMessage( + ConnectivityDiagnosticsHandler.EVENT_NETWORK_TESTED, + new ConnectivityReportEvent(results.mTimestampMillis, nai)); + m.setData(msg.getData()); + mConnectivityDiagnosticsHandler.sendMessage(m); break; } case EVENT_PROVISIONING_NOTIFICATION: { @@ -2879,6 +2829,87 @@ public class ConnectivityService extends IConnectivityManager.Stub return true; } + private void handleNetworkTested( + @NonNull NetworkAgentInfo nai, int testResult, @NonNull String redirectUrl) { + final boolean wasPartial = nai.partialConnectivity; + nai.partialConnectivity = ((testResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0); + final boolean partialConnectivityChanged = + (wasPartial != nai.partialConnectivity); + + final boolean valid = ((testResult & NETWORK_VALIDATION_RESULT_VALID) != 0); + final boolean wasValidated = nai.lastValidated; + final boolean wasDefault = isDefaultNetwork(nai); + // Only show a connected notification if the network is pending validation + // after the captive portal app was open, and it has now validated. + if (nai.captivePortalValidationPending && valid) { + // User is now logged in, network validated. + nai.captivePortalValidationPending = false; + showNetworkNotification(nai, NotificationType.LOGGED_IN); + } + + if (DBG) { + final String logMsg = !TextUtils.isEmpty(redirectUrl) + ? " with redirect to " + redirectUrl + : ""; + log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); + } + if (valid != nai.lastValidated) { + if (wasDefault) { + mDeps.getMetricsLogger() + .defaultNetworkMetrics().logDefaultNetworkValidity( + SystemClock.elapsedRealtime(), valid); + } + final int oldScore = nai.getCurrentScore(); + nai.lastValidated = valid; + nai.everValidated |= valid; + updateCapabilities(oldScore, nai, nai.networkCapabilities); + // If score has changed, rebroadcast to NetworkProviders. b/17726566 + if (oldScore != nai.getCurrentScore()) sendUpdatedScoreToFactories(nai); + if (valid) { + handleFreshlyValidatedNetwork(nai); + // Clear NO_INTERNET, PRIVATE_DNS_BROKEN, PARTIAL_CONNECTIVITY and + // LOST_INTERNET notifications if network becomes valid. + mNotifier.clearNotification(nai.network.netId, + NotificationType.NO_INTERNET); + mNotifier.clearNotification(nai.network.netId, + NotificationType.LOST_INTERNET); + mNotifier.clearNotification(nai.network.netId, + NotificationType.PARTIAL_CONNECTIVITY); + mNotifier.clearNotification(nai.network.netId, + NotificationType.PRIVATE_DNS_BROKEN); + // If network becomes valid, the hasShownBroken should be reset for + // that network so that the notification will be fired when the private + // DNS is broken again. + nai.networkAgentConfig.hasShownBroken = false; + } + } else if (partialConnectivityChanged) { + updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); + } + updateInetCondition(nai); + // Let the NetworkAgent know the state of its network + Bundle redirectUrlBundle = new Bundle(); + redirectUrlBundle.putString(NetworkAgent.REDIRECT_URL_KEY, redirectUrl); + // TODO: Evaluate to update partial connectivity to status to NetworkAgent. + nai.asyncChannel.sendMessage( + NetworkAgent.CMD_REPORT_NETWORK_STATUS, + (valid ? NetworkAgent.VALID_NETWORK : NetworkAgent.INVALID_NETWORK), + 0, redirectUrlBundle); + + // If NetworkMonitor detects partial connectivity before + // EVENT_PROMPT_UNVALIDATED arrives, show the partial connectivity notification + // immediately. Re-notify partial connectivity silently if no internet + // notification already there. + if (!wasPartial && nai.partialConnectivity) { + // Remove delayed message if there is a pending message. + mHandler.removeMessages(EVENT_PROMPT_UNVALIDATED, nai.network); + handlePromptUnvalidated(nai.network); + } + + if (wasValidated && !nai.lastValidated) { + handleNetworkUnvalidated(nai); + } + } + private int getCaptivePortalMode() { return Settings.Global.getInt(mContext.getContentResolver(), Settings.Global.CAPTIVE_PORTAL_MODE, @@ -2927,8 +2958,23 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) { - mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage(EVENT_NETWORK_TESTED, - testResult, mNetId, redirectUrl)); + notifyNetworkTestedWithExtras(testResult, redirectUrl, SystemClock.elapsedRealtime(), + PersistableBundle.EMPTY); + } + + @Override + public void notifyNetworkTestedWithExtras( + int testResult, + @Nullable String redirectUrl, + long timestampMillis, + @NonNull PersistableBundle extras) { + final Message msg = + mTrackerHandler.obtainMessage( + EVENT_NETWORK_TESTED, + new NetworkTestedResults( + mNetId, testResult, timestampMillis, redirectUrl)); + msg.setData(new Bundle(extras)); + mTrackerHandler.sendMessage(msg); } @Override @@ -7359,7 +7405,11 @@ public class ConnectivityService extends IConnectivityManager.Stub @GuardedBy("mVpns") private Vpn getVpnIfOwner() { - final int uid = Binder.getCallingUid(); + return getVpnIfOwner(Binder.getCallingUid()); + } + + @GuardedBy("mVpns") + private Vpn getVpnIfOwner(int uid) { final int user = UserHandle.getUserId(uid); final Vpn vpn = mVpns.get(user); @@ -7471,6 +7521,17 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int EVENT_UNREGISTER_CONNECTIVITY_DIAGNOSTICS_CALLBACK = 2; + /** + * Event for {@link NetworkStateTrackerHandler} to trigger ConnectivityReport callbacks + * after processing {@link #EVENT_NETWORK_TESTED} events. + * obj = {@link ConnectivityReportEvent} representing ConnectivityReport info reported from + * NetworkMonitor. + * data = PersistableBundle of extras passed from NetworkMonitor. + * + *

See {@link ConnectivityService#EVENT_NETWORK_TESTED}. + */ + private static final int EVENT_NETWORK_TESTED = ConnectivityService.EVENT_NETWORK_TESTED; + private ConnectivityDiagnosticsHandler(Looper looper) { super(looper); } @@ -7488,6 +7549,19 @@ public class ConnectivityService extends IConnectivityManager.Stub (IConnectivityDiagnosticsCallback) msg.obj, msg.arg1); break; } + case EVENT_NETWORK_TESTED: { + final ConnectivityReportEvent reportEvent = + (ConnectivityReportEvent) msg.obj; + + // This is safe because {@link + // NetworkMonitorCallbacks#notifyNetworkTestedWithExtras} receives a + // PersistableBundle and converts it to the Bundle in the incoming Message. If + // {@link NetworkMonitorCallbacks#notifyNetworkTested} is called, msg.data will + // not be set. This is also safe, as msg.getData() will return an empty Bundle. + final PersistableBundle extras = new PersistableBundle(msg.getData()); + handleNetworkTestedWithExtras(reportEvent, extras); + break; + } } } } @@ -7497,12 +7571,16 @@ public class ConnectivityService extends IConnectivityManager.Stub class ConnectivityDiagnosticsCallbackInfo implements Binder.DeathRecipient { @NonNull private final IConnectivityDiagnosticsCallback mCb; @NonNull private final NetworkRequestInfo mRequestInfo; + @NonNull private final String mCallingPackageName; @VisibleForTesting ConnectivityDiagnosticsCallbackInfo( - @NonNull IConnectivityDiagnosticsCallback cb, @NonNull NetworkRequestInfo nri) { + @NonNull IConnectivityDiagnosticsCallback cb, + @NonNull NetworkRequestInfo nri, + @NonNull String callingPackageName) { mCb = cb; mRequestInfo = nri; + mCallingPackageName = callingPackageName; } @Override @@ -7512,6 +7590,39 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Class used for sending information from {@link + * NetworkMonitorCallbacks#notifyNetworkTestedWithExtras} to the handler for processing it. + */ + private static class NetworkTestedResults { + private final int mNetId; + private final int mTestResult; + private final long mTimestampMillis; + @Nullable private final String mRedirectUrl; + + private NetworkTestedResults( + int netId, int testResult, long timestampMillis, @Nullable String redirectUrl) { + mNetId = netId; + mTestResult = testResult; + mTimestampMillis = timestampMillis; + mRedirectUrl = redirectUrl; + } + } + + /** + * Class used for sending information from {@link NetworkStateTrackerHandler} to {@link + * ConnectivityDiagnosticsHandler}. + */ + private static class ConnectivityReportEvent { + private final long mTimestampMillis; + @NonNull private final NetworkAgentInfo mNai; + + private ConnectivityReportEvent(long timestampMillis, @NonNull NetworkAgentInfo nai) { + mTimestampMillis = timestampMillis; + mNai = nai; + } + } + private void handleRegisterConnectivityDiagnosticsCallback( @NonNull ConnectivityDiagnosticsCallbackInfo cbInfo) { ensureRunningOnConnectivityServiceThread(); @@ -7559,13 +7670,80 @@ public class ConnectivityService extends IConnectivityManager.Stub cb.asBinder().unlinkToDeath(mConnectivityDiagnosticsCallbacks.remove(cb), 0); } + private void handleNetworkTestedWithExtras( + @NonNull ConnectivityReportEvent reportEvent, @NonNull PersistableBundle extras) { + final NetworkAgentInfo nai = reportEvent.mNai; + final ConnectivityReport report = + new ConnectivityReport( + reportEvent.mNai.network, + reportEvent.mTimestampMillis, + nai.linkProperties, + nai.networkCapabilities, + extras); + final List results = + getMatchingPermissionedCallbacks(nai); + for (final IConnectivityDiagnosticsCallback cb : results) { + try { + cb.onConnectivityReport(report); + } catch (RemoteException ex) { + loge("Error invoking onConnectivityReport", ex); + } + } + } + + private List getMatchingPermissionedCallbacks( + @NonNull NetworkAgentInfo nai) { + final List results = new ArrayList<>(); + for (Entry entry : + mConnectivityDiagnosticsCallbacks.entrySet()) { + final ConnectivityDiagnosticsCallbackInfo cbInfo = entry.getValue(); + final NetworkRequestInfo nri = cbInfo.mRequestInfo; + if (nai.satisfies(nri.request)) { + if (checkConnectivityDiagnosticsPermissions( + nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) { + results.add(entry.getKey()); + } + } + } + return results; + } + + @VisibleForTesting + boolean checkConnectivityDiagnosticsPermissions( + int callbackPid, int callbackUid, NetworkAgentInfo nai, String callbackPackageName) { + if (checkNetworkStackPermission(callbackPid, callbackUid)) { + return true; + } + + if (!mLocationPermissionChecker.checkLocationPermission( + callbackPackageName, null /* featureId */, callbackUid, null /* message */)) { + return false; + } + + synchronized (mVpns) { + if (getVpnIfOwner(callbackUid) != null) { + return true; + } + } + + // Administrator UIDs also contains the Owner UID + if (nai.networkCapabilities.getAdministratorUids().contains(callbackUid)) { + return true; + } + + return false; + } + @Override public void registerConnectivityDiagnosticsCallback( - @NonNull IConnectivityDiagnosticsCallback callback, @NonNull NetworkRequest request) { + @NonNull IConnectivityDiagnosticsCallback callback, + @NonNull NetworkRequest request, + @NonNull String callingPackageName) { if (request.legacyType != TYPE_NONE) { throw new IllegalArgumentException("ConnectivityManager.TYPE_* are deprecated." + " Please use NetworkCapabilities instead."); } + mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName); // This NetworkCapabilities is only used for matching to Networks. Clear out its owner uid // and administrator uids to be safe. @@ -7583,7 +7761,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // callback's binder death. final NetworkRequestInfo nri = new NetworkRequestInfo(requestWithId); final ConnectivityDiagnosticsCallbackInfo cbInfo = - new ConnectivityDiagnosticsCallbackInfo(callback, nri); + new ConnectivityDiagnosticsCallbackInfo(callback, nri, callingPackageName); mConnectivityDiagnosticsHandler.sendMessage( mConnectivityDiagnosticsHandler.obtainMessage( diff --git a/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java b/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java index 2d5df4f47e..0628691c33 100644 --- a/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java +++ b/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java @@ -38,6 +38,8 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import android.content.Context; import android.os.PersistableBundle; +import androidx.test.InstrumentationRegistry; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -58,21 +60,26 @@ public class ConnectivityDiagnosticsManagerTest { private static final Executor INLINE_EXECUTOR = x -> x.run(); - @Mock private Context mContext; @Mock private IConnectivityManager mService; @Mock private ConnectivityDiagnosticsCallback mCb; + private Context mContext; private ConnectivityDiagnosticsBinder mBinder; private ConnectivityDiagnosticsManager mManager; + private String mPackageName; + @Before public void setUp() { - mContext = mock(Context.class); + mContext = InstrumentationRegistry.getContext(); + mService = mock(IConnectivityManager.class); mCb = mock(ConnectivityDiagnosticsCallback.class); mBinder = new ConnectivityDiagnosticsBinder(mCb, INLINE_EXECUTOR); mManager = new ConnectivityDiagnosticsManager(mContext, mService); + + mPackageName = mContext.getOpPackageName(); } @After @@ -271,7 +278,7 @@ public class ConnectivityDiagnosticsManagerTest { mManager.registerConnectivityDiagnosticsCallback(request, INLINE_EXECUTOR, mCb); verify(mService).registerConnectivityDiagnosticsCallback( - any(ConnectivityDiagnosticsBinder.class), eq(request)); + any(ConnectivityDiagnosticsBinder.class), eq(request), eq(mPackageName)); assertTrue(ConnectivityDiagnosticsManager.sCallbacks.containsKey(mCb)); } @@ -302,7 +309,7 @@ public class ConnectivityDiagnosticsManagerTest { // verify that re-registering is successful mManager.registerConnectivityDiagnosticsCallback(request, INLINE_EXECUTOR, mCb); verify(mService, times(2)).registerConnectivityDiagnosticsCallback( - any(ConnectivityDiagnosticsBinder.class), eq(request)); + any(ConnectivityDiagnosticsBinder.class), eq(request), eq(mPackageName)); assertTrue(ConnectivityDiagnosticsManager.sCallbacks.containsKey(mCb)); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 028c1dad82..bd460272c8 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -23,6 +23,7 @@ import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.content.pm.PackageManager.PERMISSION_DENIED; import static android.content.pm.PackageManager.PERMISSION_GRANTED; +import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION_SUPL; @@ -119,6 +120,7 @@ import static org.mockito.Mockito.when; import android.Manifest; import android.annotation.NonNull; import android.app.AlarmManager; +import android.app.AppOpsManager; import android.app.NotificationManager; import android.app.PendingIntent; import android.content.BroadcastReceiver; @@ -132,6 +134,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.content.res.Resources; +import android.location.LocationManager; import android.net.CaptivePortalData; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; @@ -177,6 +180,7 @@ import android.net.shared.PrivateDnsConfig; import android.net.util.MultinetworkPolicyTracker; import android.os.BadParcelableException; import android.os.Binder; +import android.os.Build; import android.os.Bundle; import android.os.ConditionVariable; import android.os.Handler; @@ -187,6 +191,7 @@ import android.os.Looper; import android.os.Parcel; import android.os.ParcelFileDescriptor; import android.os.Parcelable; +import android.os.PersistableBundle; import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; @@ -218,6 +223,7 @@ import com.android.server.connectivity.DefaultNetworkMetrics; import com.android.server.connectivity.IpConnectivityMetrics; import com.android.server.connectivity.MockableSystemProperties; import com.android.server.connectivity.Nat464Xlat; +import com.android.server.connectivity.NetworkAgentInfo; import com.android.server.connectivity.NetworkNotificationManager.NotificationType; import com.android.server.connectivity.ProxyTracker; import com.android.server.connectivity.Vpn; @@ -292,6 +298,8 @@ public class ConnectivityServiceTest { private static final int UNREASONABLY_LONG_ALARM_WAIT_MS = 1000; + private static final long TIMESTAMP = 1234L; + private static final String CLAT_PREFIX = "v4-"; private static final String MOBILE_IFNAME = "test_rmnet_data0"; private static final String WIFI_IFNAME = "test_wlan0"; @@ -327,6 +335,8 @@ public class ConnectivityServiceTest { @Mock AlarmManager mAlarmManager; @Mock IConnectivityDiagnosticsCallback mConnectivityDiagnosticsCallback; @Mock IBinder mIBinder; + @Mock LocationManager mLocationManager; + @Mock AppOpsManager mAppOpsManager; private ArgumentCaptor mResolverParamsParcelCaptor = ArgumentCaptor.forClass(ResolverParamsParcel.class); @@ -412,6 +422,8 @@ public class ConnectivityServiceTest { if (Context.NETWORK_STACK_SERVICE.equals(name)) return mNetworkStack; if (Context.USER_SERVICE.equals(name)) return mUserManager; if (Context.ALARM_SERVICE.equals(name)) return mAlarmManager; + if (Context.LOCATION_SERVICE.equals(name)) return mLocationManager; + if (Context.APP_OPS_SERVICE.equals(name)) return mAppOpsManager; return super.getSystemService(name); } @@ -564,6 +576,7 @@ public class ConnectivityServiceTest { private int mProbesCompleted; private int mProbesSucceeded; private String mNmValidationRedirectUrl = null; + private PersistableBundle mValidationExtras = PersistableBundle.EMPTY; private boolean mNmProvNotificationRequested = false; private final ConditionVariable mNetworkStatusReceived = new ConditionVariable(); @@ -631,8 +644,8 @@ public class ConnectivityServiceTest { } mNmCallbacks.notifyProbeStatusChanged(mProbesCompleted, mProbesSucceeded); - mNmCallbacks.notifyNetworkTested( - mNmValidationResult, mNmValidationRedirectUrl); + mNmCallbacks.notifyNetworkTestedWithExtras( + mNmValidationResult, mNmValidationRedirectUrl, TIMESTAMP, mValidationExtras); if (mNmValidationRedirectUrl != null) { mNmCallbacks.showProvisioningNotification( @@ -970,6 +983,8 @@ public class ConnectivityServiceTest { // not inherit from NetworkAgent. private TestNetworkAgentWrapper mMockNetworkAgent; + private VpnInfo mVpnInfo; + public MockVpn(int userId) { super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService, userId); @@ -1041,6 +1056,17 @@ public class ConnectivityServiceTest { mConnected = false; mConfig = null; } + + @Override + public synchronized VpnInfo getVpnInfo() { + if (mVpnInfo != null) return mVpnInfo; + + return super.getVpnInfo(); + } + + private void setVpnInfo(VpnInfo vpnInfo) { + mVpnInfo = vpnInfo; + } } private void mockVpn(int uid) { @@ -6402,7 +6428,7 @@ public class ConnectivityServiceTest { new NetworkCapabilities(), TYPE_ETHERNET, 0, NetworkRequest.Type.NONE); try { mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, request); + mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); fail("registerConnectivityDiagnosticsCallback should throw on invalid NetworkRequest"); } catch (IllegalArgumentException expected) { } @@ -6416,7 +6442,7 @@ public class ConnectivityServiceTest { when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, wifiRequest); + mConnectivityDiagnosticsCallback, wifiRequest, mContext.getPackageName()); verify(mIBinder, timeout(TIMEOUT_MS)) .linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); @@ -6440,7 +6466,7 @@ public class ConnectivityServiceTest { when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, wifiRequest); + mConnectivityDiagnosticsCallback, wifiRequest, mContext.getPackageName()); verify(mIBinder, timeout(TIMEOUT_MS)) .linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); @@ -6451,7 +6477,7 @@ public class ConnectivityServiceTest { // Register the same callback again mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, wifiRequest); + mConnectivityDiagnosticsCallback, wifiRequest, mContext.getPackageName()); // Block until all other events are done processing. HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); @@ -6460,4 +6486,145 @@ public class ConnectivityServiceTest { mService.mConnectivityDiagnosticsCallbacks.containsKey( mConnectivityDiagnosticsCallback)); } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception { + final NetworkAgentInfo naiWithoutUid = + new NetworkAgentInfo( + null, null, null, null, null, new NetworkCapabilities(), null, + mServiceContext, null, null, mService, null, null, null, 0); + + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + assertTrue( + "NetworkStack permission not applied", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { + final NetworkAgentInfo naiWithoutUid = + new NetworkAgentInfo( + null, null, null, null, null, new NetworkCapabilities(), null, + mServiceContext, null, null, mService, null, null, null, 0); + + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + assertFalse( + "ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception { + final NetworkAgentInfo naiWithoutUid = + new NetworkAgentInfo( + null, null, null, null, null, new NetworkCapabilities(), null, + mServiceContext, null, null, mService, null, null, null, 0); + + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + // setUp() calls mockVpn() which adds a VPN with the Test Runner's uid. Configure it to be + // active + final VpnInfo info = new VpnInfo(); + info.ownerUid = Process.myUid(); + info.vpnIface = "interface"; + mMockVpn.setVpnInfo(info); + assertTrue( + "Active VPN permission not applied", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsNetworkAdministrator() throws Exception { + final NetworkCapabilities nc = new NetworkCapabilities(); + nc.setAdministratorUids(Arrays.asList(Process.myUid())); + final NetworkAgentInfo naiWithUid = + new NetworkAgentInfo( + null, null, null, null, null, nc, null, mServiceContext, null, null, + mService, null, null, null, 0); + + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + // Disconnect mock vpn so the uid check on NetworkAgentInfo is tested + mMockVpn.disconnect(); + assertTrue( + "NetworkCapabilities administrator uid permission not applied", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithUid, mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsFails() throws Exception { + final NetworkCapabilities nc = new NetworkCapabilities(); + nc.setOwnerUid(Process.myUid()); + nc.setAdministratorUids(Arrays.asList(Process.myUid())); + final NetworkAgentInfo naiWithUid = + new NetworkAgentInfo( + null, null, null, null, null, nc, null, mServiceContext, null, null, + mService, null, null, null, 0); + + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + // Use wrong pid and uid + assertFalse( + "Permissions allowed when they shouldn't be granted", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid() + 1, Process.myUid() + 1, naiWithUid, + mContext.getOpPackageName())); + } + + private void setupLocationPermissions( + int targetSdk, boolean locationToggle, String op, String perm) throws Exception { + final ApplicationInfo applicationInfo = new ApplicationInfo(); + applicationInfo.targetSdkVersion = targetSdk; + when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any())) + .thenReturn(applicationInfo); + + when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(locationToggle); + + when(mAppOpsManager.noteOp(eq(op), eq(Process.myUid()), eq(mContext.getPackageName()))) + .thenReturn(AppOpsManager.MODE_ALLOWED); + + mServiceContext.setPermission(perm, PERMISSION_GRANTED); + } + + @Test + public void testConnectivityDiagnosticsCallbackOnConnectivityReport() throws Exception { + final NetworkRequest request = new NetworkRequest.Builder().build(); + when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); + + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + + mService.registerConnectivityDiagnosticsCallback( + mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); + + // Block until all other events are done processing. + HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); + + // Connect the cell agent verify that it notifies TestNetworkCallback that it is available + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(true); + callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + callback.assertNoCallback(); + + // Wait for onConnectivityReport to fire + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onConnectivityReport(any(ConnectivityReport.class)); + } } From a1bb47a7682c1fb8342118af3acd467f8870f970 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Mon, 6 Jan 2020 16:55:35 -0800 Subject: [PATCH 6/8] Implement INetworkMonitorCallbacks#notifyDataStallSuspected. INetworkMonitorCallbacks defines notifyDataStallSuspected() for notifying ConnectivityService of networks encountering a potential data stall. A new event is introduced for ConnectivityDiagnosticsHandler to process the notification and invoke the relevant ConnectivityDiagnosticsCallbacks. Bug: 143187964 Test: compiles Test: atest CtsNetTestCases FrameworksNetTests Change-Id: I70320bdda9855dced31e08e6a0b25329fb5cb535 (cherry picked from commit 6c51dc9de39d7963a6bc6d03eacadf2ba2131e20) --- .../android/server/ConnectivityService.java | 53 +++++++++++++++++++ .../server/ConnectivityServiceTest.java | 41 ++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d0cd7ffee1..d1a5082ffd 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -66,6 +66,7 @@ import android.database.ContentObserver; import android.net.CaptivePortal; import android.net.ConnectionInfo; import android.net.ConnectivityDiagnosticsManager.ConnectivityReport; +import android.net.ConnectivityDiagnosticsManager.DataStallReport; import android.net.ConnectivityManager; import android.net.ICaptivePortal; import android.net.IConnectivityDiagnosticsCallback; @@ -3015,6 +3016,21 @@ public class ConnectivityService extends IConnectivityManager.Stub EVENT_PROVISIONING_NOTIFICATION, PROVISIONING_NOTIFICATION_HIDE, mNetId)); } + @Override + public void notifyDataStallSuspected( + long timestampMillis, int detectionMethod, PersistableBundle extras) { + final Message msg = + mConnectivityDiagnosticsHandler.obtainMessage( + ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED, + detectionMethod, mNetId, timestampMillis); + msg.setData(new Bundle(extras)); + + // NetworkStateTrackerHandler currently doesn't take any actions based on data + // stalls so send the message directly to ConnectivityDiagnosticsHandler and avoid + // the cost of going through two handlers. + mConnectivityDiagnosticsHandler.sendMessage(msg); + } + @Override public int getInterfaceVersion() { return this.VERSION; @@ -7546,6 +7562,16 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int EVENT_NETWORK_TESTED = ConnectivityService.EVENT_NETWORK_TESTED; + /** + * Event for NetworkMonitor to inform ConnectivityService that a potential data stall has + * been detected on the network. + * obj = Long the timestamp (in millis) for when the suspected data stall was detected. + * arg1 = {@link DataStallReport#DetectionMethod} indicating the detection method. + * arg2 = NetID. + * data = PersistableBundle of extras passed from NetworkMonitor. + */ + private static final int EVENT_DATA_STALL_SUSPECTED = 4; + private ConnectivityDiagnosticsHandler(Looper looper) { super(looper); } @@ -7576,6 +7602,17 @@ public class ConnectivityService extends IConnectivityManager.Stub handleNetworkTestedWithExtras(reportEvent, extras); break; } + case EVENT_DATA_STALL_SUSPECTED: { + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); + if (nai == null) break; + + // This is safe because NetworkMonitorCallbacks#notifyDataStallSuspected + // receives a PersistableBundle and converts it to the Bundle in the incoming + // Message. + final PersistableBundle extras = new PersistableBundle(msg.getData()); + handleDataStallSuspected(nai, (long) msg.obj, msg.arg1, extras); + break; + } } } } @@ -7705,6 +7742,22 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private void handleDataStallSuspected( + @NonNull NetworkAgentInfo nai, long timestampMillis, int detectionMethod, + @NonNull PersistableBundle extras) { + final DataStallReport report = + new DataStallReport(nai.network, timestampMillis, detectionMethod, extras); + final List results = + getMatchingPermissionedCallbacks(nai); + for (final IConnectivityDiagnosticsCallback cb : results) { + try { + cb.onDataStallSuspected(report); + } catch (RemoteException ex) { + loge("Error invoking onDataStallSuspected", ex); + } + } + } + private List getMatchingPermissionedCallbacks( @NonNull NetworkAgentInfo nai) { final List results = new ArrayList<>(); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 50c312cacc..5e80ede318 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -24,6 +24,7 @@ import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.content.pm.PackageManager.PERMISSION_DENIED; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; +import static android.net.ConnectivityDiagnosticsManager.DataStallReport; import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION_SUPL; @@ -570,6 +571,9 @@ public class ConnectivityServiceTest { | NETWORK_VALIDATION_RESULT_PARTIAL; private static final int VALIDATION_RESULT_INVALID = 0; + private static final long DATA_STALL_TIMESTAMP = 10L; + private static final int DATA_STALL_DETECTION_METHOD = 1; + private INetworkMonitor mNetworkMonitor; private INetworkMonitorCallbacks mNmCallbacks; private int mNmValidationResult = VALIDATION_RESULT_BASE; @@ -577,6 +581,7 @@ public class ConnectivityServiceTest { private int mProbesSucceeded; private String mNmValidationRedirectUrl = null; private PersistableBundle mValidationExtras = PersistableBundle.EMPTY; + private PersistableBundle mDataStallExtras = PersistableBundle.EMPTY; private boolean mNmProvNotificationRequested = false; private final ConditionVariable mNetworkStatusReceived = new ConditionVariable(); @@ -804,6 +809,11 @@ public class ConnectivityServiceTest { public void expectPreventReconnectReceived() { expectPreventReconnectReceived(TIMEOUT_MS); } + + void notifyDataStallSuspected() throws Exception { + mNmCallbacks.notifyDataStallSuspected( + DATA_STALL_TIMESTAMP, DATA_STALL_DETECTION_METHOD, mDataStallExtras); + } } /** @@ -6625,4 +6635,35 @@ public class ConnectivityServiceTest { verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) .onConnectivityReport(any(ConnectivityReport.class)); } + + @Test + public void testConnectivityDiagnosticsCallbackOnDataStallSuspected() throws Exception { + final NetworkRequest request = new NetworkRequest.Builder().build(); + when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); + + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + + mService.registerConnectivityDiagnosticsCallback( + mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); + + // Block until all other events are done processing. + HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); + + // Connect the cell agent verify that it notifies TestNetworkCallback that it is available + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(true); + callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + callback.assertNoCallback(); + + // Trigger notifyDataStallSuspected() on the INetworkMonitorCallbacks instance in the + // cellular network agent + mCellNetworkAgent.notifyDataStallSuspected(); + + // Wait for onDataStallSuspected to fire + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onDataStallSuspected(any(DataStallReport.class)); + } } From 3eb07d4e7fa6ea19d3d42425144780bdb24fc90f Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Tue, 7 Jan 2020 11:18:54 -0800 Subject: [PATCH 7/8] Fire ConnectivityDiagnostics callbacks on Connectivity reported. When ConnectivityService#reportNetworkTested is called, the platform needs to fire ConnectivityDiagnostics callbacks for registered callbacks that are permissioned for the network being reported on. This adds a new event to ConnectivityDiagnosticsHandler for invoking these callbacks. Bug: 143187964 Test: compiles Test: atest CtsNetTestCases ConnectivityServiceTest Change-Id: Icc6bcf7a2411133d8ecd7477bc351dad9333f24f (cherry picked from commit 3d57b0f4ff5b56780c79df6062dfaf2b9fa5ae3c) --- .../android/server/ConnectivityService.java | 45 +++++++++++++++++ .../server/ConnectivityServiceTest.java | 48 +++++++++++-------- 2 files changed, 72 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d1a5082ffd..ec3dbe9f85 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4205,6 +4205,19 @@ public class ConnectivityService extends IConnectivityManager.Stub final int connectivityInfo = encodeBool(hasConnectivity); mHandler.sendMessage( mHandler.obtainMessage(EVENT_REVALIDATE_NETWORK, uid, connectivityInfo, network)); + + final NetworkAgentInfo nai; + if (network == null) { + nai = getDefaultNetwork(); + } else { + nai = getNetworkAgentInfoForNetwork(network); + } + if (nai != null) { + mConnectivityDiagnosticsHandler.sendMessage( + mConnectivityDiagnosticsHandler.obtainMessage( + ConnectivityDiagnosticsHandler.EVENT_NETWORK_CONNECTIVITY_REPORTED, + connectivityInfo, 0, nai)); + } } private void handleReportNetworkConnectivity( @@ -7535,6 +7548,8 @@ public class ConnectivityService extends IConnectivityManager.Stub */ @VisibleForTesting class ConnectivityDiagnosticsHandler extends Handler { + private final String mTag = ConnectivityDiagnosticsHandler.class.getSimpleName(); + /** * Used to handle ConnectivityDiagnosticsCallback registration events from {@link * android.net.ConnectivityDiagnosticsManager}. @@ -7572,6 +7587,16 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int EVENT_DATA_STALL_SUSPECTED = 4; + /** + * Event for ConnectivityDiagnosticsHandler to handle network connectivity being reported to + * the platform. This event will invoke {@link + * IConnectivityDiagnosticsCallback#onNetworkConnectivityReported} for permissioned + * callbacks. + * obj = Network that was reported on + * arg1 = boolint for the quality reported + */ + private static final int EVENT_NETWORK_CONNECTIVITY_REPORTED = 5; + private ConnectivityDiagnosticsHandler(Looper looper) { super(looper); } @@ -7613,6 +7638,13 @@ public class ConnectivityService extends IConnectivityManager.Stub handleDataStallSuspected(nai, (long) msg.obj, msg.arg1, extras); break; } + case EVENT_NETWORK_CONNECTIVITY_REPORTED: { + handleNetworkConnectivityReported((NetworkAgentInfo) msg.obj, toBool(msg.arg1)); + break; + } + default: { + Log.e(mTag, "Unrecognized event in ConnectivityDiagnostics: " + msg.what); + } } } } @@ -7758,6 +7790,19 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private void handleNetworkConnectivityReported( + @NonNull NetworkAgentInfo nai, boolean connectivity) { + final List results = + getMatchingPermissionedCallbacks(nai); + for (final IConnectivityDiagnosticsCallback cb : results) { + try { + cb.onNetworkConnectivityReported(nai.network, connectivity); + } catch (RemoteException ex) { + loge("Error invoking onNetworkConnectivityReported", ex); + } + } + } + private List getMatchingPermissionedCallbacks( @NonNull NetworkAgentInfo nai) { final List results = new ArrayList<>(); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 5e80ede318..b4f32e75fd 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6609,8 +6609,7 @@ public class ConnectivityServiceTest { mServiceContext.setPermission(perm, PERMISSION_GRANTED); } - @Test - public void testConnectivityDiagnosticsCallbackOnConnectivityReport() throws Exception { + private void setUpConnectivityDiagnosticsCallback() throws Exception { final NetworkRequest request = new NetworkRequest.Builder().build(); when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); @@ -6630,6 +6629,11 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(true); callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); callback.assertNoCallback(); + } + + @Test + public void testConnectivityDiagnosticsCallbackOnConnectivityReport() throws Exception { + setUpConnectivityDiagnosticsCallback(); // Wait for onConnectivityReport to fire verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) @@ -6638,25 +6642,7 @@ public class ConnectivityServiceTest { @Test public void testConnectivityDiagnosticsCallbackOnDataStallSuspected() throws Exception { - final NetworkRequest request = new NetworkRequest.Builder().build(); - when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); - - mServiceContext.setPermission( - android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); - - mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); - - // Block until all other events are done processing. - HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); - - // Connect the cell agent verify that it notifies TestNetworkCallback that it is available - final TestNetworkCallback callback = new TestNetworkCallback(); - mCm.registerDefaultNetworkCallback(callback); - mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); - mCellNetworkAgent.connect(true); - callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); - callback.assertNoCallback(); + setUpConnectivityDiagnosticsCallback(); // Trigger notifyDataStallSuspected() on the INetworkMonitorCallbacks instance in the // cellular network agent @@ -6666,4 +6652,24 @@ public class ConnectivityServiceTest { verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) .onDataStallSuspected(any(DataStallReport.class)); } + + @Test + public void testConnectivityDiagnosticsCallbackOnConnectivityReported() throws Exception { + setUpConnectivityDiagnosticsCallback(); + + final Network n = mCellNetworkAgent.getNetwork(); + final boolean hasConnectivity = true; + mService.reportNetworkConnectivity(n, hasConnectivity); + + // Wait for onNetworkConnectivityReported to fire + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onNetworkConnectivityReported(eq(n), eq(hasConnectivity)); + + final boolean noConnectivity = false; + mService.reportNetworkConnectivity(n, noConnectivity); + + // Wait for onNetworkConnectivityReported to fire + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onNetworkConnectivityReported(eq(n), eq(noConnectivity)); + } } From 83bb5fa762672ddba171244a428c756ba09c1975 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Sun, 5 Jan 2020 14:06:39 -0800 Subject: [PATCH 8/8] Implement INetworkMonitorCallbacks#notifyNetworkTestedWithExtras. INetworkMonitorCallbacks defines notifyNetworkTestedWithExtras() for notifying ConnectivityService of networks being tested along with a PersistableBundle of extras. A new event is introduced for NetworkStateTrackerHandler to notify the ConnectivityDiagnosticsHandler before continuing with the normal processing for "network tested" notifications. The event is also used in the ConnectivityDiagnosticsHandler. Bug: 143187964 Bug: 147391402 Test: compiles. Test: atest CtsNetTestCases FrameworksNetTests Change-Id: Iab29da790c0f5faae68227770bc3a84bbc94f124 Merged-In: Iab29da790c0f5faae68227770bc3a84bbc94f124 --- .../net/ConnectivityDiagnosticsManager.java | 3 +- .../android/net/IConnectivityManager.aidl | 2 +- .../java/android/net/NetworkCapabilities.java | 4 +- .../android/server/ConnectivityService.java | 354 +++++++++++++----- .../ConnectivityDiagnosticsManagerTest.java | 15 +- .../server/ConnectivityServiceTest.java | 179 ++++++++- 6 files changed, 455 insertions(+), 102 deletions(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 140363c482..b128ea7f3e 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -676,7 +676,8 @@ public class ConnectivityDiagnosticsManager { } try { - mService.registerConnectivityDiagnosticsCallback(binder, request); + mService.registerConnectivityDiagnosticsCallback( + binder, request, mContext.getOpPackageName()); } catch (RemoteException exception) { exception.rethrowFromSystemServer(); } diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 3e9e7faccb..9df334b1e7 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -213,7 +213,7 @@ interface IConnectivityManager boolean isCallerCurrentAlwaysOnVpnLockdownApp(); void registerConnectivityDiagnosticsCallback(in IConnectivityDiagnosticsCallback callback, - in NetworkRequest request); + in NetworkRequest request, String callingPackageName); void unregisterConnectivityDiagnosticsCallback(in IConnectivityDiagnosticsCallback callback); IBinder startOrGetTestNetworkService(); diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index f94bdb767c..38f7390abf 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -858,8 +858,8 @@ public final class NetworkCapabilities implements Parcelable { * *

In general, user-supplied networks (such as WiFi networks) do not have an administrator. * - *

An app is granted owner privileges over Networks that it supplies. Owner privileges - * implicitly include administrator privileges. + *

An app is granted owner privileges over Networks that it supplies. The owner UID MUST + * always be included in administratorUids. * * @param administratorUids the UIDs to be set as administrators of this Network. * @hide diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 51f6ab0def..e916498218 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -50,8 +50,11 @@ import static android.system.OsConstants.IPPROTO_UDP; import static com.android.internal.util.Preconditions.checkNotNull; +import static java.util.Map.Entry; + import android.annotation.NonNull; import android.annotation.Nullable; +import android.app.AppOpsManager; import android.app.BroadcastOptions; import android.app.NotificationManager; import android.app.PendingIntent; @@ -64,6 +67,7 @@ import android.content.res.Configuration; import android.database.ContentObserver; import android.net.CaptivePortal; import android.net.ConnectionInfo; +import android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import android.net.ConnectivityManager; import android.net.ICaptivePortal; import android.net.IConnectivityDiagnosticsCallback; @@ -131,6 +135,7 @@ import android.os.Message; import android.os.Messenger; import android.os.ParcelFileDescriptor; import android.os.Parcelable; +import android.os.PersistableBundle; import android.os.PowerManager; import android.os.Process; import android.os.RemoteException; @@ -171,6 +176,7 @@ import com.android.internal.util.ArrayUtils; import com.android.internal.util.AsyncChannel; import com.android.internal.util.DumpUtils; import com.android.internal.util.IndentingPrintWriter; +import com.android.internal.util.LocationPermissionChecker; import com.android.internal.util.MessageUtils; import com.android.internal.util.XmlUtils; import com.android.server.am.BatteryStatsService; @@ -493,9 +499,9 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * Event for NetworkMonitor/NetworkAgentInfo to inform ConnectivityService that the network has * been tested. - * obj = String representing URL that Internet probe was redirect to, if it was redirected. - * arg1 = One of the NETWORK_TESTED_RESULT_* constants. - * arg2 = NetID. + * obj = {@link NetworkTestedResults} representing information sent from NetworkMonitor. + * data = PersistableBundle of extras passed from NetworkMonitor. If {@link + * NetworkMonitorCallbacks#notifyNetworkTested} is called, this will be null. */ private static final int EVENT_NETWORK_TESTED = 41; @@ -597,6 +603,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set mWolSupportedInterfaces; private TelephonyManager mTelephonyManager; + private final AppOpsManager mAppOpsManager; + + private final LocationPermissionChecker mLocationPermissionChecker; private KeepaliveTracker mKeepaliveTracker; private NetworkNotificationManager mNotifier; @@ -993,6 +1002,8 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetd = netd; mKeyStore = KeyStore.getInstance(); mTelephonyManager = (TelephonyManager) mContext.getSystemService(Context.TELEPHONY_SERVICE); + mAppOpsManager = (AppOpsManager) mContext.getSystemService(Context.APP_OPS_SERVICE); + mLocationPermissionChecker = new LocationPermissionChecker(mContext); // To ensure uid rules are synchronized with Network Policy, register for // NetworkPolicyManagerService events must happen prior to NetworkPolicyManagerService @@ -2093,6 +2104,12 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } + private boolean checkNetworkStackPermission(int pid, int uid) { + return checkAnyPermissionOf(pid, uid, + android.Manifest.permission.NETWORK_STACK, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); + } + private boolean checkNetworkSignalStrengthWakeupPermission(int pid, int uid) { return checkAnyPermissionOf(pid, uid, android.Manifest.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP, @@ -2739,88 +2756,21 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case EVENT_NETWORK_TESTED: { - final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(msg.arg2); + final NetworkTestedResults results = (NetworkTestedResults) msg.obj; + + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(results.mNetId); if (nai == null) break; - final boolean wasPartial = nai.partialConnectivity; - nai.partialConnectivity = ((msg.arg1 & NETWORK_VALIDATION_RESULT_PARTIAL) != 0); - final boolean partialConnectivityChanged = - (wasPartial != nai.partialConnectivity); + handleNetworkTested(nai, results.mTestResult, + (results.mRedirectUrl == null) ? "" : results.mRedirectUrl); - final boolean valid = ((msg.arg1 & NETWORK_VALIDATION_RESULT_VALID) != 0); - final boolean wasValidated = nai.lastValidated; - final boolean wasDefault = isDefaultNetwork(nai); - // Only show a connected notification if the network is pending validation - // after the captive portal app was open, and it has now validated. - if (nai.captivePortalValidationPending && valid) { - // User is now logged in, network validated. - nai.captivePortalValidationPending = false; - showNetworkNotification(nai, NotificationType.LOGGED_IN); - } - - final String redirectUrl = (msg.obj instanceof String) ? (String) msg.obj : ""; - - if (DBG) { - final String logMsg = !TextUtils.isEmpty(redirectUrl) - ? " with redirect to " + redirectUrl - : ""; - log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); - } - if (valid != nai.lastValidated) { - if (wasDefault) { - mDeps.getMetricsLogger() - .defaultNetworkMetrics().logDefaultNetworkValidity( - SystemClock.elapsedRealtime(), valid); - } - final int oldScore = nai.getCurrentScore(); - nai.lastValidated = valid; - nai.everValidated |= valid; - updateCapabilities(oldScore, nai, nai.networkCapabilities); - // If score has changed, rebroadcast to NetworkProviders. b/17726566 - if (oldScore != nai.getCurrentScore()) sendUpdatedScoreToFactories(nai); - if (valid) { - handleFreshlyValidatedNetwork(nai); - // Clear NO_INTERNET, PRIVATE_DNS_BROKEN, PARTIAL_CONNECTIVITY and - // LOST_INTERNET notifications if network becomes valid. - mNotifier.clearNotification(nai.network.netId, - NotificationType.NO_INTERNET); - mNotifier.clearNotification(nai.network.netId, - NotificationType.LOST_INTERNET); - mNotifier.clearNotification(nai.network.netId, - NotificationType.PARTIAL_CONNECTIVITY); - mNotifier.clearNotification(nai.network.netId, - NotificationType.PRIVATE_DNS_BROKEN); - // If network becomes valid, the hasShownBroken should be reset for - // that network so that the notification will be fired when the private - // DNS is broken again. - nai.networkAgentConfig.hasShownBroken = false; - } - } else if (partialConnectivityChanged) { - updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); - } - updateInetCondition(nai); - // Let the NetworkAgent know the state of its network - Bundle redirectUrlBundle = new Bundle(); - redirectUrlBundle.putString(NetworkAgent.REDIRECT_URL_KEY, redirectUrl); - // TODO: Evaluate to update partial connectivity to status to NetworkAgent. - nai.asyncChannel.sendMessage( - NetworkAgent.CMD_REPORT_NETWORK_STATUS, - (valid ? NetworkAgent.VALID_NETWORK : NetworkAgent.INVALID_NETWORK), - 0, redirectUrlBundle); - - // If NetworkMonitor detects partial connectivity before - // EVENT_PROMPT_UNVALIDATED arrives, show the partial connectivity notification - // immediately. Re-notify partial connectivity silently if no internet - // notification already there. - if (!wasPartial && nai.partialConnectivity) { - // Remove delayed message if there is a pending message. - mHandler.removeMessages(EVENT_PROMPT_UNVALIDATED, nai.network); - handlePromptUnvalidated(nai.network); - } - - if (wasValidated && !nai.lastValidated) { - handleNetworkUnvalidated(nai); - } + // Invoke ConnectivityReport generation for this Network test event. + final Message m = + mConnectivityDiagnosticsHandler.obtainMessage( + ConnectivityDiagnosticsHandler.EVENT_NETWORK_TESTED, + new ConnectivityReportEvent(results.mTimestampMillis, nai)); + m.setData(msg.getData()); + mConnectivityDiagnosticsHandler.sendMessage(m); break; } case EVENT_PROVISIONING_NOTIFICATION: { @@ -2871,6 +2821,87 @@ public class ConnectivityService extends IConnectivityManager.Stub return true; } + private void handleNetworkTested( + @NonNull NetworkAgentInfo nai, int testResult, @NonNull String redirectUrl) { + final boolean wasPartial = nai.partialConnectivity; + nai.partialConnectivity = ((testResult & NETWORK_VALIDATION_RESULT_PARTIAL) != 0); + final boolean partialConnectivityChanged = + (wasPartial != nai.partialConnectivity); + + final boolean valid = ((testResult & NETWORK_VALIDATION_RESULT_VALID) != 0); + final boolean wasValidated = nai.lastValidated; + final boolean wasDefault = isDefaultNetwork(nai); + // Only show a connected notification if the network is pending validation + // after the captive portal app was open, and it has now validated. + if (nai.captivePortalValidationPending && valid) { + // User is now logged in, network validated. + nai.captivePortalValidationPending = false; + showNetworkNotification(nai, NotificationType.LOGGED_IN); + } + + if (DBG) { + final String logMsg = !TextUtils.isEmpty(redirectUrl) + ? " with redirect to " + redirectUrl + : ""; + log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); + } + if (valid != nai.lastValidated) { + if (wasDefault) { + mDeps.getMetricsLogger() + .defaultNetworkMetrics().logDefaultNetworkValidity( + SystemClock.elapsedRealtime(), valid); + } + final int oldScore = nai.getCurrentScore(); + nai.lastValidated = valid; + nai.everValidated |= valid; + updateCapabilities(oldScore, nai, nai.networkCapabilities); + // If score has changed, rebroadcast to NetworkProviders. b/17726566 + if (oldScore != nai.getCurrentScore()) sendUpdatedScoreToFactories(nai); + if (valid) { + handleFreshlyValidatedNetwork(nai); + // Clear NO_INTERNET, PRIVATE_DNS_BROKEN, PARTIAL_CONNECTIVITY and + // LOST_INTERNET notifications if network becomes valid. + mNotifier.clearNotification(nai.network.netId, + NotificationType.NO_INTERNET); + mNotifier.clearNotification(nai.network.netId, + NotificationType.LOST_INTERNET); + mNotifier.clearNotification(nai.network.netId, + NotificationType.PARTIAL_CONNECTIVITY); + mNotifier.clearNotification(nai.network.netId, + NotificationType.PRIVATE_DNS_BROKEN); + // If network becomes valid, the hasShownBroken should be reset for + // that network so that the notification will be fired when the private + // DNS is broken again. + nai.networkAgentConfig.hasShownBroken = false; + } + } else if (partialConnectivityChanged) { + updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); + } + updateInetCondition(nai); + // Let the NetworkAgent know the state of its network + Bundle redirectUrlBundle = new Bundle(); + redirectUrlBundle.putString(NetworkAgent.REDIRECT_URL_KEY, redirectUrl); + // TODO: Evaluate to update partial connectivity to status to NetworkAgent. + nai.asyncChannel.sendMessage( + NetworkAgent.CMD_REPORT_NETWORK_STATUS, + (valid ? NetworkAgent.VALID_NETWORK : NetworkAgent.INVALID_NETWORK), + 0, redirectUrlBundle); + + // If NetworkMonitor detects partial connectivity before + // EVENT_PROMPT_UNVALIDATED arrives, show the partial connectivity notification + // immediately. Re-notify partial connectivity silently if no internet + // notification already there. + if (!wasPartial && nai.partialConnectivity) { + // Remove delayed message if there is a pending message. + mHandler.removeMessages(EVENT_PROMPT_UNVALIDATED, nai.network); + handlePromptUnvalidated(nai.network); + } + + if (wasValidated && !nai.lastValidated) { + handleNetworkUnvalidated(nai); + } + } + private int getCaptivePortalMode() { return Settings.Global.getInt(mContext.getContentResolver(), Settings.Global.CAPTIVE_PORTAL_MODE, @@ -2919,8 +2950,23 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) { - mTrackerHandler.sendMessage(mTrackerHandler.obtainMessage(EVENT_NETWORK_TESTED, - testResult, mNetId, redirectUrl)); + notifyNetworkTestedWithExtras(testResult, redirectUrl, SystemClock.elapsedRealtime(), + PersistableBundle.EMPTY); + } + + @Override + public void notifyNetworkTestedWithExtras( + int testResult, + @Nullable String redirectUrl, + long timestampMillis, + @NonNull PersistableBundle extras) { + final Message msg = + mTrackerHandler.obtainMessage( + EVENT_NETWORK_TESTED, + new NetworkTestedResults( + mNetId, testResult, timestampMillis, redirectUrl)); + msg.setData(new Bundle(extras)); + mTrackerHandler.sendMessage(msg); } @Override @@ -7271,7 +7317,11 @@ public class ConnectivityService extends IConnectivityManager.Stub @GuardedBy("mVpns") private Vpn getVpnIfOwner() { - final int uid = Binder.getCallingUid(); + return getVpnIfOwner(Binder.getCallingUid()); + } + + @GuardedBy("mVpns") + private Vpn getVpnIfOwner(int uid) { final int user = UserHandle.getUserId(uid); final Vpn vpn = mVpns.get(user); @@ -7383,6 +7433,17 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final int EVENT_UNREGISTER_CONNECTIVITY_DIAGNOSTICS_CALLBACK = 2; + /** + * Event for {@link NetworkStateTrackerHandler} to trigger ConnectivityReport callbacks + * after processing {@link #EVENT_NETWORK_TESTED} events. + * obj = {@link ConnectivityReportEvent} representing ConnectivityReport info reported from + * NetworkMonitor. + * data = PersistableBundle of extras passed from NetworkMonitor. + * + *

See {@link ConnectivityService#EVENT_NETWORK_TESTED}. + */ + private static final int EVENT_NETWORK_TESTED = ConnectivityService.EVENT_NETWORK_TESTED; + private ConnectivityDiagnosticsHandler(Looper looper) { super(looper); } @@ -7400,6 +7461,19 @@ public class ConnectivityService extends IConnectivityManager.Stub (IConnectivityDiagnosticsCallback) msg.obj, msg.arg1); break; } + case EVENT_NETWORK_TESTED: { + final ConnectivityReportEvent reportEvent = + (ConnectivityReportEvent) msg.obj; + + // This is safe because {@link + // NetworkMonitorCallbacks#notifyNetworkTestedWithExtras} receives a + // PersistableBundle and converts it to the Bundle in the incoming Message. If + // {@link NetworkMonitorCallbacks#notifyNetworkTested} is called, msg.data will + // not be set. This is also safe, as msg.getData() will return an empty Bundle. + final PersistableBundle extras = new PersistableBundle(msg.getData()); + handleNetworkTestedWithExtras(reportEvent, extras); + break; + } } } } @@ -7409,12 +7483,16 @@ public class ConnectivityService extends IConnectivityManager.Stub class ConnectivityDiagnosticsCallbackInfo implements Binder.DeathRecipient { @NonNull private final IConnectivityDiagnosticsCallback mCb; @NonNull private final NetworkRequestInfo mRequestInfo; + @NonNull private final String mCallingPackageName; @VisibleForTesting ConnectivityDiagnosticsCallbackInfo( - @NonNull IConnectivityDiagnosticsCallback cb, @NonNull NetworkRequestInfo nri) { + @NonNull IConnectivityDiagnosticsCallback cb, + @NonNull NetworkRequestInfo nri, + @NonNull String callingPackageName) { mCb = cb; mRequestInfo = nri; + mCallingPackageName = callingPackageName; } @Override @@ -7424,6 +7502,39 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Class used for sending information from {@link + * NetworkMonitorCallbacks#notifyNetworkTestedWithExtras} to the handler for processing it. + */ + private static class NetworkTestedResults { + private final int mNetId; + private final int mTestResult; + private final long mTimestampMillis; + @Nullable private final String mRedirectUrl; + + private NetworkTestedResults( + int netId, int testResult, long timestampMillis, @Nullable String redirectUrl) { + mNetId = netId; + mTestResult = testResult; + mTimestampMillis = timestampMillis; + mRedirectUrl = redirectUrl; + } + } + + /** + * Class used for sending information from {@link NetworkStateTrackerHandler} to {@link + * ConnectivityDiagnosticsHandler}. + */ + private static class ConnectivityReportEvent { + private final long mTimestampMillis; + @NonNull private final NetworkAgentInfo mNai; + + private ConnectivityReportEvent(long timestampMillis, @NonNull NetworkAgentInfo nai) { + mTimestampMillis = timestampMillis; + mNai = nai; + } + } + private void handleRegisterConnectivityDiagnosticsCallback( @NonNull ConnectivityDiagnosticsCallbackInfo cbInfo) { ensureRunningOnConnectivityServiceThread(); @@ -7471,13 +7582,80 @@ public class ConnectivityService extends IConnectivityManager.Stub cb.asBinder().unlinkToDeath(mConnectivityDiagnosticsCallbacks.remove(cb), 0); } + private void handleNetworkTestedWithExtras( + @NonNull ConnectivityReportEvent reportEvent, @NonNull PersistableBundle extras) { + final NetworkAgentInfo nai = reportEvent.mNai; + final ConnectivityReport report = + new ConnectivityReport( + reportEvent.mNai.network, + reportEvent.mTimestampMillis, + nai.linkProperties, + nai.networkCapabilities, + extras); + final List results = + getMatchingPermissionedCallbacks(nai); + for (final IConnectivityDiagnosticsCallback cb : results) { + try { + cb.onConnectivityReport(report); + } catch (RemoteException ex) { + loge("Error invoking onConnectivityReport", ex); + } + } + } + + private List getMatchingPermissionedCallbacks( + @NonNull NetworkAgentInfo nai) { + final List results = new ArrayList<>(); + for (Entry entry : + mConnectivityDiagnosticsCallbacks.entrySet()) { + final ConnectivityDiagnosticsCallbackInfo cbInfo = entry.getValue(); + final NetworkRequestInfo nri = cbInfo.mRequestInfo; + if (nai.satisfies(nri.request)) { + if (checkConnectivityDiagnosticsPermissions( + nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) { + results.add(entry.getKey()); + } + } + } + return results; + } + + @VisibleForTesting + boolean checkConnectivityDiagnosticsPermissions( + int callbackPid, int callbackUid, NetworkAgentInfo nai, String callbackPackageName) { + if (checkNetworkStackPermission(callbackPid, callbackUid)) { + return true; + } + + if (!mLocationPermissionChecker.checkLocationPermission( + callbackPackageName, null /* featureId */, callbackUid, null /* message */)) { + return false; + } + + synchronized (mVpns) { + if (getVpnIfOwner(callbackUid) != null) { + return true; + } + } + + // Administrator UIDs also contains the Owner UID + if (nai.networkCapabilities.getAdministratorUids().contains(callbackUid)) { + return true; + } + + return false; + } + @Override public void registerConnectivityDiagnosticsCallback( - @NonNull IConnectivityDiagnosticsCallback callback, @NonNull NetworkRequest request) { + @NonNull IConnectivityDiagnosticsCallback callback, + @NonNull NetworkRequest request, + @NonNull String callingPackageName) { if (request.legacyType != TYPE_NONE) { throw new IllegalArgumentException("ConnectivityManager.TYPE_* are deprecated." + " Please use NetworkCapabilities instead."); } + mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName); // This NetworkCapabilities is only used for matching to Networks. Clear out its owner uid // and administrator uids to be safe. @@ -7495,7 +7673,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // callback's binder death. final NetworkRequestInfo nri = new NetworkRequestInfo(requestWithId); final ConnectivityDiagnosticsCallbackInfo cbInfo = - new ConnectivityDiagnosticsCallbackInfo(callback, nri); + new ConnectivityDiagnosticsCallbackInfo(callback, nri, callingPackageName); mConnectivityDiagnosticsHandler.sendMessage( mConnectivityDiagnosticsHandler.obtainMessage( diff --git a/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java b/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java index 2d5df4f47e..0628691c33 100644 --- a/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java +++ b/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java @@ -38,6 +38,8 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import android.content.Context; import android.os.PersistableBundle; +import androidx.test.InstrumentationRegistry; + import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -58,21 +60,26 @@ public class ConnectivityDiagnosticsManagerTest { private static final Executor INLINE_EXECUTOR = x -> x.run(); - @Mock private Context mContext; @Mock private IConnectivityManager mService; @Mock private ConnectivityDiagnosticsCallback mCb; + private Context mContext; private ConnectivityDiagnosticsBinder mBinder; private ConnectivityDiagnosticsManager mManager; + private String mPackageName; + @Before public void setUp() { - mContext = mock(Context.class); + mContext = InstrumentationRegistry.getContext(); + mService = mock(IConnectivityManager.class); mCb = mock(ConnectivityDiagnosticsCallback.class); mBinder = new ConnectivityDiagnosticsBinder(mCb, INLINE_EXECUTOR); mManager = new ConnectivityDiagnosticsManager(mContext, mService); + + mPackageName = mContext.getOpPackageName(); } @After @@ -271,7 +278,7 @@ public class ConnectivityDiagnosticsManagerTest { mManager.registerConnectivityDiagnosticsCallback(request, INLINE_EXECUTOR, mCb); verify(mService).registerConnectivityDiagnosticsCallback( - any(ConnectivityDiagnosticsBinder.class), eq(request)); + any(ConnectivityDiagnosticsBinder.class), eq(request), eq(mPackageName)); assertTrue(ConnectivityDiagnosticsManager.sCallbacks.containsKey(mCb)); } @@ -302,7 +309,7 @@ public class ConnectivityDiagnosticsManagerTest { // verify that re-registering is successful mManager.registerConnectivityDiagnosticsCallback(request, INLINE_EXECUTOR, mCb); verify(mService, times(2)).registerConnectivityDiagnosticsCallback( - any(ConnectivityDiagnosticsBinder.class), eq(request)); + any(ConnectivityDiagnosticsBinder.class), eq(request), eq(mPackageName)); assertTrue(ConnectivityDiagnosticsManager.sCallbacks.containsKey(mCb)); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 50f1bbeed0..46bc1b5f3c 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -23,6 +23,7 @@ import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.content.pm.PackageManager.PERMISSION_DENIED; import static android.content.pm.PackageManager.PERMISSION_GRANTED; +import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION_SUPL; @@ -119,6 +120,7 @@ import static org.mockito.Mockito.when; import android.Manifest; import android.annotation.NonNull; import android.app.AlarmManager; +import android.app.AppOpsManager; import android.app.NotificationManager; import android.app.PendingIntent; import android.content.BroadcastReceiver; @@ -132,6 +134,7 @@ import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.content.pm.UserInfo; import android.content.res.Resources; +import android.location.LocationManager; import android.net.CaptivePortalData; import android.net.ConnectivityManager; import android.net.ConnectivityManager.NetworkCallback; @@ -177,6 +180,7 @@ import android.net.shared.PrivateDnsConfig; import android.net.util.MultinetworkPolicyTracker; import android.os.BadParcelableException; import android.os.Binder; +import android.os.Build; import android.os.Bundle; import android.os.ConditionVariable; import android.os.Handler; @@ -187,6 +191,7 @@ import android.os.Looper; import android.os.Parcel; import android.os.ParcelFileDescriptor; import android.os.Parcelable; +import android.os.PersistableBundle; import android.os.Process; import android.os.RemoteException; import android.os.SystemClock; @@ -218,6 +223,7 @@ import com.android.server.connectivity.DefaultNetworkMetrics; import com.android.server.connectivity.IpConnectivityMetrics; import com.android.server.connectivity.MockableSystemProperties; import com.android.server.connectivity.Nat464Xlat; +import com.android.server.connectivity.NetworkAgentInfo; import com.android.server.connectivity.NetworkNotificationManager.NotificationType; import com.android.server.connectivity.ProxyTracker; import com.android.server.connectivity.Vpn; @@ -292,6 +298,8 @@ public class ConnectivityServiceTest { private static final int UNREASONABLY_LONG_ALARM_WAIT_MS = 1000; + private static final long TIMESTAMP = 1234L; + private static final String CLAT_PREFIX = "v4-"; private static final String MOBILE_IFNAME = "test_rmnet_data0"; private static final String WIFI_IFNAME = "test_wlan0"; @@ -327,6 +335,8 @@ public class ConnectivityServiceTest { @Mock AlarmManager mAlarmManager; @Mock IConnectivityDiagnosticsCallback mConnectivityDiagnosticsCallback; @Mock IBinder mIBinder; + @Mock LocationManager mLocationManager; + @Mock AppOpsManager mAppOpsManager; private ArgumentCaptor mResolverParamsParcelCaptor = ArgumentCaptor.forClass(ResolverParamsParcel.class); @@ -412,6 +422,8 @@ public class ConnectivityServiceTest { if (Context.NETWORK_STACK_SERVICE.equals(name)) return mNetworkStack; if (Context.USER_SERVICE.equals(name)) return mUserManager; if (Context.ALARM_SERVICE.equals(name)) return mAlarmManager; + if (Context.LOCATION_SERVICE.equals(name)) return mLocationManager; + if (Context.APP_OPS_SERVICE.equals(name)) return mAppOpsManager; return super.getSystemService(name); } @@ -564,6 +576,7 @@ public class ConnectivityServiceTest { private int mProbesCompleted; private int mProbesSucceeded; private String mNmValidationRedirectUrl = null; + private PersistableBundle mValidationExtras = PersistableBundle.EMPTY; private boolean mNmProvNotificationRequested = false; private final ConditionVariable mNetworkStatusReceived = new ConditionVariable(); @@ -631,8 +644,8 @@ public class ConnectivityServiceTest { } mNmCallbacks.notifyProbeStatusChanged(mProbesCompleted, mProbesSucceeded); - mNmCallbacks.notifyNetworkTested( - mNmValidationResult, mNmValidationRedirectUrl); + mNmCallbacks.notifyNetworkTestedWithExtras( + mNmValidationResult, mNmValidationRedirectUrl, TIMESTAMP, mValidationExtras); if (mNmValidationRedirectUrl != null) { mNmCallbacks.showProvisioningNotification( @@ -970,6 +983,8 @@ public class ConnectivityServiceTest { // not inherit from NetworkAgent. private TestNetworkAgentWrapper mMockNetworkAgent; + private VpnInfo mVpnInfo; + public MockVpn(int userId) { super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService, userId); @@ -1041,6 +1056,17 @@ public class ConnectivityServiceTest { mConnected = false; mConfig = null; } + + @Override + public synchronized VpnInfo getVpnInfo() { + if (mVpnInfo != null) return mVpnInfo; + + return super.getVpnInfo(); + } + + private void setVpnInfo(VpnInfo vpnInfo) { + mVpnInfo = vpnInfo; + } } private void mockVpn(int uid) { @@ -6368,7 +6394,7 @@ public class ConnectivityServiceTest { new NetworkCapabilities(), TYPE_ETHERNET, 0, NetworkRequest.Type.NONE); try { mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, request); + mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); fail("registerConnectivityDiagnosticsCallback should throw on invalid NetworkRequest"); } catch (IllegalArgumentException expected) { } @@ -6382,7 +6408,7 @@ public class ConnectivityServiceTest { when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, wifiRequest); + mConnectivityDiagnosticsCallback, wifiRequest, mContext.getPackageName()); verify(mIBinder, timeout(TIMEOUT_MS)) .linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); @@ -6406,7 +6432,7 @@ public class ConnectivityServiceTest { when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, wifiRequest); + mConnectivityDiagnosticsCallback, wifiRequest, mContext.getPackageName()); verify(mIBinder, timeout(TIMEOUT_MS)) .linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); @@ -6417,7 +6443,7 @@ public class ConnectivityServiceTest { // Register the same callback again mService.registerConnectivityDiagnosticsCallback( - mConnectivityDiagnosticsCallback, wifiRequest); + mConnectivityDiagnosticsCallback, wifiRequest, mContext.getPackageName()); // Block until all other events are done processing. HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); @@ -6426,4 +6452,145 @@ public class ConnectivityServiceTest { mService.mConnectivityDiagnosticsCallbacks.containsKey( mConnectivityDiagnosticsCallback)); } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception { + final NetworkAgentInfo naiWithoutUid = + new NetworkAgentInfo( + null, null, null, null, null, new NetworkCapabilities(), null, + mServiceContext, null, null, mService, null, null, null, 0); + + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + assertTrue( + "NetworkStack permission not applied", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { + final NetworkAgentInfo naiWithoutUid = + new NetworkAgentInfo( + null, null, null, null, null, new NetworkCapabilities(), null, + mServiceContext, null, null, mService, null, null, null, 0); + + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + assertFalse( + "ACCESS_FINE_LOCATION permission necessary for Connectivity Diagnostics", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception { + final NetworkAgentInfo naiWithoutUid = + new NetworkAgentInfo( + null, null, null, null, null, new NetworkCapabilities(), null, + mServiceContext, null, null, mService, null, null, null, 0); + + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + // setUp() calls mockVpn() which adds a VPN with the Test Runner's uid. Configure it to be + // active + final VpnInfo info = new VpnInfo(); + info.ownerUid = Process.myUid(); + info.vpnIface = "interface"; + mMockVpn.setVpnInfo(info); + assertTrue( + "Active VPN permission not applied", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsNetworkAdministrator() throws Exception { + final NetworkCapabilities nc = new NetworkCapabilities(); + nc.setAdministratorUids(Arrays.asList(Process.myUid())); + final NetworkAgentInfo naiWithUid = + new NetworkAgentInfo( + null, null, null, null, null, nc, null, mServiceContext, null, null, + mService, null, null, null, 0); + + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + // Disconnect mock vpn so the uid check on NetworkAgentInfo is tested + mMockVpn.disconnect(); + assertTrue( + "NetworkCapabilities administrator uid permission not applied", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithUid, mContext.getOpPackageName())); + } + + @Test + public void testCheckConnectivityDiagnosticsPermissionsFails() throws Exception { + final NetworkCapabilities nc = new NetworkCapabilities(); + nc.setOwnerUid(Process.myUid()); + nc.setAdministratorUids(Arrays.asList(Process.myUid())); + final NetworkAgentInfo naiWithUid = + new NetworkAgentInfo( + null, null, null, null, null, nc, null, mServiceContext, null, null, + mService, null, null, null, 0); + + setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, + Manifest.permission.ACCESS_FINE_LOCATION); + mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); + + // Use wrong pid and uid + assertFalse( + "Permissions allowed when they shouldn't be granted", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid() + 1, Process.myUid() + 1, naiWithUid, + mContext.getOpPackageName())); + } + + private void setupLocationPermissions( + int targetSdk, boolean locationToggle, String op, String perm) throws Exception { + final ApplicationInfo applicationInfo = new ApplicationInfo(); + applicationInfo.targetSdkVersion = targetSdk; + when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any())) + .thenReturn(applicationInfo); + + when(mLocationManager.isLocationEnabledForUser(any())).thenReturn(locationToggle); + + when(mAppOpsManager.noteOp(eq(op), eq(Process.myUid()), eq(mContext.getPackageName()))) + .thenReturn(AppOpsManager.MODE_ALLOWED); + + mServiceContext.setPermission(perm, PERMISSION_GRANTED); + } + + @Test + public void testConnectivityDiagnosticsCallbackOnConnectivityReport() throws Exception { + final NetworkRequest request = new NetworkRequest.Builder().build(); + when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); + + mServiceContext.setPermission( + android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); + + mService.registerConnectivityDiagnosticsCallback( + mConnectivityDiagnosticsCallback, request, mContext.getPackageName()); + + // Block until all other events are done processing. + HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); + + // Connect the cell agent verify that it notifies TestNetworkCallback that it is available + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(true); + callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + callback.assertNoCallback(); + + // Wait for onConnectivityReport to fire + verify(mConnectivityDiagnosticsCallback, timeout(TIMEOUT_MS)) + .onConnectivityReport(any(ConnectivityReport.class)); + } }