From 1a407651019718d0c3db426b9ef96c49632453ba Mon Sep 17 00:00:00 2001 From: paulhu Date: Fri, 22 Mar 2019 16:35:06 +0800 Subject: [PATCH] Address leftover comments on 923337 and 930217. - Restrict unprivileged apps to use NetworkRequest.Builder#setSignalStrength. - Remove the "throws NullPointerException" in CaptivePortalProbeSpec constructor. - Remove the null check in LinkProperties. - Add annotataion into all ConnectivityManager.NetworkCallback methods. Change-Id: Id275cac1d6a30d7515cd7b113394f5e8a0179314 Fix: 129097486 Test: atest FrameworksNetTests --- .../java/android/net/ConnectivityManager.java | 24 ++++++----- core/java/android/net/LinkProperties.java | 8 +--- core/java/android/net/NetworkRequest.java | 5 +++ .../android/server/ConnectivityService.java | 25 ++++++++++- .../server/ConnectivityServiceTest.java | 41 +++++++++++++++++++ 5 files changed, 83 insertions(+), 20 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index ae93cf0197..79560f50b4 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3231,7 +3231,7 @@ public class ConnectivityManager { * * @hide */ - public void onPreCheck(Network network) {} + public void onPreCheck(@NonNull Network network) {} /** * Called when the framework connects and has declared a new network ready for use. @@ -3244,8 +3244,9 @@ public class ConnectivityManager { * @param blocked Whether access to the {@link Network} is blocked due to system policy. * @hide */ - public void onAvailable(Network network, NetworkCapabilities networkCapabilities, - LinkProperties linkProperties, boolean blocked) { + public void onAvailable(@NonNull Network network, + @NonNull NetworkCapabilities networkCapabilities, + @NonNull LinkProperties linkProperties, boolean blocked) { // Internally only this method is called when a new network is available, and // it calls the callback in the same way and order that older versions used // to call so as not to change the behavior. @@ -3269,7 +3270,7 @@ public class ConnectivityManager { * * @param network The {@link Network} of the satisfying network. */ - public void onAvailable(Network network) {} + public void onAvailable(@NonNull Network network) {} /** * Called when the network is about to be disconnected. Often paired with an @@ -3285,7 +3286,7 @@ public class ConnectivityManager { * network connected. Note that the network may suffer a * hard loss at any time. */ - public void onLosing(Network network, int maxMsToLive) {} + public void onLosing(@NonNull Network network, int maxMsToLive) {} /** * Called when the framework has a hard loss of the network or when the @@ -3293,7 +3294,7 @@ public class ConnectivityManager { * * @param network The {@link Network} lost. */ - public void onLost(Network network) {} + public void onLost(@NonNull Network network) {} /** * Called if no network is found in the timeout time specified in @@ -3313,8 +3314,8 @@ public class ConnectivityManager { * @param networkCapabilities The new {@link android.net.NetworkCapabilities} for this * network. */ - public void onCapabilitiesChanged(Network network, - NetworkCapabilities networkCapabilities) {} + public void onCapabilitiesChanged(@NonNull Network network, + @NonNull NetworkCapabilities networkCapabilities) {} /** * Called when the network the framework connected to for this request @@ -3323,7 +3324,8 @@ public class ConnectivityManager { * @param network The {@link Network} whose link properties have changed. * @param linkProperties The new {@link LinkProperties} for this network. */ - public void onLinkPropertiesChanged(Network network, LinkProperties linkProperties) {} + public void onLinkPropertiesChanged(@NonNull Network network, + @NonNull LinkProperties linkProperties) {} /** * Called when the network the framework connected to for this request @@ -3334,7 +3336,7 @@ public class ConnectivityManager { * a tunnel, etc. * @hide */ - public void onNetworkSuspended(Network network) {} + public void onNetworkSuspended(@NonNull Network network) {} /** * Called when the network the framework connected to for this request @@ -3342,7 +3344,7 @@ public class ConnectivityManager { * preceded by a matching {@link NetworkCallback#onNetworkSuspended} call. * @hide */ - public void onNetworkResumed(Network network) {} + public void onNetworkResumed(@NonNull Network network) {} /** * Called when access to the specified network is blocked or unblocked. diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 03d6d48bc8..ad67763ac3 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -316,9 +316,6 @@ public final class LinkProperties implements Parcelable { @SystemApi @TestApi public boolean removeLinkAddress(@NonNull LinkAddress toRemove) { - if (toRemove == null) { - return false; - } int i = findLinkAddressIndex(toRemove); if (i >= 0) { mLinkAddresses.remove(i); @@ -391,10 +388,7 @@ public final class LinkProperties implements Parcelable { @TestApi @SystemApi public boolean removeDnsServer(@NonNull InetAddress dnsServer) { - if (dnsServer != null) { - return mDnses.remove(dnsServer); - } - return false; + return mDnses.remove(dnsServer); } /** diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 3a41a079da..acafa1354a 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -17,6 +17,7 @@ package android.net; import android.annotation.NonNull; +import android.annotation.RequiresPermission; import android.annotation.SystemApi; import android.annotation.UnsupportedAppUsage; import android.net.NetworkCapabilities.NetCapability; @@ -343,10 +344,14 @@ public class NetworkRequest implements Parcelable { * current value. A value of {@code SIGNAL_STRENGTH_UNSPECIFIED} means no value when * received and has no effect when requesting a callback. * + *

This method requires the caller to hold the + * {@link android.Manifest.permission#NETWORK_SIGNAL_STRENGTH_WAKEUP} permission + * * @param signalStrength the bearer-specific signal strength. * @hide */ @SystemApi + @RequiresPermission(android.Manifest.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP) public @NonNull Builder setSignalStrength(int signalStrength) { mNetworkCapabilities.setSignalStrength(signalStrength); return this; diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index bca2df47cd..5f3c67fb35 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1882,6 +1882,15 @@ public class ConnectivityService extends IConnectivityManager.Stub return false; } + private boolean checkAnyPermissionOf(int pid, int uid, String... permissions) { + for (String permission : permissions) { + if (mContext.checkPermission(permission, pid, uid) == PERMISSION_GRANTED) { + return true; + } + } + return false; + } + private void enforceAnyPermissionOf(String... permissions) { if (!checkAnyPermissionOf(permissions)) { throw new SecurityException("Requires one of the following permissions: " @@ -1956,6 +1965,12 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); } + private boolean checkNetworkSignalStrengthWakeupPermission(int pid, int uid) { + return checkAnyPermissionOf(pid, uid, + android.Manifest.permission.NETWORK_SIGNAL_STRENGTH_WAKEUP, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); + } + private void enforceConnectivityRestrictedNetworksPermission() { try { mContext.enforceCallingOrSelfPermission( @@ -4952,13 +4967,19 @@ 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. + // This checks that the passed capabilities either do not request a specific SSID/SignalStrength + // , or the calling app has permission to do so. private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, int callerPid, int callerUid) { if (null != nc.getSSID() && !checkSettingsPermission(callerPid, callerUid)) { throw new SecurityException("Insufficient permissions to request a specific SSID"); } + + if (nc.hasSignalStrength() + && !checkNetworkSignalStrengthWakeupPermission(callerPid, callerUid)) { + throw new SecurityException( + "Insufficient permissions to request a specific signal strength"); + } } private ArrayList getSignalStrengthThresholds(NetworkAgentInfo nai) { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 92a865a3dc..34dafa7723 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -3022,6 +3022,47 @@ public class ConnectivityServiceTest { } } + @Test + public void testInvalidSignalStrength() { + NetworkRequest r = new NetworkRequest.Builder() + .addCapability(NET_CAPABILITY_INTERNET) + .addTransportType(TRANSPORT_WIFI) + .setSignalStrength(-75) + .build(); + // Registering a NetworkCallback with signal strength but w/o NETWORK_SIGNAL_STRENGTH_WAKEUP + // permission should get SecurityException. + try { + mCm.registerNetworkCallback(r, new NetworkCallback()); + fail("Expected SecurityException filing a callback with signal strength"); + } catch (SecurityException expected) { + // expected + } + + try { + mCm.registerNetworkCallback(r, PendingIntent.getService( + mServiceContext, 0, new Intent(), 0)); + fail("Expected SecurityException filing a callback with signal strength"); + } catch (SecurityException expected) { + // expected + } + + // Requesting a Network with signal strength should get IllegalArgumentException. + try { + mCm.requestNetwork(r, new NetworkCallback()); + fail("Expected IllegalArgumentException filing a request with signal strength"); + } catch (IllegalArgumentException expected) { + // expected + } + + try { + mCm.requestNetwork(r, PendingIntent.getService( + mServiceContext, 0, new Intent(), 0)); + fail("Expected IllegalArgumentException filing a request with signal strength"); + } catch (IllegalArgumentException expected) { + // expected + } + } + @Test public void testRegisterDefaultNetworkCallback() throws Exception { final TestNetworkCallback defaultNetworkCallback = new TestNetworkCallback();