From fe059d8c302d464f80113e2a9178e69ee5b65bc1 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Wed, 12 Feb 2020 14:50:58 -0800 Subject: [PATCH 1/8] Invoke onConnectivityReport on registering ConnectivityDiagnostics. This change updates the behavior for registering ConnectivityDiagnosticsCallbacks. Now, after a successful register() call, callbacks will receive cached ConnectivityReports for all matching, permissioned networks. This allows registrants to be updated with the network state for their networks without having to wait for the next network validation. Bug: 147849853 Test: atest FrameworksNetTests Change-Id: I924ba8fdcc847f453557021591bde38602fe089c Merged-In: I924ba8fdcc847f453557021591bde38602fe089c (cherry picked from commit 95ec0b206b759e1d26bc1dbb2255a515bb24358a) --- .../net/ConnectivityDiagnosticsManager.java | 3 +- .../android/server/ConnectivityService.java | 29 ++++++++++++++++ .../server/connectivity/NetworkAgentInfo.java | 28 +++++++++++++++ .../server/ConnectivityServiceTest.java | 34 +++++++++++++++++++ 4 files changed, 93 insertions(+), 1 deletion(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 6f0a4f9744..aa35852f78 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -659,7 +659,8 @@ public class ConnectivityDiagnosticsManager { public abstract static class ConnectivityDiagnosticsCallback { /** * Called when the platform completes a data connectivity check. This will also be invoked - * upon registration with the latest report. + * immediately upon registration with the latest report, if a report has already been + * generated for this network. * *

The Network specified in the ConnectivityReport may not be active any more when this * method is invoked. diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d684f0c199..2b5f21a172 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7855,6 +7855,34 @@ public class ConnectivityService extends IConnectivityManager.Stub cb.asBinder().linkToDeath(cbInfo, 0); } catch (RemoteException e) { cbInfo.binderDied(); + return; + } + + // Once registered, provide ConnectivityReports for matching Networks + final List matchingNetworks = new ArrayList<>(); + synchronized (mNetworkForNetId) { + for (int i = 0; i < mNetworkForNetId.size(); i++) { + final NetworkAgentInfo nai = mNetworkForNetId.valueAt(i); + if (nai.satisfies(nri.request)) { + matchingNetworks.add(nai); + } + } + } + for (final NetworkAgentInfo nai : matchingNetworks) { + final ConnectivityReport report = nai.getConnectivityReport(); + if (report == null) { + continue; + } + if (!checkConnectivityDiagnosticsPermissions( + nri.mPid, nri.mUid, nai, cbInfo.mCallingPackageName)) { + continue; + } + + try { + cb.onConnectivityReportAvailable(report); + } catch (RemoteException e) { + // Exception while sending the ConnectivityReport. Move on to the next network. + } } } @@ -7890,6 +7918,7 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.linkProperties, networkCapabilities, extras); + nai.setConnectivityReport(report); final List results = getMatchingPermissionedCallbacks(nai); for (final IConnectivityDiagnosticsCallback cb : results) { diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 2f047157d4..d2086138a1 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -16,6 +16,7 @@ package com.android.server.connectivity; +import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport; import static android.net.NetworkCapabilities.transportNamesOf; import android.annotation.NonNull; @@ -243,6 +244,9 @@ public class NetworkAgentInfo implements Comparable { // How many of the satisfied requests are of type BACKGROUND_REQUEST. private int mNumBackgroundNetworkRequests = 0; + // The last ConnectivityReport made available for this network. + private ConnectivityReport mConnectivityReport; + public final Messenger messenger; public final AsyncChannel asyncChannel; @@ -621,6 +625,30 @@ public class NetworkAgentInfo implements Comparable { for (LingerTimer timer : mLingerTimers) { pw.println(timer); } } + /** + * Sets the most recent ConnectivityReport for this network. + * + *

This should only be called from the ConnectivityService thread. + * + * @hide + */ + public void setConnectivityReport(@NonNull ConnectivityReport connectivityReport) { + mConnectivityReport = connectivityReport; + } + + /** + * Returns the most recent ConnectivityReport for this network, or null if none have been + * reported yet. + * + *

This should only be called from the ConnectivityService thread. + * + * @hide + */ + @Nullable + public ConnectivityReport getConnectivityReport() { + return mConnectivityReport; + } + // TODO: Print shorter members first and only print the boolean variable which value is true // to improve readability. public String toString() { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 86d8a820e4..50b61fb40f 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -317,6 +317,8 @@ public class ConnectivityServiceTest { private static final String TEST_PACKAGE_NAME = "com.android.test.package"; private static final String[] EMPTY_STRING_ARRAY = new String[0]; + private static final String INTERFACE_NAME = "interface"; + private MockContext mServiceContext; private HandlerThread mCsHandlerThread; private ConnectivityService mService; @@ -6915,6 +6917,38 @@ public class ConnectivityServiceTest { mContext.getOpPackageName())); } + @Test + public void testRegisterConnectivityDiagnosticsCallbackCallsOnConnectivityReport() + throws Exception { + // Set up the Network, which leads to a ConnectivityReport being cached for the network. + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + final LinkProperties linkProperties = new LinkProperties(); + linkProperties.setInterfaceName(INTERFACE_NAME); + mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, linkProperties); + mCellNetworkAgent.connect(true); + callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + callback.assertNoCallback(); + + 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); + + verify(mConnectivityDiagnosticsCallback) + .onConnectivityReportAvailable(argThat(report -> { + return INTERFACE_NAME.equals(report.getLinkProperties().getInterfaceName()) + && report.getNetworkCapabilities().hasTransport(TRANSPORT_CELLULAR); + })); + } + private void setUpConnectivityDiagnosticsCallback() throws Exception { final NetworkRequest request = new NetworkRequest.Builder().build(); when(mConnectivityDiagnosticsCallback.asBinder()).thenReturn(mIBinder); From aebda983d39cb83ac1d8280535f3956e3d51f701 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Wed, 4 Mar 2020 13:35:20 -0800 Subject: [PATCH 2/8] Decrement networkRequestPerUid when callbacks are unregistered. ConnectivityDiagnosticsCallbacks are tied to NetworkRequestInfo objects when registered with the platform. Each NetworkRequestInfo is tied to a specific uid, and ConnectivityService enforces a limit on the number of network requests that can be associated with each uid. When ConnectivityDiagnosticsCallbacks are unregistered from the platform, their NetworkRequestInfo is freed and the number of network requests per the user's uid should be decremented. Bug: 150802582 Test: atest android.net.cts.ConnectivityDiagnosticsManagerTest Change-Id: Ia5ed39c1d8e6221cd402be4f8baf69fa643a6113 Merged-In: Ia5ed39c1d8e6221cd402be4f8baf69fa643a6113 (cherry picked from commit 662076b1a7c0f064fa746fc7b8d3204c966c8e48) --- .../core/java/com/android/server/ConnectivityService.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 2b5f21a172..63f7cb3b7d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7902,6 +7902,11 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } + // Decrement the reference count for this NetworkRequestInfo. The reference count is + // incremented when the NetworkRequestInfo is created as part of + // enforceRequestCountLimit(). + decrementNetworkRequestPerUidCount(nri); + cb.asBinder().unlinkToDeath(mConnectivityDiagnosticsCallbacks.remove(cb), 0); } From b65e18e638fdd14ad1a2cfa88fa8ea3c8e22434f Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Thu, 5 Mar 2020 10:46:02 -0800 Subject: [PATCH 3/8] Use IBinder as key for ConnectivityDiagnostics storage in CS. This change updates ConnectivityService to use IBinder instances as keys when storing ConnectivityDiagnosticsCallbacks. When storing ConnectivityDiagnosticsCallbacks in ConnectivityService, the IConnectivityDiagnsoticsCallback is used as the key for ConnectivityService.mConnectivityDiagnosticsCallbacks. However, IConnectivityDiagnosticsCallback instances are received as different objects. This causes them to produce different hashCode() values, so attempts to remove an IConnectivityDiagnosticsCallback fail. Bug: 150867635 Test: atest FrameworksNetTests Change-Id: Ib99e68d5ae47fa27e12428f9a60a2c1204ac59a2 Merged-In: Ib99e68d5ae47fa27e12428f9a60a2c1204ac59a2 (cherry picked from commit c7c6a4ac12beb7c216076958612869426da06da0) --- .../android/server/ConnectivityService.java | 24 +++++++++++-------- .../server/ConnectivityServiceTest.java | 16 ++++--------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 63f7cb3b7d..e3f574f366 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -653,8 +653,8 @@ public class ConnectivityService extends IConnectivityManager.Stub final MultipathPolicyTracker mMultipathPolicyTracker; @VisibleForTesting - final Map - mConnectivityDiagnosticsCallbacks = new HashMap<>(); + final Map mConnectivityDiagnosticsCallbacks = + new HashMap<>(); /** * Implements support for the legacy "one network per network type" model. @@ -7835,11 +7835,12 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureRunningOnConnectivityServiceThread(); final IConnectivityDiagnosticsCallback cb = cbInfo.mCb; + final IBinder iCb = cb.asBinder(); final NetworkRequestInfo nri = cbInfo.mRequestInfo; // This means that the client registered the same callback multiple times. Do // not override the previous entry, and exit silently. - if (mConnectivityDiagnosticsCallbacks.containsKey(cb)) { + if (mConnectivityDiagnosticsCallbacks.containsKey(iCb)) { if (VDBG) log("Diagnostics callback is already registered"); // Decrement the reference count for this NetworkRequestInfo. The reference count is @@ -7849,10 +7850,10 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } - mConnectivityDiagnosticsCallbacks.put(cb, cbInfo); + mConnectivityDiagnosticsCallbacks.put(iCb, cbInfo); try { - cb.asBinder().linkToDeath(cbInfo, 0); + iCb.linkToDeath(cbInfo, 0); } catch (RemoteException e) { cbInfo.binderDied(); return; @@ -7889,13 +7890,14 @@ public class ConnectivityService extends IConnectivityManager.Stub private void handleUnregisterConnectivityDiagnosticsCallback( @NonNull IConnectivityDiagnosticsCallback cb, int uid) { ensureRunningOnConnectivityServiceThread(); + final IBinder iCb = cb.asBinder(); - if (!mConnectivityDiagnosticsCallbacks.containsKey(cb)) { + if (!mConnectivityDiagnosticsCallbacks.containsKey(iCb)) { if (VDBG) log("Removing diagnostics callback that is not currently registered"); return; } - final NetworkRequestInfo nri = mConnectivityDiagnosticsCallbacks.get(cb).mRequestInfo; + final NetworkRequestInfo nri = mConnectivityDiagnosticsCallbacks.get(iCb).mRequestInfo; if (uid != nri.mUid) { if (VDBG) loge("Different uid than registrant attempting to unregister cb"); @@ -7907,7 +7909,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // enforceRequestCountLimit(). decrementNetworkRequestPerUidCount(nri); - cb.asBinder().unlinkToDeath(mConnectivityDiagnosticsCallbacks.remove(cb), 0); + final ConnectivityDiagnosticsCallbackInfo cbInfo = + mConnectivityDiagnosticsCallbacks.remove(iCb); + iCb.unlinkToDeath(cbInfo, 0); } private void handleNetworkTestedWithExtras( @@ -7982,14 +7986,14 @@ public class ConnectivityService extends IConnectivityManager.Stub private List getMatchingPermissionedCallbacks( @NonNull NetworkAgentInfo nai) { final List results = new ArrayList<>(); - for (Entry entry : + 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()); + results.add(entry.getValue().mCb); } } } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 50b61fb40f..b02398d1f1 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6750,16 +6750,12 @@ public class ConnectivityServiceTest { verify(mIBinder).linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); verify(mConnectivityDiagnosticsCallback).asBinder(); - assertTrue( - mService.mConnectivityDiagnosticsCallbacks.containsKey( - mConnectivityDiagnosticsCallback)); + assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder)); mService.unregisterConnectivityDiagnosticsCallback(mConnectivityDiagnosticsCallback); verify(mIBinder, timeout(TIMEOUT_MS)) .unlinkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); - assertFalse( - mService.mConnectivityDiagnosticsCallbacks.containsKey( - mConnectivityDiagnosticsCallback)); + assertFalse(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder)); verify(mConnectivityDiagnosticsCallback, atLeastOnce()).asBinder(); } @@ -6777,9 +6773,7 @@ public class ConnectivityServiceTest { verify(mIBinder).linkToDeath(any(ConnectivityDiagnosticsCallbackInfo.class), anyInt()); verify(mConnectivityDiagnosticsCallback).asBinder(); - assertTrue( - mService.mConnectivityDiagnosticsCallbacks.containsKey( - mConnectivityDiagnosticsCallback)); + assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder)); // Register the same callback again mService.registerConnectivityDiagnosticsCallback( @@ -6788,9 +6782,7 @@ public class ConnectivityServiceTest { // Block until all other events are done processing. HandlerUtilsKt.waitForIdle(mCsHandlerThread, TIMEOUT_MS); - assertTrue( - mService.mConnectivityDiagnosticsCallbacks.containsKey( - mConnectivityDiagnosticsCallback)); + assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder)); } @Test From 0d667c5508e2c54f6dc2d575c5653de08703c0a7 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 6 Feb 2020 18:31:19 +0900 Subject: [PATCH 4/8] Add combine() and equals() for NetworkCapabilities admin UIDs. NetworkCapabilities#mAdministratorUids should be checked for equality when combining NetworkCapabilities. Administrator UIDs should also be included in NetworkCapabilities equals() and hashCode(). Bug: 147903575 Test: FrameworksNetTests Change-Id: I803bdec80e27ee80d3a39844c5fb7aed584ab07d Merged-In: I803bdec80e27ee80d3a39844c5fb7aed584ab07d (cherry picked from commit 5fad8aa761336012bb228afc3b6f7d42fa274242) --- .../java/android/net/NetworkCapabilities.java | 65 +++++++++++++++++-- .../android/net/NetworkCapabilitiesTest.java | 21 ++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 4e4ff4a63a..18bbb07da4 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -16,6 +16,8 @@ package android.net; +import static com.android.internal.annotations.VisibleForTesting.Visibility.PRIVATE; + import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; @@ -118,7 +120,7 @@ public final class NetworkCapabilities implements Parcelable { mTransportInfo = nc.mTransportInfo; mSignalStrength = nc.mSignalStrength; setUids(nc.mUids); // Will make the defensive copy - setAdministratorUids(nc.mAdministratorUids); + setAdministratorUids(nc.getAdministratorUids()); mOwnerUid = nc.mOwnerUid; mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; mSSID = nc.mSSID; @@ -919,6 +921,9 @@ public final class NetworkCapabilities implements Parcelable { *

For NetworkCapability instances being sent from the System Server, this value MUST be * empty unless the destination is 1) the System Server, or 2) Telephony. In either case, the * receiving entity must have the ACCESS_FINE_LOCATION permission and target R+. + * + *

When received from an app in a NetworkRequest this is always cleared out by the system + * server. This field is never used for matching NetworkRequests to NetworkAgents. */ @NonNull private int[] mAdministratorUids = new int[0]; @@ -927,10 +932,11 @@ public final class NetworkCapabilities implements Parcelable { * *

UIDs included in administratorUids gain administrator privileges over this Network. * Examples of UIDs that should be included in administratorUids are: + * *

* *

In general, user-supplied networks (such as WiFi networks) do not have an administrator. @@ -938,7 +944,10 @@ public final class NetworkCapabilities implements Parcelable { *

An app is granted owner privileges over Networks that it supplies. The owner UID MUST * always be included in administratorUids. * + *

The administrator UIDs are set by network agents. + * * @param administratorUids the UIDs to be set as administrators of this Network. + * @see #mAdministratorUids * @hide */ @NonNull @@ -950,7 +959,12 @@ public final class NetworkCapabilities implements Parcelable { /** * Retrieves the UIDs that are administrators of this Network. * + *

This is only populated in NetworkCapabilities objects that come from network agents for + * networks that are managed by specific apps on the system, such as carrier privileged apps or + * wifi suggestion apps. This will include the network owner. + * * @return the int[] of UIDs that are administrators of this Network + * @see #mAdministratorUids * @hide */ @NonNull @@ -960,6 +974,40 @@ public final class NetworkCapabilities implements Parcelable { return Arrays.copyOf(mAdministratorUids, mAdministratorUids.length); } + /** + * Tests if the set of administrator UIDs of this network is the same as that of the passed one. + * + *

The administrator UIDs must be in sorted order. + * + *

nc is assumed non-null. Else, NPE. + * + * @hide + */ + @VisibleForTesting(visibility = PRIVATE) + public boolean equalsAdministratorUids(@NonNull final NetworkCapabilities nc) { + return Arrays.equals(mAdministratorUids, nc.mAdministratorUids); + } + + /** + * Combine the administrator UIDs of the capabilities. + * + *

This is only legal if either of the administrators lists are empty, or if they are equal. + * Combining administrator UIDs is only possible for combining non-overlapping sets of UIDs. + * + *

If both administrator lists are non-empty but not equal, they conflict with each other. In + * this case, it would not make sense to add them together. + */ + private void combineAdministratorUids(@NonNull final NetworkCapabilities nc) { + if (nc.mAdministratorUids.length == 0) return; + if (mAdministratorUids.length == 0) { + mAdministratorUids = Arrays.copyOf(nc.mAdministratorUids, nc.mAdministratorUids.length); + return; + } + if (!equalsAdministratorUids(nc)) { + throw new IllegalStateException("Can't combine two different administrator UID lists"); + } + } + /** * Value indicating that link bandwidth is unspecified. * @hide @@ -1455,6 +1503,7 @@ public final class NetworkCapabilities implements Parcelable { combineUids(nc); combineSSIDs(nc); combineRequestor(nc); + combineAdministratorUids(nc); } /** @@ -1568,7 +1617,8 @@ public final class NetworkCapabilities implements Parcelable { && equalsUids(that) && equalsSSID(that) && equalsPrivateDnsBroken(that) - && equalsRequestor(that); + && equalsRequestor(that) + && equalsAdministratorUids(that); } @Override @@ -1588,7 +1638,8 @@ public final class NetworkCapabilities implements Parcelable { + Objects.hashCode(mTransportInfo) * 41 + Objects.hashCode(mPrivateDnsBroken) * 43 + Objects.hashCode(mRequestorUid) * 47 - + Objects.hashCode(mRequestorPackageName) * 53; + + Objects.hashCode(mRequestorPackageName) * 53 + + Arrays.hashCode(mAdministratorUids) * 59; } @Override @@ -1609,7 +1660,7 @@ public final class NetworkCapabilities implements Parcelable { dest.writeArraySet(mUids); dest.writeString(mSSID); dest.writeBoolean(mPrivateDnsBroken); - dest.writeIntArray(mAdministratorUids); + dest.writeIntArray(getAdministratorUids()); dest.writeInt(mOwnerUid); dest.writeInt(mRequestorUid); dest.writeString(mRequestorPackageName); diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index 916c339811..db3372de16 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -58,6 +58,7 @@ import androidx.test.runner.AndroidJUnit4; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.Arrays; import java.util.Set; @RunWith(AndroidJUnit4.class) @@ -280,6 +281,7 @@ public class NetworkCapabilitiesTest { .addCapability(NET_CAPABILITY_NOT_METERED); if (isAtLeastR()) { netCap.setOwnerUid(123); + netCap.setAdministratorUids(new int[] {5, 11}); } assertParcelingIsLossless(netCap); netCap.setSSID(TEST_SSID); @@ -491,6 +493,25 @@ public class NetworkCapabilitiesTest { assertFalse(nc2.appliesToUid(12)); assertTrue(nc1.appliesToUid(22)); assertTrue(nc2.appliesToUid(22)); + + final int[] adminUids = {3, 6, 12}; + nc1.setAdministratorUids(adminUids); + nc2.combineCapabilities(nc1); + assertTrue(nc2.equalsAdministratorUids(nc1)); + assertArrayEquals(nc2.getAdministratorUids(), adminUids); + + final int[] adminUidsOtherOrder = {3, 12, 6}; + nc1.setAdministratorUids(adminUids); + assertTrue(nc2.equalsAdministratorUids(nc1)); + + final int[] adminUids2 = {11, 1, 12, 3, 6}; + nc1.setAdministratorUids(adminUids2); + assertFalse(nc2.equalsAdministratorUids(nc1)); + assertFalse(Arrays.equals(nc2.getAdministratorUids(), adminUids2)); + try { + nc2.combineCapabilities(nc1); + fail("Shouldn't be able to combine different lists of admin UIDs"); + } catch (IllegalStateException expected) { } } @Test From 9c341963ac91b861f407ffec7a73e180de845bad Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Tue, 24 Mar 2020 11:53:30 -0700 Subject: [PATCH 5/8] Sort administrator UIDs for NetworkCapabilities. Administrator UIDs stored in NetworkCapabilities should be sorted. This allows for easier equals checks and hashCode computation. Additionally, duplicate UIDs should be prevented. Bug: 147903575 Test: atest FrameworksNetTests Change-Id: Ia5387ca2ce7c3fcbd04dc7fbff5266f7bcc71694 Merged-In: Ia5387ca2ce7c3fcbd04dc7fbff5266f7bcc71694 (cherry picked from commit 2091bd9059d1d24f8c6bd1cd345361f185cab1ea) --- core/java/android/net/NetworkCapabilities.java | 7 +++++++ .../android/net/NetworkCapabilitiesTest.java | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 18bbb07da4..9ff7ebee6d 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -947,12 +947,19 @@ public final class NetworkCapabilities implements Parcelable { *

The administrator UIDs are set by network agents. * * @param administratorUids the UIDs to be set as administrators of this Network. + * @throws IllegalArgumentException if duplicate UIDs are contained in administratorUids * @see #mAdministratorUids * @hide */ @NonNull public NetworkCapabilities setAdministratorUids(@NonNull final int[] administratorUids) { mAdministratorUids = Arrays.copyOf(administratorUids, administratorUids.length); + Arrays.sort(mAdministratorUids); + for (int i = 0; i < mAdministratorUids.length - 1; i++) { + if (mAdministratorUids[i] >= mAdministratorUids[i + 1]) { + throw new IllegalArgumentException("All administrator UIDs must be unique"); + } + } return this; } diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index db3372de16..316a83adf4 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -441,6 +441,23 @@ public class NetworkCapabilitiesTest { return range; } + @Test + public void testSetAdministratorUids() { + NetworkCapabilities nc = + new NetworkCapabilities().setAdministratorUids(new int[] {2, 1, 3}); + + assertArrayEquals(new int[] {1, 2, 3}, nc.getAdministratorUids()); + } + + @Test + public void testSetAdministratorUidsWithDuplicates() { + try { + new NetworkCapabilities().setAdministratorUids(new int[] {1, 1}); + fail("Expected IllegalArgumentException for duplicate uids"); + } catch (IllegalArgumentException expected) { + } + } + @Test public void testCombineCapabilities() { NetworkCapabilities nc1 = new NetworkCapabilities(); From 7d4ef210105e8068ee0e669b78aba4245eaef845 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Mon, 30 Mar 2020 12:03:21 -0700 Subject: [PATCH 6/8] Clarify comments for Connectivity Diagnostics reports. Clarify when ConnectivityDiagnosticsCallback#onConnectivityReportAvailable will be invoked. Clarify when NetworkAgentInfo#mConnectivityReport will be null vs non-null. Bug: 147849853 Test: atest FrameworksNetTests Change-Id: I748bd9ded72a34d89f13bd4362d6d4da62b910b8 Merged-In: I748bd9ded72a34d89f13bd4362d6d4da62b910b8 (cherry picked from commit 604dd40cf077f42c2d4b6ff80ff41d89cfbcacee) --- core/java/android/net/ConnectivityDiagnosticsManager.java | 4 ++-- .../com/android/server/connectivity/NetworkAgentInfo.java | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index aa35852f78..9086d49231 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -659,8 +659,8 @@ public class ConnectivityDiagnosticsManager { public abstract static class ConnectivityDiagnosticsCallback { /** * Called when the platform completes a data connectivity check. This will also be invoked - * immediately upon registration with the latest report, if a report has already been - * generated for this network. + * immediately upon registration for each network matching the request with the latest + * report, if a report has already been generated for that network. * *

The Network specified in the ConnectivityReport may not be active any more when this * method is invoked. diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index d2086138a1..15628f03ba 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -244,8 +244,9 @@ public class NetworkAgentInfo implements Comparable { // How many of the satisfied requests are of type BACKGROUND_REQUEST. private int mNumBackgroundNetworkRequests = 0; - // The last ConnectivityReport made available for this network. - private ConnectivityReport mConnectivityReport; + // The last ConnectivityReport made available for this network. This value is only null before a + // report is generated. Once non-null, it will never be null again. + @Nullable private ConnectivityReport mConnectivityReport; public final Messenger messenger; public final AsyncChannel asyncChannel; From 5d51a917f533614e2065495ba9ec59315078491c Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Mon, 30 Mar 2020 12:43:49 -0700 Subject: [PATCH 7/8] Simplify unregister logic for Connectivity Diagnostics callbacks. ConnectivityService is updated to simplify the logic for unregistering ConnectivityDiagnosticsCallback instances. This change removes the given callback from ConnectivityService's data structure. If the callback was not registered with ConnectivityService, it is logged and the function exits; else, the unregister() operation continues. Bug: 150867635 Test: atest FrameworksNetTests Change-Id: I9096969a1bf33da72b117f5bbc88257df805e688 Merged-In: I9096969a1bf33da72b117f5bbc88257df805e688 (cherry picked from commit f047313940b5af49a3b0e72a5f2d94fc1dda9c9d) --- .../core/java/com/android/server/ConnectivityService.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e3f574f366..2e5a844c88 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7892,12 +7892,14 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureRunningOnConnectivityServiceThread(); final IBinder iCb = cb.asBinder(); - if (!mConnectivityDiagnosticsCallbacks.containsKey(iCb)) { + final ConnectivityDiagnosticsCallbackInfo cbInfo = + mConnectivityDiagnosticsCallbacks.remove(iCb); + if (cbInfo == null) { if (VDBG) log("Removing diagnostics callback that is not currently registered"); return; } - final NetworkRequestInfo nri = mConnectivityDiagnosticsCallbacks.get(iCb).mRequestInfo; + final NetworkRequestInfo nri = cbInfo.mRequestInfo; if (uid != nri.mUid) { if (VDBG) loge("Different uid than registrant attempting to unregister cb"); @@ -7909,8 +7911,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // enforceRequestCountLimit(). decrementNetworkRequestPerUidCount(nri); - final ConnectivityDiagnosticsCallbackInfo cbInfo = - mConnectivityDiagnosticsCallbacks.remove(iCb); iCb.unlinkToDeath(cbInfo, 0); } From 85308a5a14e84ed78e2ef9b843f33fbe956c4961 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Thu, 5 Mar 2020 22:13:31 -0800 Subject: [PATCH 8/8] Update CS helper for clearing NetworkCapabilities UIDs. NetworkCapabilities needs to have its UIDs cleared (UID ranges, owner UID, and administrator UIDs) before it can be shared with apps via ConnectivityDiagnosticsCallback invocations. The previous helper used for clearing these values mutated the provided NetworkCapabilities. This is updated to instead return a sanitized copy of the provided NetworkCapabilities Bug: 148942124 Test: atest FrameworksNetTests Change-Id: I2431a6d273d0d73432919baf41b4f66397f4b7dc Merged-In: I2431a6d273d0d73432919baf41b4f66397f4b7dc (cherry picked from commit 45bbc4f6ac910a2ea87eb6b2197e34db50d3ada8) --- .../com/android/server/ConnectivityService.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 2e5a844c88..4ab035e7f4 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7918,8 +7918,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @NonNull ConnectivityReportEvent reportEvent, @NonNull PersistableBundle extras) { final NetworkAgentInfo nai = reportEvent.mNai; final NetworkCapabilities networkCapabilities = - new NetworkCapabilities(nai.networkCapabilities); - clearNetworkCapabilitiesUids(networkCapabilities); + getNetworkCapabilitiesWithoutUids(nai.networkCapabilities); final ConnectivityReport report = new ConnectivityReport( reportEvent.mNai.network, @@ -7943,8 +7942,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @NonNull NetworkAgentInfo nai, long timestampMillis, int detectionMethod, @NonNull PersistableBundle extras) { final NetworkCapabilities networkCapabilities = - new NetworkCapabilities(nai.networkCapabilities); - clearNetworkCapabilitiesUids(networkCapabilities); + getNetworkCapabilitiesWithoutUids(nai.networkCapabilities); final DataStallReport report = new DataStallReport( nai.network, @@ -7977,10 +7975,12 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void clearNetworkCapabilitiesUids(@NonNull NetworkCapabilities nc) { - nc.setUids(null); - nc.setAdministratorUids(new int[0]); - nc.setOwnerUid(Process.INVALID_UID); + private NetworkCapabilities getNetworkCapabilitiesWithoutUids(@NonNull NetworkCapabilities nc) { + final NetworkCapabilities sanitized = new NetworkCapabilities(nc); + sanitized.setUids(null); + sanitized.setAdministratorUids(new int[0]); + sanitized.setOwnerUid(Process.INVALID_UID); + return sanitized; } private List getMatchingPermissionedCallbacks(