From cf7bb0ee0634dbcce05d11e7f8b91383663cd964 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Wed, 18 Mar 2020 21:11:29 +0000 Subject: [PATCH 1/8] Restrict VPN Diagnostics callbacks to underlying networks. ConnectivityDiagnosticsCallbacks should only be invoked for the underlying networks declared by active VPNs. This encourages VPN apps to declare their underlying networks. The previous permission model for VPNs allowed active VPNs to receive callbacks on any network. Bug: 148903617 Test: atest FrameworksNetTests Change-Id: Ic08cdd2e2532580fda0fd3034e2bdff27e0ff84b Merged-In: Ic08cdd2e2532580fda0fd3034e2bdff27e0ff84b (cherry picked from commit e1f0c56f74593d3781bfa4ee4871a5efbabe303c) --- .../android/server/ConnectivityService.java | 9 ++++-- .../server/ConnectivityServiceTest.java | 28 +++++++++++++++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4d504e7423..84f144eda7 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -7949,10 +7949,13 @@ public class ConnectivityService extends IConnectivityManager.Stub return false; } + final Network[] underlyingNetworks; synchronized (mVpns) { - if (getVpnIfOwner(callbackUid) != null) { - return true; - } + final Vpn vpn = getVpnIfOwner(callbackUid); + underlyingNetworks = (vpn == null) ? null : vpn.getUnderlyingNetworks(); + } + if (underlyingNetworks != null) { + if (Arrays.asList(underlyingNetworks).contains(nai.network)) return true; } // Administrator UIDs also contains the Owner UID diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 4bfb51bca2..2f86d2ceda 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -307,6 +307,8 @@ public class ConnectivityServiceTest { private static final long TIMESTAMP = 1234L; + private static final int NET_ID = 110; + private static final String CLAT_PREFIX = "v4-"; private static final String MOBILE_IFNAME = "test_rmnet_data0"; private static final String WIFI_IFNAME = "test_wlan0"; @@ -1015,6 +1017,7 @@ public class ConnectivityServiceTest { private int mVpnType = VpnManager.TYPE_VPN_SERVICE; private VpnInfo mVpnInfo; + private Network[] mUnderlyingNetworks; public MockVpn(int userId) { super(startHandlerThreadAndReturnLooper(), mServiceContext, mNetworkManagementService, @@ -1104,9 +1107,21 @@ public class ConnectivityServiceTest { return super.getVpnInfo(); } - private void setVpnInfo(VpnInfo vpnInfo) { + private synchronized void setVpnInfo(VpnInfo vpnInfo) { mVpnInfo = vpnInfo; } + + @Override + public synchronized Network[] getUnderlyingNetworks() { + if (mUnderlyingNetworks != null) return mUnderlyingNetworks; + + return super.getUnderlyingNetworks(); + } + + /** Don't override behavior for {@link Vpn#setUnderlyingNetworks}. */ + private synchronized void overrideUnderlyingNetworks(Network[] underlyingNetworks) { + mUnderlyingNetworks = underlyingNetworks; + } } private void mockVpn(int uid) { @@ -6824,9 +6839,10 @@ public class ConnectivityServiceTest { @Test public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception { + final Network network = new Network(NET_ID); final NetworkAgentInfo naiWithoutUid = new NetworkAgentInfo( - null, null, null, null, null, new NetworkCapabilities(), 0, + null, null, network, null, null, new NetworkCapabilities(), 0, mServiceContext, null, null, mService, null, null, null, 0); setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, @@ -6839,11 +6855,19 @@ public class ConnectivityServiceTest { info.ownerUid = Process.myUid(); info.vpnIface = "interface"; mMockVpn.setVpnInfo(info); + mMockVpn.overrideUnderlyingNetworks(new Network[] {network}); assertTrue( "Active VPN permission not applied", mService.checkConnectivityDiagnosticsPermissions( Process.myPid(), Process.myUid(), naiWithoutUid, mContext.getOpPackageName())); + + mMockVpn.overrideUnderlyingNetworks(null); + assertFalse( + "VPN shouldn't receive callback on non-underlying network", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid(), Process.myUid(), naiWithoutUid, + mContext.getOpPackageName())); } @Test From 21105c9771d1f65db9b3de5a40cbe5d62e196277 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Mon, 16 Mar 2020 20:37:44 +0000 Subject: [PATCH 2/8] Clean up unit testing for ConnectivityDiagnostics. Connectivity Diagnostics included an unnecessary try-catch for verifying that the permissions check for invoking ConnectivityDiagnosticsCallbacks doesn't throw when the uid and package name do not match. Bug: 149119324 Test: atest FrameworksNetTests Change-Id: Ie302b1f4f437e819fdd15ec28adb0b56750c2c53 Merged-In: Ie302b1f4f437e819fdd15ec28adb0b56750c2c53 (cherry picked from commit 66b5e081b785a3b7e2a032342d4d424905581cf9) --- .../android/server/ConnectivityServiceTest.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 2f86d2ceda..83399b8076 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6810,15 +6810,11 @@ public class ConnectivityServiceTest { mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); - try { - assertFalse( - "Mismatched uid/package name should not pass the location permission check", - mService.checkConnectivityDiagnosticsPermissions( - Process.myPid() + 1, Process.myUid() + 1, naiWithoutUid, - mContext.getOpPackageName())); - } catch (SecurityException e) { - fail("checkConnectivityDiagnosticsPermissions shouldn't surface a SecurityException"); - } + assertFalse( + "Mismatched uid/package name should not pass the location permission check", + mService.checkConnectivityDiagnosticsPermissions( + Process.myPid() + 1, Process.myUid() + 1, naiWithoutUid, + mContext.getOpPackageName())); } @Test From 68d0e42789c51b030b1496d202ea82b4c4e8bb61 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 24 Mar 2020 23:16:35 +0900 Subject: [PATCH 3/8] Update the NetworkAgent API for council comments Bug: 152238712 Test: FrameworksNetTests NetworkStackTests Change-Id: I9a2691f783f4449348c3f767568e05620f0b9df5 --- core/java/android/net/NetworkAgent.java | 135 ++++++++++++++++----- core/java/android/net/NetworkRequest.java | 2 - core/java/android/net/SocketKeepalive.java | 10 ++ 3 files changed, 118 insertions(+), 29 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 5c754a1b97..8119df9217 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -16,6 +16,8 @@ package android.net; +import android.annotation.IntDef; +import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.SystemApi; @@ -32,18 +34,52 @@ import android.util.Log; import com.android.internal.util.AsyncChannel; import com.android.internal.util.Protocol; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; /** - * A Utility class for handling for communicating between bearer-specific + * A utility class for handling for communicating between bearer-specific * code and ConnectivityService. * + * An agent manages the life cycle of a network. A network starts its + * life cycle when {@link register} is called on NetworkAgent. The network + * is then connecting. When full L3 connectivity has been established, + * the agent shoud call {@link setConnected} to inform the system that + * this network is ready to use. When the network disconnects its life + * ends and the agent should call {@link unregister}, at which point the + * system will clean up and free resources. + * Any reconnection becomes a new logical network, so after a network + * is disconnected the agent cannot be used any more. Network providers + * should create a new NetworkAgent instance to handle new connections. + * * A bearer may have more than one NetworkAgent if it can simultaneously * support separate networks (IMS / Internet / MMS Apns on cellular, or * perhaps connections with different SSID or P2P for Wi-Fi). * + * This class supports methods to start and stop sending keepalive packets. + * Keepalive packets are typically sent at periodic intervals over a network + * with NAT when there is no other traffic to avoid the network forcefully + * closing the connection. NetworkAgents that manage technologies that + * have hardware support for keepalive should implement the related + * methods to save battery life. NetworkAgent that cannot get support + * without waking up the CPU should not, as this would be prohibitive in + * terms of battery - these agents should simply not override the related + * methods, which results in the implementation returning + * {@link SocketKeepalive.ERROR_UNSUPPORTED} as appropriate. + * + * Keepalive packets need to be sent at relatively frequent intervals + * (a few seconds to a few minutes). As the contents of keepalive packets + * depend on the current network status, hardware needs to be configured + * to send them and has a limited amount of memory to do so. The HAL + * formalizes this as slots that an implementation can configure to send + * the correct packets. Devices typically have a small number of slots + * per radio technology, and the specific number of slots for each + * technology is specified in configuration files. + * {@see SocketKeepalive} for details. + * * @hide */ @SystemApi @@ -65,7 +101,7 @@ public abstract class NetworkAgent { private final String LOG_TAG; private static final boolean DBG = true; private static final boolean VDBG = false; - private final ArrayListmPreConnectedQueue = new ArrayList(); + private final ArrayList mPreConnectedQueue = new ArrayList(); private volatile long mLastBwRefreshTime = 0; private static final long BW_REFRESH_MIN_WIN_MS = 500; private boolean mBandwidthUpdateScheduled = false; @@ -74,6 +110,8 @@ public abstract class NetworkAgent { // into the internal API of ConnectivityService. @NonNull private NetworkInfo mNetworkInfo; + @NonNull + private final Object mRegisterLock = new Object(); /** * The ID of the {@link NetworkProvider} that created this object, or @@ -158,6 +196,14 @@ public abstract class NetworkAgent { */ public static final int VALIDATION_STATUS_NOT_VALID = 2; + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef(prefix = { "VALIDATION_STATUS_" }, value = { + VALIDATION_STATUS_VALID, + VALIDATION_STATUS_NOT_VALID + }) + public @interface ValidationStatus {} + // TODO: remove. /** @hide */ public static final int VALID_NETWORK = 1; @@ -202,7 +248,7 @@ public abstract class NetworkAgent { * Sent by ConnectivityService to the NetworkAgent to request that the specified packet be sent * periodically on the given interval. * - * arg1 = the slot number of the keepalive to start + * arg1 = the hardware slot number of the keepalive to start * arg2 = interval in seconds * obj = KeepalivePacketData object describing the data to be sent * @@ -214,7 +260,7 @@ public abstract class NetworkAgent { /** * Requests that the specified keepalive packet be stopped. * - * arg1 = slot number of the keepalive to stop. + * arg1 = hardware slot number of the keepalive to stop. * * Also used internally by ConnectivityService / KeepaliveTracker, with different semantics. * @hide @@ -229,7 +275,7 @@ public abstract class NetworkAgent { * This is also sent by KeepaliveTracker to the app's {@link SocketKeepalive}, * so that the app's {@link SocketKeepalive.Callback} methods can be called. * - * arg1 = slot number of the keepalive + * arg1 = hardware slot number of the keepalive * arg2 = error code * @hide */ @@ -259,7 +305,7 @@ public abstract class NetworkAgent { * remote site will send ACK packets in response to the keepalive packets, the firmware also * needs to be configured to properly filter the ACKs to prevent the system from waking up. * This does not happen with UDP, so this message is TCP-specific. - * arg1 = slot number of the keepalive to filter for. + * arg1 = hardware slot number of the keepalive to filter for. * obj = the keepalive packet to send repeatedly. * @hide */ @@ -268,7 +314,7 @@ public abstract class NetworkAgent { /** * Sent by the KeepaliveTracker to NetworkAgent to remove a packet filter. See * {@link #CMD_ADD_KEEPALIVE_PACKET_FILTER}. - * arg1 = slot number of the keepalive packet filter to remove. + * arg1 = hardware slot number of the keepalive packet filter to remove. * @hide */ public static final int CMD_REMOVE_KEEPALIVE_PACKET_FILTER = BASE + 17; @@ -441,7 +487,15 @@ public abstract class NetworkAgent { + (msg.arg1 == VALID_NETWORK ? "VALID, " : "INVALID, ") + redirectUrl); } - onValidationStatus(msg.arg1 /* status */, redirectUrl); + Uri uri = null; + try { + if (null != redirectUrl) { + uri = Uri.parse(redirectUrl); + } + } catch (Exception e) { + Log.wtf(LOG_TAG, "Surprising URI : " + redirectUrl, e); + } + onValidationStatus(msg.arg1 /* status */, uri); break; } case CMD_SAVE_ACCEPT_UNVALIDATED: { @@ -489,19 +543,29 @@ public abstract class NetworkAgent { /** * Register this network agent with ConnectivityService. + * + * This method can only be called once per network agent. + * * @return the Network associated with this network agent (which can also be obtained later * by calling getNetwork() on this agent). + * @throws IllegalStateException thrown by the system server if this network agent is + * already registered. */ @NonNull public Network register() { if (VDBG) log("Registering NetworkAgent"); final ConnectivityManager cm = (ConnectivityManager) mInitialConfiguration.context .getSystemService(Context.CONNECTIVITY_SERVICE); - mNetwork = cm.registerNetworkAgent(new Messenger(mHandler), - new NetworkInfo(mInitialConfiguration.info), - mInitialConfiguration.properties, mInitialConfiguration.capabilities, - mInitialConfiguration.score, mInitialConfiguration.config, providerId); - mInitialConfiguration = null; // All this memory can now be GC'd + synchronized (mRegisterLock) { + if (mNetwork != null) { + throw new IllegalStateException("Agent already registered"); + } + mNetwork = cm.registerNetworkAgent(new Messenger(mHandler), + new NetworkInfo(mInitialConfiguration.info), + mInitialConfiguration.properties, mInitialConfiguration.capabilities, + mInitialConfiguration.score, mInitialConfiguration.config, providerId); + mInitialConfiguration = null; // All this memory can now be GC'd + } return mNetwork; } @@ -544,13 +608,14 @@ public abstract class NetworkAgent { * Must be called by the agent when the network's {@link LinkProperties} change. * @param linkProperties the new LinkProperties. */ - public void sendLinkProperties(@NonNull LinkProperties linkProperties) { + public final void sendLinkProperties(@NonNull LinkProperties linkProperties) { Objects.requireNonNull(linkProperties); queueOrSendMessage(EVENT_NETWORK_PROPERTIES_CHANGED, new LinkProperties(linkProperties)); } /** * Inform ConnectivityService that this agent has now connected. + * Call {@link #unregister} to disconnect. */ public void setConnected() { if (mIsLegacy) { @@ -569,8 +634,7 @@ public abstract class NetworkAgent { */ public void unregister() { if (mIsLegacy) { - throw new UnsupportedOperationException( - "Legacy agents can't call unregister."); + throw new UnsupportedOperationException("Legacy agents can't call unregister."); } mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, null, null); queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo); @@ -626,7 +690,7 @@ public abstract class NetworkAgent { * @hide TODO: expose something better. */ @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023) - public void sendNetworkInfo(NetworkInfo networkInfo) { + public final void sendNetworkInfo(NetworkInfo networkInfo) { if (!mIsLegacy) { throw new UnsupportedOperationException("Only legacy agents can call sendNetworkInfo."); } @@ -637,7 +701,7 @@ public abstract class NetworkAgent { * Must be called by the agent when the network's {@link NetworkCapabilities} change. * @param networkCapabilities the new NetworkCapabilities. */ - public void sendNetworkCapabilities(@NonNull NetworkCapabilities networkCapabilities) { + public final void sendNetworkCapabilities(@NonNull NetworkCapabilities networkCapabilities) { Objects.requireNonNull(networkCapabilities); mBandwidthUpdatePending.set(false); mLastBwRefreshTime = System.currentTimeMillis(); @@ -647,9 +711,10 @@ public abstract class NetworkAgent { /** * Must be called by the agent to update the score of this network. - * @param score the new score. + * + * @param score the new score, between 0 and 99. */ - public void sendNetworkScore(int score) { + public final void sendNetworkScore(@IntRange(from = 0, to = 99) int score) { if (score < 0) { throw new IllegalArgumentException("Score must be >= 0"); } @@ -737,11 +802,11 @@ public abstract class NetworkAgent { * subsequent attempts to validate connectivity that fail. * * @param status one of {@code VALIDATION_STATUS_VALID} or {@code VALIDATION_STATUS_NOT_VALID}. - * @param redirectUrl If Internet connectivity is being redirected (e.g., on a captive portal), + * @param redirectUri If Internet connectivity is being redirected (e.g., on a captive portal), * this is the destination the probes are being redirected to, otherwise {@code null}. */ - public void onValidationStatus(int status, @Nullable String redirectUrl) { - networkStatus(status, redirectUrl); + public void onValidationStatus(@ValidationStatus int status, @Nullable Uri redirectUri) { + networkStatus(status, redirectUri.toString()); } /** @hide TODO delete once subclasses have moved to onValidationStatus */ protected void networkStatus(int status, String redirectUrl) { @@ -770,7 +835,12 @@ public abstract class NetworkAgent { * @param intervalSeconds the interval between packets * @param packet the packet to send. */ - public void onStartSocketKeepalive(int slot, int intervalSeconds, + // seconds is from SocketKeepalive.MIN_INTERVAL_SEC to MAX_INTERVAL_SEC, but these should + // not be exposed as constants because they may change in the future (API guideline 4.8) + // and should have getters if exposed at all. Getters can't be used in the annotation, + // so the values unfortunately need to be copied. + public void onStartSocketKeepalive(int slot, + @IntRange(from = 10, to = 3600) int intervalSeconds, @NonNull KeepalivePacketData packet) { Message msg = mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, slot, intervalSeconds, packet); @@ -801,9 +871,11 @@ public abstract class NetworkAgent { * Must be called by the agent when a socket keepalive event occurs. * * @param slot the hardware slot on which the event occurred. - * @param event the event that occurred. + * @param event the event that occurred, as one of the SocketKeepalive.ERROR_* + * or SocketKeepalive.SUCCESS constants. */ - public void sendSocketKeepaliveEvent(int slot, int event) { + public final void sendSocketKeepaliveEvent(int slot, + @SocketKeepalive.KeepaliveEvent int event) { queueOrSendMessage(EVENT_SOCKET_KEEPALIVE, slot, event); } /** @hide TODO delete once callers have moved to sendSocketKeepaliveEvent */ @@ -845,9 +917,18 @@ public abstract class NetworkAgent { } /** - * Called by ConnectivityService to inform this network transport of signal strength thresholds + * Called by ConnectivityService to inform this network agent of signal strength thresholds * that when crossed should trigger a system wakeup and a NetworkCapabilities update. * + * When the system updates the list of thresholds that should wake up the CPU for a + * given agent it will call this method on the agent. The agent that implement this + * should implement it in hardware so as to ensure the CPU will be woken up on breach. + * Agents are expected to react to a breach by sending an updated NetworkCapabilities + * object with the appropriate signal strength to sendNetworkCapabilities. + * + * The specific units are bearer-dependent. See details on the units and requests in + * {@link NetworkCapabilities.Builder#setSignalStrength}. + * * @param thresholds the array of thresholds that should trigger wakeups. */ public void onSignalStrengthThresholdsUpdated(@NonNull int[] thresholds) { diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 964f13f39e..798856d13b 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -473,9 +473,7 @@ public class NetworkRequest implements Parcelable { * * @param nc Capabilities that should satisfy this NetworkRequest. null capabilities do not * satisfy any request. - * @hide */ - @SystemApi public boolean satisfiedBy(@Nullable NetworkCapabilities nc) { return networkCapabilities.satisfiedByNetworkCapabilities(nc); } diff --git a/core/java/android/net/SocketKeepalive.java b/core/java/android/net/SocketKeepalive.java index fc9a8f63c1..8ff8f4c48c 100644 --- a/core/java/android/net/SocketKeepalive.java +++ b/core/java/android/net/SocketKeepalive.java @@ -109,6 +109,16 @@ public abstract class SocketKeepalive implements AutoCloseable { }) public @interface ErrorCode {} + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef(value = { + SUCCESS, + ERROR_INVALID_LENGTH, + ERROR_UNSUPPORTED, + ERROR_INSUFFICIENT_RESOURCES + }) + public @interface KeepaliveEvent {} + /** * The minimum interval in seconds between keepalive packet transmissions. * From 15a3adbdf3789269b9e91022ca4c3f99bef9dfda Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 25 Mar 2020 01:24:04 +0900 Subject: [PATCH 4/8] Update NetworkAgentConfig API for council comments Bug: 152238712 Test: FrameworksNetTests NetworkStackTests Change-Id: Idca9f243a5c955f4caa30921ee520e1a93b0d11a --- .../java/android/net/ConnectivityManager.java | 30 +++++++++++++++++++ core/java/android/net/NetworkAgentConfig.java | 3 +- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 2a323e5ec9..7332ede0b9 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -705,6 +705,36 @@ public class ConnectivityManager { @Deprecated public static final int TYPE_TEST = 18; // TODO: Remove this once NetworkTypes are unused. + /** + * @deprecated Use {@link NetworkCapabilities} instead. + * @hide + */ + @Deprecated + @Retention(RetentionPolicy.SOURCE) + @IntDef(prefix = { "TYPE_" }, value = { + TYPE_NONE, + TYPE_MOBILE, + TYPE_WIFI, + TYPE_MOBILE_MMS, + TYPE_MOBILE_SUPL, + TYPE_MOBILE_DUN, + TYPE_MOBILE_HIPRI, + TYPE_WIMAX, + TYPE_BLUETOOTH, + TYPE_DUMMY, + TYPE_ETHERNET, + TYPE_MOBILE_FOTA, + TYPE_MOBILE_IMS, + TYPE_MOBILE_CBS, + TYPE_WIFI_P2P, + TYPE_MOBILE_IA, + TYPE_MOBILE_EMERGENCY, + TYPE_PROXY, + TYPE_VPN, + TYPE_TEST + }) + public @interface LegacyNetworkType {} + // Deprecated constants for return values of startUsingNetworkFeature. They used to live // in com.android.internal.telephony.PhoneConstants until they were made inaccessible. private static final int DEPRECATED_PHONE_CONSTANT_APN_ALREADY_ACTIVE = 0; diff --git a/core/java/android/net/NetworkAgentConfig.java b/core/java/android/net/NetworkAgentConfig.java index ca9328a713..fee868a93b 100644 --- a/core/java/android/net/NetworkAgentConfig.java +++ b/core/java/android/net/NetworkAgentConfig.java @@ -155,6 +155,7 @@ public final class NetworkAgentConfig implements Parcelable { /** * @return the legacy type */ + @ConnectivityManager.LegacyNetworkType public int getLegacyType() { return legacyType; } @@ -206,7 +207,7 @@ public final class NetworkAgentConfig implements Parcelable { /** * Builder class to facilitate constructing {@link NetworkAgentConfig} objects. */ - public static class Builder { + public static final class Builder { private final NetworkAgentConfig mConfig = new NetworkAgentConfig(); /** From fd23dbbf07475954fa34b6f46c802a026b4e81b8 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 25 Mar 2020 02:09:26 +0900 Subject: [PATCH 5/8] Update the NetworkProvider API for council comments Bug: 152238712 Test: FrameworksNetTests NetworkStackTests Change-Id: I6b086572cfc72a0727f4510351cff0e74cbc4302 --- core/java/android/net/NetworkProvider.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/core/java/android/net/NetworkProvider.java b/core/java/android/net/NetworkProvider.java index 418d6915d4..75086cf82b 100644 --- a/core/java/android/net/NetworkProvider.java +++ b/core/java/android/net/NetworkProvider.java @@ -16,6 +16,7 @@ package android.net; +import android.annotation.IntRange; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresPermission; @@ -33,8 +34,8 @@ import android.util.Log; * {@link NetworkAgent}s. The networks can then provide connectivity to apps and can be interacted * with via networking APIs such as {@link ConnectivityManager}. * - * Subclasses should implement {@link #onNetworkRequested} and {@link #onRequestWithdrawn} to - * receive {@link NetworkRequest}s sent by the system and by apps. A network that is not the + * Subclasses should implement {@link #onNetworkRequested} and {@link #onNetworkRequestWithdrawn} + * to receive {@link NetworkRequest}s sent by the system and by apps. A network that is not the * best (highest-scoring) network for any request is generally not used by the system, and torn * down. * @@ -77,7 +78,7 @@ public class NetworkProvider { * Constructs a new NetworkProvider. * * @param looper the Looper on which to run {@link #onNetworkRequested} and - * {@link #onRequestWithdrawn}. + * {@link #onNetworkRequestWithdrawn}. * @param name the name of the listener, used only for debugging. * * @hide @@ -94,7 +95,7 @@ public class NetworkProvider { onNetworkRequested((NetworkRequest) m.obj, m.arg1, m.arg2); break; case CMD_CANCEL_REQUEST: - onRequestWithdrawn((NetworkRequest) m.obj); + onNetworkRequestWithdrawn((NetworkRequest) m.obj); break; default: Log.e(mName, "Unhandled message: " + m.what); @@ -142,14 +143,15 @@ public class NetworkProvider { * @hide */ @SystemApi - public void onNetworkRequested(@NonNull NetworkRequest request, int score, int providerId) {} + public void onNetworkRequested(@NonNull NetworkRequest request, + @IntRange(from = 0, to = 99) int score, int providerId) {} /** * Called when a NetworkRequest is withdrawn. * @hide */ @SystemApi - public void onRequestWithdrawn(@NonNull NetworkRequest request) {} + public void onNetworkRequestWithdrawn(@NonNull NetworkRequest request) {} /** * Asserts that no provider will ever be able to satisfy the specified request. The provider @@ -157,7 +159,7 @@ public class NetworkProvider { * satisfying this request, and that the request cannot be satisfied. The application filing the * request will receive an {@link NetworkCallback#onUnavailable()} callback. * - * @param request the request that cannot be fulfilled + * @param request the request that permanently cannot be fulfilled * @hide */ @SystemApi From 812e8cb87e72169b71cfa9d7759f0da92529f2e3 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 27 Mar 2020 15:00:38 +0900 Subject: [PATCH 6/8] Address further API council comments. Test: FrameworksNetTests NetworkStackTests Bug: 152238712 Change-Id: I8a785ae0e74e659c317deaaa28c203356c7766ed --- core/java/android/net/NetworkAgent.java | 31 ++++++++++++++-------- core/java/android/net/SocketKeepalive.java | 3 ++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 8119df9217..fe90a84579 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -36,6 +36,7 @@ import com.android.internal.util.Protocol; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.time.Duration; import java.util.ArrayList; import java.util.Objects; import java.util.concurrent.atomic.AtomicBoolean; @@ -47,7 +48,7 @@ import java.util.concurrent.atomic.AtomicBoolean; * An agent manages the life cycle of a network. A network starts its * life cycle when {@link register} is called on NetworkAgent. The network * is then connecting. When full L3 connectivity has been established, - * the agent shoud call {@link setConnected} to inform the system that + * the agent shoud call {@link markConnected} to inform the system that * this network is ready to use. When the network disconnects its life * ends and the agent should call {@link unregister}, at which point the * system will clean up and free resources. @@ -503,7 +504,8 @@ public abstract class NetworkAgent { break; } case CMD_START_SOCKET_KEEPALIVE: { - onStartSocketKeepalive(msg.arg1 /* slot */, msg.arg2 /* interval */, + onStartSocketKeepalive(msg.arg1 /* slot */, + Duration.ofSeconds(msg.arg2) /* interval */, (KeepalivePacketData) msg.obj /* packet */); break; } @@ -617,10 +619,10 @@ public abstract class NetworkAgent { * Inform ConnectivityService that this agent has now connected. * Call {@link #unregister} to disconnect. */ - public void setConnected() { + public void markConnected() { if (mIsLegacy) { throw new UnsupportedOperationException( - "Legacy agents can't call setConnected."); + "Legacy agents can't call markConnected."); } mNetworkInfo.setDetailedState(NetworkInfo.DetailedState.CONNECTED, null, null); queueOrSendMessage(EVENT_NETWORK_INFO_CHANGED, mNetworkInfo); @@ -798,8 +800,8 @@ public abstract class NetworkAgent { * {@code VALIDATION_STATUS_VALID} if Internet connectivity was validated, * {@code VALIDATION_STATUS_NOT_VALID} if Internet connectivity was not validated. * - * This may be called multiple times as network status changes, or if there are multiple - * subsequent attempts to validate connectivity that fail. + * This is guaranteed to be called again when the network status changes, but the system + * may also call this multiple times even if the status does not change. * * @param status one of {@code VALIDATION_STATUS_VALID} or {@code VALIDATION_STATUS_NOT_VALID}. * @param redirectUri If Internet connectivity is being redirected (e.g., on a captive portal), @@ -832,18 +834,25 @@ public abstract class NetworkAgent { * Requests that the network hardware send the specified packet at the specified interval. * * @param slot the hardware slot on which to start the keepalive. - * @param intervalSeconds the interval between packets + * @param interval the interval between packets, between 10 and 3600. Note that this API + * does not support sub-second precision and will round off the request. * @param packet the packet to send. */ // seconds is from SocketKeepalive.MIN_INTERVAL_SEC to MAX_INTERVAL_SEC, but these should // not be exposed as constants because they may change in the future (API guideline 4.8) // and should have getters if exposed at all. Getters can't be used in the annotation, // so the values unfortunately need to be copied. - public void onStartSocketKeepalive(int slot, - @IntRange(from = 10, to = 3600) int intervalSeconds, + public void onStartSocketKeepalive(int slot, @NonNull Duration interval, @NonNull KeepalivePacketData packet) { - Message msg = mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, slot, intervalSeconds, - packet); + final long intervalSeconds = interval.getSeconds(); + if (intervalSeconds < SocketKeepalive.MIN_INTERVAL_SEC + || intervalSeconds > SocketKeepalive.MAX_INTERVAL_SEC) { + throw new IllegalArgumentException("Interval needs to be comprised between " + + SocketKeepalive.MIN_INTERVAL_SEC + " and " + SocketKeepalive.MAX_INTERVAL_SEC + + " but was " + intervalSeconds); + } + final Message msg = mHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, slot, + (int) intervalSeconds, packet); startSocketKeepalive(msg); msg.recycle(); } diff --git a/core/java/android/net/SocketKeepalive.java b/core/java/android/net/SocketKeepalive.java index 8ff8f4c48c..46aef10258 100644 --- a/core/java/android/net/SocketKeepalive.java +++ b/core/java/android/net/SocketKeepalive.java @@ -115,7 +115,8 @@ public abstract class SocketKeepalive implements AutoCloseable { SUCCESS, ERROR_INVALID_LENGTH, ERROR_UNSUPPORTED, - ERROR_INSUFFICIENT_RESOURCES + ERROR_INSUFFICIENT_RESOURCES, + ERROR_HARDWARE_UNSUPPORTED }) public @interface KeepaliveEvent {} From 51b8e287ad9964123217deeb8d78973476233874 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 27 Mar 2020 17:57:34 +0900 Subject: [PATCH 7/8] Rename satisfiedBy into canBeSatisfiedBy Test: FrameworksNetTests NetworkStackTests Bug: 152238712 Change-Id: I076876a6662bde143ef7d315ce3767acafff93c1 --- core/java/android/net/NetworkCapabilities.java | 2 +- core/java/android/net/NetworkRequest.java | 2 +- .../java/com/android/server/ConnectivityServiceTest.java | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index af9414c566..91ef911ef2 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -1122,7 +1122,7 @@ public final class NetworkCapabilities implements Parcelable { } private boolean satisfiedBySpecifier(NetworkCapabilities nc) { - return mNetworkSpecifier == null || mNetworkSpecifier.satisfiedBy(nc.mNetworkSpecifier) + return mNetworkSpecifier == null || mNetworkSpecifier.canBeSatisfiedBy(nc.mNetworkSpecifier) || nc.mNetworkSpecifier instanceof MatchAllNetworkSpecifier; } diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 798856d13b..a6bd74a587 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -474,7 +474,7 @@ public class NetworkRequest implements Parcelable { * @param nc Capabilities that should satisfy this NetworkRequest. null capabilities do not * satisfy any request. */ - public boolean satisfiedBy(@Nullable NetworkCapabilities nc) { + public boolean canBeSatisfiedBy(@Nullable NetworkCapabilities nc) { return networkCapabilities.satisfiedByNetworkCapabilities(nc); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 4bfb51bca2..83f4c9817d 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -2896,7 +2896,7 @@ public class ConnectivityServiceTest { class ConfidentialMatchAllNetworkSpecifier extends NetworkSpecifier implements Parcelable { @Override - public boolean satisfiedBy(NetworkSpecifier other) { + public boolean canBeSatisfiedBy(NetworkSpecifier other) { return true; } @@ -2924,7 +2924,7 @@ public class ConnectivityServiceTest { } @Override - public boolean satisfiedBy(NetworkSpecifier other) { + public boolean canBeSatisfiedBy(NetworkSpecifier other) { if (other instanceof LocalStringNetworkSpecifier) { return TextUtils.equals(mString, ((LocalStringNetworkSpecifier) other).mString); @@ -3045,7 +3045,10 @@ public class ConnectivityServiceTest { }); class NonParcelableSpecifier extends NetworkSpecifier { - public boolean satisfiedBy(NetworkSpecifier other) { return false; } + @Override + public boolean canBeSatisfiedBy(NetworkSpecifier other) { + return false; + } }; class ParcelableSpecifier extends NonParcelableSpecifier implements Parcelable { @Override public int describeContents() { return 0; } From f4667b4925ffefe55a5f59b759f4705e57152ae4 Mon Sep 17 00:00:00 2001 From: Hall Liu Date: Thu, 26 Mar 2020 19:09:30 -0700 Subject: [PATCH 8/8] Add gating, logging for PhoneStateListener's limit Add gating via PlatformCompat and DeviceConfig and logging via PlatformCompat to the limit instituted on per-process listeners Fixes: 152074216 Test: atest CtsTelephonyHostCases Change-Id: I4d6681d90705b68c3349f4124e434a29b50fd3a2 --- tests/net/java/com/android/server/ConnectivityServiceTest.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 6985415a6a..3904b6f8b5 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -205,6 +205,7 @@ import android.os.UserManager; import android.provider.Settings; import android.security.KeyStore; import android.system.Os; +import android.telephony.TelephonyManager; import android.test.mock.MockContentResolver; import android.text.TextUtils; import android.util.ArraySet; @@ -345,6 +346,7 @@ public class ConnectivityServiceTest { @Mock IBinder mIBinder; @Mock LocationManager mLocationManager; @Mock AppOpsManager mAppOpsManager; + @Mock TelephonyManager mTelephonyManager; private ArgumentCaptor mResolverParamsParcelCaptor = ArgumentCaptor.forClass(ResolverParamsParcel.class); @@ -432,6 +434,7 @@ public class ConnectivityServiceTest { if (Context.ALARM_SERVICE.equals(name)) return mAlarmManager; if (Context.LOCATION_SERVICE.equals(name)) return mLocationManager; if (Context.APP_OPS_SERVICE.equals(name)) return mAppOpsManager; + if (Context.TELEPHONY_SERVICE.equals(name)) return mTelephonyManager; return super.getSystemService(name); }