From 24e3fe60a559308f9918f727fc58e8529ecb3f33 Mon Sep 17 00:00:00 2001 From: Eric Enslen Date: Wed, 7 Feb 2018 18:03:53 -0800 Subject: [PATCH 01/20] fix isActiveNetworkMetered with VPNs Clean cherry-pick of ag/3580901 Bug: 72871435 Test: flashed and verified, also ran runtest framework-net Merged-In: I177eff1237dd59514ccf91397a3d307148bc37b1 Change-Id: Ic5919a32f91f7baee5f1370703ad166e6ea52b58 --- services/core/java/com/android/server/ConnectivityService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 9838de1474..f2a25eb022 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1415,7 +1415,8 @@ public class ConnectivityService extends IConnectivityManager.Stub public boolean isActiveNetworkMetered() { enforceAccessPermission(); - final NetworkCapabilities caps = getNetworkCapabilities(getActiveNetwork()); + final int uid = Binder.getCallingUid(); + final NetworkCapabilities caps = getUnfilteredActiveNetworkState(uid).networkCapabilities; if (caps != null) { return !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED); } else { From e90c0e624f0f0481ed537a1276d78fafbef2b7ba Mon Sep 17 00:00:00 2001 From: Chad Brubaker Date: Thu, 8 Mar 2018 10:37:09 -0800 Subject: [PATCH 02/20] Expose CONNECTIVITY_ACTION to Instant Apps Clean cherry-pick of ag/3710436 Test: Broadcast is visible to Instant Apps Bug: 69421898 Change-Id: Ibac92b5aa16bf1538776b90df5dc05362667e785 Merged-In: I0434f8c7292a85e25df3da2858e4d89cf55fab3d Merged-In: I62ed2107d0b0712267a903e465cbeba6a4f0b346 --- services/core/java/com/android/server/ConnectivityService.java | 1 + 1 file changed, 1 insertion(+) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f2a25eb022..b7e8503106 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1738,6 +1738,7 @@ public class ConnectivityService extends IConnectivityManager.Stub ni != null ? ni.getState().toString() : "?"); } catch (RemoteException e) { } + intent.addFlags(Intent.FLAG_RECEIVER_VISIBLE_TO_INSTANT_APPS); } try { mContext.sendStickyBroadcastAsUser(intent, UserHandle.ALL, options); From 04f08d4daeb5a9429ef9ee05f24b0b76cef0efdf Mon Sep 17 00:00:00 2001 From: mswest46 Date: Mon, 12 Mar 2018 10:34:34 -0700 Subject: [PATCH 03/20] add airplane mode shell commands to connectivity Clean cherry-pick of ag/3718273 Change-Id: I302802afc952b9df7a5544b12d9015091997bd67 Fixes: 74410990 Test: Manually checked that commands enable/disable airplane mode. Merged-In: I8787d642594e6852bff5b902e8d0fa380ce7c37f --- .../android/server/ConnectivityService.java | 61 ++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b7e8503106..34d9a8bf49 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -102,6 +102,8 @@ import android.os.RemoteException; import android.os.ResultReceiver; import android.os.ServiceManager; import android.os.ServiceSpecificException; +import android.os.ShellCallback; +import android.os.ShellCommand; import android.os.SystemClock; import android.os.UserHandle; import android.os.UserManager; @@ -5814,4 +5816,61 @@ public class ConnectivityService extends IConnectivityManager.Stub private static int encodeBool(boolean b) { return b ? 1 : 0; } -} + + @Override + public void onShellCommand(FileDescriptor in, FileDescriptor out, + FileDescriptor err, String[] args, ShellCallback callback, + ResultReceiver resultReceiver) { + (new ShellCmd()).exec(this, in, out, err, args, callback, resultReceiver); + } + + private class ShellCmd extends ShellCommand { + + @Override + public int onCommand(String cmd) { + if (cmd == null) { + return handleDefaultCommands(cmd); + } + final PrintWriter pw = getOutPrintWriter(); + try { + switch (cmd) { + case "airplane-mode": + final String action = getNextArg(); + if ("enable".equals(action)) { + setAirplaneMode(true); + return 0; + } else if ("disable".equals(action)) { + setAirplaneMode(false); + return 0; + } else if (action == null) { + final ContentResolver cr = mContext.getContentResolver(); + final int enabled = Settings.Global.getInt(cr, + Settings.Global.AIRPLANE_MODE_ON); + pw.println(enabled == 0 ? "disabled" : "enabled"); + return 0; + } else { + onHelp(); + return -1; + } + default: + return handleDefaultCommands(cmd); + } + } catch (Exception e) { + pw.println(e); + } + return -1; + } + + @Override + public void onHelp() { + PrintWriter pw = getOutPrintWriter(); + pw.println("Connectivity service commands:"); + pw.println(" help"); + pw.println(" Print this help text."); + pw.println(" airplane-mode [enable|disable]"); + pw.println(" Turn airplane mode on or off."); + pw.println(" airplane-mode"); + pw.println(" Get airplane mode."); + } + } +} \ No newline at end of file From e023bc215df787278eeadb3f6d000a492a33cb22 Mon Sep 17 00:00:00 2001 From: Pavel Maltsev Date: Thu, 22 Mar 2018 11:41:32 -0700 Subject: [PATCH 04/20] Add OEM_PAID capability to system api Some system apps should be able to request OEM_PAID networks. This makes a lot of sense when Android is used as in-vehicle infotainment systems. Clean cherry-pick of ag/3782591 Bug: 68762530 Test: runtest -x frameworks/base/tests/net/ -c android.net.NetworkCapabilitiesTest Change-Id: I306f060c5a386ff4b82cd99a03dc037ce60ded6a Merged-In: Ic164c4a29cd449a31b2f1c12c8c345bcc5dc77fa Merged-In: I6e9c4130db23a4f1c89ce7e9071ae519a2b0b7ec --- core/java/android/net/NetworkCapabilities.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 19f0c90e32..d93971c75c 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -17,6 +17,7 @@ package android.net; import android.annotation.IntDef; +import android.annotation.SystemApi; import android.net.ConnectivityManager.NetworkCallback; import android.os.Parcel; import android.os.Parcelable; @@ -275,6 +276,7 @@ public final class NetworkCapabilities implements Parcelable { * this network can be used by system apps to upload telemetry data. * @hide */ + @SystemApi public static final int NET_CAPABILITY_OEM_PAID = 22; private static final int MIN_NET_CAPABILITY = NET_CAPABILITY_MMS; From 20329ac8bb798c163d55c5d7167ef59ac206f888 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 28 Mar 2018 14:01:55 -0600 Subject: [PATCH 05/20] API council requested tweaks to NetworkRequest. If you put values into the Builder, you should be able to observe those values on the built object. Clean cherry-pick of ag/3813257 Test: atest android.net.cts.NetworkRequestTest Bug: 74945408 Change-Id: Ib28de279efb8b33ab46aa64f580e10fe5f8720e3 Merged-In: I0d090ebb7d57689a061badcf593ae9a37d88f7ce Merged-In: I539184f7385c1f288cfb77be8307e4463e07e9e6 --- core/java/android/net/NetworkRequest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 1ee0ed7d90..9c28ed7a04 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -17,6 +17,8 @@ package android.net; import android.annotation.NonNull; +import android.net.NetworkCapabilities.NetCapability; +import android.net.NetworkCapabilities.Transport; import android.os.Parcel; import android.os.Parcelable; import android.os.Process; @@ -426,6 +428,20 @@ public class NetworkRequest implements Parcelable { return type == Type.BACKGROUND_REQUEST; } + /** + * @see Builder#addCapability(int) + */ + public boolean hasCapability(@NetCapability int capability) { + return networkCapabilities.hasCapability(capability); + } + + /** + * @see Builder#addTransportType(int) + */ + public boolean hasTransport(@Transport int transportType) { + return networkCapabilities.hasTransport(transportType); + } + public String toString() { return "NetworkRequest [ " + type + " id=" + requestId + (legacyType != ConnectivityManager.TYPE_NONE ? ", legacyType=" + legacyType : "") + From 5b7ec405f40cc87526ead20a22239d4965c6866c Mon Sep 17 00:00:00 2001 From: Pavel Maltsev Date: Mon, 2 Apr 2018 11:10:13 -0700 Subject: [PATCH 06/20] Actually @hide unwanted capability methods. Cherry-pick of ag/3943779 Bug: 77601789 Test: builds, boots Test: make doc-comment-check-docs Merged-In: I80a88123b16c54734306da7e5dc0670972041648 Merged-In: I923e5377a1abe761217612452cbfdba752e53de2 --- core/java/android/net/NetworkRequest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 9c28ed7a04..ff39ed15a9 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -171,6 +171,8 @@ public class NetworkRequest implements Parcelable { * If the given capability was previously added to the list of unwanted capabilities * then the capability will also be removed from the list of unwanted capabilities. * + * @see #addUnwantedCapability(int) + * * @param capability The capability to add. * @return The builder to facilitate chaining * {@code builder.addCapability(...).addCapability();}. @@ -230,6 +232,7 @@ public class NetworkRequest implements Parcelable { * * @param capability The capability to add to unwanted capability list. * @return The builder to facilitate chaining. + * * @hide */ public Builder addUnwantedCapability(@NetworkCapabilities.NetCapability int capability) { @@ -435,6 +438,15 @@ public class NetworkRequest implements Parcelable { return networkCapabilities.hasCapability(capability); } + /** + * @see Builder#addUnwantedCapability(int) + * + * @hide + */ + public boolean hasUnwantedCapability(@NetCapability int capability) { + return networkCapabilities.hasUnwantedCapability(capability); + } + /** * @see Builder#addTransportType(int) */ From 4fc3638fa707b75fe3b1cf98e39bbce3a8323a0d Mon Sep 17 00:00:00 2001 From: Pavel Maltsev Date: Mon, 9 Apr 2018 13:10:11 -0700 Subject: [PATCH 07/20] Remove unwanted capability from the java-doc Per API council feedback remove unwanted capaibility from public API java docs Clean cherry-pick from ag/3868438 Bug: 77601789 Test: make docs Test: build and flash Change-Id: I4f3b8b558e8dab2bcc9ef4cc6cfc3135c264c291 Merged-In: I7fef43cce3cfe17dae6a5e4f564ad8857371502a Merged-In: Ib02988daf44dabfaef7a0b788385b7f7c655b8b2 --- core/java/android/net/NetworkRequest.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index ff39ed15a9..b0b58cd524 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -167,11 +167,6 @@ public class NetworkRequest implements Parcelable { * the requested network's required capabilities. Note that when searching * for a network to satisfy a request, all capabilities requested must be * satisfied. - *

- * If the given capability was previously added to the list of unwanted capabilities - * then the capability will also be removed from the list of unwanted capabilities. - * - * @see #addUnwantedCapability(int) * * @param capability The capability to add. * @return The builder to facilitate chaining @@ -183,8 +178,7 @@ public class NetworkRequest implements Parcelable { } /** - * Removes (if found) the given capability from this builder instance from both required - * and unwanted capabilities lists. + * Removes (if found) the given capability from this builder instance. * * @param capability The capability to remove. * @return The builder to facilitate chaining. From dd7d9f4c841b42d4143f621a3930cb126167e92b Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 12 Apr 2018 11:52:37 +0900 Subject: [PATCH 08/20] Notif manager and captive portal app to read SSID again Cherry-picked from ag/3887738 ; almost clean CP, only had to add an import. Bug: 77114259 Test: frameworks-net pass manual test shows the SSID is now displayed again Change-Id: I5cb2b4777ad78d972031e8f2ff22e2155f4ab894 Merged-In: I588fedba49ea5d08e40bd2b3ea8ba2c2383958ec Merged-In: I663a59ff2847a9f44ea1395326f6cb00e97237b6 --- .../server/connectivity/NetworkNotificationManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java index 0d935dba22..02459bde09 100644 --- a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java +++ b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java @@ -23,6 +23,7 @@ import android.content.Context; import android.content.Intent; import android.content.res.Resources; import android.net.NetworkCapabilities; +import android.net.wifi.WifiInfo; import android.os.UserHandle; import android.telephony.TelephonyManager; import android.util.Slog; @@ -176,7 +177,8 @@ public class NetworkNotificationManager { switch (transportType) { case TRANSPORT_WIFI: title = r.getString(R.string.wifi_available_sign_in, 0); - details = r.getString(R.string.network_available_sign_in_detailed, extraInfo); + details = r.getString(R.string.network_available_sign_in_detailed, + WifiInfo.removeDoubleQuotes(nai.networkCapabilities.getSSID())); break; case TRANSPORT_CELLULAR: title = r.getString(R.string.network_available_sign_in, 0); From 390bee5f2cbd13be9c110e679e0bf98ffbccd913 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 11 Apr 2018 21:09:10 +0900 Subject: [PATCH 09/20] Add a new ssid field in NetworkCapabilities. Clean cherry-pick of ag/3887737 Bug: 77891227 Test: frameworks-net Change-Id: Icefe1657bca52b913a72b56705342a7285769658 Merged-In: Ia1f4e51b7c2e9118789242cf6d9e7576c4167cda Merged-In: I6dd1aba6fde5a502b4a9145cf16393d8ce623c89 --- .../java/android/net/NetworkCapabilities.java | 78 +++++++++++++++++-- .../android/server/ConnectivityService.java | 35 +++++++-- .../android/net/NetworkCapabilitiesTest.java | 34 ++++++++ 3 files changed, 137 insertions(+), 10 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index d93971c75c..3f0f334772 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -70,6 +70,7 @@ public final class NetworkCapabilities implements Parcelable { mUids = nc.mUids; mEstablishingVpnAppUid = nc.mEstablishingVpnAppUid; mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; + mSSID = nc.mSSID; } } @@ -85,6 +86,7 @@ public final class NetworkCapabilities implements Parcelable { mSignalStrength = SIGNAL_STRENGTH_UNSPECIFIED; mUids = null; mEstablishingVpnAppUid = INVALID_UID; + mSSID = null; } /** @@ -922,7 +924,7 @@ public final class NetworkCapabilities implements Parcelable { /** * Sets the signal strength. This is a signed integer, with higher values indicating a stronger * signal. The exact units are bearer-dependent. For example, Wi-Fi uses the same RSSI units - * reported by WifiManager. + * reported by wifi code. *

* Note that when used to register a network callback, this specifies the minimum acceptable * signal strength. When received as the state of an existing network it specifies the current @@ -1054,7 +1056,7 @@ public final class NetworkCapabilities implements Parcelable { } /** - * Tests if the set of UIDs that this network applies to is the same of the passed set of UIDs. + * Tests if the set of UIDs that this network applies to is the same as the passed network. *

* This test only checks whether equal range objects are in both sets. It will * return false if the ranges are not exactly the same, even if the covered UIDs @@ -1144,6 +1146,62 @@ public final class NetworkCapabilities implements Parcelable { mUids.addAll(nc.mUids); } + + /** + * The SSID of the network, or null if not applicable or unknown. + *

+ * This is filled in by wifi code. + * @hide + */ + private String mSSID; + + /** + * Sets the SSID of this network. + * @hide + */ + public NetworkCapabilities setSSID(String ssid) { + mSSID = ssid; + return this; + } + + /** + * Gets the SSID of this network, or null if none or unknown. + * @hide + */ + public String getSSID() { + return mSSID; + } + + /** + * Tests if the SSID of this network is the same as the SSID of the passed network. + * @hide + */ + public boolean equalsSSID(NetworkCapabilities nc) { + return Objects.equals(mSSID, nc.mSSID); + } + + /** + * Check if the SSID requirements of this object are matched by the passed object. + * @hide + */ + public boolean satisfiedBySSID(NetworkCapabilities nc) { + return mSSID == null || mSSID.equals(nc.mSSID); + } + + /** + * Combine SSIDs of the capabilities. + *

+ * This is only legal if either the SSID of this object is null, or both SSIDs are + * equal. + * @hide + */ + private void combineSSIDs(NetworkCapabilities nc) { + if (mSSID != null && !mSSID.equals(nc.mSSID)) { + throw new IllegalStateException("Can't combine two SSIDs"); + } + setSSID(nc.mSSID); + } + /** * Combine a set of Capabilities to this one. Useful for coming up with the complete set. *

@@ -1159,6 +1217,7 @@ public final class NetworkCapabilities implements Parcelable { combineSpecifiers(nc); combineSignalStrength(nc); combineUids(nc); + combineSSIDs(nc); } /** @@ -1177,7 +1236,8 @@ public final class NetworkCapabilities implements Parcelable { && (onlyImmutable || satisfiedByLinkBandwidths(nc)) && satisfiedBySpecifier(nc) && (onlyImmutable || satisfiedBySignalStrength(nc)) - && (onlyImmutable || satisfiedByUids(nc))); + && (onlyImmutable || satisfiedByUids(nc)) + && (onlyImmutable || satisfiedBySSID(nc))); } /** @@ -1266,7 +1326,8 @@ public final class NetworkCapabilities implements Parcelable { && equalsLinkBandwidths(that) && equalsSignalStrength(that) && equalsSpecifier(that) - && equalsUids(that)); + && equalsUids(that) + && equalsSSID(that)); } @Override @@ -1281,7 +1342,8 @@ public final class NetworkCapabilities implements Parcelable { + (mLinkDownBandwidthKbps * 19) + Objects.hashCode(mNetworkSpecifier) * 23 + (mSignalStrength * 29) - + Objects.hashCode(mUids) * 31; + + Objects.hashCode(mUids) * 31 + + Objects.hashCode(mSSID) * 37; } @Override @@ -1298,6 +1360,7 @@ public final class NetworkCapabilities implements Parcelable { dest.writeParcelable((Parcelable) mNetworkSpecifier, flags); dest.writeInt(mSignalStrength); dest.writeArraySet(mUids); + dest.writeString(mSSID); } public static final Creator CREATOR = @@ -1315,6 +1378,7 @@ public final class NetworkCapabilities implements Parcelable { netCap.mSignalStrength = in.readInt(); netCap.mUids = (ArraySet) in.readArraySet( null /* ClassLoader, null for default */); + netCap.mSSID = in.readString(); return netCap; } @Override @@ -1365,6 +1429,10 @@ public final class NetworkCapabilities implements Parcelable { sb.append(" EstablishingAppUid: ").append(mEstablishingVpnAppUid); } + if (null != mSSID) { + sb.append(" SSID: ").append(mSSID); + } + sb.append("]"); return sb.toString(); } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 34d9a8bf49..6f91ede8a1 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1354,7 +1354,8 @@ public class ConnectivityService extends IConnectivityManager.Stub if (nai != null) { synchronized (nai) { if (nai.networkCapabilities != null) { - return networkCapabilitiesWithoutUidsUnlessAllowed(nai.networkCapabilities, + return networkCapabilitiesRestrictedForCallerPermissions( + nai.networkCapabilities, Binder.getCallingPid(), Binder.getCallingUid()); } } @@ -1368,10 +1369,12 @@ public class ConnectivityService extends IConnectivityManager.Stub return getNetworkCapabilitiesInternal(getNetworkAgentInfoForNetwork(network)); } - private NetworkCapabilities networkCapabilitiesWithoutUidsUnlessAllowed( + private NetworkCapabilities networkCapabilitiesRestrictedForCallerPermissions( NetworkCapabilities nc, int callerPid, int callerUid) { - if (checkSettingsPermission(callerPid, callerUid)) return new NetworkCapabilities(nc); - return new NetworkCapabilities(nc).setUids(null); + final NetworkCapabilities newNc = new NetworkCapabilities(nc); + if (!checkSettingsPermission(callerPid, callerUid)) newNc.setUids(null); + if (!checkNetworkStackPermission(callerPid, callerUid)) newNc.setSSID(null); + return newNc; } private void restrictRequestUidsForCaller(NetworkCapabilities nc) { @@ -1630,6 +1633,11 @@ public class ConnectivityService extends IConnectivityManager.Stub android.Manifest.permission.NETWORK_SETTINGS, pid, uid); } + private boolean checkNetworkStackPermission(int pid, int uid) { + return PERMISSION_GRANTED == mContext.checkPermission( + android.Manifest.permission.NETWORK_STACK, pid, uid); + } + private void enforceTetherAccessPermission() { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.ACCESS_NETWORK_STATE, @@ -4185,6 +4193,15 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // This checks that the passed capabilities either do not request a specific SSID, or the + // calling app has permission to do so. + private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, + int callerPid, int callerUid) { + if (null != nc.getSSID() && !checkNetworkStackPermission(callerPid, callerUid)) { + throw new SecurityException("Insufficient permissions to request a specific SSID"); + } + } + private ArrayList getSignalStrengthThresholds(NetworkAgentInfo nai) { final SortedSet thresholds = new TreeSet(); synchronized (nai) { @@ -4254,6 +4271,8 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceMeteredApnPolicy(networkCapabilities); } ensureRequestableCapabilities(networkCapabilities); + ensureSufficientPermissionsForRequest(networkCapabilities, + 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 @@ -4332,6 +4351,8 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceNetworkRequestPermissions(networkCapabilities); enforceMeteredApnPolicy(networkCapabilities); ensureRequestableCapabilities(networkCapabilities); + ensureSufficientPermissionsForRequest(networkCapabilities, + Binder.getCallingPid(), Binder.getCallingUid()); ensureValidNetworkSpecifier(networkCapabilities); restrictRequestUidsForCaller(networkCapabilities); @@ -4387,6 +4408,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); + ensureSufficientPermissionsForRequest(networkCapabilities, + 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 @@ -4413,6 +4436,8 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceAccessPermission(); } ensureValidNetworkSpecifier(networkCapabilities); + ensureSufficientPermissionsForRequest(networkCapabilities, + Binder.getCallingPid(), Binder.getCallingUid()); final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); restrictRequestUidsForCaller(nc); @@ -4984,7 +5009,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } case ConnectivityManager.CALLBACK_CAP_CHANGED: { // networkAgent can't be null as it has been accessed a few lines above. - final NetworkCapabilities nc = networkCapabilitiesWithoutUidsUnlessAllowed( + final NetworkCapabilities nc = networkCapabilitiesRestrictedForCallerPermissions( networkAgent.networkCapabilities, nri.mPid, nri.mUid); putParcelable(bundle, nc); break; diff --git a/tests/net/java/android/net/NetworkCapabilitiesTest.java b/tests/net/java/android/net/NetworkCapabilitiesTest.java index 06965925e4..c17f31e701 100644 --- a/tests/net/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/java/android/net/NetworkCapabilitiesTest.java @@ -40,12 +40,14 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import android.os.Parcel; import android.support.test.runner.AndroidJUnit4; import android.test.suitebuilder.annotation.SmallTest; import android.util.ArraySet; + import org.junit.Test; import org.junit.runner.RunWith; @@ -54,6 +56,8 @@ import java.util.Set; @RunWith(AndroidJUnit4.class) @SmallTest public class NetworkCapabilitiesTest { + private static final String TEST_SSID = "TEST_SSID"; + @Test public void testMaybeMarkCapabilitiesRestricted() { // verify EIMS is restricted @@ -268,6 +272,8 @@ public class NetworkCapabilitiesTest { .addCapability(NET_CAPABILITY_EIMS) .addCapability(NET_CAPABILITY_NOT_METERED); assertEqualsThroughMarshalling(netCap); + netCap.setSSID(TEST_SSID); + assertEqualsThroughMarshalling(netCap); } @Test @@ -361,6 +367,21 @@ public class NetworkCapabilitiesTest { assertTrue(nc1.equalsNetCapabilities(nc2)); } + @Test + public void testSSID() { + NetworkCapabilities nc1 = new NetworkCapabilities(); + NetworkCapabilities nc2 = new NetworkCapabilities(); + assertTrue(nc2.satisfiedBySSID(nc1)); + + nc1.setSSID(TEST_SSID); + assertTrue(nc2.satisfiedBySSID(nc1)); + nc2.setSSID("different " + TEST_SSID); + assertFalse(nc2.satisfiedBySSID(nc1)); + + assertTrue(nc1.satisfiedByImmutableNetworkCapabilities(nc2)); + assertFalse(nc1.satisfiedByNetworkCapabilities(nc2)); + } + @Test public void testCombineCapabilities() { NetworkCapabilities nc1 = new NetworkCapabilities(); @@ -382,6 +403,19 @@ public class NetworkCapabilitiesTest { // will never be satisfied. assertTrue(nc2.hasCapability(NET_CAPABILITY_NOT_ROAMING)); assertTrue(nc2.hasUnwantedCapability(NET_CAPABILITY_NOT_ROAMING)); + + nc1.setSSID(TEST_SSID); + nc2.combineCapabilities(nc1); + assertTrue(TEST_SSID.equals(nc2.getSSID())); + + // Because they now have the same SSID, the folllowing call should not throw + nc2.combineCapabilities(nc1); + + nc1.setSSID("different " + TEST_SSID); + try { + nc2.combineCapabilities(nc1); + fail("Expected IllegalStateException: can't combine different SSIDs"); + } catch (IllegalStateException expected) {} } @Test From 4ea84827b3c81be7f27df594c970cd03324230f5 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Mon, 16 Apr 2018 12:25:22 +0900 Subject: [PATCH 10/20] Guard the SSID with NETWORK_SETTINGS Clean cherry-pick of ag/3904260 Bug: 77865258 Test: manual Change-Id: I2a2e236041797df495759dd4e07648545cad6c7c Merged-In: Iba59e93875c28b8e30db0c013575bc2f117cb16c Merged-In: I6cf364f0815a2eaab60f5de5e1d5ccc4908e9eca --- .../java/com/android/server/ConnectivityService.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 6f91ede8a1..d9b4602bfc 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1373,7 +1373,7 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkCapabilities nc, int callerPid, int callerUid) { final NetworkCapabilities newNc = new NetworkCapabilities(nc); if (!checkSettingsPermission(callerPid, callerUid)) newNc.setUids(null); - if (!checkNetworkStackPermission(callerPid, callerUid)) newNc.setSSID(null); + if (!checkSettingsPermission(callerPid, callerUid)) newNc.setSSID(null); return newNc; } @@ -1633,11 +1633,6 @@ public class ConnectivityService extends IConnectivityManager.Stub android.Manifest.permission.NETWORK_SETTINGS, pid, uid); } - private boolean checkNetworkStackPermission(int pid, int uid) { - return PERMISSION_GRANTED == mContext.checkPermission( - android.Manifest.permission.NETWORK_STACK, pid, uid); - } - private void enforceTetherAccessPermission() { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.ACCESS_NETWORK_STATE, @@ -4197,7 +4192,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // calling app has permission to do so. private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, int callerPid, int callerUid) { - if (null != nc.getSSID() && !checkNetworkStackPermission(callerPid, callerUid)) { + if (null != nc.getSSID() && !checkSettingsPermission(callerPid, callerUid)) { throw new SecurityException("Insufficient permissions to request a specific SSID"); } } From d94e48ff95115ab64cffc15b6e159fe29e6f8e1e Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 18 Apr 2018 19:18:58 +0900 Subject: [PATCH 11/20] Add tests for getActiveNetwork. Clean cherry-pick of ag/3918295 One-line adjustment for ag/3638326 which has not been put in AOSP. Bug: 77737389 Test: runtest frameworks-net Change-Id: I03ae2bbb08559f2cd44979e291c1f5d50eb215da Merged-In: Iaa0285825735d3f16bba6e4946723a437fd9b0b9 Merged-In: Ia8f985b448251f911484e6bd63fa562bffc1b0e4 --- .../android/server/ConnectivityService.java | 3 +- .../server/ConnectivityServiceTest.java | 94 ++++++++++++++++++- 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index d9b4602bfc..7eaea8dca3 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -236,8 +236,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private KeyStore mKeyStore; + @VisibleForTesting @GuardedBy("mVpns") - private final SparseArray mVpns = new SparseArray(); + protected final SparseArray mVpns = new SparseArray(); // TODO: investigate if mLockdownEnabled can be removed and replaced everywhere by // a direct call to LockdownVpnTracker.isEnabled(). diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 53c62c2402..c96b0159a6 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -84,6 +84,7 @@ import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; +import android.content.pm.UserInfo; import android.content.res.Resources; import android.net.CaptivePortal; import android.net.ConnectivityManager; @@ -202,6 +203,7 @@ public class ConnectivityServiceTest { @Mock DefaultNetworkMetrics mDefaultNetworkMetrics; @Mock INetworkManagementService mNetworkManagementService; @Mock INetworkStatsService mStatsService; + @Mock Vpn mMockVpn; private ArgumentCaptor mStringArrayCaptor = ArgumentCaptor.forClass(String[].class); @@ -506,6 +508,7 @@ public class ConnectivityServiceTest { mWrappedNetworkMonitor.gen204ProbeResult = 204; NetworkRequest request = new NetworkRequest.Builder() .addTransportType(mNetworkCapabilities.getTransportTypes()[0]) + .clearCapabilities() .build(); callback = new NetworkCallback() { public void onCapabilitiesChanged(Network network, @@ -889,6 +892,15 @@ public class ConnectivityServiceTest { return mLastCreatedNetworkMonitor; } + public void mockVpn(int uid) { + synchronized (mVpns) { + // This has no effect unless the VPN is actually connected, because things like + // getActiveNetworkForUidInternal call getNetworkAgentInfoForNetId on the VPN + // netId, and check if that network is actually connected. + mVpns.put(UserHandle.getUserId(Process.myUid()), mMockVpn); + } + } + public void waitForIdle(int timeoutMs) { waitForIdleHandler(mHandlerThread, timeoutMs); } @@ -915,6 +927,7 @@ public class ConnectivityServiceTest { MockitoAnnotations.initMocks(this); when(mMetricsService.defaultNetworkMetrics()).thenReturn(mDefaultNetworkMetrics); + when(mMockVpn.appliesToUid(Process.myUid())).thenReturn(true); // InstrumentationTestRunner prepares a looper, but AndroidJUnitRunner does not. // http://b/25897652 . @@ -932,8 +945,9 @@ public class ConnectivityServiceTest { mock(INetworkPolicyManager.class), mock(IpConnectivityLog.class)); - mService.systemReady(); mCm = new WrappedConnectivityManager(InstrumentationRegistry.getContext(), mService); + mService.systemReady(); + mService.mockVpn(Process.myUid()); mCm.bindProcessToNetwork(null); // Ensure that the default setting for Captive Portals is used for most tests @@ -1346,6 +1360,7 @@ public class ConnectivityServiceTest { private final static int TIMEOUT_MS = 100; private final LinkedBlockingQueue mCallbacks = new LinkedBlockingQueue<>(); + private Network mLastAvailableNetwork; protected void setLastCallback(CallbackState state, Network network, Object o) { mCallbacks.offer(new CallbackInfo(state, network, o)); @@ -1353,6 +1368,7 @@ public class ConnectivityServiceTest { @Override public void onAvailable(Network network) { + mLastAvailableNetwork = network; setLastCallback(CallbackState.AVAILABLE, network, null); } @@ -1388,9 +1404,14 @@ public class ConnectivityServiceTest { @Override public void onLost(Network network) { + mLastAvailableNetwork = null; setLastCallback(CallbackState.LOST, network, null); } + public Network getLastAvailableNetwork() { + return mLastAvailableNetwork; + } + CallbackInfo nextCallback(int timeoutMs) { CallbackInfo cb = null; try { @@ -1657,6 +1678,7 @@ public class ConnectivityServiceTest { callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); defaultCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mWiFiNetworkAgent.connect(true); // We get AVAILABLE on wifi when wifi connects and satisfies our unmetered request. @@ -1667,6 +1689,7 @@ public class ConnectivityServiceTest { callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent); defaultCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mEthernetNetworkAgent.connect(true); callback.expectAvailableCallbacksUnvalidated(mEthernetNetworkAgent); @@ -1675,11 +1698,13 @@ public class ConnectivityServiceTest { callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mEthernetNetworkAgent); defaultCallback.expectAvailableDoubleValidatedCallbacks(mEthernetNetworkAgent); assertEquals(mEthernetNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mEthernetNetworkAgent.disconnect(); callback.expectCallback(CallbackState.LOST, mEthernetNetworkAgent); defaultCallback.expectCallback(CallbackState.LOST, mEthernetNetworkAgent); defaultCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); for (int i = 0; i < 4; i++) { MockNetworkAgent oldNetwork, newNetwork; @@ -1708,6 +1733,7 @@ public class ConnectivityServiceTest { defaultCallback.expectCapabilitiesWithout(NET_CAPABILITY_NOT_METERED, mWiFiNetworkAgent); defaultCallback.assertNoCallback(); callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Wifi no longer satisfies our listen, which is for an unmetered network. // But because its score is 55, it's still up (and the default network). @@ -1717,8 +1743,11 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.disconnect(); defaultCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mCellNetworkAgent.disconnect(); defaultCallback.expectCallback(CallbackState.LOST, mCellNetworkAgent); + waitForIdle(); + assertEquals(null, mCm.getActiveNetwork()); mCm.unregisterNetworkCallback(callback); waitForIdle(); @@ -1735,6 +1764,7 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up wifi with a score of 20. // Cell stays up because it would satisfy the default request if it validated. @@ -1743,12 +1773,14 @@ public class ConnectivityServiceTest { callback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mWiFiNetworkAgent.disconnect(); callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); defaultCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up wifi with a score of 70. // Cell is lingered because it would not satisfy any request, even if it validated. @@ -1759,6 +1791,7 @@ public class ConnectivityServiceTest { callback.expectCallback(CallbackState.LOSING, mCellNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Tear down wifi. mWiFiNetworkAgent.disconnect(); @@ -1766,6 +1799,7 @@ public class ConnectivityServiceTest { defaultCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent); assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up wifi, then validate it. Previous versions would immediately tear down cell, but // it's arguably correct to linger it, since it was the default network before it validated. @@ -1777,6 +1811,7 @@ public class ConnectivityServiceTest { callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent); defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); assertEquals(mWiFiNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mWiFiNetworkAgent.disconnect(); callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); @@ -1785,12 +1820,15 @@ public class ConnectivityServiceTest { mCellNetworkAgent.disconnect(); callback.expectCallback(CallbackState.LOST, mCellNetworkAgent); defaultCallback.expectCallback(CallbackState.LOST, mCellNetworkAgent); + waitForIdle(); + assertEquals(null, mCm.getActiveNetwork()); // If a network is lingering, and we add and remove a request from it, resume lingering. mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); mCellNetworkAgent.connect(true); callback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); defaultCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(true); defaultCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); @@ -1798,6 +1836,7 @@ public class ConnectivityServiceTest { // TODO: Investigate sending validated before losing. callback.expectCallback(CallbackState.LOSING, mCellNetworkAgent); callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mWiFiNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); NetworkRequest cellRequest = new NetworkRequest.Builder() .addTransportType(TRANSPORT_CELLULAR).build(); @@ -1814,6 +1853,7 @@ public class ConnectivityServiceTest { callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); defaultCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); defaultCallback.expectAvailableCallbacksValidated(mCellNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Cell is now the default network. Pin it with a cell-specific request. noopCallback = new NetworkCallback(); // Can't reuse NetworkCallbacks. http://b/20701525 @@ -1824,6 +1864,7 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent.connect(true); callback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); defaultCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // The default request is lingering on cell, but nothing happens to cell, and we send no // callbacks for it, because it's kept up by cellRequest. callback.assertNoCallback(); @@ -1847,6 +1888,7 @@ public class ConnectivityServiceTest { callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, mEthernetNetworkAgent); trackDefaultCallback.expectAvailableDoubleValidatedCallbacks(mEthernetNetworkAgent); defaultCallback.expectAvailableDoubleValidatedCallbacks(mEthernetNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Let linger run its course. callback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent, lingerTimeoutMs); @@ -2495,23 +2537,27 @@ public class ConnectivityServiceTest { mCellNetworkAgent.connect(true); cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); defaultNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); + assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up wifi and expect CALLBACK_AVAILABLE. mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(true); cellNetworkCallback.assertNoCallback(); defaultNetworkCallback.expectAvailableDoubleValidatedCallbacks(mWiFiNetworkAgent); + assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring down cell. Expect no default network callback, since it wasn't the default. mCellNetworkAgent.disconnect(); cellNetworkCallback.expectCallback(CallbackState.LOST, mCellNetworkAgent); defaultNetworkCallback.assertNoCallback(); + assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring up cell. Expect no default network callback, since it won't be the default. mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); mCellNetworkAgent.connect(true); cellNetworkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); defaultNetworkCallback.assertNoCallback(); + assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); // Bring down wifi. Expect the default network callback to notified of LOST wifi // followed by AVAILABLE cell. @@ -2522,6 +2568,23 @@ public class ConnectivityServiceTest { mCellNetworkAgent.disconnect(); cellNetworkCallback.expectCallback(CallbackState.LOST, mCellNetworkAgent); defaultNetworkCallback.expectCallback(CallbackState.LOST, mCellNetworkAgent); + waitForIdle(); + assertEquals(null, mCm.getActiveNetwork()); + + final int uid = Process.myUid(); + final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + final ArraySet ranges = new ArraySet<>(); + ranges.add(new UidRange(uid, uid)); + when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); + vpnNetworkAgent.setUids(ranges); + vpnNetworkAgent.connect(true); + defaultNetworkCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); + assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + + vpnNetworkAgent.disconnect(); + defaultNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + waitForIdle(); + assertEquals(null, mCm.getActiveNetwork()); } @Test @@ -4012,6 +4075,7 @@ public class ConnectivityServiceTest { final TestNetworkCallback genericNotVpnNetworkCallback = new TestNetworkCallback(); final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback(); final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback(); + final TestNetworkCallback defaultCallback = new TestNetworkCallback(); final NetworkRequest genericNotVpnRequest = new NetworkRequest.Builder().build(); final NetworkRequest genericRequest = new NetworkRequest.Builder() .removeCapability(NET_CAPABILITY_NOT_VPN).build(); @@ -4024,6 +4088,8 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(genericNotVpnRequest, genericNotVpnNetworkCallback); mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback); + mCm.registerDefaultNetworkCallback(defaultCallback); + defaultCallback.assertNoCallback(); mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(false); @@ -4031,15 +4097,14 @@ public class ConnectivityServiceTest { genericNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); genericNotVpnNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); wifiNetworkCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); + defaultCallback.expectAvailableCallbacksUnvalidated(mWiFiNetworkAgent); vpnNetworkCallback.assertNoCallback(); - - // TODO : check callbacks agree with the return value of mCm.getActiveNetwork(). - // Right now this is not possible because establish() is not adequately instrumented - // in this test. + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); final ArraySet ranges = new ArraySet<>(); ranges.add(new UidRange(uid, uid)); + when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); vpnNetworkAgent.setUids(ranges); vpnNetworkAgent.connect(false); @@ -4047,10 +4112,14 @@ public class ConnectivityServiceTest { genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); + defaultCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); genericNetworkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); genericNotVpnNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCapabilitiesLike(nc -> null == nc.getUids(), vpnNetworkAgent); + defaultCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); ranges.clear(); vpnNetworkAgent.setUids(ranges); @@ -4060,6 +4129,14 @@ public class ConnectivityServiceTest { wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + // TODO : The default network callback should actually get a LOST call here (also see the + // comment below for AVAILABLE). This is because ConnectivityService does not look at UID + // ranges at all when determining whether a network should be rematched. In practice, VPNs + // can't currently update their UIDs without disconnecting, so this does not matter too + // much, but that is the reason the test here has to check for an update to the + // capabilities instead of the expected LOST then AVAILABLE. + defaultCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); + ranges.add(new UidRange(uid, uid)); vpnNetworkAgent.setUids(ranges); @@ -4067,6 +4144,9 @@ public class ConnectivityServiceTest { genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent); + // TODO : Here like above, AVAILABLE would be correct, but because this can't actually + // happen outside of the test, ConnectivityService does not rematch callbacks. + defaultCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); mWiFiNetworkAgent.disconnect(); @@ -4074,6 +4154,7 @@ public class ConnectivityServiceTest { genericNotVpnNetworkCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); wifiNetworkCallback.expectCallback(CallbackState.LOST, mWiFiNetworkAgent); vpnNetworkCallback.assertNoCallback(); + defaultCallback.assertNoCallback(); vpnNetworkAgent.disconnect(); @@ -4081,9 +4162,12 @@ public class ConnectivityServiceTest { genericNotVpnNetworkCallback.assertNoCallback(); wifiNetworkCallback.assertNoCallback(); vpnNetworkCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + defaultCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + assertEquals(null, mCm.getActiveNetwork()); mCm.unregisterNetworkCallback(genericNetworkCallback); mCm.unregisterNetworkCallback(wifiNetworkCallback); mCm.unregisterNetworkCallback(vpnNetworkCallback); + mCm.unregisterNetworkCallback(defaultCallback); } } From 818f6b5c6ad9b440aba281e9cbae60bd81ab42e3 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 18 Apr 2018 20:18:38 +0900 Subject: [PATCH 12/20] Make sure getActiveNetwork is consistent with default callbacks Almost clean cherry-pick of ag/3889538. Bug: 77737389 Test: runtest framework-net new test don't pass without the main code change, but they do with it Change-Id: I0cd83a935ab0b349aa47e065b830e5a43ab9a091 Merged-In: Iaa0285825735d3f16bba6e4946723a437fd9b0b9 Merged-In: Ia8f985b448251f911484e6bd63fa562bffc1b0e4 --- .../android/server/ConnectivityService.java | 30 +++++++++--- .../server/ConnectivityServiceTest.java | 49 +++++++++++++++++++ 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 7eaea8dca3..0cbd20c5ea 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -901,6 +901,15 @@ public class ConnectivityService extends IConnectivityManager.Stub deps); } + private static NetworkCapabilities createDefaultNetworkCapabilitiesForUid(int uid) { + final NetworkCapabilities netCap = new NetworkCapabilities(); + netCap.addCapability(NET_CAPABILITY_INTERNET); + netCap.addCapability(NET_CAPABILITY_NOT_RESTRICTED); + netCap.removeCapability(NET_CAPABILITY_NOT_VPN); + netCap.setSingleUid(uid); + return netCap; + } + private NetworkRequest createDefaultInternetRequestForTransport( int transportType, NetworkRequest.Type type) { NetworkCapabilities netCap = new NetworkCapabilities(); @@ -1153,12 +1162,20 @@ public class ConnectivityService extends IConnectivityManager.Stub int vpnNetId = NETID_UNSET; synchronized (mVpns) { final Vpn vpn = mVpns.get(user); + // TODO : now that capabilities contain the UID, the appliesToUid test should + // be removed as the satisfying test below should be enough. if (vpn != null && vpn.appliesToUid(uid)) vpnNetId = vpn.getNetId(); } NetworkAgentInfo nai; if (vpnNetId != NETID_UNSET) { nai = getNetworkAgentInfoForNetId(vpnNetId); - if (nai != null) return nai.network; + if (nai != null) { + final NetworkCapabilities requiredCaps = + createDefaultNetworkCapabilitiesForUid(uid); + if (requiredCaps.satisfiedByNetworkCapabilities(nai.networkCapabilities)) { + return nai.network; + } + } } nai = getDefaultNetwork(); if (nai != null @@ -1373,8 +1390,10 @@ public class ConnectivityService extends IConnectivityManager.Stub private NetworkCapabilities networkCapabilitiesRestrictedForCallerPermissions( NetworkCapabilities nc, int callerPid, int callerUid) { final NetworkCapabilities newNc = new NetworkCapabilities(nc); - if (!checkSettingsPermission(callerPid, callerUid)) newNc.setUids(null); - if (!checkSettingsPermission(callerPid, callerUid)) newNc.setSSID(null); + if (!checkSettingsPermission(callerPid, callerUid)) { + newNc.setUids(null); + newNc.setSSID(null); + } return newNc; } @@ -4255,8 +4274,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 = new NetworkCapabilities(mDefaultRequest.networkCapabilities); - networkCapabilities.removeCapability(NET_CAPABILITY_NOT_VPN); + networkCapabilities = createDefaultNetworkCapabilitiesForUid(Binder.getCallingUid()); enforceAccessPermission(); } else { networkCapabilities = new NetworkCapabilities(networkCapabilities); @@ -4513,7 +4531,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Note: if mDefaultRequest is changed, NetworkMonitor needs to be updated. private final NetworkRequest mDefaultRequest; - + // Request used to optionally keep mobile data active even when higher // priority networks like Wi-Fi are active. private final NetworkRequest mDefaultMobileDataRequest; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index c96b0159a6..52104c2658 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4170,4 +4170,53 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(vpnNetworkCallback); mCm.unregisterNetworkCallback(defaultCallback); } + + @Test + public void testVpnWithAndWithoutInternet() { + final int uid = Process.myUid(); + + final TestNetworkCallback defaultCallback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(defaultCallback); + defaultCallback.assertNoCallback(); + + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(true); + + defaultCallback.expectAvailableThenValidatedCallbacks(mWiFiNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + + MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + final ArraySet ranges = new ArraySet<>(); + ranges.add(new UidRange(uid, uid)); + when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); + vpnNetworkAgent.setUids(ranges); + vpnNetworkAgent.connect(true /* validated */, false /* hasInternet */); + + defaultCallback.assertNoCallback(); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + + vpnNetworkAgent.disconnect(); + defaultCallback.assertNoCallback(); + + vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); + vpnNetworkAgent.setUids(ranges); + vpnNetworkAgent.connect(true /* validated */, true /* hasInternet */); + defaultCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); + assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); + + vpnNetworkAgent.disconnect(); + defaultCallback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + defaultCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent); + + vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); + ranges.clear(); + vpnNetworkAgent.setUids(ranges); + + vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */); + defaultCallback.assertNoCallback(); + + mCm.unregisterNetworkCallback(defaultCallback); + } } From d082c6c12884e0b4f25b51eace629edcf3909154 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Thu, 26 Apr 2018 16:16:10 +0900 Subject: [PATCH 13/20] Fix SSID not being logged by the validation logs Also add it in the logs of the notification manager. Clean cherry-pick of ag/4022397 Bug: 78547904 Test: manual Change-Id: I0afc18c94adf97154c61af2a5bdf933fb5f0e622 Merged-In: Iad5388a31a1502bc1944346276bb9600ac1386bd Merged-In: I8bdd4a020e9d04f46847ef3c7e80ccf5c5cd19ea --- .../android/server/ConnectivityService.java | 18 ++++++++++-------- .../NetworkNotificationManager.java | 15 ++++++++------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0cbd20c5ea..ff301487e2 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -497,24 +497,24 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int MAX_VALIDATION_LOGS = 10; private static class ValidationLog { final Network mNetwork; - final String mNetworkExtraInfo; + final String mName; final ReadOnlyLocalLog mLog; - ValidationLog(Network network, String networkExtraInfo, ReadOnlyLocalLog log) { + ValidationLog(Network network, String name, ReadOnlyLocalLog log) { mNetwork = network; - mNetworkExtraInfo = networkExtraInfo; + mName = name; mLog = log; } } private final ArrayDeque mValidationLogs = new ArrayDeque(MAX_VALIDATION_LOGS); - private void addValidationLogs(ReadOnlyLocalLog log, Network network, String networkExtraInfo) { + private void addValidationLogs(ReadOnlyLocalLog log, Network network, String name) { synchronized (mValidationLogs) { while (mValidationLogs.size() >= MAX_VALIDATION_LOGS) { mValidationLogs.removeLast(); } - mValidationLogs.addFirst(new ValidationLog(network, networkExtraInfo, log)); + mValidationLogs.addFirst(new ValidationLog(network, name, log)); } } @@ -2059,7 +2059,7 @@ public class ConnectivityService extends IConnectivityManager.Stub synchronized (mValidationLogs) { pw.println("mValidationLogs (most recent first):"); for (ValidationLog p : mValidationLogs) { - pw.println(p.mNetwork + " - " + p.mNetworkExtraInfo); + pw.println(p.mNetwork + " - " + p.mName); pw.increaseIndent(); p.mLog.dump(fd, pw, args); pw.decreaseIndent(); @@ -4584,8 +4584,10 @@ public class ConnectivityService extends IConnectivityManager.Stub synchronized (this) { nai.networkMonitor.systemReady = mSystemReady; } - addValidationLogs(nai.networkMonitor.getValidationLogs(), nai.network, - networkInfo.getExtraInfo()); + final String extraInfo = networkInfo.getExtraInfo(); + final String name = TextUtils.isEmpty(extraInfo) + ? nai.networkCapabilities.getSSID() : extraInfo; + addValidationLogs(nai.networkMonitor.getValidationLogs(), nai.network, name); if (DBG) log("registerNetworkAgent " + nai); mHandler.sendMessage(mHandler.obtainMessage(EVENT_REGISTER_NETWORK_AGENT, nai)); return nai.network.netId; diff --git a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java index 02459bde09..36a2476d2c 100644 --- a/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java +++ b/services/core/java/com/android/server/connectivity/NetworkNotificationManager.java @@ -26,6 +26,7 @@ import android.net.NetworkCapabilities; import android.net.wifi.WifiInfo; import android.os.UserHandle; import android.telephony.TelephonyManager; +import android.text.TextUtils; import android.util.Slog; import android.util.SparseArray; import android.util.SparseIntArray; @@ -131,16 +132,17 @@ public class NetworkNotificationManager { final String tag = tagFor(id); final int eventId = notifyType.eventId; final int transportType; - final String extraInfo; + final String name; if (nai != null) { transportType = getFirstTransportType(nai); - extraInfo = nai.networkInfo.getExtraInfo(); + final String extraInfo = nai.networkInfo.getExtraInfo(); + name = TextUtils.isEmpty(extraInfo) ? nai.networkCapabilities.getSSID() : extraInfo; // Only notify for Internet-capable networks. if (!nai.networkCapabilities.hasCapability(NET_CAPABILITY_INTERNET)) return; } else { // Legacy notifications. transportType = TRANSPORT_CELLULAR; - extraInfo = null; + name = null; } // Clear any previous notification with lower priority, otherwise return. http://b/63676954. @@ -157,9 +159,8 @@ public class NetworkNotificationManager { if (DBG) { Slog.d(TAG, String.format( - "showNotification tag=%s event=%s transport=%s extraInfo=%s highPrioriy=%s", - tag, nameOf(eventId), getTransportName(transportType), extraInfo, - highPriority)); + "showNotification tag=%s event=%s transport=%s name=%s highPriority=%s", + tag, nameOf(eventId), getTransportName(transportType), name, highPriority)); } Resources r = Resources.getSystem(); @@ -188,7 +189,7 @@ public class NetworkNotificationManager { break; default: title = r.getString(R.string.network_available_sign_in, 0); - details = r.getString(R.string.network_available_sign_in_detailed, extraInfo); + details = r.getString(R.string.network_available_sign_in_detailed, name); break; } } else if (notifyType == NotificationType.NETWORK_SWITCH) { From 52a6d547b7bd207b0f9c8479fdfd37eb39dfb211 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 18 May 2018 23:48:49 +0900 Subject: [PATCH 14/20] Fix setCapabilities. P introduced setSSID, UIDs and unwanted capabilities. None of these exhibit commutative behavior through combineCapabilities because their semantics don't allow it. Therefore NetworkRequest.setCapabilities() is badly broken around any of these. Look at the comments in the new tests to realize the extent of the damage. Clean cherry-pick of ag/4083952 Bug: 79748782 Test: new tests written, old tests pass Change-Id: Iafe074126132a82af37f4bf056c4a7b8d56bdc83 Merged-In: Ia5bebf8a233775367bbf1b788870528934ecbcfb Merged-In: I13d7782a6c0c7b1f94137995bbb0d257a58d89c1 --- .../java/android/net/NetworkCapabilities.java | 28 ++++++--- core/java/android/net/NetworkRequest.java | 3 +- .../android/net/NetworkCapabilitiesTest.java | 61 ++++++++++++++++++- 3 files changed, 78 insertions(+), 14 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 3f0f334772..0f2bfba86c 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -61,16 +61,7 @@ public final class NetworkCapabilities implements Parcelable { public NetworkCapabilities(NetworkCapabilities nc) { if (nc != null) { - mNetworkCapabilities = nc.mNetworkCapabilities; - mTransportTypes = nc.mTransportTypes; - mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps; - mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps; - mNetworkSpecifier = nc.mNetworkSpecifier; - mSignalStrength = nc.mSignalStrength; - mUids = nc.mUids; - mEstablishingVpnAppUid = nc.mEstablishingVpnAppUid; - mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; - mSSID = nc.mSSID; + set(nc); } } @@ -89,6 +80,23 @@ public final class NetworkCapabilities implements Parcelable { mSSID = null; } + /** + * Set all contents of this object to the contents of a NetworkCapabilities. + * @hide + */ + public void set(NetworkCapabilities nc) { + mNetworkCapabilities = nc.mNetworkCapabilities; + mTransportTypes = nc.mTransportTypes; + mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps; + mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps; + mNetworkSpecifier = nc.mNetworkSpecifier; + mSignalStrength = nc.mSignalStrength; + setUids(nc.mUids); // Will make the defensive copy + mEstablishingVpnAppUid = nc.mEstablishingVpnAppUid; + mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; + mSSID = nc.mSSID; + } + /** * Represents the network's capabilities. If any are specified they will be satisfied * by any Network that matches all of them. diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index b0b58cd524..f3669fb3e2 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -197,8 +197,7 @@ public class NetworkRequest implements Parcelable { * @hide */ public Builder setCapabilities(NetworkCapabilities nc) { - mNetworkCapabilities.clearAll(); - mNetworkCapabilities.combineCapabilities(nc); + mNetworkCapabilities.set(nc); return this; } diff --git a/tests/net/java/android/net/NetworkCapabilitiesTest.java b/tests/net/java/android/net/NetworkCapabilitiesTest.java index c17f31e701..2f4d69ec78 100644 --- a/tests/net/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/java/android/net/NetworkCapabilitiesTest.java @@ -57,6 +57,7 @@ import java.util.Set; @SmallTest public class NetworkCapabilitiesTest { private static final String TEST_SSID = "TEST_SSID"; + private static final String DIFFERENT_TEST_SSID = "DIFFERENT_TEST_SSID"; @Test public void testMaybeMarkCapabilitiesRestricted() { @@ -382,6 +383,12 @@ public class NetworkCapabilitiesTest { assertFalse(nc1.satisfiedByNetworkCapabilities(nc2)); } + private ArraySet uidRange(int from, int to) { + final ArraySet range = new ArraySet<>(1); + range.add(new UidRange(from, to)); + return range; + } + @Test public void testCombineCapabilities() { NetworkCapabilities nc1 = new NetworkCapabilities(); @@ -408,14 +415,30 @@ public class NetworkCapabilitiesTest { nc2.combineCapabilities(nc1); assertTrue(TEST_SSID.equals(nc2.getSSID())); - // Because they now have the same SSID, the folllowing call should not throw + // Because they now have the same SSID, the following call should not throw nc2.combineCapabilities(nc1); - nc1.setSSID("different " + TEST_SSID); + nc1.setSSID(DIFFERENT_TEST_SSID); try { nc2.combineCapabilities(nc1); fail("Expected IllegalStateException: can't combine different SSIDs"); } catch (IllegalStateException expected) {} + nc1.setSSID(TEST_SSID); + + nc1.setUids(uidRange(10, 13)); + assertNotEquals(nc1, nc2); + nc2.combineCapabilities(nc1); // Everything + 10~13 is still everything. + assertNotEquals(nc1, nc2); + nc1.combineCapabilities(nc2); // 10~13 + everything is everything. + assertEquals(nc1, nc2); + nc1.setUids(uidRange(10, 13)); + nc2.setUids(uidRange(20, 23)); + assertNotEquals(nc1, nc2); + nc1.combineCapabilities(nc2); + assertTrue(nc1.appliesToUid(12)); + assertFalse(nc2.appliesToUid(12)); + assertTrue(nc1.appliesToUid(22)); + assertTrue(nc2.appliesToUid(22)); } @Test @@ -454,4 +477,38 @@ public class NetworkCapabilitiesTest { p.setDataPosition(0); assertEquals(NetworkCapabilities.CREATOR.createFromParcel(p), netCap); } + + @Test + public void testSet() { + NetworkCapabilities nc1 = new NetworkCapabilities(); + NetworkCapabilities nc2 = new NetworkCapabilities(); + + nc1.addUnwantedCapability(NET_CAPABILITY_CAPTIVE_PORTAL); + nc1.addCapability(NET_CAPABILITY_NOT_ROAMING); + assertNotEquals(nc1, nc2); + nc2.set(nc1); + assertEquals(nc1, nc2); + assertTrue(nc2.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(nc2.hasUnwantedCapability(NET_CAPABILITY_CAPTIVE_PORTAL)); + + // This will effectively move NOT_ROAMING capability from required to unwanted for nc1. + nc1.addUnwantedCapability(NET_CAPABILITY_NOT_ROAMING); + nc1.setSSID(TEST_SSID); + nc2.set(nc1); + assertEquals(nc1, nc2); + // Contrary to combineCapabilities, set() will have removed the NOT_ROAMING capability + // from nc2. + assertFalse(nc2.hasCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(nc2.hasUnwantedCapability(NET_CAPABILITY_NOT_ROAMING)); + assertTrue(TEST_SSID.equals(nc2.getSSID())); + + nc1.setSSID(DIFFERENT_TEST_SSID); + nc2.set(nc1); + assertEquals(nc1, nc2); + assertTrue(DIFFERENT_TEST_SSID.equals(nc2.getSSID())); + + nc1.setUids(uidRange(10, 13)); + nc2.set(nc1); // Overwrites, as opposed to combineCapabilities + assertEquals(nc1, nc2); + } } From 32f9daa029b0813b9e4615c1160104e1a0c3a6fc Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 18 May 2018 21:47:45 +0900 Subject: [PATCH 15/20] Add tests for setUnderlyingNetworks. Fixes come later. This is complex enough as it is. Clean cherry-pick of ag/4083953 Bug: 79748782 Test: new test passes, old tests still pass Change-Id: If7276fe1f751be7b9c18f689e97699e566e5bde0 Merged-In: I12c948ebeb2b74290908f8320ff77220dc4a9fb9 --- .../server/ConnectivityServiceTest.java | 235 ++++++++++++++++-- 1 file changed, 219 insertions(+), 16 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 52104c2658..572cb521ed 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -112,6 +112,7 @@ import android.net.NetworkUtils; import android.net.RouteInfo; import android.net.StringNetworkSpecifier; import android.net.UidRange; +import android.net.VpnService; import android.net.captiveportal.CaptivePortalProbeResult; import android.net.metrics.IpConnectivityLog; import android.net.util.MultinetworkPolicyTracker; @@ -134,6 +135,7 @@ import android.test.mock.MockContentResolver; import android.util.ArraySet; import android.util.Log; +import com.android.internal.net.VpnConfig; import com.android.internal.util.ArrayUtils; import com.android.internal.util.WakeupMessage; import com.android.internal.util.test.BroadcastInterceptingContext; @@ -197,13 +199,13 @@ public class ConnectivityServiceTest { private MockNetworkAgent mWiFiNetworkAgent; private MockNetworkAgent mCellNetworkAgent; private MockNetworkAgent mEthernetNetworkAgent; + private MockVpn mMockVpn; private Context mContext; @Mock IpConnectivityMetrics.Logger mMetricsService; @Mock DefaultNetworkMetrics mDefaultNetworkMetrics; @Mock INetworkManagementService mNetworkManagementService; @Mock INetworkStatsService mStatsService; - @Mock Vpn mMockVpn; private ArgumentCaptor mStringArrayCaptor = ArgumentCaptor.forClass(String[].class); @@ -479,6 +481,14 @@ public class ConnectivityServiceTest { mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); } + public void setNetworkCapabilities(NetworkCapabilities nc, + boolean sendToConnectivityService) { + mNetworkCapabilities.set(nc); + if (sendToConnectivityService) { + mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities); + } + } + public void connectWithoutInternet() { mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, null); mNetworkAgent.sendNetworkInfo(mNetworkInfo); @@ -594,6 +604,10 @@ public class ConnectivityServiceTest { return mRedirectUrl; } + public NetworkAgent getNetworkAgent() { + return mNetworkAgent; + } + public NetworkCapabilities getNetworkCapabilities() { return mNetworkCapabilities; } @@ -726,6 +740,87 @@ public class ConnectivityServiceTest { } } + private static Looper startHandlerThreadAndReturnLooper() { + final HandlerThread handlerThread = new HandlerThread("MockVpnThread"); + handlerThread.start(); + return handlerThread.getLooper(); + } + + private class MockVpn extends Vpn { + // TODO : the interactions between this mock and the mock network agent are too + // hard to get right at this moment, because it's unclear in which case which + // target needs to get a method call or both, and in what order. It's because + // MockNetworkAgent wants to manage its own NetworkCapabilities, but the Vpn + // parent class of MockVpn agent wants that responsibility. + // That being said inside the test it should be possible to make the interactions + // harder to get wrong with precise speccing, judicious comments, helper methods + // and a few sprinkled assertions. + + private boolean mConnected = false; + // Careful ! This is different from mNetworkAgent, because MockNetworkAgent does + // not inherit from NetworkAgent. + private MockNetworkAgent mMockNetworkAgent; + + public MockVpn(int userId) { + super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService, + userId); + } + + public void setNetworkAgent(MockNetworkAgent agent) { + waitForIdle(agent, TIMEOUT_MS); + mMockNetworkAgent = agent; + mNetworkAgent = agent.getNetworkAgent(); + mNetworkCapabilities.set(agent.getNetworkCapabilities()); + } + + public void setUids(Set uids) { + mNetworkCapabilities.setUids(uids); + updateCapabilities(); + } + + @Override + public int getNetId() { + return mMockNetworkAgent.getNetwork().netId; + } + + @Override + public boolean appliesToUid(int uid) { + return mConnected; // Trickery to simplify testing. + } + + @Override + protected boolean isCallerEstablishedOwnerLocked() { + return mConnected; // Similar trickery + } + + public void connect() { + mNetworkCapabilities.set(mMockNetworkAgent.getNetworkCapabilities()); + mConnected = true; + mConfig = new VpnConfig(); + } + + @Override + public void updateCapabilities() { + if (!mConnected) return; + super.updateCapabilities(); + // Because super.updateCapabilities will update the capabilities of the agent but not + // the mock agent, the mock agent needs to know about them. + copyCapabilitiesToNetworkAgent(); + } + + private void copyCapabilitiesToNetworkAgent() { + if (null != mMockNetworkAgent) { + mMockNetworkAgent.setNetworkCapabilities(mNetworkCapabilities, + false /* sendToConnectivityService */); + } + } + + public void disconnect() { + mConnected = false; + mConfig = null; + } + } + private class FakeWakeupMessage extends WakeupMessage { private static final int UNREASONABLY_LONG_WAIT = 1000; @@ -894,10 +989,12 @@ public class ConnectivityServiceTest { public void mockVpn(int uid) { synchronized (mVpns) { + int userId = UserHandle.getUserId(uid); + mMockVpn = new MockVpn(userId); // This has no effect unless the VPN is actually connected, because things like // getActiveNetworkForUidInternal call getNetworkAgentInfoForNetId on the VPN // netId, and check if that network is actually connected. - mVpns.put(UserHandle.getUserId(Process.myUid()), mMockVpn); + mVpns.put(userId, mMockVpn); } } @@ -927,7 +1024,6 @@ public class ConnectivityServiceTest { MockitoAnnotations.initMocks(this); when(mMetricsService.defaultNetworkMetrics()).thenReturn(mDefaultNetworkMetrics); - when(mMockVpn.appliesToUid(Process.myUid())).thenReturn(true); // InstrumentationTestRunner prepares a looper, but AndroidJUnitRunner does not. // http://b/25897652 . @@ -1547,7 +1643,8 @@ public class ConnectivityServiceTest { void expectCapabilitiesLike(Predicate fn, MockNetworkAgent agent) { CallbackInfo cbi = expectCallback(CallbackState.NETWORK_CAPABILITIES, agent); - assertTrue(fn.test((NetworkCapabilities) cbi.arg)); + assertTrue("Received capabilities don't match expectations : " + cbi.arg, + fn.test((NetworkCapabilities) cbi.arg)); } void assertNoCallback() { @@ -2575,9 +2672,10 @@ public class ConnectivityServiceTest { final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); final ArraySet ranges = new ArraySet<>(); ranges.add(new UidRange(uid, uid)); - when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); - vpnNetworkAgent.setUids(ranges); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.setUids(ranges); vpnNetworkAgent.connect(true); + mMockVpn.connect(); defaultNetworkCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); assertEquals(defaultNetworkCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); @@ -4104,9 +4202,10 @@ public class ConnectivityServiceTest { final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); final ArraySet ranges = new ArraySet<>(); ranges.add(new UidRange(uid, uid)); - when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); - vpnNetworkAgent.setUids(ranges); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.setUids(ranges); vpnNetworkAgent.connect(false); + mMockVpn.connect(); genericNetworkCallback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); genericNotVpnNetworkCallback.assertNoCallback(); @@ -4138,7 +4237,7 @@ public class ConnectivityServiceTest { defaultCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); ranges.add(new UidRange(uid, uid)); - vpnNetworkAgent.setUids(ranges); + mMockVpn.setUids(ranges); genericNetworkCallback.expectAvailableCallbacksValidated(vpnNetworkAgent); genericNotVpnNetworkCallback.assertNoCallback(); @@ -4188,9 +4287,10 @@ public class ConnectivityServiceTest { MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); final ArraySet ranges = new ArraySet<>(); ranges.add(new UidRange(uid, uid)); - when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); - vpnNetworkAgent.setUids(ranges); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.setUids(ranges); vpnNetworkAgent.connect(true /* validated */, false /* hasInternet */); + mMockVpn.connect(); defaultCallback.assertNoCallback(); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); @@ -4199,9 +4299,10 @@ public class ConnectivityServiceTest { defaultCallback.assertNoCallback(); vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); - when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); - vpnNetworkAgent.setUids(ranges); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.setUids(ranges); vpnNetworkAgent.connect(true /* validated */, true /* hasInternet */); + mMockVpn.connect(); defaultCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); assertEquals(defaultCallback.getLastAvailableNetwork(), mCm.getActiveNetwork()); @@ -4210,13 +4311,115 @@ public class ConnectivityServiceTest { defaultCallback.expectAvailableCallbacksValidated(mWiFiNetworkAgent); vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); - when(mMockVpn.getNetId()).thenReturn(vpnNetworkAgent.getNetwork().netId); ranges.clear(); - vpnNetworkAgent.setUids(ranges); - + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.setUids(ranges); vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */); + mMockVpn.connect(); defaultCallback.assertNoCallback(); mCm.unregisterNetworkCallback(defaultCallback); } + + @Test + public void testVpnSetUnderlyingNetworks() { + final int uid = Process.myUid(); + + final TestNetworkCallback vpnNetworkCallback = new TestNetworkCallback(); + final NetworkRequest vpnNetworkRequest = new NetworkRequest.Builder() + .removeCapability(NET_CAPABILITY_NOT_VPN) + .addTransportType(TRANSPORT_VPN) + .build(); + NetworkCapabilities nc; + mCm.registerNetworkCallback(vpnNetworkRequest, vpnNetworkCallback); + vpnNetworkCallback.assertNoCallback(); + + final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + final ArraySet ranges = new ArraySet<>(); + ranges.add(new UidRange(uid, uid)); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.connect(); + mMockVpn.setUids(ranges); + vpnNetworkAgent.connect(true /* validated */, false /* hasInternet */); + + vpnNetworkCallback.expectAvailableThenValidatedCallbacks(vpnNetworkAgent); + nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork()); + assertTrue(nc.hasTransport(TRANSPORT_VPN)); + assertFalse(nc.hasTransport(TRANSPORT_CELLULAR)); + assertFalse(nc.hasTransport(TRANSPORT_WIFI)); + // For safety reasons a VPN without underlying networks is considered metered. + assertFalse(nc.hasCapability(NET_CAPABILITY_NOT_METERED)); + + // Connect cell and use it as an underlying network. + mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR); + mCellNetworkAgent.connect(true); + + mService.setUnderlyingNetworksForVpn( + new Network[] { mCellNetworkAgent.getNetwork() }); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.addCapability(NET_CAPABILITY_NOT_METERED); + mWiFiNetworkAgent.connect(true); + + mService.setUnderlyingNetworksForVpn( + new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() }); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + // Don't disconnect, but note the VPN is not using wifi any more. + mService.setUnderlyingNetworksForVpn( + new Network[] { mCellNetworkAgent.getNetwork() }); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + // Use Wifi but not cell. Note the VPN is now unmetered. + mService.setUnderlyingNetworksForVpn( + new Network[] { mWiFiNetworkAgent.getNetwork() }); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) + && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + // Use both again. + mService.setUnderlyingNetworksForVpn( + new Network[] { mCellNetworkAgent.getNetwork(), mWiFiNetworkAgent.getNetwork() }); + + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + if (false) { // TODO : reactivate this ; in the current state, the below fail. + // Disconnect cell. Receive update without even removing the dead network from the + // underlying networks – it's dead anyway. Not metered any more. + mCellNetworkAgent.disconnect(); + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) + && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + + // Disconnect wifi too. No underlying networks should mean this is now metered, + // unfortunately a discrepancy in the current implementation has this unmetered. + // TODO : fix this. + mWiFiNetworkAgent.disconnect(); + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) + && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); + } + + mMockVpn.disconnect(); + } } From 22670f9b93fe220a32d72136c55b437dc2e48f9a Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 18 May 2018 22:02:56 +0900 Subject: [PATCH 16/20] Fix: VPNs update caps upon underlying network disconnect. Clean cherry-pick of ag/4083954 Bug: 79748782 Test: ConnectivityServiceTests still pass Change-Id: I21e866c723099e5c3dee54ff13e830d44427fc7a Merged-In: I12c948ebeb2b74290908f8320ff77220dc4a9fb9 --- .../android/server/ConnectivityService.java | 32 +++++++++++++++---- .../server/ConnectivityServiceTest.java | 32 +++++++++---------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ff301487e2..fe0f6d23b1 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2465,6 +2465,9 @@ public class ConnectivityService extends IConnectivityManager.Stub ensureNetworkTransitionWakelock(nai.name()); } mLegacyTypeTracker.remove(nai, wasDefault); + if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { + updateAllVpnsCapabilities(); + } rematchAllNetworksAndRequests(null, 0); mLingerMonitor.noteDisconnect(nai); if (nai.created) { @@ -3734,6 +3737,26 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Ask all VPN objects to recompute and update their capabilities. + * + * When underlying networks change, VPNs may have to update capabilities to reflect things + * like the metered bit, their transports, and so on. This asks the VPN objects to update + * their capabilities, and as this will cause them to send messages to the ConnectivityService + * handler thread through their agent, this is asynchronous. When the capabilities objects + * are computed they will be up-to-date as they are computed synchronously from here and + * this is running on the ConnectivityService thread. + * TODO : Fix this and call updateCapabilities inline to remove out-of-order events. + */ + private void updateAllVpnsCapabilities() { + synchronized (mVpns) { + for (int i = 0; i < mVpns.size(); i++) { + final Vpn vpn = mVpns.valueAt(i); + vpn.updateCapabilities(); + } + } + } + @Override public boolean updateLockdownVpn() { if (Binder.getCallingUid() != Process.SYSTEM_UID) { @@ -4531,7 +4554,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // Note: if mDefaultRequest is changed, NetworkMonitor needs to be updated. private final NetworkRequest mDefaultRequest; - + // Request used to optionally keep mobile data active even when higher // priority networks like Wi-Fi are active. private final NetworkRequest mDefaultMobileDataRequest; @@ -4891,12 +4914,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (!newNc.hasTransport(TRANSPORT_VPN)) { // Tell VPNs about updated capabilities, since they may need to // bubble those changes through. - synchronized (mVpns) { - for (int i = 0; i < mVpns.size(); i++) { - final Vpn vpn = mVpns.valueAt(i); - vpn.updateCapabilities(); - } - } + updateAllVpnsCapabilities(); } } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 572cb521ed..fef5417f1a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4401,24 +4401,22 @@ public class ConnectivityServiceTest { && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), vpnNetworkAgent); - if (false) { // TODO : reactivate this ; in the current state, the below fail. - // Disconnect cell. Receive update without even removing the dead network from the - // underlying networks – it's dead anyway. Not metered any more. - mCellNetworkAgent.disconnect(); - vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) - && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) - && caps.hasCapability(NET_CAPABILITY_NOT_METERED), - vpnNetworkAgent); + // Disconnect cell. Receive update without even removing the dead network from the + // underlying networks – it's dead anyway. Not metered any more. + mCellNetworkAgent.disconnect(); + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && caps.hasTransport(TRANSPORT_WIFI) + && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); - // Disconnect wifi too. No underlying networks should mean this is now metered, - // unfortunately a discrepancy in the current implementation has this unmetered. - // TODO : fix this. - mWiFiNetworkAgent.disconnect(); - vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) - && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) - && caps.hasCapability(NET_CAPABILITY_NOT_METERED), - vpnNetworkAgent); - } + // Disconnect wifi too. No underlying networks should mean this is now metered, + // unfortunately a discrepancy in the current implementation has this unmetered. + // TODO : fix this. + mWiFiNetworkAgent.disconnect(); + vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) + && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) + && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + vpnNetworkAgent); mMockVpn.disconnect(); } From ec3d62faa5f4cee654ac9d45719b56d3de0492d8 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Mon, 21 May 2018 15:30:56 +0900 Subject: [PATCH 17/20] Unify behavior of various cases of "no underlying networks" Before this change, VPNs having no underlying networks would be marked as metered as the safe option, but VPNs having only disconnected underlying networks would be marked as unmetered. Fix this discrepancy. Clean cherry-pick of ag/4113809 Bug: 79748782 Test: runtest frameworks-net Change-Id: Ie6ace6bd95139605ffcfa8cd6c15cf28f8fa28c8 Merged-In: If19b85325e7d684e645470293b3c8a674084c641 Merged-in: I22f80a6a39d4a19ff74aa61fcbd66f1a041b1003 --- .../java/com/android/server/ConnectivityServiceTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index fef5417f1a..d6018f1b13 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4409,13 +4409,11 @@ public class ConnectivityServiceTest { && caps.hasCapability(NET_CAPABILITY_NOT_METERED), vpnNetworkAgent); - // Disconnect wifi too. No underlying networks should mean this is now metered, - // unfortunately a discrepancy in the current implementation has this unmetered. - // TODO : fix this. + // Disconnect wifi too. No underlying networks means this is now metered. mWiFiNetworkAgent.disconnect(); vpnNetworkCallback.expectCapabilitiesLike((caps) -> caps.hasTransport(TRANSPORT_VPN) && !caps.hasTransport(TRANSPORT_CELLULAR) && !caps.hasTransport(TRANSPORT_WIFI) - && caps.hasCapability(NET_CAPABILITY_NOT_METERED), + && !caps.hasCapability(NET_CAPABILITY_NOT_METERED), vpnNetworkAgent); mMockVpn.disconnect(); From 8cfa434e53cdb7d95809fc0e10bb834f9146503f Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 23 May 2018 09:07:51 +0900 Subject: [PATCH 18/20] Fix a ConcurrentModificationException crash. This is a pinpoint fix against the bug listed below. While a client is synchronously reading the LinkProperties of a network, the ConnectivityServiceThread is updating its properties. Make sure that update is done atomically. This is a stopgap countermeasure against a problem that is pervasive with usage of LinkProperties, but fixing the problem itself will happen later. Clean cherry-pick of ag/4174798 Bug: 80077223 Test: runtest frameworks-net Change-Id: I61b262d824c98b4ced36395a597b73de9193a199 Merged-In: I25007ac26349e451bb47f966af70d590d699c347 Merged-In: I03526187645b6955eb89ca4d2e4a930ebac236b8 --- .../core/java/com/android/server/ConnectivityService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index fe0f6d23b1..789092d1cf 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4630,7 +4630,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void updateLinkProperties(NetworkAgentInfo networkAgent, LinkProperties oldLp) { - LinkProperties newLp = networkAgent.linkProperties; + LinkProperties newLp = new LinkProperties(networkAgent.linkProperties); int netId = networkAgent.network.netId; // The NetworkAgentInfo does not know whether clatd is running on its network or not. Before @@ -4664,6 +4664,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } // TODO - move this check to cover the whole function if (!Objects.equals(newLp, oldLp)) { + synchronized (networkAgent) { + networkAgent.linkProperties = newLp; + } notifyIfacesChangedForNetworkStats(); notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_IP_CHANGED); } From 43847bc10cbab7922691b868990bb289895ea428 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 11 May 2018 20:19:20 +0900 Subject: [PATCH 19/20] Destroy networks as soon as they are disconnected. ...as opposed to after the async channel finished disconnecting. Clean cherry-pick of ag/4043255 Bug: 78308259 Test: runtest frameworks-net also used a device with this patch over the weekend and tried all I could think of Merged-In: Ic4c7520e907de353a01c2a3a8a50d661dee4a994 Merged-In: I0617f0ff6e46a1d3764335a1e7ad01b34c8cc5a8 Change-Id: I4e4b41bbdf25d7d7bea4124cb58da004d47f1090 --- .../android/server/ConnectivityService.java | 175 ++++++++++-------- 1 file changed, 93 insertions(+), 82 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 789092d1cf..2593690e4b 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2404,97 +2404,107 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // This is a no-op if it's called with a message designating a network that has + // already been destroyed, because its reference will not be found in the relevant + // maps. private void handleAsyncChannelDisconnected(Message msg) { NetworkAgentInfo nai = mNetworkAgentInfos.get(msg.replyTo); if (nai != null) { - if (DBG) { - log(nai.name() + " got DISCONNECTED, was satisfying " + nai.numNetworkRequests()); - } - // A network agent has disconnected. - // TODO - if we move the logic to the network agent (have them disconnect - // because they lost all their requests or because their score isn't good) - // then they would disconnect organically, report their new state and then - // disconnect the channel. - if (nai.networkInfo.isConnected()) { - nai.networkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, - null, null); - } - final boolean wasDefault = isDefaultNetwork(nai); - if (wasDefault) { - mDefaultInetConditionPublished = 0; - // Log default network disconnection before required book-keeping. - // Let rematchAllNetworksAndRequests() below record a new default network event - // if there is a fallback. Taken together, the two form a X -> 0, 0 -> Y sequence - // whose timestamps tell how long it takes to recover a default network. - long now = SystemClock.elapsedRealtime(); - metricsLogger().defaultNetworkMetrics().logDefaultNetworkEvent(now, null, nai); - } - notifyIfacesChangedForNetworkStats(); - // TODO - we shouldn't send CALLBACK_LOST to requests that can be satisfied - // by other networks that are already connected. Perhaps that can be done by - // sending all CALLBACK_LOST messages (for requests, not listens) at the end - // of rematchAllNetworksAndRequests - notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOST); - mKeepaliveTracker.handleStopAllKeepalives(nai, - ConnectivityManager.PacketKeepalive.ERROR_INVALID_NETWORK); - for (String iface : nai.linkProperties.getAllInterfaceNames()) { - // Disable wakeup packet monitoring for each interface. - wakeupModifyInterface(iface, nai.networkCapabilities, false); - } - nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); - mNetworkAgentInfos.remove(msg.replyTo); - nai.maybeStopClat(); - synchronized (mNetworkForNetId) { - // Remove the NetworkAgent, but don't mark the netId as - // available until we've told netd to delete it below. - mNetworkForNetId.remove(nai.network.netId); - } - // Remove all previously satisfied requests. - for (int i = 0; i < nai.numNetworkRequests(); i++) { - NetworkRequest request = nai.requestAt(i); - NetworkAgentInfo currentNetwork = getNetworkForRequest(request.requestId); - if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) { - clearNetworkForRequest(request.requestId); - sendUpdatedScoreToFactories(request, 0); - } - } - nai.clearLingerState(); - if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) { - removeDataActivityTracking(nai); - notifyLockdownVpn(nai); - ensureNetworkTransitionWakelock(nai.name()); - } - mLegacyTypeTracker.remove(nai, wasDefault); - if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { - updateAllVpnsCapabilities(); - } - rematchAllNetworksAndRequests(null, 0); - mLingerMonitor.noteDisconnect(nai); - if (nai.created) { - // Tell netd to clean up the configuration for this network - // (routing rules, DNS, etc). - // This may be slow as it requires a lot of netd shelling out to ip and - // ip[6]tables to flush routes and remove the incoming packet mark rule, so do it - // after we've rematched networks with requests which should make a potential - // fallback network the default or requested a new network from the - // NetworkFactories, so network traffic isn't interrupted for an unnecessarily - // long time. - try { - mNetd.removeNetwork(nai.network.netId); - } catch (Exception e) { - loge("Exception removing network: " + e); - } - mDnsManager.removeNetwork(nai.network); - } - synchronized (mNetworkForNetId) { - mNetIdInUse.delete(nai.network.netId); - } + disconnectAndDestroyNetwork(nai); } else { NetworkFactoryInfo nfi = mNetworkFactoryInfos.remove(msg.replyTo); if (DBG && nfi != null) log("unregisterNetworkFactory for " + nfi.name); } } + // Destroys a network, remove references to it from the internal state managed by + // ConnectivityService, free its interfaces and clean up. + // Must be called on the Handler thread. + private void disconnectAndDestroyNetwork(NetworkAgentInfo nai) { + if (DBG) { + log(nai.name() + " got DISCONNECTED, was satisfying " + nai.numNetworkRequests()); + } + // A network agent has disconnected. + // TODO - if we move the logic to the network agent (have them disconnect + // because they lost all their requests or because their score isn't good) + // then they would disconnect organically, report their new state and then + // disconnect the channel. + if (nai.networkInfo.isConnected()) { + nai.networkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, + null, null); + } + final boolean wasDefault = isDefaultNetwork(nai); + if (wasDefault) { + mDefaultInetConditionPublished = 0; + // Log default network disconnection before required book-keeping. + // Let rematchAllNetworksAndRequests() below record a new default network event + // if there is a fallback. Taken together, the two form a X -> 0, 0 -> Y sequence + // whose timestamps tell how long it takes to recover a default network. + long now = SystemClock.elapsedRealtime(); + metricsLogger().defaultNetworkMetrics().logDefaultNetworkEvent(now, null, nai); + } + notifyIfacesChangedForNetworkStats(); + // TODO - we shouldn't send CALLBACK_LOST to requests that can be satisfied + // by other networks that are already connected. Perhaps that can be done by + // sending all CALLBACK_LOST messages (for requests, not listens) at the end + // of rematchAllNetworksAndRequests + notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOST); + mKeepaliveTracker.handleStopAllKeepalives(nai, + ConnectivityManager.PacketKeepalive.ERROR_INVALID_NETWORK); + for (String iface : nai.linkProperties.getAllInterfaceNames()) { + // Disable wakeup packet monitoring for each interface. + wakeupModifyInterface(iface, nai.networkCapabilities, false); + } + nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); + mNetworkAgentInfos.remove(nai.messenger); + nai.maybeStopClat(); + synchronized (mNetworkForNetId) { + // Remove the NetworkAgent, but don't mark the netId as + // available until we've told netd to delete it below. + mNetworkForNetId.remove(nai.network.netId); + } + // Remove all previously satisfied requests. + for (int i = 0; i < nai.numNetworkRequests(); i++) { + NetworkRequest request = nai.requestAt(i); + NetworkAgentInfo currentNetwork = getNetworkForRequest(request.requestId); + if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) { + clearNetworkForRequest(request.requestId); + sendUpdatedScoreToFactories(request, 0); + } + } + nai.clearLingerState(); + if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) { + removeDataActivityTracking(nai); + notifyLockdownVpn(nai); + ensureNetworkTransitionWakelock(nai.name()); + } + mLegacyTypeTracker.remove(nai, wasDefault); + if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { + updateAllVpnsCapabilities(); + } + rematchAllNetworksAndRequests(null, 0); + mLingerMonitor.noteDisconnect(nai); + if (nai.created) { + // Tell netd to clean up the configuration for this network + // (routing rules, DNS, etc). + // This may be slow as it requires a lot of netd shelling out to ip and + // ip[6]tables to flush routes and remove the incoming packet mark rule, so do it + // after we've rematched networks with requests which should make a potential + // fallback network the default or requested a new network from the + // NetworkFactories, so network traffic isn't interrupted for an unnecessarily + // long time. + try { + mNetd.removeNetwork(nai.network.netId); + } catch (Exception e) { + loge("Exception removing network: " + e); + } + mDnsManager.removeNetwork(nai.network); + } + synchronized (mNetworkForNetId) { + mNetIdInUse.delete(nai.network.netId); + } + } + // If this method proves to be too slow then we can maintain a separate // pendingIntent => NetworkRequestInfo map. // This method assumes that every non-null PendingIntent maps to exactly 1 NetworkRequestInfo. @@ -5577,6 +5587,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } updateUids(networkAgent, networkAgent.networkCapabilities, null); } + disconnectAndDestroyNetwork(networkAgent); } else if ((oldInfo != null && oldInfo.getState() == NetworkInfo.State.SUSPENDED) || state == NetworkInfo.State.SUSPENDED) { // going into or coming out of SUSPEND: rescore and notify From 6ae4b8847276106db917d528ac0904b6da71e340 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Mon, 14 May 2018 13:49:07 -0600 Subject: [PATCH 20/20] Let tests enumerate all transports/capabilities. This gives them a way to collect all included values without resorting to manual probing of each newly added value. Cherry-pick of ag/4052941 with minor conflicts in the imports. Bug: 16207332 Test: atest com.android.cts.net.HostsideVpnTests Change-Id: Ia764b3412bf834890612378e0c3846913f4e0a06 Merged-In: Ie5cd22cfa2b6a60510fd1e31d7ebcd8f6cc890a0 Merged-In: If07e77c92046807235229a4f67ee087bdd7bccf1 --- core/java/android/net/NetworkCapabilities.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 0f2bfba86c..83553dff2f 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -18,6 +18,7 @@ package android.net; import android.annotation.IntDef; import android.annotation.SystemApi; +import android.annotation.TestApi; import android.net.ConnectivityManager.NetworkCallback; import android.os.Parcel; import android.os.Parcelable; @@ -435,6 +436,7 @@ public final class NetworkCapabilities implements Parcelable { * @return an array of capability values for this instance. * @hide */ + @TestApi public @NetCapability int[] getCapabilities() { return BitUtils.unpackBits(mNetworkCapabilities); } @@ -699,6 +701,7 @@ public final class NetworkCapabilities implements Parcelable { * @return an array of transport type values for this instance. * @hide */ + @TestApi public @Transport int[] getTransportTypes() { return BitUtils.unpackBits(mTransportTypes); }