From 0900154ef542785af7175748e1ef57cf749366fa Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 15 Jan 2020 04:01:53 +0900 Subject: [PATCH 01/17] Add a systemapi constructor for NetworkAgent Bug: 138306002 Bug: 139268426 Test: atest FrameworksNetTests FrameworksWifiTests FrameworksTelephonyTests make doc-comment-check-docs Change-Id: I288ea32fac07a9a486e2ea451a2c9b098446a74c Merged-In: I288ea32fac07a9a486e2ea451a2c9b098446a74c --- core/java/android/net/NetworkAgent.java | 44 +++++++++++--- core/java/android/net/NetworkAgentConfig.java | 58 ++++++++++++++++++- 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index aae9fd4725..61a1484cef 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -262,32 +262,60 @@ public abstract class NetworkAgent { */ public static final int CMD_REMOVE_KEEPALIVE_PACKET_FILTER = BASE + 17; - // TODO : remove these two constructors. They are a stopgap measure to help sheperding a number - // of dependent changes that would conflict throughout the automerger graph. Having these - // temporarily helps with the process of going through with all these dependent changes across - // the entire tree. - /** @hide TODO: decide which of these to expose. */ + /** @hide TODO: remove and replace usage with the public constructor. */ public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni, NetworkCapabilities nc, LinkProperties lp, int score) { this(looper, context, logTag, ni, nc, lp, score, null, NetworkProvider.ID_NONE); } - /** @hide TODO: decide which of these to expose. */ + /** @hide TODO: remove and replace usage with the public constructor. */ public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni, NetworkCapabilities nc, LinkProperties lp, int score, NetworkAgentConfig config) { this(looper, context, logTag, ni, nc, lp, score, config, NetworkProvider.ID_NONE); } - /** @hide TODO: decide which of these to expose. */ + /** @hide TODO: remove and replace usage with the public constructor. */ public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni, NetworkCapabilities nc, LinkProperties lp, int score, int providerId) { this(looper, context, logTag, ni, nc, lp, score, null, providerId); } - /** @hide TODO: decide which of these to expose. */ + /** @hide TODO: remove and replace usage with the public constructor. */ public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni, NetworkCapabilities nc, LinkProperties lp, int score, NetworkAgentConfig config, int providerId) { + this(looper, context, logTag, nc, lp, score, config, providerId, ni); + } + + private static NetworkInfo getLegacyNetworkInfo(final NetworkAgentConfig config) { + // The subtype can be changed with (TODO) setLegacySubtype, but it starts + // with the type and an empty description. + return new NetworkInfo(config.legacyType, config.legacyType, config.legacyTypeName, ""); + } + + /** + * Create a new network agent. + * @param context a {@link Context} to get system services from. + * @param looper the {@link Looper} on which to invoke the callbacks. + * @param logTag the tag for logs + * @param nc the initial {@link NetworkCapabilities} of this network. Update with + * sendNetworkCapabilities. + * @param lp the initial {@link LinkProperties} of this network. Update with sendLinkProperties. + * @param score the initial score of this network. Update with sendNetworkScore. + * @param config an immutable {@link NetworkAgentConfig} for this agent. + * @param provider the {@link NetworkProvider} managing this agent. + */ + public NetworkAgent(@NonNull Context context, @NonNull Looper looper, @NonNull String logTag, + @NonNull NetworkCapabilities nc, @NonNull LinkProperties lp, int score, + @NonNull NetworkAgentConfig config, @Nullable NetworkProvider provider) { + this(looper, context, logTag, nc, lp, score, config, + provider == null ? NetworkProvider.ID_NONE : provider.getProviderId(), + getLegacyNetworkInfo(config)); + } + + private NetworkAgent(Looper looper, Context context, String logTag, NetworkCapabilities nc, + LinkProperties lp, int score, NetworkAgentConfig config, int providerId, + NetworkInfo ni) { mHandler = new NetworkAgentHandler(looper); LOG_TAG = logTag; mContext = context; diff --git a/core/java/android/net/NetworkAgentConfig.java b/core/java/android/net/NetworkAgentConfig.java index abc6b67efb..1e5af673b8 100644 --- a/core/java/android/net/NetworkAgentConfig.java +++ b/core/java/android/net/NetworkAgentConfig.java @@ -21,12 +21,11 @@ import android.annotation.Nullable; import android.annotation.SystemApi; import android.os.Parcel; import android.os.Parcelable; -import android.text.TextUtils; /** * Allows a network transport to provide the system with policy and configuration information about - * a particular network when registering a {@link NetworkAgent}. This information cannot change once - * the agent is registered. + * a particular network when registering a {@link NetworkAgent}. + * @note This information cannot change once the agent is registered. * * @hide */ @@ -119,6 +118,19 @@ public final class NetworkAgentConfig implements Parcelable { return !skip464xlat; } + /** + * The legacy type of this network agent, or TYPE_NONE if unset. + * @hide + */ + public int legacyType = ConnectivityManager.TYPE_NONE; + + /** + * @return the legacy type + */ + public int getLegacyType() { + return legacyType; + } + /** * Set to true if the PRIVATE_DNS_BROKEN notification has shown for this network. * Reset this bit when private DNS mode is changed from strict mode to opportunistic/off mode. @@ -127,6 +139,21 @@ public final class NetworkAgentConfig implements Parcelable { */ public boolean hasShownBroken; + /** + * The name of the legacy network type. It's a free-form string used in logging. + * @hide + */ + @NonNull + public String legacyTypeName = ""; + + /** + * @return the name of the legacy network type. It's a free-form string used in logging. + */ + @NonNull + public String getLegacyTypeName() { + return legacyTypeName; + } + /** @hide */ public NetworkAgentConfig() { } @@ -140,6 +167,8 @@ public final class NetworkAgentConfig implements Parcelable { subscriberId = nac.subscriberId; provisioningNotificationDisabled = nac.provisioningNotificationDisabled; skip464xlat = nac.skip464xlat; + legacyType = nac.legacyType; + legacyTypeName = nac.legacyTypeName; } } @@ -184,6 +213,29 @@ public final class NetworkAgentConfig implements Parcelable { return this; } + /** + * Sets the legacy type for this network. + * + * @param legacyType the type + * @return this builder, to facilitate chaining. + */ + @NonNull + public Builder setLegacyType(int legacyType) { + mConfig.legacyType = legacyType; + return this; + } + + /** + * Sets the name of the legacy type of the agent. It's a free-form string used in logging. + * @param legacyTypeName the name + * @return this builder, to facilitate chaining. + */ + @NonNull + public Builder setLegacyTypeName(@NonNull String legacyTypeName) { + mConfig.legacyTypeName = legacyTypeName; + return this; + } + /** * Returns the constructed {@link NetworkAgentConfig} object. */ From f2852480a0b0b3f822fa1eb1c6766047ebd9874d Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Tue, 4 Feb 2020 21:52:09 -0800 Subject: [PATCH 02/17] Include NetworkCapabilities and LinkProperties in DataStallReport. DataStallReport is updated to include the NetworkCapabilities and Link Properties for the Network being reported on. This provides a more complete picture of the Network conditions when the suspected data stall was detected. Bug: 148966398 Test: atest FrameworksNetTests Change-Id: I913cf18c348b9f688f9d2a3d25a71bc94eb8f000 --- .../net/ConnectivityDiagnosticsManager.java | 51 +++++++- .../android/server/ConnectivityService.java | 8 +- .../ConnectivityDiagnosticsManagerTest.java | 110 ++++++++++++++---- 3 files changed, 141 insertions(+), 28 deletions(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 3c39d1558e..d009144034 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -252,8 +252,8 @@ public class ConnectivityDiagnosticsManager { @NonNull PersistableBundle additionalInfo) { mNetwork = network; mReportTimestamp = reportTimestamp; - mLinkProperties = linkProperties; - mNetworkCapabilities = networkCapabilities; + mLinkProperties = new LinkProperties(linkProperties); + mNetworkCapabilities = new NetworkCapabilities(networkCapabilities); mAdditionalInfo = additionalInfo; } @@ -433,6 +433,12 @@ public class ConnectivityDiagnosticsManager { /** The detection method used to identify the suspected data stall */ @DetectionMethod private final int mDetectionMethod; + /** LinkProperties available on the Network at the reported timestamp */ + @NonNull private final LinkProperties mLinkProperties; + + /** NetworkCapabilities available on the Network at the reported timestamp */ + @NonNull private final NetworkCapabilities mNetworkCapabilities; + /** PersistableBundle that may contain additional information on the suspected data stall */ @NonNull private final PersistableBundle mStallDetails; @@ -446,16 +452,23 @@ public class ConnectivityDiagnosticsManager { * @param network The Network for which this DataStallReport applies * @param reportTimestamp The timestamp for the report * @param detectionMethod The detection method used to identify this data stall + * @param linkProperties The LinkProperties available on network at reportTimestamp + * @param networkCapabilities The NetworkCapabilities available on network at + * reportTimestamp * @param stallDetails A PersistableBundle that may contain additional info about the report */ public DataStallReport( @NonNull Network network, long reportTimestamp, @DetectionMethod int detectionMethod, + @NonNull LinkProperties linkProperties, + @NonNull NetworkCapabilities networkCapabilities, @NonNull PersistableBundle stallDetails) { mNetwork = network; mReportTimestamp = reportTimestamp; mDetectionMethod = detectionMethod; + mLinkProperties = new LinkProperties(linkProperties); + mNetworkCapabilities = new NetworkCapabilities(networkCapabilities); mStallDetails = stallDetails; } @@ -487,6 +500,26 @@ public class ConnectivityDiagnosticsManager { return mDetectionMethod; } + /** + * Returns the LinkProperties available when this report was taken. + * + * @return LinkProperties available on the Network at the reported timestamp + */ + @NonNull + public LinkProperties getLinkProperties() { + return new LinkProperties(mLinkProperties); + } + + /** + * Returns the NetworkCapabilities when this report was taken. + * + * @return NetworkCapabilities available on the Network at the reported timestamp + */ + @NonNull + public NetworkCapabilities getNetworkCapabilities() { + return new NetworkCapabilities(mNetworkCapabilities); + } + /** * Returns a PersistableBundle with additional info for this report. * @@ -513,12 +546,20 @@ public class ConnectivityDiagnosticsManager { return mReportTimestamp == that.mReportTimestamp && mDetectionMethod == that.mDetectionMethod && mNetwork.equals(that.mNetwork) + && mLinkProperties.equals(that.mLinkProperties) + && mNetworkCapabilities.equals(that.mNetworkCapabilities) && persistableBundleEquals(mStallDetails, that.mStallDetails); } @Override public int hashCode() { - return Objects.hash(mNetwork, mReportTimestamp, mDetectionMethod, mStallDetails); + return Objects.hash( + mNetwork, + mReportTimestamp, + mDetectionMethod, + mLinkProperties, + mNetworkCapabilities, + mStallDetails); } /** {@inheritDoc} */ @@ -533,6 +574,8 @@ public class ConnectivityDiagnosticsManager { dest.writeParcelable(mNetwork, flags); dest.writeLong(mReportTimestamp); dest.writeInt(mDetectionMethod); + dest.writeParcelable(mLinkProperties, flags); + dest.writeParcelable(mNetworkCapabilities, flags); dest.writeParcelable(mStallDetails, flags); } @@ -544,6 +587,8 @@ public class ConnectivityDiagnosticsManager { in.readParcelable(null), in.readLong(), in.readInt(), + in.readParcelable(null), + in.readParcelable(null), in.readParcelable(null)); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b3b1722664..6227d81f98 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7821,7 +7821,13 @@ public class ConnectivityService extends IConnectivityManager.Stub @NonNull NetworkAgentInfo nai, long timestampMillis, int detectionMethod, @NonNull PersistableBundle extras) { final DataStallReport report = - new DataStallReport(nai.network, timestampMillis, detectionMethod, extras); + new DataStallReport( + nai.network, + timestampMillis, + detectionMethod, + nai.linkProperties, + nai.networkCapabilities, + extras); final List results = getMatchingPermissionedCallbacks(nai); for (final IConnectivityDiagnosticsCallback cb : results) { diff --git a/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java b/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java index 0628691c33..8eb5cfa48c 100644 --- a/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java +++ b/tests/net/java/android/net/ConnectivityDiagnosticsManagerTest.java @@ -146,17 +146,14 @@ public class ConnectivityDiagnosticsManagerTest { @Test public void testConnectivityReportEquals() { - assertEquals(createSampleConnectivityReport(), createSampleConnectivityReport()); - assertEquals(createDefaultConnectivityReport(), createDefaultConnectivityReport()); + final ConnectivityReport defaultReport = createDefaultConnectivityReport(); + final ConnectivityReport sampleReport = createSampleConnectivityReport(); + assertEquals(sampleReport, createSampleConnectivityReport()); + assertEquals(defaultReport, createDefaultConnectivityReport()); - final LinkProperties linkProperties = new LinkProperties(); - linkProperties.setInterfaceName(INTERFACE_NAME); - - final NetworkCapabilities networkCapabilities = new NetworkCapabilities(); - networkCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_IMS); - - final PersistableBundle bundle = new PersistableBundle(); - bundle.putString(BUNDLE_KEY, BUNDLE_VALUE); + final LinkProperties linkProperties = sampleReport.getLinkProperties(); + final NetworkCapabilities networkCapabilities = sampleReport.getNetworkCapabilities(); + final PersistableBundle bundle = sampleReport.getAdditionalInfo(); assertNotEquals( createDefaultConnectivityReport(), @@ -206,39 +203,104 @@ public class ConnectivityDiagnosticsManagerTest { } private DataStallReport createSampleDataStallReport() { + final LinkProperties linkProperties = new LinkProperties(); + linkProperties.setInterfaceName(INTERFACE_NAME); + final PersistableBundle bundle = new PersistableBundle(); bundle.putString(BUNDLE_KEY, BUNDLE_VALUE); - return new DataStallReport(new Network(NET_ID), TIMESTAMP, DETECTION_METHOD, bundle); + + final NetworkCapabilities networkCapabilities = new NetworkCapabilities(); + networkCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_IMS); + + return new DataStallReport( + new Network(NET_ID), + TIMESTAMP, + DETECTION_METHOD, + linkProperties, + networkCapabilities, + bundle); } private DataStallReport createDefaultDataStallReport() { - return new DataStallReport(new Network(0), 0L, 0, PersistableBundle.EMPTY); + return new DataStallReport( + new Network(0), + 0L, + 0, + new LinkProperties(), + new NetworkCapabilities(), + PersistableBundle.EMPTY); } @Test public void testDataStallReportEquals() { - assertEquals(createSampleDataStallReport(), createSampleDataStallReport()); - assertEquals(createDefaultDataStallReport(), createDefaultDataStallReport()); + final DataStallReport defaultReport = createDefaultDataStallReport(); + final DataStallReport sampleReport = createSampleDataStallReport(); + assertEquals(sampleReport, createSampleDataStallReport()); + assertEquals(defaultReport, createDefaultDataStallReport()); - final PersistableBundle bundle = new PersistableBundle(); - bundle.putString(BUNDLE_KEY, BUNDLE_VALUE); + final LinkProperties linkProperties = sampleReport.getLinkProperties(); + final NetworkCapabilities networkCapabilities = sampleReport.getNetworkCapabilities(); + final PersistableBundle bundle = sampleReport.getStallDetails(); assertNotEquals( - createDefaultDataStallReport(), - new DataStallReport(new Network(NET_ID), 0L, 0, PersistableBundle.EMPTY)); + defaultReport, + new DataStallReport( + new Network(NET_ID), + 0L, + 0, + new LinkProperties(), + new NetworkCapabilities(), + PersistableBundle.EMPTY)); assertNotEquals( - createDefaultDataStallReport(), - new DataStallReport(new Network(0), TIMESTAMP, 0, PersistableBundle.EMPTY)); + defaultReport, + new DataStallReport( + new Network(0), + TIMESTAMP, + 0, + new LinkProperties(), + new NetworkCapabilities(), + PersistableBundle.EMPTY)); assertNotEquals( - createDefaultDataStallReport(), - new DataStallReport(new Network(0), 0L, DETECTION_METHOD, PersistableBundle.EMPTY)); + defaultReport, + new DataStallReport( + new Network(0), + 0L, + DETECTION_METHOD, + new LinkProperties(), + new NetworkCapabilities(), + PersistableBundle.EMPTY)); assertNotEquals( - createDefaultDataStallReport(), new DataStallReport(new Network(0), 0L, 0, bundle)); + defaultReport, + new DataStallReport( + new Network(0), + 0L, + 0, + linkProperties, + new NetworkCapabilities(), + PersistableBundle.EMPTY)); + assertNotEquals( + defaultReport, + new DataStallReport( + new Network(0), + 0L, + 0, + new LinkProperties(), + networkCapabilities, + PersistableBundle.EMPTY)); + assertNotEquals( + defaultReport, + new DataStallReport( + new Network(0), + 0L, + 0, + new LinkProperties(), + new NetworkCapabilities(), + bundle)); } @Test public void testDataStallReportParcelUnparcel() { - assertParcelSane(createSampleDataStallReport(), 4); + assertParcelSane(createSampleDataStallReport(), 6); } @Test From d499517306750bfaacdeae5d15f490cbc8d98d97 Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Thu, 16 Jan 2020 12:17:17 -0800 Subject: [PATCH 03/17] NetworkRequest: Embed requestor uid & packageName Add the requestorUid & requestorPackageName fields to NetworkCapabilities. This is populated by CS when a new network request is received. These 2 requestor fields are also optionally used for network matching. All of the regular app initiated requests will have the requestor uid and package name set by connectivity service. Network agents can optionally set the requestorUid and requestorPackageName to restrict the network created only to the app that requested the network. This will help removing the necessity for the various specifiers to embed the uid & package name info in the specifier for network matching. Note: NetworkSpecifier.assertValidFromUid() is deprecated & removed in favor of setting the uid/package name on the agent to restrict the network to a certain app (useful for wifi peer to peer API & wifi aware). Bug: 144102365 Test: Verified that wifi network request related CTS verifier tests pass. Test: Device boots up and connects to wifi networks Change-Id: I207c446108afdac7ee2c25e6bbcbc37c4e3f6529 Merged-In: I207c446108afdac7ee2c25e6bbcbc37c4e3f6529 --- .../java/android/net/ConnectivityManager.java | 14 +- .../android/net/IConnectivityManager.aidl | 9 +- .../java/android/net/NetworkCapabilities.java | 161 +++++++++++++++++- core/java/android/net/NetworkRequest.java | 26 +++ .../android/server/ConnectivityService.java | 52 +++--- .../android/net/NetworkCapabilitiesTest.java | 16 +- .../android/net/ConnectivityManagerTest.java | 12 +- .../server/ConnectivityServiceTest.java | 31 +--- 8 files changed, 256 insertions(+), 65 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 94eda01410..f24de88a3f 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3747,6 +3747,7 @@ public class ConnectivityManager { checkCallbackNotNull(callback); Preconditions.checkArgument(action == REQUEST || need != null, "null NetworkCapabilities"); final NetworkRequest request; + final String callingPackageName = mContext.getOpPackageName(); try { synchronized(sCallbacks) { if (callback.networkRequest != null @@ -3758,10 +3759,11 @@ public class ConnectivityManager { Messenger messenger = new Messenger(handler); Binder binder = new Binder(); if (action == LISTEN) { - request = mService.listenForNetwork(need, messenger, binder); + request = mService.listenForNetwork( + need, messenger, binder, callingPackageName); } else { request = mService.requestNetwork( - need, messenger, timeoutMs, binder, legacyType); + need, messenger, timeoutMs, binder, legacyType, callingPackageName); } if (request != null) { sCallbacks.put(request, callback); @@ -4034,8 +4036,10 @@ public class ConnectivityManager { @NonNull PendingIntent operation) { printStackTrace(); checkPendingIntentNotNull(operation); + final String callingPackageName = mContext.getOpPackageName(); try { - mService.pendingRequestForNetwork(request.networkCapabilities, operation); + mService.pendingRequestForNetwork( + request.networkCapabilities, operation, callingPackageName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (ServiceSpecificException e) { @@ -4147,8 +4151,10 @@ public class ConnectivityManager { @NonNull PendingIntent operation) { printStackTrace(); checkPendingIntentNotNull(operation); + final String callingPackageName = mContext.getOpPackageName(); try { - mService.pendingListenForNetwork(request.networkCapabilities, operation); + mService.pendingListenForNetwork( + request.networkCapabilities, operation, callingPackageName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (ServiceSpecificException e) { diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index c871c456dc..3a55461a77 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -166,18 +166,19 @@ interface IConnectivityManager in int factorySerialNumber); NetworkRequest requestNetwork(in NetworkCapabilities networkCapabilities, - in Messenger messenger, int timeoutSec, in IBinder binder, int legacy); + in Messenger messenger, int timeoutSec, in IBinder binder, int legacy, + String callingPackageName); NetworkRequest pendingRequestForNetwork(in NetworkCapabilities networkCapabilities, - in PendingIntent operation); + in PendingIntent operation, String callingPackageName); void releasePendingNetworkRequest(in PendingIntent operation); NetworkRequest listenForNetwork(in NetworkCapabilities networkCapabilities, - in Messenger messenger, in IBinder binder); + in Messenger messenger, in IBinder binder, String callingPackageName); void pendingListenForNetwork(in NetworkCapabilities networkCapabilities, - in PendingIntent operation); + in PendingIntent operation, String callingPackageName); void releaseNetworkRequest(in NetworkRequest networkRequest); diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 38f7390abf..ef4a9e5f3b 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -27,6 +27,7 @@ import android.os.Build; import android.os.Parcel; import android.os.Parcelable; import android.os.Process; +import android.text.TextUtils; import android.util.ArraySet; import android.util.proto.ProtoOutputStream; @@ -63,6 +64,16 @@ public final class NetworkCapabilities implements Parcelable { // Set to true when private DNS is broken. private boolean mPrivateDnsBroken; + /** + * Uid of the app making the request. + */ + private int mRequestorUid; + + /** + * Package name of the app making the request. + */ + private String mRequestorPackageName; + public NetworkCapabilities() { clearAll(); mNetworkCapabilities = DEFAULT_CAPABILITIES; @@ -89,6 +100,8 @@ public final class NetworkCapabilities implements Parcelable { mOwnerUid = Process.INVALID_UID; mSSID = null; mPrivateDnsBroken = false; + mRequestorUid = Process.INVALID_UID; + mRequestorPackageName = null; } /** @@ -109,6 +122,8 @@ public final class NetworkCapabilities implements Parcelable { mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; mSSID = nc.mSSID; mPrivateDnsBroken = nc.mPrivateDnsBroken; + mRequestorUid = nc.mRequestorUid; + mRequestorPackageName = nc.mRequestorPackageName; } /** @@ -810,7 +825,7 @@ public final class NetworkCapabilities implements Parcelable { } /** - * UID of the app that owns this network, or INVALID_UID if none/unknown. + * UID of the app that owns this network, or Process#INVALID_UID if none/unknown. * *

This field keeps track of the UID of the app that created this network and is in charge of * its lifecycle. This could be the UID of apps such as the Wifi network suggestor, the running @@ -821,8 +836,9 @@ public final class NetworkCapabilities implements Parcelable { /** * Set the UID of the owner app. */ - public void setOwnerUid(final int uid) { + public @NonNull NetworkCapabilities setOwnerUid(final int uid) { mOwnerUid = uid; + return this; } /** @@ -865,9 +881,11 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ @SystemApi - public void setAdministratorUids(@NonNull final List administratorUids) { + public @NonNull NetworkCapabilities setAdministratorUids( + @NonNull final List administratorUids) { mAdministratorUids.clear(); mAdministratorUids.addAll(administratorUids); + return this; } /** @@ -1385,6 +1403,7 @@ public final class NetworkCapabilities implements Parcelable { combineSignalStrength(nc); combineUids(nc); combineSSIDs(nc); + combineRequestor(nc); } /** @@ -1404,7 +1423,8 @@ public final class NetworkCapabilities implements Parcelable { && satisfiedBySpecifier(nc) && (onlyImmutable || satisfiedBySignalStrength(nc)) && (onlyImmutable || satisfiedByUids(nc)) - && (onlyImmutable || satisfiedBySSID(nc))); + && (onlyImmutable || satisfiedBySSID(nc))) + && (onlyImmutable || satisfiedByRequestor(nc)); } /** @@ -1488,7 +1508,7 @@ public final class NetworkCapabilities implements Parcelable { public boolean equals(@Nullable Object obj) { if (obj == null || (obj instanceof NetworkCapabilities == false)) return false; NetworkCapabilities that = (NetworkCapabilities) obj; - return (equalsNetCapabilities(that) + return equalsNetCapabilities(that) && equalsTransportTypes(that) && equalsLinkBandwidths(that) && equalsSignalStrength(that) @@ -1496,7 +1516,8 @@ public final class NetworkCapabilities implements Parcelable { && equalsTransportInfo(that) && equalsUids(that) && equalsSSID(that) - && equalsPrivateDnsBroken(that)); + && equalsPrivateDnsBroken(that) + && equalsRequestor(that); } @Override @@ -1514,7 +1535,9 @@ public final class NetworkCapabilities implements Parcelable { + Objects.hashCode(mUids) * 31 + Objects.hashCode(mSSID) * 37 + Objects.hashCode(mTransportInfo) * 41 - + Objects.hashCode(mPrivateDnsBroken) * 43; + + Objects.hashCode(mPrivateDnsBroken) * 43 + + Objects.hashCode(mRequestorUid) * 47 + + Objects.hashCode(mRequestorPackageName) * 53; } @Override @@ -1537,6 +1560,8 @@ public final class NetworkCapabilities implements Parcelable { dest.writeBoolean(mPrivateDnsBroken); dest.writeList(mAdministratorUids); dest.writeInt(mOwnerUid); + dest.writeInt(mRequestorUid); + dest.writeString(mRequestorPackageName); } public static final @android.annotation.NonNull Creator CREATOR = @@ -1559,6 +1584,8 @@ public final class NetworkCapabilities implements Parcelable { netCap.mPrivateDnsBroken = in.readBoolean(); netCap.setAdministratorUids(in.readArrayList(null)); netCap.mOwnerUid = in.readInt(); + netCap.mRequestorUid = in.readInt(); + netCap.mRequestorPackageName = in.readString(); return netCap; } @Override @@ -1624,6 +1651,9 @@ public final class NetworkCapabilities implements Parcelable { sb.append(" Private DNS is broken"); } + sb.append(" RequestorUid: ").append(mRequestorUid); + sb.append(" RequestorPackageName: ").append(mRequestorPackageName); + sb.append("]"); return sb.toString(); } @@ -1632,6 +1662,7 @@ public final class NetworkCapabilities implements Parcelable { private interface NameOf { String nameOf(int value); } + /** * @hide */ @@ -1799,4 +1830,120 @@ public final class NetworkCapabilities implements Parcelable { private boolean equalsPrivateDnsBroken(NetworkCapabilities nc) { return mPrivateDnsBroken == nc.mPrivateDnsBroken; } + + /** + * Set the uid of the app making the request. + * + * Note: This works only for {@link NetworkAgent} instances. Any capabilities passed in + * via the public {@link ConnectivityManager} API's will have this field overwritten. + * + * @param uid UID of the app. + * @hide + */ + @SystemApi + public @NonNull NetworkCapabilities setRequestorUid(int uid) { + mRequestorUid = uid; + return this; + } + + /** + * @return the uid of the app making the request. + * + * Note: This could return {@link Process#INVALID_UID} if the {@link NetworkRequest} + * object was not obtained from {@link ConnectivityManager}. + * @hide + */ + public int getRequestorUid() { + return mRequestorUid; + } + + /** + * Set the package name of the app making the request. + * + * Note: This works only for {@link NetworkAgent} instances. Any capabilities passed in + * via the public {@link ConnectivityManager} API's will have this field overwritten. + * + * @param packageName package name of the app. + * @hide + */ + @SystemApi + public @NonNull NetworkCapabilities setRequestorPackageName(@NonNull String packageName) { + mRequestorPackageName = packageName; + return this; + } + + /** + * @return the package name of the app making the request. + * + * Note: This could return {@code null} if the {@link NetworkRequest} object was not obtained + * from {@link ConnectivityManager}. + * @hide + */ + @Nullable + public String getRequestorPackageName() { + return mRequestorPackageName; + } + + /** + * Set the uid and package name of the app making the request. + * + * Note: This is intended to be only invoked from within connectivitiy service. + * + * @param uid UID of the app. + * @param packageName package name of the app. + * @hide + */ + public @NonNull NetworkCapabilities setRequestorUidAndPackageName( + int uid, @NonNull String packageName) { + return setRequestorUid(uid).setRequestorPackageName(packageName); + } + + /** + * Test whether the passed NetworkCapabilities satisfies the requestor restrictions of this + * capabilities. + * + * This method is called on the NetworkCapabilities embedded in a request with the + * capabilities of an available network. If the available network, sets a specific + * requestor (by uid and optionally package name), then this will only match a request from the + * same app. If either of the capabilities have an unset uid or package name, then it matches + * everything. + *

+ * nc is assumed nonnull. Else, NPE. + */ + private boolean satisfiedByRequestor(NetworkCapabilities nc) { + // No uid set, matches everything. + if (mRequestorUid == Process.INVALID_UID || nc.mRequestorUid == Process.INVALID_UID) { + return true; + } + // uids don't match. + if (mRequestorUid != nc.mRequestorUid) return false; + // No package names set, matches everything + if (null == nc.mRequestorPackageName || null == mRequestorPackageName) return true; + // check for package name match. + return TextUtils.equals(mRequestorPackageName, nc.mRequestorPackageName); + } + + /** + * Combine requestor info of the capabilities. + *

+ * This is only legal if either the requestor info of this object is reset, or both info are + * equal. + * nc is assumed nonnull. + */ + private void combineRequestor(@NonNull NetworkCapabilities nc) { + if (mRequestorUid != Process.INVALID_UID && mRequestorUid != nc.mOwnerUid) { + throw new IllegalStateException("Can't combine two uids"); + } + if (mRequestorPackageName != null + && !mRequestorPackageName.equals(nc.mRequestorPackageName)) { + throw new IllegalStateException("Can't combine two package names"); + } + setRequestorUid(nc.mRequestorUid); + setRequestorPackageName(nc.mRequestorPackageName); + } + + private boolean equalsRequestor(NetworkCapabilities nc) { + return mRequestorUid == nc.mRequestorUid + && TextUtils.equals(mRequestorPackageName, nc.mRequestorPackageName); + } } diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index ee4379a85b..b0bf64ecec 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -380,6 +380,7 @@ public class NetworkRequest implements Parcelable { dest.writeInt(requestId); dest.writeString(type.name()); } + public static final @android.annotation.NonNull Creator CREATOR = new Creator() { public NetworkRequest createFromParcel(Parcel in) { @@ -494,6 +495,31 @@ public class NetworkRequest implements Parcelable { return networkCapabilities.getNetworkSpecifier(); } + /** + * @return the uid of the app making the request. + * + * Note: This could return {@link Process#INVALID_UID} if the {@link NetworkRequest} object was + * not obtained from {@link ConnectivityManager}. + * @hide + */ + @SystemApi + public int getRequestorUid() { + return networkCapabilities.getRequestorUid(); + } + + /** + * @return the package name of the app making the request. + * + * Note: This could return {@code null} if the {@link NetworkRequest} object was not obtained + * from {@link ConnectivityManager}. + * @hide + */ + @SystemApi + @Nullable + public String getRequestorPackageName() { + return networkCapabilities.getRequestorPackageName(); + } + public String toString() { return "NetworkRequest [ " + type + " id=" + requestId + (legacyType != ConnectivityManager.TYPE_NONE ? ", legacyType=" + legacyType : "") + diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d304152192..61ef76e559 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -604,7 +604,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set mWolSupportedInterfaces; - private TelephonyManager mTelephonyManager; + private final TelephonyManager mTelephonyManager; private final AppOpsManager mAppOpsManager; private final LocationPermissionChecker mLocationPermissionChecker; @@ -1171,6 +1171,7 @@ public class ConnectivityService extends IConnectivityManager.Stub int transportType, NetworkRequest.Type type) { final NetworkCapabilities netCap = new NetworkCapabilities(); netCap.addCapability(NET_CAPABILITY_INTERNET); + netCap.setRequestorUidAndPackageName(Process.myUid(), mContext.getPackageName()); if (transportType > -1) { netCap.addTransportType(transportType); } @@ -1701,10 +1702,12 @@ public class ConnectivityService extends IConnectivityManager.Stub return newLp; } - private void restrictRequestUidsForCaller(NetworkCapabilities nc) { + private void restrictRequestUidsForCallerAndSetRequestorInfo(NetworkCapabilities nc, + int callerUid, String callerPackageName) { if (!checkSettingsPermission()) { - nc.setSingleUid(Binder.getCallingUid()); + nc.setSingleUid(callerUid); } + nc.setRequestorUidAndPackageName(callerUid, callerPackageName); nc.setAdministratorUids(Collections.EMPTY_LIST); // Clear owner UID; this can never come from an app. @@ -5297,7 +5300,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // This checks that the passed capabilities either do not request a // specific SSID/SignalStrength, or the calling app has permission to do so. private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, - int callerPid, int callerUid) { + int callerPid, int callerUid, String callerPackageName) { if (null != nc.getSSID() && !checkSettingsPermission(callerPid, callerUid)) { throw new SecurityException("Insufficient permissions to request a specific SSID"); } @@ -5307,6 +5310,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new SecurityException( "Insufficient permissions to request a specific signal strength"); } + mAppOpsManager.checkPackage(callerUid, callerPackageName); } private ArrayList getSignalStrengthThresholds(NetworkAgentInfo nai) { @@ -5353,7 +5357,6 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(ns); - ns.assertValidFromUid(Binder.getCallingUid()); } private void ensureValid(NetworkCapabilities nc) { @@ -5365,7 +5368,9 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest requestNetwork(NetworkCapabilities networkCapabilities, - Messenger messenger, int timeoutMs, IBinder binder, int legacyType) { + Messenger messenger, int timeoutMs, IBinder binder, int legacyType, + @NonNull String callingPackageName) { + final int callingUid = Binder.getCallingUid(); final NetworkRequest.Type type = (networkCapabilities == null) ? NetworkRequest.Type.TRACK_DEFAULT : NetworkRequest.Type.REQUEST; @@ -5373,7 +5378,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // the default network request. This allows callers to keep track of // the system default network. if (type == NetworkRequest.Type.TRACK_DEFAULT) { - networkCapabilities = createDefaultNetworkCapabilitiesForUid(Binder.getCallingUid()); + networkCapabilities = createDefaultNetworkCapabilitiesForUid(callingUid); enforceAccessPermission(); } else { networkCapabilities = new NetworkCapabilities(networkCapabilities); @@ -5385,13 +5390,14 @@ public class ConnectivityService extends IConnectivityManager.Stub } ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); + Binder.getCallingPid(), callingUid, callingPackageName); // Set the UID range for this request to the single UID of the requester, or to an empty // set of UIDs if the caller has the appropriate permission and UIDs have not been set. // This will overwrite any allowed UIDs in the requested capabilities. Though there // are no visible methods to set the UIDs, an app could use reflection to try and get // networks for other apps so it's essential that the UIDs are overwritten. - restrictRequestUidsForCaller(networkCapabilities); + restrictRequestUidsForCallerAndSetRequestorInfo(networkCapabilities, + callingUid, callingPackageName); if (timeoutMs < 0) { throw new IllegalArgumentException("Bad timeout specified"); @@ -5466,16 +5472,18 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest pendingRequestForNetwork(NetworkCapabilities networkCapabilities, - PendingIntent operation) { + PendingIntent operation, @NonNull String callingPackageName) { checkNotNull(operation, "PendingIntent cannot be null."); + final int callingUid = Binder.getCallingUid(); networkCapabilities = new NetworkCapabilities(networkCapabilities); enforceNetworkRequestPermissions(networkCapabilities); enforceMeteredApnPolicy(networkCapabilities); ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); + Binder.getCallingPid(), callingUid, callingPackageName); ensureValidNetworkSpecifier(networkCapabilities); - restrictRequestUidsForCaller(networkCapabilities); + restrictRequestUidsForCallerAndSetRequestorInfo(networkCapabilities, + callingUid, callingPackageName); NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.REQUEST); @@ -5523,15 +5531,16 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest listenForNetwork(NetworkCapabilities networkCapabilities, - Messenger messenger, IBinder binder) { + Messenger messenger, IBinder binder, @NonNull String callingPackageName) { + final int callingUid = Binder.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); - restrictRequestUidsForCaller(nc); + Binder.getCallingPid(), callingUid, callingPackageName); + restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); // Apps without the CHANGE_NETWORK_STATE permission can't use background networks, so // make all their listens include NET_CAPABILITY_FOREGROUND. That way, they will get // onLost and onAvailable callbacks when networks move in and out of the background. @@ -5551,17 +5560,17 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void pendingListenForNetwork(NetworkCapabilities networkCapabilities, - PendingIntent operation) { + PendingIntent operation, @NonNull String callingPackageName) { checkNotNull(operation, "PendingIntent cannot be null."); + final int callingUid = Binder.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } ensureValid(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); - + Binder.getCallingPid(), callingUid, callingPackageName); final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); - restrictRequestUidsForCaller(nc); + restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); NetworkRequest networkRequest = new NetworkRequest(nc, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.LISTEN); @@ -7868,12 +7877,13 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new IllegalArgumentException("ConnectivityManager.TYPE_* are deprecated." + " Please use NetworkCapabilities instead."); } - mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName); + final int callingUid = Binder.getCallingUid(); + mAppOpsManager.checkPackage(callingUid, callingPackageName); // This NetworkCapabilities is only used for matching to Networks. Clear out its owner uid // and administrator uids to be safe. final NetworkCapabilities nc = new NetworkCapabilities(request.networkCapabilities); - restrictRequestUidsForCaller(nc); + restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); final NetworkRequest requestWithId = new NetworkRequest( diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index 3e4f3d8188..efea91ab91 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -272,9 +272,23 @@ public class NetworkCapabilitiesTest { netCap.setOwnerUid(123); assertParcelingIsLossless(netCap); netCap.setSSID(TEST_SSID); - assertParcelSane(netCap, 13); + assertParcelSane(netCap, 15); } + @Test + public void testParcelNetworkCapabilitiesWithRequestorUidAndPackageName() { + final NetworkCapabilities netCap = new NetworkCapabilities() + .addCapability(NET_CAPABILITY_INTERNET) + .setRequestorUid(9304) + .setRequestorPackageName("com.android.test") + .addCapability(NET_CAPABILITY_EIMS) + .addCapability(NET_CAPABILITY_NOT_METERED); + assertParcelingIsLossless(netCap); + netCap.setSSID(TEST_SSID); + assertParcelSane(netCap, 15); + } + + @Test public void testOemPaid() { NetworkCapabilities nc = new NetworkCapabilities(); diff --git a/tests/net/java/android/net/ConnectivityManagerTest.java b/tests/net/java/android/net/ConnectivityManagerTest.java index 7ede14428a..d6bf334ee5 100644 --- a/tests/net/java/android/net/ConnectivityManagerTest.java +++ b/tests/net/java/android/net/ConnectivityManagerTest.java @@ -212,7 +212,8 @@ public class ConnectivityManagerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Messenger.class); // register callback - when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) + when(mService.requestNetwork( + any(), captor.capture(), anyInt(), any(), anyInt(), any())) .thenReturn(request); manager.requestNetwork(request, callback, handler); @@ -240,7 +241,8 @@ public class ConnectivityManagerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Messenger.class); // register callback - when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) + when(mService.requestNetwork( + any(), captor.capture(), anyInt(), any(), anyInt(), any())) .thenReturn(req1); manager.requestNetwork(req1, callback, handler); @@ -258,7 +260,8 @@ public class ConnectivityManagerTest { verify(callback, timeout(100).times(0)).onLosing(any(), anyInt()); // callback can be registered again - when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) + when(mService.requestNetwork( + any(), captor.capture(), anyInt(), any(), anyInt(), any())) .thenReturn(req2); manager.requestNetwork(req2, callback, handler); @@ -282,7 +285,8 @@ public class ConnectivityManagerTest { info.targetSdkVersion = VERSION_CODES.N_MR1 + 1; when(mCtx.getApplicationInfo()).thenReturn(info); - when(mService.requestNetwork(any(), any(), anyInt(), any(), anyInt())).thenReturn(request); + when(mService.requestNetwork(any(), any(), anyInt(), any(), anyInt(), any())) + .thenReturn(request); Handler handler = new Handler(Looper.getMainLooper()); manager.requestNetwork(request, callback, handler); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 8da1a5b655..f40e57fe46 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -107,6 +107,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -305,6 +306,7 @@ public class ConnectivityServiceTest { private static final String MOBILE_IFNAME = "test_rmnet_data0"; private static final String WIFI_IFNAME = "test_wlan0"; private static final String WIFI_WOL_IFNAME = "test_wlan_wol"; + private static final String TEST_PACKAGE_NAME = "com.android.test.package"; private static final String[] EMPTY_STRING_ARRAY = new String[0]; private MockContext mServiceContext; @@ -654,7 +656,7 @@ public class ConnectivityServiceTest { if (mNmValidationRedirectUrl != null) { mNmCallbacks.showProvisioningNotification( - "test_provisioning_notif_action", "com.android.test.package"); + "test_provisioning_notif_action", TEST_PACKAGE_NAME); mNmProvNotificationRequested = true; } } @@ -2972,7 +2974,7 @@ public class ConnectivityServiceTest { networkCapabilities.addTransportType(TRANSPORT_WIFI) .setNetworkSpecifier(new MatchAllNetworkSpecifier()); mService.requestNetwork(networkCapabilities, null, 0, null, - ConnectivityManager.TYPE_WIFI); + ConnectivityManager.TYPE_WIFI, TEST_PACKAGE_NAME); }); class NonParcelableSpecifier extends NetworkSpecifier { @@ -3011,31 +3013,12 @@ public class ConnectivityServiceTest { } @Test - public void testNetworkSpecifierUidSpoofSecurityException() throws Exception { - class UidAwareNetworkSpecifier extends NetworkSpecifier implements Parcelable { - @Override - public boolean satisfiedBy(NetworkSpecifier other) { - return true; - } - - @Override - public void assertValidFromUid(int requestorUid) { - throw new SecurityException("failure"); - } - - @Override - public int describeContents() { return 0; } - @Override - public void writeToParcel(Parcel dest, int flags) {} - } - + public void testNetworkRequestUidSpoofSecurityException() throws Exception { mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(false); - - UidAwareNetworkSpecifier networkSpecifier = new UidAwareNetworkSpecifier(); - NetworkRequest networkRequest = newWifiRequestBuilder().setNetworkSpecifier( - networkSpecifier).build(); + NetworkRequest networkRequest = newWifiRequestBuilder().build(); TestNetworkCallback networkCallback = new TestNetworkCallback(); + doThrow(new SecurityException()).when(mAppOpsManager).checkPackage(anyInt(), anyString()); assertThrows(SecurityException.class, () -> { mCm.requestNetwork(networkRequest, networkCallback); }); From c2b200e46b8ee7cf3a14bb0dfb52d701b59dd7a9 Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Tue, 11 Feb 2020 17:13:11 +0800 Subject: [PATCH 04/17] Fix invalid usage of javadoc CP from ag/10125651 Test: this change removes invalid usage Change-Id: Ibc9b1965ec7aa545c0dae71d741c3802d3a9fa12 Merged-In: I35fc4a814238635fa95981649c27230dda319afa --- core/java/android/net/NetworkAgentConfig.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/java/android/net/NetworkAgentConfig.java b/core/java/android/net/NetworkAgentConfig.java index 1e5af673b8..2c5a113a93 100644 --- a/core/java/android/net/NetworkAgentConfig.java +++ b/core/java/android/net/NetworkAgentConfig.java @@ -24,8 +24,7 @@ import android.os.Parcelable; /** * Allows a network transport to provide the system with policy and configuration information about - * a particular network when registering a {@link NetworkAgent}. - * @note This information cannot change once the agent is registered. + * a particular network when registering a {@link NetworkAgent}. This information cannot change once the agent is registered. * * @hide */ From 6a4dfac6b8c17d1f252668cefb3d1da378aa4b40 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 4 Dec 2019 20:01:46 +0900 Subject: [PATCH 05/17] [NS A44 1/2] Update linger state before processing listens To compute accurately whether a network is in the background, the linger state needs to be updated. Do that before computing whether a network is in the background and possibly calling applyBackgroundChangeForRematch. However ! As of this patch, rematchNetworksAndRequests computes a wrong value when adding to the list of affected networks, because it is looking at intermediate global state. Somehow this used to compensate exactly for the way reading back the state was wrong. There might have been a few undetected bugs there, but none is known. As such, as of this patch, rematchNetworksAndRequests still computes a wrong value while the computation when applying that state now computes the right one, so the tests do not pass. This patch must be checked in together with A44 2/2 which will fix the computation in rematchNetworksAndRequests, but is kept separate for ease of review. Bug: 113554781 Test: ConnectivityServiceTest Change-Id: Iaeed0d11bfa09f292f232ae020e944e430bc0184 --- .../android/server/ConnectivityService.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b3b1722664..197b0f14a8 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6777,17 +6777,10 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - for (final NetworkReassignment.NetworkBgStatePair event : changes.getRematchedNetworks()) { - // Process listen requests and update capabilities if the background state has - // changed for this network. For consistency with previous behavior, send onLost - // callbacks before onAvailable. - processNewlyLostListenRequests(event.mNetwork); - if (event.mOldBackground != event.mNetwork.isBackgroundNetwork()) { - applyBackgroundChangeForRematch(event.mNetwork); - } - processNewlySatisfiedListenRequests(event.mNetwork); - } - + // Update the linger state before processing listen callbacks, because the background + // computation depends on whether the network is lingering. Don't send the LOSING callbacks + // just yet though, because they have to be sent after the listens are processed to keep + // backward compatibility. final ArrayList lingeredNetworks = new ArrayList<>(); for (final NetworkAgentInfo nai : nais) { // Rematching may have altered the linger state of some networks, so update all linger @@ -6800,6 +6793,17 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + for (final NetworkReassignment.NetworkBgStatePair event : changes.getRematchedNetworks()) { + // Process listen requests and update capabilities if the background state has + // changed for this network. For consistency with previous behavior, send onLost + // callbacks before onAvailable. + processNewlyLostListenRequests(event.mNetwork); + if (event.mOldBackground != event.mNetwork.isBackgroundNetwork()) { + applyBackgroundChangeForRematch(event.mNetwork); + } + processNewlySatisfiedListenRequests(event.mNetwork); + } + for (final NetworkAgentInfo nai : lingeredNetworks) { notifyNetworkLosing(nai, now); } @@ -7200,8 +7204,6 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkRequest nr = networkAgent.requestAt(i); NetworkRequestInfo nri = mNetworkRequests.get(nr); if (VDBG) log(" sending notification for " + nr); - // TODO: if we're in the middle of a rematch, can we send a CAP_CHANGED callback for - // a network that no longer satisfies the listen? if (nri.mPendingIntent == null) { callCallbackForRequest(nri, networkAgent, notifyType, arg1); } else { From 64520dc48819996db0bfcb1b39505800f2781197 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 4 Dec 2019 19:55:32 +0900 Subject: [PATCH 06/17] [NS A44 2/2] Apply requests after all networks rematching is computed This patch finally separates completely computing the rematch from all the side effects. A collateral effect of this is to compute correctly the background network state in rematchNetworkAndRequests, which compensates the breakage from the previous patch. Bug: 113554781 Test: ConnectivityServiceTest Change-Id: I998c729c385940168fcd6ba3f2e01911f1844ce1 --- .../android/server/ConnectivityService.java | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 197b0f14a8..b9c718c269 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6601,11 +6601,9 @@ public class ConnectivityService extends IConnectivityManager.Stub return change; } - private ArrayMap computeRequestReassignmentForNetwork( - @NonNull final NetworkReassignment changes, + private void computeRequestReassignmentForNetwork(@NonNull final NetworkReassignment changes, @NonNull final NetworkAgentInfo newNetwork) { final int score = newNetwork.getCurrentScore(); - final ArrayMap reassignedRequests = new ArrayMap<>(); for (NetworkRequestInfo nri : mNetworkRequests.values()) { // Process requests in the first pass and listens in the second pass. This allows us to // change a network's capabilities depending on which requests it has. This is only @@ -6631,18 +6629,14 @@ public class ConnectivityService extends IConnectivityManager.Stub + ", newScore = " + score); } if (currentNetwork == null || currentNetwork.getCurrentScore() < score) { - reassignedRequests.put(nri, newNetwork); changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( nri, currentNetwork, newNetwork)); } } else if (newNetwork == currentNetwork) { - reassignedRequests.put(nri, null); changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( nri, currentNetwork, null)); } } - - return reassignedRequests; } // Handles a network appearing or improving its score. @@ -6661,7 +6655,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // @param newNetwork is the network to be matched against NetworkRequests. // @param now the time the rematch starts, as returned by SystemClock.elapsedRealtime(); private void rematchNetworkAndRequests(@NonNull final NetworkReassignment changes, - @NonNull final NetworkAgentInfo newNetwork, final long now) { + @NonNull final NetworkAgentInfo newNetwork) { ensureRunningOnConnectivityServiceThread(); if (!newNetwork.everConnected) return; @@ -6670,18 +6664,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (VDBG || DDBG) log("rematching " + newNetwork.name()); - final ArrayMap reassignedRequests = - computeRequestReassignmentForNetwork(changes, newNetwork); - - // Find and migrate to this Network any NetworkRequests for - // which this network is now the best. - for (final Map.Entry entry : - reassignedRequests.entrySet()) { - final NetworkRequestInfo nri = entry.getKey(); - final NetworkAgentInfo previousSatisfier = nri.mSatisfier; - final NetworkAgentInfo newSatisfier = entry.getValue(); - updateSatisfiersForRematchRequest(nri, previousSatisfier, newSatisfier, now); - } + computeRequestReassignmentForNetwork(changes, newNetwork); } private void updateSatisfiersForRematchRequest(@NonNull final NetworkRequestInfo nri, @@ -6734,7 +6717,20 @@ public class ConnectivityService extends IConnectivityManager.Stub Arrays.sort(nais); final NetworkReassignment changes = computeInitialReassignment(); for (final NetworkAgentInfo nai : nais) { - rematchNetworkAndRequests(changes, nai, now); + rematchNetworkAndRequests(changes, nai); + } + + // Now that the entire rematch is computed, update the lists of satisfied requests in + // the network agents. This is necessary because some code later depends on this state + // to be correct, most prominently computing the linger status. + for (final NetworkReassignment.RequestReassignment event : + changes.getRequestReassignments()) { + // The rematch is seeded with an entry for each request, and requests that don't + // change satisfiers have the same network as old and new. + // TODO : remove these entries when they are not needed any more. + if (event.mOldNetwork == event.mNewNetwork) continue; + updateSatisfiersForRematchRequest(event.mRequest, event.mOldNetwork, + event.mNewNetwork, now); } final NetworkRequestInfo defaultRequestInfo = mNetworkRequests.get(mDefaultRequest); From 57cc7cb461fecc5c6a25238056ff18a65a71075e Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 18:56:30 +0900 Subject: [PATCH 07/17] [NS B01] Move the computation loop to a separate function Bug: 113554781 Test: FrameworksNetTests Change-Id: I6c28c7af5c600d35aa1e9328b6c988dadb921f51 --- .../android/server/ConnectivityService.java | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b9c718c269..30419aebd0 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6656,7 +6656,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // @param now the time the rematch starts, as returned by SystemClock.elapsedRealtime(); private void rematchNetworkAndRequests(@NonNull final NetworkReassignment changes, @NonNull final NetworkAgentInfo newNetwork) { - ensureRunningOnConnectivityServiceThread(); if (!newNetwork.everConnected) return; changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(newNetwork, @@ -6696,6 +6695,22 @@ public class ConnectivityService extends IConnectivityManager.Stub nri.mSatisfier = newSatisfier; } + @NonNull + private NetworkReassignment computeNetworkReassignment() { + ensureRunningOnConnectivityServiceThread(); + final NetworkAgentInfo[] nais = mNetworkAgentInfos.values().toArray( + new NetworkAgentInfo[mNetworkAgentInfos.size()]); + // Rematch higher scoring networks first to prevent requests first matching a lower + // scoring network and then a higher scoring network, which could produce multiple + // callbacks. + Arrays.sort(nais); + final NetworkReassignment changes = computeInitialReassignment(); + for (final NetworkAgentInfo nai : nais) { + rematchNetworkAndRequests(changes, nai); + } + return changes; + } + /** * Attempt to rematch all Networks with NetworkRequests. This may result in Networks * being disconnected. @@ -6709,16 +6724,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final long now = SystemClock.elapsedRealtime(); final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); - final NetworkAgentInfo[] nais = mNetworkAgentInfos.values().toArray( - new NetworkAgentInfo[mNetworkAgentInfos.size()]); - // Rematch higher scoring networks first to prevent requests first matching a lower - // scoring network and then a higher scoring network, which could produce multiple - // callbacks. - Arrays.sort(nais); - final NetworkReassignment changes = computeInitialReassignment(); - for (final NetworkAgentInfo nai : nais) { - rematchNetworkAndRequests(changes, nai); - } + final NetworkReassignment changes = computeNetworkReassignment(); // Now that the entire rematch is computed, update the lists of satisfied requests in // the network agents. This is necessary because some code later depends on this state @@ -6773,6 +6779,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + final Collection nais = mNetworkAgentInfos.values(); + // Update the linger state before processing listen callbacks, because the background // computation depends on whether the network is lingering. Don't send the LOSING callbacks // just yet though, because they have to be sent after the listens are processed to keep @@ -6849,7 +6857,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void updateLegacyTypeTrackerAndVpnLockdownForRematch( @Nullable final NetworkAgentInfo oldDefaultNetwork, @Nullable final NetworkAgentInfo newDefaultNetwork, - @NonNull final NetworkAgentInfo[] nais) { + @NonNull final Collection nais) { if (oldDefaultNetwork != newDefaultNetwork) { // Maintain the illusion : since the legacy API only understands one network at a time, // if the default network changed, apps should see a disconnected broadcast for the From d7f762d7be9298fe46ae58627ad420889f0713f4 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 19:01:29 +0900 Subject: [PATCH 08/17] [NS B02] Split out a function to apply a NetworkReassignment This makes the rematchAllNetworksAndRequests function, which is the nexus of the rematching code, very straightforward and easy to read. Bug: 113554781 Test: FrameworksNetTests Change-Id: I5cea4ed7e06439494700d88ab202b696402fa360 --- .../java/com/android/server/ConnectivityService.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 30419aebd0..ea6fe2b918 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6723,12 +6723,15 @@ public class ConnectivityService extends IConnectivityManager.Stub // be optimized to only do the processing needed. final long now = SystemClock.elapsedRealtime(); final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); - final NetworkReassignment changes = computeNetworkReassignment(); + applyNetworkReassignment(changes, oldDefaultNetwork, now); + } - // Now that the entire rematch is computed, update the lists of satisfied requests in - // the network agents. This is necessary because some code later depends on this state - // to be correct, most prominently computing the linger status. + private void applyNetworkReassignment(@NonNull final NetworkReassignment changes, + @Nullable final NetworkAgentInfo oldDefaultNetwork, final long now) { + // First, update the lists of satisfied requests in the network agents. This is necessary + // because some code later depends on this state to be correct, most prominently computing + // the linger status. for (final NetworkReassignment.RequestReassignment event : changes.getRequestReassignments()) { // The rematch is seeded with an entry for each request, and requests that don't From 4970757c26bb2a6fdc0225feb316fe189bc93e21 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 21:07:02 +0900 Subject: [PATCH 09/17] [NS B03] Add debug log showing the reassignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dumpString for a reassignment looks like : NetworkReassignment : Rematched networks : [100 CELLULAR], [101 WIFI] 6 : 100 → 101 8 : null → 101 toString looks like : NetReassign [4 : 100 → 101, 5 : null → 101] If no changes, then it looks like NetworkReassignment : no changes Bug: 113554781 Test: Manual Change-Id: If9eeadb7ee317dee2d91ca1feca3091ae39e9bae --- .../android/server/ConnectivityService.java | 118 ++++++++++++------ .../server/connectivity/KeepaliveTracker.java | 19 +-- .../server/connectivity/LingerMonitor.java | 16 +-- .../server/connectivity/Nat464Xlat.java | 2 +- .../server/connectivity/NetworkAgentInfo.java | 32 +++-- 5 files changed, 124 insertions(+), 63 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ea6fe2b918..d4d870f2d7 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -231,6 +231,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.SortedSet; +import java.util.StringJoiner; import java.util.TreeSet; import java.util.concurrent.atomic.AtomicInteger; @@ -727,9 +728,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private void maybeLogBroadcast(NetworkAgentInfo nai, DetailedState state, int type, boolean isDefaultNetwork) { if (DBG) { - log("Sending " + state + - " broadcast for type " + type + " " + nai.name() + - " isDefaultNetwork=" + isDefaultNetwork); + log("Sending " + state + + " broadcast for type " + type + " " + nai.toShortString() + + " isDefaultNetwork=" + isDefaultNetwork); } } @@ -809,14 +810,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private String naiToString(NetworkAgentInfo nai) { - String name = nai.name(); - String state = (nai.networkInfo != null) ? - nai.networkInfo.getState() + "/" + nai.networkInfo.getDetailedState() : - "???/???"; - return name + " " + state; - } - public void dump(IndentingPrintWriter pw) { pw.println("mLegacyTypeTracker:"); pw.increaseIndent(); @@ -831,7 +824,7 @@ public class ConnectivityService extends IConnectivityManager.Stub for (int type = 0; type < mTypeLists.length; type++) { if (mTypeLists[type] == null || mTypeLists[type].isEmpty()) continue; for (NetworkAgentInfo nai : mTypeLists[type]) { - pw.println(type + " " + naiToString(nai)); + pw.println(type + " " + nai.toShortString()); } } } @@ -2786,7 +2779,7 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.everCaptivePortalDetected |= visible; if (nai.lastCaptivePortalDetected && Settings.Global.CAPTIVE_PORTAL_MODE_AVOID == getCaptivePortalMode()) { - if (DBG) log("Avoiding captive portal network: " + nai.name()); + if (DBG) log("Avoiding captive portal network: " + nai.toShortString()); nai.asyncChannel.sendMessage( NetworkAgent.CMD_PREVENT_AUTOMATIC_RECONNECT); teardownUnneededNetwork(nai); @@ -2845,7 +2838,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final String logMsg = !TextUtils.isEmpty(redirectUrl) ? " with redirect to " + redirectUrl : ""; - log(nai.name() + " validation " + (valid ? "passed" : "failed") + logMsg); + log(nai.toShortString() + " validation " + (valid ? "passed" : "failed") + logMsg); } if (valid != nai.lastValidated) { if (wasDefault) { @@ -3126,13 +3119,13 @@ public class ConnectivityService extends IConnectivityManager.Stub // one lingered request, start lingering. nai.updateLingerTimer(); if (nai.isLingering() && nai.numForegroundNetworkRequests() > 0) { - if (DBG) log("Unlingering " + nai.name()); + if (DBG) log("Unlingering " + nai.toShortString()); nai.unlinger(); logNetworkEvent(nai, NetworkEvent.NETWORK_UNLINGER); } else if (unneeded(nai, UnneededFor.LINGER) && nai.getLingerExpiry() > 0) { if (DBG) { final int lingerTime = (int) (nai.getLingerExpiry() - now); - log("Lingering " + nai.name() + " for " + lingerTime + "ms"); + log("Lingering " + nai.toShortString() + " for " + lingerTime + "ms"); } nai.linger(); logNetworkEvent(nai, NetworkEvent.NETWORK_LINGER); @@ -3196,7 +3189,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void disconnectAndDestroyNetwork(NetworkAgentInfo nai) { ensureRunningOnConnectivityServiceThread(); if (DBG) { - log(nai.name() + " got DISCONNECTED, was satisfying " + nai.numNetworkRequests()); + log(nai.toShortString() + " disconnected, was satisfying " + nai.numNetworkRequests()); } // Clear all notifications of this network. mNotifier.clearNotification(nai.network.netId); @@ -3254,7 +3247,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mDefaultNetworkNai = null; updateDataActivityTracking(null /* newNetwork */, nai); notifyLockdownVpn(nai); - ensureNetworkTransitionWakelock(nai.name()); + ensureNetworkTransitionWakelock(nai.toShortString()); } mLegacyTypeTracker.remove(nai, wasDefault); if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { @@ -3477,8 +3470,8 @@ public class ConnectivityService extends IConnectivityManager.Stub boolean wasBackgroundNetwork = nai.isBackgroundNetwork(); nai.removeRequest(nri.request.requestId); if (VDBG || DDBG) { - log(" Removing from current network " + nai.name() + - ", leaving " + nai.numNetworkRequests() + " requests."); + log(" Removing from current network " + nai.toShortString() + + ", leaving " + nai.numNetworkRequests() + " requests."); } // If there are still lingered requests on this network, don't tear it down, // but resume lingering instead. @@ -3487,7 +3480,7 @@ public class ConnectivityService extends IConnectivityManager.Stub notifyNetworkLosing(nai, now); } if (unneeded(nai, UnneededFor.TEARDOWN)) { - if (DBG) log("no live requests for " + nai.name() + "; disconnecting"); + if (DBG) log("no live requests for " + nai.toShortString() + "; disconnecting"); teardownUnneededNetwork(nai); } else { wasKept = true; @@ -3822,7 +3815,7 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.increaseIndent(); for (NetworkAgentInfo nai : networksSortedById()) { if (nai.avoidUnvalidated) { - pw.println(nai.name()); + pw.println(nai.toShortString()); } } pw.decreaseIndent(); @@ -3934,7 +3927,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private void handleNetworkUnvalidated(NetworkAgentInfo nai) { NetworkCapabilities nc = nai.networkCapabilities; - if (DBG) log("handleNetworkUnvalidated " + nai.name() + " cap=" + nc); + if (DBG) log("handleNetworkUnvalidated " + nai.toShortString() + " cap=" + nc); if (!nc.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) { return; @@ -5348,7 +5341,7 @@ public class ConnectivityService extends IConnectivityManager.Stub detail = reason; } log(String.format("updateSignalStrengthThresholds: %s, sending %s to %s", - detail, Arrays.toString(thresholdsArray.toArray()), nai.name())); + detail, Arrays.toString(thresholdsArray.toArray()), nai.toShortString())); } nai.asyncChannel.sendMessage( @@ -6272,9 +6265,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // newLp is already a defensive copy. newLp.ensureDirectlyConnectedRoutes(); if (VDBG || DDBG) { - log("Update of LinkProperties for " + nai.name() + - "; created=" + nai.created + - "; everConnected=" + nai.everConnected); + log("Update of LinkProperties for " + nai.toShortString() + + "; created=" + nai.created + + "; everConnected=" + nai.everConnected); } updateLinkProperties(nai, newLp, new LinkProperties(nai.linkProperties)); } @@ -6444,7 +6437,7 @@ public class ConnectivityService extends IConnectivityManager.Stub loge("Unknown NetworkAgentInfo in handleLingerComplete"); return; } - if (DBG) log("handleLingerComplete for " + oldNetwork.name()); + if (DBG) log("handleLingerComplete for " + oldNetwork.toShortString()); // If we get here it means that the last linger timeout for this network expired. So there // must be no other active linger timers, and we must stop lingering. @@ -6527,6 +6520,11 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetwork = network; mOldBackground = oldBackground; } + + public String toString() { + return "[" + NetworkAgentInfo.toShortString(mNetwork) + + " oldBg=" + mOldBackground + "]"; + } } static class RequestReassignment { @@ -6540,6 +6538,12 @@ public class ConnectivityService extends IConnectivityManager.Stub mOldNetwork = oldNetwork; mNewNetwork = newNetwork; } + + public String toString() { + return mRequest.request.requestId + " : " + + (null != mOldNetwork ? mOldNetwork.network.netId : "null") + + " → " + (null != mNewNetwork ? mNewNetwork.network.netId : "null"); + } } @NonNull private final Set mRematchedNetworks = new ArraySet<>(); @@ -6589,6 +6593,36 @@ public class ConnectivityService extends IConnectivityManager.Stub } return null; } + + public String toString() { + final StringJoiner sj = new StringJoiner(", " /* delimiter */, + "NetReassign [" /* prefix */, "]" /* suffix */); + if (mRematchedNetworks.isEmpty() && mReassignments.isEmpty()) { + return sj.add("no changes").toString(); + } + for (final RequestReassignment rr : getRequestReassignments()) { + sj.add(rr.toString()); + } + return sj.toString(); + } + + public String debugString() { + final StringBuilder sb = new StringBuilder(); + sb.append("NetworkReassignment :"); + if (mRematchedNetworks.isEmpty() && mReassignments.isEmpty()) { + return sb.append(" no changes").toString(); + } + final StringJoiner sj = new StringJoiner(", " /* delimiter */, + "\n Rematched networks : " /* prefix */, "" /* suffix */); + for (final NetworkBgStatePair rr : mRematchedNetworks) { + sj.add(rr.mNetwork.toShortString()); + } + sb.append(sj.toString()); + for (final RequestReassignment rr : getRequestReassignments()) { + sb.append("\n ").append(rr); + } + return sb.append("\n").toString(); + } } // TODO : remove this when it's useless @@ -6661,7 +6695,7 @@ public class ConnectivityService extends IConnectivityManager.Stub changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(newNetwork, newNetwork.isBackgroundNetwork())); - if (VDBG || DDBG) log("rematching " + newNetwork.name()); + if (VDBG || DDBG) log("rematching " + newNetwork.toShortString()); computeRequestReassignmentForNetwork(changes, newNetwork); } @@ -6671,10 +6705,10 @@ public class ConnectivityService extends IConnectivityManager.Stub @Nullable final NetworkAgentInfo newSatisfier, final long now) { if (newSatisfier != null) { - if (VDBG) log("rematch for " + newSatisfier.name()); + if (VDBG) log("rematch for " + newSatisfier.toShortString()); if (previousSatisfier != null) { if (VDBG || DDBG) { - log(" accepting network in place of " + previousSatisfier.name()); + log(" accepting network in place of " + previousSatisfier.toShortString()); } previousSatisfier.removeRequest(nri.request.requestId); previousSatisfier.lingerRequest(nri.request, now, mLingerDelayMs); @@ -6683,11 +6717,12 @@ public class ConnectivityService extends IConnectivityManager.Stub } newSatisfier.unlingerRequest(nri.request); if (!newSatisfier.addRequest(nri.request)) { - Slog.wtf(TAG, "BUG: " + newSatisfier.name() + " already has " + nri.request); + Slog.wtf(TAG, "BUG: " + newSatisfier.toShortString() + " already has " + + nri.request); } } else { if (DBG) { - log("Network " + previousSatisfier.name() + " stopped satisfying" + log("Network " + previousSatisfier.toShortString() + " stopped satisfying" + " request " + nri.request.requestId); } previousSatisfier.removeRequest(nri.request.requestId); @@ -6724,6 +6759,11 @@ public class ConnectivityService extends IConnectivityManager.Stub final long now = SystemClock.elapsedRealtime(); final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); final NetworkReassignment changes = computeNetworkReassignment(); + if (VDBG || DDBG) { + log(changes.debugString()); + } else if (DBG) { + log(changes.toString()); // Shorter form, only one line of log + } applyNetworkReassignment(changes, oldDefaultNetwork, now); } @@ -6832,7 +6872,7 @@ public class ConnectivityService extends IConnectivityManager.Stub notifyNetworkLosing(nai, now); } } else { - if (DBG) log("Reaping " + nai.name()); + if (DBG) log("Reaping " + nai.toShortString()); teardownUnneededNetwork(nai); } } @@ -6980,8 +7020,8 @@ public class ConnectivityService extends IConnectivityManager.Stub notifyLockdownVpn(networkAgent); if (DBG) { - log(networkAgent.name() + " EVENT_NETWORK_INFO_CHANGED, going from " + - oldInfo.getState() + " to " + state); + log(networkAgent.toShortString() + " EVENT_NETWORK_INFO_CHANGED, going from " + + oldInfo.getState() + " to " + state); } if (!networkAgent.created @@ -6999,7 +7039,7 @@ public class ConnectivityService extends IConnectivityManager.Stub networkAgent.everConnected = true; if (networkAgent.linkProperties == null) { - Slog.wtf(TAG, networkAgent.name() + " connected with null LinkProperties"); + Slog.wtf(TAG, networkAgent.toShortString() + " connected with null LinkProperties"); } // NetworkCapabilities need to be set before sending the private DNS config to @@ -7059,7 +7099,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void updateNetworkScore(NetworkAgentInfo nai, NetworkScore ns) { - if (VDBG || DDBG) log("updateNetworkScore for " + nai.name() + " to " + ns); + if (VDBG || DDBG) log("updateNetworkScore for " + nai.toShortString() + " to " + ns); nai.setNetworkScore(ns); rematchAllNetworksAndRequests(); sendUpdatedScoreToFactories(nai); @@ -7205,7 +7245,7 @@ public class ConnectivityService extends IConnectivityManager.Stub protected void notifyNetworkCallbacks(NetworkAgentInfo networkAgent, int notifyType, int arg1) { if (VDBG || DDBG) { String notification = ConnectivityManager.getCallbackName(notifyType); - log("notifyType " + notification + " for " + networkAgent.name()); + log("notifyType " + notification + " for " + networkAgent.toShortString()); } for (int i = 0; i < networkAgent.numNetworkRequests(); i++) { NetworkRequest nr = networkAgent.requestAt(i); diff --git a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java index af8a3666e9..5059a4861a 100644 --- a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java +++ b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java @@ -325,7 +325,7 @@ public class KeepaliveTracker { mSlot = slot; int error = isValid(); if (error == SUCCESS) { - Log.d(TAG, "Starting keepalive " + mSlot + " on " + mNai.name()); + Log.d(TAG, "Starting keepalive " + mSlot + " on " + mNai.toShortString()); switch (mType) { case TYPE_NATT: mNai.asyncChannel.sendMessage( @@ -365,7 +365,8 @@ public class KeepaliveTracker { Log.e(TAG, "Cannot stop unowned keepalive " + mSlot + " on " + mNai.network); } } - Log.d(TAG, "Stopping keepalive " + mSlot + " on " + mNai.name() + ": " + reason); + Log.d(TAG, "Stopping keepalive " + mSlot + " on " + mNai.toShortString() + + ": " + reason); switch (mStartedState) { case NOT_STARTED: // Remove the reference of the keepalive that meet error before starting, @@ -476,7 +477,7 @@ public class KeepaliveTracker { } public void handleStopKeepalive(NetworkAgentInfo nai, int slot, int reason) { - String networkName = (nai == null) ? "(null)" : nai.name(); + final String networkName = NetworkAgentInfo.toShortString(nai); HashMap networkKeepalives = mKeepalives.get(nai); if (networkKeepalives == null) { Log.e(TAG, "Attempt to stop keepalive on nonexistent network " + networkName); @@ -493,7 +494,7 @@ public class KeepaliveTracker { } private void cleanupStoppedKeepalive(NetworkAgentInfo nai, int slot) { - String networkName = (nai == null) ? "(null)" : nai.name(); + final String networkName = NetworkAgentInfo.toShortString(nai); HashMap networkKeepalives = mKeepalives.get(nai); if (networkKeepalives == null) { Log.e(TAG, "Attempt to remove keepalive on nonexistent network " + networkName); @@ -540,7 +541,7 @@ public class KeepaliveTracker { } catch(NullPointerException e) {} if (ki == null) { Log.e(TAG, "Event " + message.what + "," + slot + "," + reason - + " for unknown keepalive " + slot + " on " + nai.name()); + + " for unknown keepalive " + slot + " on " + nai.toShortString()); return; } @@ -562,7 +563,7 @@ public class KeepaliveTracker { if (KeepaliveInfo.STARTING == ki.mStartedState) { if (SUCCESS == reason) { // Keepalive successfully started. - Log.d(TAG, "Started keepalive " + slot + " on " + nai.name()); + Log.d(TAG, "Started keepalive " + slot + " on " + nai.toShortString()); ki.mStartedState = KeepaliveInfo.STARTED; try { ki.mCallback.onStarted(slot); @@ -570,14 +571,14 @@ public class KeepaliveTracker { Log.w(TAG, "Discarded onStarted(" + slot + ") callback"); } } else { - Log.d(TAG, "Failed to start keepalive " + slot + " on " + nai.name() + Log.d(TAG, "Failed to start keepalive " + slot + " on " + nai.toShortString() + ": " + reason); // The message indicated some error trying to start: do call handleStopKeepalive. handleStopKeepalive(nai, slot, reason); } } else if (KeepaliveInfo.STOPPING == ki.mStartedState) { // The message indicated result of stopping : clean up keepalive slots. - Log.d(TAG, "Stopped keepalive " + slot + " on " + nai.name() + Log.d(TAG, "Stopped keepalive " + slot + " on " + nai.toShortString() + " stopped: " + reason); ki.mStartedState = KeepaliveInfo.NOT_STARTED; cleanupStoppedKeepalive(nai, slot); @@ -733,7 +734,7 @@ public class KeepaliveTracker { pw.println("Socket keepalives:"); pw.increaseIndent(); for (NetworkAgentInfo nai : mKeepalives.keySet()) { - pw.println(nai.name()); + pw.println(nai.toShortString()); pw.increaseIndent(); for (int slot : mKeepalives.get(nai).keySet()) { KeepaliveInfo ki = mKeepalives.get(nai).get(slot); diff --git a/services/core/java/com/android/server/connectivity/LingerMonitor.java b/services/core/java/com/android/server/connectivity/LingerMonitor.java index 7071510598..04c000f645 100644 --- a/services/core/java/com/android/server/connectivity/LingerMonitor.java +++ b/services/core/java/com/android/server/connectivity/LingerMonitor.java @@ -200,8 +200,9 @@ public class LingerMonitor { } if (DBG) { - Log.d(TAG, "Notifying switch from=" + fromNai.name() + " to=" + toNai.name() + - " type=" + sNotifyTypeNames.get(notifyType, "unknown(" + notifyType + ")")); + Log.d(TAG, "Notifying switch from=" + fromNai.toShortString() + + " to=" + toNai.toShortString() + + " type=" + sNotifyTypeNames.get(notifyType, "unknown(" + notifyType + ")")); } mNotifications.put(fromNai.network.netId, toNai.network.netId); @@ -222,10 +223,10 @@ public class LingerMonitor { public void noteLingerDefaultNetwork(@NonNull final NetworkAgentInfo fromNai, @Nullable final NetworkAgentInfo toNai) { if (VDBG) { - Log.d(TAG, "noteLingerDefaultNetwork from=" + fromNai.name() + - " everValidated=" + fromNai.everValidated + - " lastValidated=" + fromNai.lastValidated + - " to=" + toNai.name()); + Log.d(TAG, "noteLingerDefaultNetwork from=" + fromNai.toShortString() + + " everValidated=" + fromNai.everValidated + + " lastValidated=" + fromNai.lastValidated + + " to=" + toNai.toShortString()); } // If we are currently notifying the user because the device switched to fromNai, now that @@ -270,7 +271,8 @@ public class LingerMonitor { // TODO: should we do this? if (everNotified(fromNai)) { if (VDBG) { - Log.d(TAG, "Not notifying handover from " + fromNai.name() + ", already notified"); + Log.d(TAG, "Not notifying handover from " + fromNai.toShortString() + + ", already notified"); } return; } diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index f636d67c3d..82465f8a09 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -174,7 +174,7 @@ public class Nat464Xlat extends BaseNetworkObserver { try { mNMService.registerObserver(this); } catch (RemoteException e) { - Slog.e(TAG, "Can't register interface observer for clat on " + mNetwork.name()); + Slog.e(TAG, "Can't register iface observer for clat on " + mNetwork.toShortString()); return; } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index d66aec5761..d30a32041c 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -16,7 +16,10 @@ package com.android.server.connectivity; +import static android.net.NetworkCapabilities.transportNamesOf; + import android.annotation.NonNull; +import android.annotation.Nullable; import android.content.Context; import android.net.IDnsResolver; import android.net.INetd; @@ -372,7 +375,7 @@ public class NetworkAgentInfo implements Comparable { // Should only happen if the requestId wraps. If that happens lots of other things will // be broken as well. Log.wtf(TAG, String.format("Duplicate requestId for %s and %s on %s", - networkRequest, existing, name())); + networkRequest, existing, toShortString())); updateRequestCounts(REMOVE, existing); } mNetworkRequests.put(networkRequest.requestId, networkRequest); @@ -542,11 +545,11 @@ public class NetworkAgentInfo implements Comparable { // Cannot happen. Once a request is lingering on a particular network, we cannot // re-linger it unless that network becomes the best for that request again, in which // case we should have unlingered it. - Log.wtf(TAG, this.name() + ": request " + request.requestId + " already lingered"); + Log.wtf(TAG, toShortString() + ": request " + request.requestId + " already lingered"); } final long expiryMs = now + duration; LingerTimer timer = new LingerTimer(request, expiryMs); - if (VDBG) Log.d(TAG, "Adding LingerTimer " + timer + " to " + this.name()); + if (VDBG) Log.d(TAG, "Adding LingerTimer " + timer + " to " + toShortString()); mLingerTimers.add(timer); mLingerTimerForRequest.put(request.requestId, timer); } @@ -558,7 +561,7 @@ public class NetworkAgentInfo implements Comparable { public boolean unlingerRequest(NetworkRequest request) { LingerTimer timer = mLingerTimerForRequest.get(request.requestId); if (timer != null) { - if (VDBG) Log.d(TAG, "Removing LingerTimer " + timer + " from " + this.name()); + if (VDBG) Log.d(TAG, "Removing LingerTimer " + timer + " from " + toShortString()); mLingerTimers.remove(timer); mLingerTimerForRequest.remove(request.requestId); return true; @@ -645,9 +648,16 @@ public class NetworkAgentInfo implements Comparable { + "}"; } - public String name() { - return "NetworkAgentInfo [" + networkInfo.getTypeName() + " (" + - networkInfo.getSubtypeName() + ") - " + Objects.toString(network) + "]"; + /** + * Show a short string representing a Network. + * + * This is often not enough for debugging purposes for anything complex, but the full form + * is very long and hard to read, so this is useful when there isn't a lot of ambiguity. + * This represents the network with something like "[100 WIFI|VPN]" or "[108 MOBILE]". + */ + public String toShortString() { + return "[" + network.netId + " " + + transportNamesOf(networkCapabilities.getTransportTypes()) + "]"; } // Enables sorting in descending order of score. @@ -655,4 +665,12 @@ public class NetworkAgentInfo implements Comparable { public int compareTo(NetworkAgentInfo other) { return other.getCurrentScore() - getCurrentScore(); } + + /** + * Null-guarding version of NetworkAgentInfo#toShortString() + */ + @NonNull + public static String toShortString(@Nullable final NetworkAgentInfo nai) { + return null != nai ? nai.toShortString() : "[null]"; + } } From 857a1719a1851f640646c9253a3333c33592ba59 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 21:08:07 +0900 Subject: [PATCH 10/17] [NS B04] Make the network selection request-major. This patch marks the most important turning point of this refactoring. Cleanups removing unused code will follow. Replace the old network-major reassignment computation with a much simpler and faster loop that takes each request and assigns it the highest-scoring network. All tests pass, of course. Bug: 113554781 Test: FrameworksNetTests Change-Id: Ie143802995155151a38a4eb1d2f26c3f29e556bd --- .../android/server/ConnectivityService.java | 46 +++++++++++++------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d4d870f2d7..e65c8305cc 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6733,15 +6733,37 @@ public class ConnectivityService extends IConnectivityManager.Stub @NonNull private NetworkReassignment computeNetworkReassignment() { ensureRunningOnConnectivityServiceThread(); - final NetworkAgentInfo[] nais = mNetworkAgentInfos.values().toArray( - new NetworkAgentInfo[mNetworkAgentInfos.size()]); - // Rematch higher scoring networks first to prevent requests first matching a lower - // scoring network and then a higher scoring network, which could produce multiple - // callbacks. - Arrays.sort(nais); - final NetworkReassignment changes = computeInitialReassignment(); - for (final NetworkAgentInfo nai : nais) { - rematchNetworkAndRequests(changes, nai); + final NetworkReassignment changes = new NetworkReassignment(); + + // Gather the list of all relevant agents and sort them by score. + final ArrayList nais = new ArrayList<>(); + for (final NetworkAgentInfo nai : mNetworkAgentInfos.values()) { + if (!nai.everConnected) continue; + nais.add(nai); + changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(nai, + nai.isBackgroundNetwork())); + } + Collections.sort(nais); + + for (final NetworkRequestInfo nri : mNetworkRequests.values()) { + if (nri.request.isListen()) continue; + // Find the top scoring network satisfying this request. + NetworkAgentInfo bestNetwork = null; + for (final NetworkAgentInfo nai : nais) { + if (!nai.satisfies(nri.request)) continue; + bestNetwork = nai; + // As the nais are sorted by score, this is the top-scoring network that can + // satisfy this request. The best network for this request has been found, + // go process the next NRI + break; + } + // If no NAI satisfies this request, bestNetwork is still null. That's fine : it + // means no network can satisfy the request. If nri.mSatisfier is not null, it just + // means the network that used to satisfy the request stopped satisfying it. + if (nri.mSatisfier != bestNetwork) { + changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( + nri, nri.mSatisfier, bestNetwork)); + } } return changes; } @@ -6751,11 +6773,7 @@ public class ConnectivityService extends IConnectivityManager.Stub * being disconnected. */ private void rematchAllNetworksAndRequests() { - // TODO: This may be slow, and should be optimized. Unfortunately at this moment the - // processing is network-major instead of request-major (the code iterates through all - // networks, then for each it iterates for all requests), which is a problem for re-scoring - // requests. Once the code has switched to a request-major iteration style, this can - // be optimized to only do the processing needed. + // TODO: This may be slow, and should be optimized. final long now = SystemClock.elapsedRealtime(); final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); final NetworkReassignment changes = computeNetworkReassignment(); From 7feb6ed384b1bb677181a646d92872d8ba4f7ad5 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 21:10:48 +0900 Subject: [PATCH 11/17] [NS B05] Remove old dead code Test: FrameworksNetTests Change-Id: I553721b327b76ede0e76b9fb7a0130fcae012175 --- .../android/server/ConnectivityService.java | 75 ------------------- 1 file changed, 75 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e65c8305cc..bcac2b5036 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6625,81 +6625,6 @@ 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 void computeRequestReassignmentForNetwork(@NonNull final NetworkReassignment changes, - @NonNull final NetworkAgentInfo newNetwork) { - final int score = newNetwork.getCurrentScore(); - for (NetworkRequestInfo nri : mNetworkRequests.values()) { - // Process requests in the first pass and listens in the second pass. This allows us to - // change a network's capabilities depending on which requests it has. This is only - // correct if the change in capabilities doesn't affect whether the network satisfies - // requests or not, and doesn't affect the network's score. - if (nri.request.isListen()) continue; - - // The reassignment has been seeded with the initial assignment, therefore - // getReassignment can't be null and mNewNetwork is only null if there was no - // satisfier in the first place or there was an explicit reassignment to null. - final NetworkAgentInfo currentNetwork = changes.getReassignment(nri).mNewNetwork; - final boolean satisfies = newNetwork.satisfies(nri.request); - if (newNetwork == currentNetwork && satisfies) continue; - - // check if it satisfies the NetworkCapabilities - if (VDBG) log(" checking if request is satisfied: " + nri.request); - if (satisfies) { - // next check if it's better than any current network we're using for - // this request - if (VDBG || DDBG) { - log("currentScore = " - + (currentNetwork != null ? currentNetwork.getCurrentScore() : 0) - + ", newScore = " + score); - } - if (currentNetwork == null || currentNetwork.getCurrentScore() < score) { - changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( - nri, currentNetwork, newNetwork)); - } - } else if (newNetwork == currentNetwork) { - changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( - nri, currentNetwork, null)); - } - } - } - - // Handles a network appearing or improving its score. - // - // - Evaluates all current NetworkRequests that can be - // satisfied by newNetwork, and reassigns to newNetwork - // any such requests for which newNetwork is the best. - // - // - Writes into the passed reassignment object all changes that should be done for - // rematching this network with all requests, to be applied later. - // - // TODO : stop writing to the passed reassignment. This is temporarily more useful, but - // it's unidiomatic Java and it's hard to read. - // - // @param changes a currently-building list of changes to write to - // @param newNetwork is the network to be matched against NetworkRequests. - // @param now the time the rematch starts, as returned by SystemClock.elapsedRealtime(); - private void rematchNetworkAndRequests(@NonNull final NetworkReassignment changes, - @NonNull final NetworkAgentInfo newNetwork) { - if (!newNetwork.everConnected) return; - - changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(newNetwork, - newNetwork.isBackgroundNetwork())); - - if (VDBG || DDBG) log("rematching " + newNetwork.toShortString()); - - computeRequestReassignmentForNetwork(changes, newNetwork); - } - private void updateSatisfiersForRematchRequest(@NonNull final NetworkRequestInfo nri, @Nullable final NetworkAgentInfo previousSatisfier, @Nullable final NetworkAgentInfo newSatisfier, From 46a62378ecbc2991554fcb3ca823d877cdf12480 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 21:25:24 +0900 Subject: [PATCH 12/17] [NS B06] Simplification This check is now unnecessary, seeing how the code adding these changes is now guaranteed to only add at most one change for each request. Test: FrameworksNetTests Change-Id: Ia0443602d9c89ee413e956df9c7b79f8f74813f7 --- .../android/server/ConnectivityService.java | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index bcac2b5036..90c38fce13 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -155,7 +155,6 @@ import android.security.Credentials; import android.security.KeyStore; import android.telephony.TelephonyManager; import android.text.TextUtils; -import android.util.ArrayMap; import android.util.ArraySet; import android.util.LocalLog; import android.util.Log; @@ -6547,37 +6546,30 @@ public class ConnectivityService extends IConnectivityManager.Stub } @NonNull private final Set mRematchedNetworks = new ArraySet<>(); - @NonNull private final Map mReassignments = - new ArrayMap<>(); + @NonNull private final ArrayList mReassignments = new ArrayList<>(); @NonNull Iterable getRematchedNetworks() { return mRematchedNetworks; } @NonNull Iterable getRequestReassignments() { - return mReassignments.values(); + return mReassignments; } void addRequestReassignment(@NonNull final RequestReassignment reassignment) { - final RequestReassignment oldChange = mReassignments.get(reassignment.mRequest); - if (null == oldChange) { - mReassignments.put(reassignment.mRequest, reassignment); - return; + if (!Build.IS_USER) { + // The code is never supposed to add two reassignments of the same request. Make + // sure this stays true, but without imposing this expensive check on all + // reassignments on all user devices. + for (final RequestReassignment existing : mReassignments) { + if (existing.mRequest.equals(reassignment.mRequest)) { + throw new IllegalStateException("Trying to reassign [" + + reassignment + "] but already have [" + + existing + "]"); + } + } } - 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)); + mReassignments.add(reassignment); } void addRematchedNetwork(@NonNull final NetworkBgStatePair network) { From a6014db9659354ca73514c8b65940493e6f8fe2e Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 21:58:23 +0900 Subject: [PATCH 13/17] [NS B07] More simplification The new reassignment does not contain these useless lines any more. Test: FrameworksNetTests Change-Id: I1583aebe94e529ce2b36e191a6e1f49c976bf29a --- .../core/java/com/android/server/ConnectivityService.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 90c38fce13..4acedd9496 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6709,10 +6709,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // the linger status. for (final NetworkReassignment.RequestReassignment event : changes.getRequestReassignments()) { - // The rematch is seeded with an entry for each request, and requests that don't - // change satisfiers have the same network as old and new. - // TODO : remove these entries when they are not needed any more. - if (event.mOldNetwork == event.mNewNetwork) continue; updateSatisfiersForRematchRequest(event.mRequest, event.mOldNetwork, event.mNewNetwork, now); } @@ -6741,8 +6737,6 @@ 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 f955f8eb6cad2b6a32fcf5e1aa4aef2f57b69881 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 22:01:31 +0900 Subject: [PATCH 14/17] [NS B08] More simplification Only computing the reassignment does not actually change the default network. Test: FrameworksNetTests Change-Id: I21ddf5cc1e3d3817055dbda4246e38ceb0732407 --- .../core/java/com/android/server/ConnectivityService.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4acedd9496..feea30fe52 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6692,18 +6692,17 @@ public class ConnectivityService extends IConnectivityManager.Stub private void rematchAllNetworksAndRequests() { // TODO: This may be slow, and should be optimized. final long now = SystemClock.elapsedRealtime(); - final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); final NetworkReassignment changes = computeNetworkReassignment(); if (VDBG || DDBG) { log(changes.debugString()); } else if (DBG) { log(changes.toString()); // Shorter form, only one line of log } - applyNetworkReassignment(changes, oldDefaultNetwork, now); + applyNetworkReassignment(changes, now); } private void applyNetworkReassignment(@NonNull final NetworkReassignment changes, - @Nullable final NetworkAgentInfo oldDefaultNetwork, final long now) { + final long now) { // First, update the lists of satisfied requests in the network agents. This is necessary // because some code later depends on this state to be correct, most prominently computing // the linger status. @@ -6713,6 +6712,7 @@ public class ConnectivityService extends IConnectivityManager.Stub event.mNewNetwork, now); } + final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); final NetworkRequestInfo defaultRequestInfo = mNetworkRequests.get(mDefaultRequest); final NetworkReassignment.RequestReassignment reassignment = changes.getReassignment(defaultRequestInfo); From 96a4f4b8de7cc383ce403d9106800d166445aad1 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 10 Dec 2019 22:16:53 +0900 Subject: [PATCH 15/17] [NS B09] Create NetworkRanker Bug: 113554781 Test: FrameworksNetTests Change-Id: Ia534247144f479fe896e1a6e05b906103cd10005 --- .../android/server/ConnectivityService.java | 21 ++--- .../server/connectivity/NetworkRanker.java | 50 +++++++++++ .../server/connectivity/NetworkRankerTest.kt | 84 +++++++++++++++++++ 3 files changed, 140 insertions(+), 15 deletions(-) create mode 100644 services/core/java/com/android/server/connectivity/NetworkRanker.java create mode 100644 tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index feea30fe52..a06cc95bc8 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -194,6 +194,7 @@ import com.android.server.connectivity.NetworkAgentInfo; import com.android.server.connectivity.NetworkDiagnostics; import com.android.server.connectivity.NetworkNotificationManager; import com.android.server.connectivity.NetworkNotificationManager.NotificationType; +import com.android.server.connectivity.NetworkRanker; import com.android.server.connectivity.PermissionMonitor; import com.android.server.connectivity.ProxyTracker; import com.android.server.connectivity.Vpn; @@ -579,6 +580,7 @@ public class ConnectivityService extends IConnectivityManager.Stub final ConnectivityDiagnosticsHandler mConnectivityDiagnosticsHandler; private final DnsManager mDnsManager; + private final NetworkRanker mNetworkRanker; private boolean mSystemReady; private Intent mInitialBroadcast; @@ -958,6 +960,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mMetricsLog = logger; mDefaultRequest = createDefaultInternetRequestForTransport(-1, NetworkRequest.Type.REQUEST); + mNetworkRanker = new NetworkRanker(); NetworkRequestInfo defaultNRI = new NetworkRequestInfo(null, mDefaultRequest, new Binder()); mNetworkRequests.put(mDefaultRequest, defaultNRI); mNetworkRequestInfoLogs.log("REGISTER " + defaultNRI); @@ -6660,24 +6663,12 @@ public class ConnectivityService extends IConnectivityManager.Stub changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(nai, nai.isBackgroundNetwork())); } - Collections.sort(nais); for (final NetworkRequestInfo nri : mNetworkRequests.values()) { if (nri.request.isListen()) continue; - // Find the top scoring network satisfying this request. - NetworkAgentInfo bestNetwork = null; - for (final NetworkAgentInfo nai : nais) { - if (!nai.satisfies(nri.request)) continue; - bestNetwork = nai; - // As the nais are sorted by score, this is the top-scoring network that can - // satisfy this request. The best network for this request has been found, - // go process the next NRI - break; - } - // If no NAI satisfies this request, bestNetwork is still null. That's fine : it - // means no network can satisfy the request. If nri.mSatisfier is not null, it just - // means the network that used to satisfy the request stopped satisfying it. - if (nri.mSatisfier != bestNetwork) { + final NetworkAgentInfo bestNetwork = mNetworkRanker.getBestNetwork(nri.request, nais); + if (bestNetwork != nri.mSatisfier) { + // bestNetwork may be null if no network can satisfy this request. changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( nri, nri.mSatisfier, bestNetwork)); } diff --git a/services/core/java/com/android/server/connectivity/NetworkRanker.java b/services/core/java/com/android/server/connectivity/NetworkRanker.java new file mode 100644 index 0000000000..d0aabf95d5 --- /dev/null +++ b/services/core/java/com/android/server/connectivity/NetworkRanker.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.connectivity; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.net.NetworkRequest; + +import java.util.Collection; + +/** + * A class that knows how to find the best network matching a request out of a list of networks. + */ +public class NetworkRanker { + public NetworkRanker() { } + + /** + * Find the best network satisfying this request among the list of passed networks. + */ + // Almost equivalent to Collections.max(nais), but allows returning null if no network + // satisfies the request. + @Nullable + public NetworkAgentInfo getBestNetwork(@NonNull final NetworkRequest request, + @NonNull final Collection nais) { + NetworkAgentInfo bestNetwork = null; + int bestScore = Integer.MIN_VALUE; + for (final NetworkAgentInfo nai : nais) { + if (!nai.satisfies(request)) continue; + if (nai.getCurrentScore() > bestScore) { + bestNetwork = nai; + bestScore = nai.getCurrentScore(); + } + } + return bestNetwork; + } +} diff --git a/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt new file mode 100644 index 0000000000..86c91165f6 --- /dev/null +++ b/tests/net/java/com/android/server/connectivity/NetworkRankerTest.kt @@ -0,0 +1,84 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.connectivity + +import android.net.NetworkRequest +import androidx.test.filters.SmallTest +import androidx.test.runner.AndroidJUnit4 +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any +import org.mockito.Mockito.doReturn +import org.mockito.Mockito.mock +import kotlin.test.assertEquals +import kotlin.test.assertNull + +@RunWith(AndroidJUnit4::class) +@SmallTest +class NetworkRankerTest { + private val ranker = NetworkRanker() + + private fun makeNai(satisfy: Boolean, score: Int) = mock(NetworkAgentInfo::class.java).also { + doReturn(satisfy).`when`(it).satisfies(any()) + doReturn(score).`when`(it).currentScore + } + + @Test + fun testGetBestNetwork() { + val scores = listOf(20, 50, 90, 60, 23, 68) + val nais = scores.map { makeNai(true, it) } + val bestNetwork = nais[2] // The one with the top score + val someRequest = mock(NetworkRequest::class.java) + assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais)) + } + + @Test + fun testIgnoreNonSatisfying() { + val nais = listOf(makeNai(true, 20), makeNai(true, 50), makeNai(false, 90), + makeNai(false, 60), makeNai(true, 23), makeNai(false, 68)) + val bestNetwork = nais[1] // Top score that's satisfying + val someRequest = mock(NetworkRequest::class.java) + assertEquals(bestNetwork, ranker.getBestNetwork(someRequest, nais)) + } + + @Test + fun testNoMatch() { + val nais = listOf(makeNai(false, 20), makeNai(false, 50), makeNai(false, 90)) + val someRequest = mock(NetworkRequest::class.java) + assertNull(ranker.getBestNetwork(someRequest, nais)) + } + + @Test + fun testEmpty() { + val someRequest = mock(NetworkRequest::class.java) + assertNull(ranker.getBestNetwork(someRequest, emptyList())) + } + + // Make sure the ranker is "stable" (as in stable sort), that is, it always returns the FIRST + // network satisfying the request if multiple of them have the same score. + @Test + fun testStable() { + val nais1 = listOf(makeNai(true, 30), makeNai(true, 30), makeNai(true, 30), + makeNai(true, 30), makeNai(true, 30), makeNai(true, 30)) + val someRequest = mock(NetworkRequest::class.java) + assertEquals(nais1[0], ranker.getBestNetwork(someRequest, nais1)) + + val nais2 = listOf(makeNai(true, 30), makeNai(true, 50), makeNai(true, 20), + makeNai(true, 50), makeNai(true, 50), makeNai(true, 40)) + assertEquals(nais2[1], ranker.getBestNetwork(someRequest, nais2)) + } +} From b10ab413503e47a56e6373f1e2d420eb57f14316 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 11 Dec 2019 14:12:30 +0900 Subject: [PATCH 16/17] [NS B10] Cleanup : remove mRematchedNetworks This is better computed by the code that applies the change than by the code that computes the reassignment Test: FrameworksNetTests Change-Id: I13e2764fd9b29145499085c3bb56de88a97d6c3c --- .../android/server/ConnectivityService.java | 68 ++++++------------- 1 file changed, 21 insertions(+), 47 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index a06cc95bc8..175c33b662 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6512,23 +6512,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } // An accumulator class to gather the list of changes that result from a rematch. - // TODO : enrich to represent an entire set of changes to apply. private static class NetworkReassignment { - static class NetworkBgStatePair { - @NonNull final NetworkAgentInfo mNetwork; - final boolean mOldBackground; - NetworkBgStatePair(@NonNull final NetworkAgentInfo network, - final boolean oldBackground) { - mNetwork = network; - mOldBackground = oldBackground; - } - - public String toString() { - return "[" + NetworkAgentInfo.toShortString(mNetwork) - + " oldBg=" + mOldBackground + "]"; - } - } - static class RequestReassignment { @NonNull public final NetworkRequestInfo mRequest; @Nullable public final NetworkAgentInfo mOldNetwork; @@ -6548,13 +6532,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - @NonNull private final Set mRematchedNetworks = new ArraySet<>(); @NonNull private final ArrayList mReassignments = new ArrayList<>(); - @NonNull Iterable getRematchedNetworks() { - return mRematchedNetworks; - } - @NonNull Iterable getRequestReassignments() { return mReassignments; } @@ -6575,10 +6554,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mReassignments.add(reassignment); } - void addRematchedNetwork(@NonNull final NetworkBgStatePair network) { - mRematchedNetworks.add(network); - } - // Will return null if this reassignment does not change the network assigned to // the passed request. @Nullable @@ -6592,9 +6567,7 @@ public class ConnectivityService extends IConnectivityManager.Stub public String toString() { final StringJoiner sj = new StringJoiner(", " /* delimiter */, "NetReassign [" /* prefix */, "]" /* suffix */); - if (mRematchedNetworks.isEmpty() && mReassignments.isEmpty()) { - return sj.add("no changes").toString(); - } + if (mReassignments.isEmpty()) return sj.add("no changes").toString(); for (final RequestReassignment rr : getRequestReassignments()) { sj.add(rr.toString()); } @@ -6604,15 +6577,7 @@ public class ConnectivityService extends IConnectivityManager.Stub public String debugString() { final StringBuilder sb = new StringBuilder(); sb.append("NetworkReassignment :"); - if (mRematchedNetworks.isEmpty() && mReassignments.isEmpty()) { - return sb.append(" no changes").toString(); - } - final StringJoiner sj = new StringJoiner(", " /* delimiter */, - "\n Rematched networks : " /* prefix */, "" /* suffix */); - for (final NetworkBgStatePair rr : mRematchedNetworks) { - sj.add(rr.mNetwork.toShortString()); - } - sb.append(sj.toString()); + if (mReassignments.isEmpty()) return sb.append(" no changes").toString(); for (final RequestReassignment rr : getRequestReassignments()) { sb.append("\n ").append(rr); } @@ -6660,8 +6625,6 @@ public class ConnectivityService extends IConnectivityManager.Stub for (final NetworkAgentInfo nai : mNetworkAgentInfos.values()) { if (!nai.everConnected) continue; nais.add(nai); - changes.addRematchedNetwork(new NetworkReassignment.NetworkBgStatePair(nai, - nai.isBackgroundNetwork())); } for (final NetworkRequestInfo nri : mNetworkRequests.values()) { @@ -6694,6 +6657,15 @@ public class ConnectivityService extends IConnectivityManager.Stub private void applyNetworkReassignment(@NonNull final NetworkReassignment changes, final long now) { + final Collection nais = mNetworkAgentInfos.values(); + + // Since most of the time there are only 0 or 1 background networks, it would probably + // be more efficient to just use an ArrayList here. TODO : measure performance + final ArraySet oldBgNetworks = new ArraySet<>(); + for (final NetworkAgentInfo nai : nais) { + if (nai.isBackgroundNetwork()) oldBgNetworks.add(nai); + } + // First, update the lists of satisfied requests in the network agents. This is necessary // because some code later depends on this state to be correct, most prominently computing // the linger status. @@ -6742,8 +6714,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - final Collection nais = mNetworkAgentInfos.values(); - // Update the linger state before processing listen callbacks, because the background // computation depends on whether the network is lingering. Don't send the LOSING callbacks // just yet though, because they have to be sent after the listens are processed to keep @@ -6760,15 +6730,17 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - for (final NetworkReassignment.NetworkBgStatePair event : changes.getRematchedNetworks()) { + for (final NetworkAgentInfo nai : nais) { + if (!nai.everConnected) continue; + final boolean oldBackground = oldBgNetworks.contains(nai); // Process listen requests and update capabilities if the background state has // changed for this network. For consistency with previous behavior, send onLost // callbacks before onAvailable. - processNewlyLostListenRequests(event.mNetwork); - if (event.mOldBackground != event.mNetwork.isBackgroundNetwork()) { - applyBackgroundChangeForRematch(event.mNetwork); + processNewlyLostListenRequests(nai); + if (oldBackground != nai.isBackgroundNetwork()) { + applyBackgroundChangeForRematch(nai); } - processNewlySatisfiedListenRequests(event.mNetwork); + processNewlySatisfiedListenRequests(nai); } for (final NetworkAgentInfo nai : lingeredNetworks) { @@ -6859,7 +6831,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // they may get old info. Reverse this after the old startUsing api is removed. // This is on top of the multiple intent sequencing referenced in the todo above. for (NetworkAgentInfo nai : nais) { - addNetworkToLegacyTypeTracker(nai); + if (nai.everConnected) { + addNetworkToLegacyTypeTracker(nai); + } } } From d85de75bfa30eae5a95bec81741e87b61e58e55a Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Wed, 12 Feb 2020 23:31:16 +0000 Subject: [PATCH 17/17] Revert "NetworkRequest: Embed requestor uid & packageName" This reverts commit d499517306750bfaacdeae5d15f490cbc8d98d97. Reason for revert: b/149425896 Bug: b/149425896 Change-Id: I9fe31509c040cc421ccb00ea60f15e6f199e8bc6 --- .../java/android/net/ConnectivityManager.java | 14 +- .../android/net/IConnectivityManager.aidl | 9 +- .../java/android/net/NetworkCapabilities.java | 161 +----------------- core/java/android/net/NetworkRequest.java | 26 --- .../android/server/ConnectivityService.java | 52 +++--- .../android/net/NetworkCapabilitiesTest.java | 16 +- .../android/net/ConnectivityManagerTest.java | 12 +- .../server/ConnectivityServiceTest.java | 31 +++- 8 files changed, 65 insertions(+), 256 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index f24de88a3f..94eda01410 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3747,7 +3747,6 @@ public class ConnectivityManager { checkCallbackNotNull(callback); Preconditions.checkArgument(action == REQUEST || need != null, "null NetworkCapabilities"); final NetworkRequest request; - final String callingPackageName = mContext.getOpPackageName(); try { synchronized(sCallbacks) { if (callback.networkRequest != null @@ -3759,11 +3758,10 @@ public class ConnectivityManager { Messenger messenger = new Messenger(handler); Binder binder = new Binder(); if (action == LISTEN) { - request = mService.listenForNetwork( - need, messenger, binder, callingPackageName); + request = mService.listenForNetwork(need, messenger, binder); } else { request = mService.requestNetwork( - need, messenger, timeoutMs, binder, legacyType, callingPackageName); + need, messenger, timeoutMs, binder, legacyType); } if (request != null) { sCallbacks.put(request, callback); @@ -4036,10 +4034,8 @@ public class ConnectivityManager { @NonNull PendingIntent operation) { printStackTrace(); checkPendingIntentNotNull(operation); - final String callingPackageName = mContext.getOpPackageName(); try { - mService.pendingRequestForNetwork( - request.networkCapabilities, operation, callingPackageName); + mService.pendingRequestForNetwork(request.networkCapabilities, operation); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (ServiceSpecificException e) { @@ -4151,10 +4147,8 @@ public class ConnectivityManager { @NonNull PendingIntent operation) { printStackTrace(); checkPendingIntentNotNull(operation); - final String callingPackageName = mContext.getOpPackageName(); try { - mService.pendingListenForNetwork( - request.networkCapabilities, operation, callingPackageName); + mService.pendingListenForNetwork(request.networkCapabilities, operation); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (ServiceSpecificException e) { diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 3a55461a77..c871c456dc 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -166,19 +166,18 @@ interface IConnectivityManager in int factorySerialNumber); NetworkRequest requestNetwork(in NetworkCapabilities networkCapabilities, - in Messenger messenger, int timeoutSec, in IBinder binder, int legacy, - String callingPackageName); + in Messenger messenger, int timeoutSec, in IBinder binder, int legacy); NetworkRequest pendingRequestForNetwork(in NetworkCapabilities networkCapabilities, - in PendingIntent operation, String callingPackageName); + in PendingIntent operation); void releasePendingNetworkRequest(in PendingIntent operation); NetworkRequest listenForNetwork(in NetworkCapabilities networkCapabilities, - in Messenger messenger, in IBinder binder, String callingPackageName); + in Messenger messenger, in IBinder binder); void pendingListenForNetwork(in NetworkCapabilities networkCapabilities, - in PendingIntent operation, String callingPackageName); + in PendingIntent operation); void releaseNetworkRequest(in NetworkRequest networkRequest); diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index ef4a9e5f3b..38f7390abf 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -27,7 +27,6 @@ import android.os.Build; import android.os.Parcel; import android.os.Parcelable; import android.os.Process; -import android.text.TextUtils; import android.util.ArraySet; import android.util.proto.ProtoOutputStream; @@ -64,16 +63,6 @@ public final class NetworkCapabilities implements Parcelable { // Set to true when private DNS is broken. private boolean mPrivateDnsBroken; - /** - * Uid of the app making the request. - */ - private int mRequestorUid; - - /** - * Package name of the app making the request. - */ - private String mRequestorPackageName; - public NetworkCapabilities() { clearAll(); mNetworkCapabilities = DEFAULT_CAPABILITIES; @@ -100,8 +89,6 @@ public final class NetworkCapabilities implements Parcelable { mOwnerUid = Process.INVALID_UID; mSSID = null; mPrivateDnsBroken = false; - mRequestorUid = Process.INVALID_UID; - mRequestorPackageName = null; } /** @@ -122,8 +109,6 @@ public final class NetworkCapabilities implements Parcelable { mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; mSSID = nc.mSSID; mPrivateDnsBroken = nc.mPrivateDnsBroken; - mRequestorUid = nc.mRequestorUid; - mRequestorPackageName = nc.mRequestorPackageName; } /** @@ -825,7 +810,7 @@ public final class NetworkCapabilities implements Parcelable { } /** - * UID of the app that owns this network, or Process#INVALID_UID if none/unknown. + * UID of the app that owns this network, or INVALID_UID if none/unknown. * *

This field keeps track of the UID of the app that created this network and is in charge of * its lifecycle. This could be the UID of apps such as the Wifi network suggestor, the running @@ -836,9 +821,8 @@ public final class NetworkCapabilities implements Parcelable { /** * Set the UID of the owner app. */ - public @NonNull NetworkCapabilities setOwnerUid(final int uid) { + public void setOwnerUid(final int uid) { mOwnerUid = uid; - return this; } /** @@ -881,11 +865,9 @@ public final class NetworkCapabilities implements Parcelable { * @hide */ @SystemApi - public @NonNull NetworkCapabilities setAdministratorUids( - @NonNull final List administratorUids) { + public void setAdministratorUids(@NonNull final List administratorUids) { mAdministratorUids.clear(); mAdministratorUids.addAll(administratorUids); - return this; } /** @@ -1403,7 +1385,6 @@ public final class NetworkCapabilities implements Parcelable { combineSignalStrength(nc); combineUids(nc); combineSSIDs(nc); - combineRequestor(nc); } /** @@ -1423,8 +1404,7 @@ public final class NetworkCapabilities implements Parcelable { && satisfiedBySpecifier(nc) && (onlyImmutable || satisfiedBySignalStrength(nc)) && (onlyImmutable || satisfiedByUids(nc)) - && (onlyImmutable || satisfiedBySSID(nc))) - && (onlyImmutable || satisfiedByRequestor(nc)); + && (onlyImmutable || satisfiedBySSID(nc))); } /** @@ -1508,7 +1488,7 @@ public final class NetworkCapabilities implements Parcelable { public boolean equals(@Nullable Object obj) { if (obj == null || (obj instanceof NetworkCapabilities == false)) return false; NetworkCapabilities that = (NetworkCapabilities) obj; - return equalsNetCapabilities(that) + return (equalsNetCapabilities(that) && equalsTransportTypes(that) && equalsLinkBandwidths(that) && equalsSignalStrength(that) @@ -1516,8 +1496,7 @@ public final class NetworkCapabilities implements Parcelable { && equalsTransportInfo(that) && equalsUids(that) && equalsSSID(that) - && equalsPrivateDnsBroken(that) - && equalsRequestor(that); + && equalsPrivateDnsBroken(that)); } @Override @@ -1535,9 +1514,7 @@ public final class NetworkCapabilities implements Parcelable { + Objects.hashCode(mUids) * 31 + Objects.hashCode(mSSID) * 37 + Objects.hashCode(mTransportInfo) * 41 - + Objects.hashCode(mPrivateDnsBroken) * 43 - + Objects.hashCode(mRequestorUid) * 47 - + Objects.hashCode(mRequestorPackageName) * 53; + + Objects.hashCode(mPrivateDnsBroken) * 43; } @Override @@ -1560,8 +1537,6 @@ public final class NetworkCapabilities implements Parcelable { dest.writeBoolean(mPrivateDnsBroken); dest.writeList(mAdministratorUids); dest.writeInt(mOwnerUid); - dest.writeInt(mRequestorUid); - dest.writeString(mRequestorPackageName); } public static final @android.annotation.NonNull Creator CREATOR = @@ -1584,8 +1559,6 @@ public final class NetworkCapabilities implements Parcelable { netCap.mPrivateDnsBroken = in.readBoolean(); netCap.setAdministratorUids(in.readArrayList(null)); netCap.mOwnerUid = in.readInt(); - netCap.mRequestorUid = in.readInt(); - netCap.mRequestorPackageName = in.readString(); return netCap; } @Override @@ -1651,9 +1624,6 @@ public final class NetworkCapabilities implements Parcelable { sb.append(" Private DNS is broken"); } - sb.append(" RequestorUid: ").append(mRequestorUid); - sb.append(" RequestorPackageName: ").append(mRequestorPackageName); - sb.append("]"); return sb.toString(); } @@ -1662,7 +1632,6 @@ public final class NetworkCapabilities implements Parcelable { private interface NameOf { String nameOf(int value); } - /** * @hide */ @@ -1830,120 +1799,4 @@ public final class NetworkCapabilities implements Parcelable { private boolean equalsPrivateDnsBroken(NetworkCapabilities nc) { return mPrivateDnsBroken == nc.mPrivateDnsBroken; } - - /** - * Set the uid of the app making the request. - * - * Note: This works only for {@link NetworkAgent} instances. Any capabilities passed in - * via the public {@link ConnectivityManager} API's will have this field overwritten. - * - * @param uid UID of the app. - * @hide - */ - @SystemApi - public @NonNull NetworkCapabilities setRequestorUid(int uid) { - mRequestorUid = uid; - return this; - } - - /** - * @return the uid of the app making the request. - * - * Note: This could return {@link Process#INVALID_UID} if the {@link NetworkRequest} - * object was not obtained from {@link ConnectivityManager}. - * @hide - */ - public int getRequestorUid() { - return mRequestorUid; - } - - /** - * Set the package name of the app making the request. - * - * Note: This works only for {@link NetworkAgent} instances. Any capabilities passed in - * via the public {@link ConnectivityManager} API's will have this field overwritten. - * - * @param packageName package name of the app. - * @hide - */ - @SystemApi - public @NonNull NetworkCapabilities setRequestorPackageName(@NonNull String packageName) { - mRequestorPackageName = packageName; - return this; - } - - /** - * @return the package name of the app making the request. - * - * Note: This could return {@code null} if the {@link NetworkRequest} object was not obtained - * from {@link ConnectivityManager}. - * @hide - */ - @Nullable - public String getRequestorPackageName() { - return mRequestorPackageName; - } - - /** - * Set the uid and package name of the app making the request. - * - * Note: This is intended to be only invoked from within connectivitiy service. - * - * @param uid UID of the app. - * @param packageName package name of the app. - * @hide - */ - public @NonNull NetworkCapabilities setRequestorUidAndPackageName( - int uid, @NonNull String packageName) { - return setRequestorUid(uid).setRequestorPackageName(packageName); - } - - /** - * Test whether the passed NetworkCapabilities satisfies the requestor restrictions of this - * capabilities. - * - * This method is called on the NetworkCapabilities embedded in a request with the - * capabilities of an available network. If the available network, sets a specific - * requestor (by uid and optionally package name), then this will only match a request from the - * same app. If either of the capabilities have an unset uid or package name, then it matches - * everything. - *

- * nc is assumed nonnull. Else, NPE. - */ - private boolean satisfiedByRequestor(NetworkCapabilities nc) { - // No uid set, matches everything. - if (mRequestorUid == Process.INVALID_UID || nc.mRequestorUid == Process.INVALID_UID) { - return true; - } - // uids don't match. - if (mRequestorUid != nc.mRequestorUid) return false; - // No package names set, matches everything - if (null == nc.mRequestorPackageName || null == mRequestorPackageName) return true; - // check for package name match. - return TextUtils.equals(mRequestorPackageName, nc.mRequestorPackageName); - } - - /** - * Combine requestor info of the capabilities. - *

- * This is only legal if either the requestor info of this object is reset, or both info are - * equal. - * nc is assumed nonnull. - */ - private void combineRequestor(@NonNull NetworkCapabilities nc) { - if (mRequestorUid != Process.INVALID_UID && mRequestorUid != nc.mOwnerUid) { - throw new IllegalStateException("Can't combine two uids"); - } - if (mRequestorPackageName != null - && !mRequestorPackageName.equals(nc.mRequestorPackageName)) { - throw new IllegalStateException("Can't combine two package names"); - } - setRequestorUid(nc.mRequestorUid); - setRequestorPackageName(nc.mRequestorPackageName); - } - - private boolean equalsRequestor(NetworkCapabilities nc) { - return mRequestorUid == nc.mRequestorUid - && TextUtils.equals(mRequestorPackageName, nc.mRequestorPackageName); - } } diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index b0bf64ecec..ee4379a85b 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -380,7 +380,6 @@ public class NetworkRequest implements Parcelable { dest.writeInt(requestId); dest.writeString(type.name()); } - public static final @android.annotation.NonNull Creator CREATOR = new Creator() { public NetworkRequest createFromParcel(Parcel in) { @@ -495,31 +494,6 @@ public class NetworkRequest implements Parcelable { return networkCapabilities.getNetworkSpecifier(); } - /** - * @return the uid of the app making the request. - * - * Note: This could return {@link Process#INVALID_UID} if the {@link NetworkRequest} object was - * not obtained from {@link ConnectivityManager}. - * @hide - */ - @SystemApi - public int getRequestorUid() { - return networkCapabilities.getRequestorUid(); - } - - /** - * @return the package name of the app making the request. - * - * Note: This could return {@code null} if the {@link NetworkRequest} object was not obtained - * from {@link ConnectivityManager}. - * @hide - */ - @SystemApi - @Nullable - public String getRequestorPackageName() { - return networkCapabilities.getRequestorPackageName(); - } - public String toString() { return "NetworkRequest [ " + type + " id=" + requestId + (legacyType != ConnectivityManager.TYPE_NONE ? ", legacyType=" + legacyType : "") + diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 61ef76e559..d304152192 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -604,7 +604,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set mWolSupportedInterfaces; - private final TelephonyManager mTelephonyManager; + private TelephonyManager mTelephonyManager; private final AppOpsManager mAppOpsManager; private final LocationPermissionChecker mLocationPermissionChecker; @@ -1171,7 +1171,6 @@ public class ConnectivityService extends IConnectivityManager.Stub int transportType, NetworkRequest.Type type) { final NetworkCapabilities netCap = new NetworkCapabilities(); netCap.addCapability(NET_CAPABILITY_INTERNET); - netCap.setRequestorUidAndPackageName(Process.myUid(), mContext.getPackageName()); if (transportType > -1) { netCap.addTransportType(transportType); } @@ -1702,12 +1701,10 @@ public class ConnectivityService extends IConnectivityManager.Stub return newLp; } - private void restrictRequestUidsForCallerAndSetRequestorInfo(NetworkCapabilities nc, - int callerUid, String callerPackageName) { + private void restrictRequestUidsForCaller(NetworkCapabilities nc) { if (!checkSettingsPermission()) { - nc.setSingleUid(callerUid); + nc.setSingleUid(Binder.getCallingUid()); } - nc.setRequestorUidAndPackageName(callerUid, callerPackageName); nc.setAdministratorUids(Collections.EMPTY_LIST); // Clear owner UID; this can never come from an app. @@ -5300,7 +5297,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // This checks that the passed capabilities either do not request a // specific SSID/SignalStrength, or the calling app has permission to do so. private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, - int callerPid, int callerUid, String callerPackageName) { + int callerPid, int callerUid) { if (null != nc.getSSID() && !checkSettingsPermission(callerPid, callerUid)) { throw new SecurityException("Insufficient permissions to request a specific SSID"); } @@ -5310,7 +5307,6 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new SecurityException( "Insufficient permissions to request a specific signal strength"); } - mAppOpsManager.checkPackage(callerUid, callerPackageName); } private ArrayList getSignalStrengthThresholds(NetworkAgentInfo nai) { @@ -5357,6 +5353,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(ns); + ns.assertValidFromUid(Binder.getCallingUid()); } private void ensureValid(NetworkCapabilities nc) { @@ -5368,9 +5365,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest requestNetwork(NetworkCapabilities networkCapabilities, - Messenger messenger, int timeoutMs, IBinder binder, int legacyType, - @NonNull String callingPackageName) { - final int callingUid = Binder.getCallingUid(); + Messenger messenger, int timeoutMs, IBinder binder, int legacyType) { final NetworkRequest.Type type = (networkCapabilities == null) ? NetworkRequest.Type.TRACK_DEFAULT : NetworkRequest.Type.REQUEST; @@ -5378,7 +5373,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // the default network request. This allows callers to keep track of // the system default network. if (type == NetworkRequest.Type.TRACK_DEFAULT) { - networkCapabilities = createDefaultNetworkCapabilitiesForUid(callingUid); + networkCapabilities = createDefaultNetworkCapabilitiesForUid(Binder.getCallingUid()); enforceAccessPermission(); } else { networkCapabilities = new NetworkCapabilities(networkCapabilities); @@ -5390,14 +5385,13 @@ public class ConnectivityService extends IConnectivityManager.Stub } ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), callingUid, callingPackageName); + Binder.getCallingPid(), Binder.getCallingUid()); // Set the UID range for this request to the single UID of the requester, or to an empty // set of UIDs if the caller has the appropriate permission and UIDs have not been set. // This will overwrite any allowed UIDs in the requested capabilities. Though there // are no visible methods to set the UIDs, an app could use reflection to try and get // networks for other apps so it's essential that the UIDs are overwritten. - restrictRequestUidsForCallerAndSetRequestorInfo(networkCapabilities, - callingUid, callingPackageName); + restrictRequestUidsForCaller(networkCapabilities); if (timeoutMs < 0) { throw new IllegalArgumentException("Bad timeout specified"); @@ -5472,18 +5466,16 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest pendingRequestForNetwork(NetworkCapabilities networkCapabilities, - PendingIntent operation, @NonNull String callingPackageName) { + PendingIntent operation) { checkNotNull(operation, "PendingIntent cannot be null."); - final int callingUid = Binder.getCallingUid(); networkCapabilities = new NetworkCapabilities(networkCapabilities); enforceNetworkRequestPermissions(networkCapabilities); enforceMeteredApnPolicy(networkCapabilities); ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), callingUid, callingPackageName); + Binder.getCallingPid(), Binder.getCallingUid()); ensureValidNetworkSpecifier(networkCapabilities); - restrictRequestUidsForCallerAndSetRequestorInfo(networkCapabilities, - callingUid, callingPackageName); + restrictRequestUidsForCaller(networkCapabilities); NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.REQUEST); @@ -5531,16 +5523,15 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest listenForNetwork(NetworkCapabilities networkCapabilities, - Messenger messenger, IBinder binder, @NonNull String callingPackageName) { - final int callingUid = Binder.getCallingUid(); + Messenger messenger, IBinder binder) { if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), callingUid, callingPackageName); - restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); + Binder.getCallingPid(), Binder.getCallingUid()); + restrictRequestUidsForCaller(nc); // Apps without the CHANGE_NETWORK_STATE permission can't use background networks, so // make all their listens include NET_CAPABILITY_FOREGROUND. That way, they will get // onLost and onAvailable callbacks when networks move in and out of the background. @@ -5560,17 +5551,17 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void pendingListenForNetwork(NetworkCapabilities networkCapabilities, - PendingIntent operation, @NonNull String callingPackageName) { + PendingIntent operation) { checkNotNull(operation, "PendingIntent cannot be null."); - final int callingUid = Binder.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } ensureValid(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), callingUid, callingPackageName); + Binder.getCallingPid(), Binder.getCallingUid()); + final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); - restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); + restrictRequestUidsForCaller(nc); NetworkRequest networkRequest = new NetworkRequest(nc, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.LISTEN); @@ -7877,13 +7868,12 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new IllegalArgumentException("ConnectivityManager.TYPE_* are deprecated." + " Please use NetworkCapabilities instead."); } - final int callingUid = Binder.getCallingUid(); - mAppOpsManager.checkPackage(callingUid, callingPackageName); + 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. final NetworkCapabilities nc = new NetworkCapabilities(request.networkCapabilities); - restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); + restrictRequestUidsForCaller(nc); final NetworkRequest requestWithId = new NetworkRequest( diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index efea91ab91..3e4f3d8188 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -272,23 +272,9 @@ public class NetworkCapabilitiesTest { netCap.setOwnerUid(123); assertParcelingIsLossless(netCap); netCap.setSSID(TEST_SSID); - assertParcelSane(netCap, 15); + assertParcelSane(netCap, 13); } - @Test - public void testParcelNetworkCapabilitiesWithRequestorUidAndPackageName() { - final NetworkCapabilities netCap = new NetworkCapabilities() - .addCapability(NET_CAPABILITY_INTERNET) - .setRequestorUid(9304) - .setRequestorPackageName("com.android.test") - .addCapability(NET_CAPABILITY_EIMS) - .addCapability(NET_CAPABILITY_NOT_METERED); - assertParcelingIsLossless(netCap); - netCap.setSSID(TEST_SSID); - assertParcelSane(netCap, 15); - } - - @Test public void testOemPaid() { NetworkCapabilities nc = new NetworkCapabilities(); diff --git a/tests/net/java/android/net/ConnectivityManagerTest.java b/tests/net/java/android/net/ConnectivityManagerTest.java index d6bf334ee5..7ede14428a 100644 --- a/tests/net/java/android/net/ConnectivityManagerTest.java +++ b/tests/net/java/android/net/ConnectivityManagerTest.java @@ -212,8 +212,7 @@ public class ConnectivityManagerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Messenger.class); // register callback - when(mService.requestNetwork( - any(), captor.capture(), anyInt(), any(), anyInt(), any())) + when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) .thenReturn(request); manager.requestNetwork(request, callback, handler); @@ -241,8 +240,7 @@ public class ConnectivityManagerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Messenger.class); // register callback - when(mService.requestNetwork( - any(), captor.capture(), anyInt(), any(), anyInt(), any())) + when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) .thenReturn(req1); manager.requestNetwork(req1, callback, handler); @@ -260,8 +258,7 @@ public class ConnectivityManagerTest { verify(callback, timeout(100).times(0)).onLosing(any(), anyInt()); // callback can be registered again - when(mService.requestNetwork( - any(), captor.capture(), anyInt(), any(), anyInt(), any())) + when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) .thenReturn(req2); manager.requestNetwork(req2, callback, handler); @@ -285,8 +282,7 @@ public class ConnectivityManagerTest { info.targetSdkVersion = VERSION_CODES.N_MR1 + 1; when(mCtx.getApplicationInfo()).thenReturn(info); - when(mService.requestNetwork(any(), any(), anyInt(), any(), anyInt(), any())) - .thenReturn(request); + when(mService.requestNetwork(any(), any(), anyInt(), any(), anyInt())).thenReturn(request); Handler handler = new Handler(Looper.getMainLooper()); manager.requestNetwork(request, callback, handler); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index f40e57fe46..8da1a5b655 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -107,7 +107,6 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -306,7 +305,6 @@ public class ConnectivityServiceTest { private static final String MOBILE_IFNAME = "test_rmnet_data0"; private static final String WIFI_IFNAME = "test_wlan0"; private static final String WIFI_WOL_IFNAME = "test_wlan_wol"; - private static final String TEST_PACKAGE_NAME = "com.android.test.package"; private static final String[] EMPTY_STRING_ARRAY = new String[0]; private MockContext mServiceContext; @@ -656,7 +654,7 @@ public class ConnectivityServiceTest { if (mNmValidationRedirectUrl != null) { mNmCallbacks.showProvisioningNotification( - "test_provisioning_notif_action", TEST_PACKAGE_NAME); + "test_provisioning_notif_action", "com.android.test.package"); mNmProvNotificationRequested = true; } } @@ -2974,7 +2972,7 @@ public class ConnectivityServiceTest { networkCapabilities.addTransportType(TRANSPORT_WIFI) .setNetworkSpecifier(new MatchAllNetworkSpecifier()); mService.requestNetwork(networkCapabilities, null, 0, null, - ConnectivityManager.TYPE_WIFI, TEST_PACKAGE_NAME); + ConnectivityManager.TYPE_WIFI); }); class NonParcelableSpecifier extends NetworkSpecifier { @@ -3013,12 +3011,31 @@ public class ConnectivityServiceTest { } @Test - public void testNetworkRequestUidSpoofSecurityException() throws Exception { + public void testNetworkSpecifierUidSpoofSecurityException() throws Exception { + class UidAwareNetworkSpecifier extends NetworkSpecifier implements Parcelable { + @Override + public boolean satisfiedBy(NetworkSpecifier other) { + return true; + } + + @Override + public void assertValidFromUid(int requestorUid) { + throw new SecurityException("failure"); + } + + @Override + public int describeContents() { return 0; } + @Override + public void writeToParcel(Parcel dest, int flags) {} + } + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(false); - NetworkRequest networkRequest = newWifiRequestBuilder().build(); + + UidAwareNetworkSpecifier networkSpecifier = new UidAwareNetworkSpecifier(); + NetworkRequest networkRequest = newWifiRequestBuilder().setNetworkSpecifier( + networkSpecifier).build(); TestNetworkCallback networkCallback = new TestNetworkCallback(); - doThrow(new SecurityException()).when(mAppOpsManager).checkPackage(anyInt(), anyString()); assertThrows(SecurityException.class, () -> { mCm.requestNetwork(networkRequest, networkCallback); });