From 7c9b1c757a407760de93e442c71d5dd988a70a8c Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Sat, 26 Oct 2019 01:20:57 +0900 Subject: [PATCH 1/7] Add test coverage for strict mode private DNS. Support faking out the DNS lookups used by NetworkMonitor to resolve strict mode DNS, and add more test coverage. These tests were partly adapted from tests we have in Q but also contain new coverage. This is because in Q the interface between ConnectivityService and NetworkMonitor changed substantially, and it is impractical to backport NetworkMonitorTest. Bug: 122652057 Test: atest FrameworksNetTests Change-Id: I6497b7efa539267576d38d3036eef0af0df4e9cb Merged-In: Iaa78a7edcf23755c89d7b354edbc28d37d74d891 --- .../server/ConnectivityServiceTest.java | 117 +++++++++++++++++- 1 file changed, 111 insertions(+), 6 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index e6a889b1c2..d7a64dd38d 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -165,6 +165,7 @@ import org.mockito.MockitoAnnotations; import org.mockito.Spy; import java.net.InetAddress; +import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -455,6 +456,10 @@ public class ConnectivityServiceTest { mNetworkAgent.sendNetworkScore(mScore); } + public int getScore() { + return mScore; + } + public void explicitlySelected(boolean acceptUnvalidated) { mNetworkAgent.explicitlySelected(acceptUnvalidated); } @@ -869,6 +874,7 @@ public class ConnectivityServiceTest { // HTTP response code fed back to NetworkMonitor for Internet connectivity probe. public int gen204ProbeResult = 500; public String gen204ProbeRedirectUrl = null; + public volatile InetAddress[] dnsLookupResults = null; public WrappedNetworkMonitor(Context context, Handler handler, NetworkAgentInfo networkAgentInfo, NetworkRequest defaultRequest, @@ -883,6 +889,25 @@ public class ConnectivityServiceTest { if (!mIsCaptivePortalCheckEnabled) { return new CaptivePortalProbeResult(204); } return new CaptivePortalProbeResult(gen204ProbeResult, gen204ProbeRedirectUrl, null); } + + private InetAddress[] fakeDnsLookup() throws UnknownHostException { + if (dnsLookupResults == null) { + throw new UnknownHostException(); + } + return dnsLookupResults; + } + + @Override + protected InetAddress[] getAllByName(Network network, String hostname) + throws UnknownHostException { + return fakeDnsLookup(); + } + + @Override + protected InetAddress[] resolveAllLocally(Network network, String hostname, int flags) + throws UnknownHostException { + return fakeDnsLookup(); + } } private class WrappedMultinetworkPolicyTracker extends MultinetworkPolicyTracker { @@ -4021,7 +4046,7 @@ public class ConnectivityServiceTest { cellLp.addDnsServer(InetAddress.getByName("192.0.2.1")); mCellNetworkAgent.sendLinkProperties(cellLp); - mCellNetworkAgent.connect(false); + mCellNetworkAgent.connect(true); waitForIdle(); verify(mNetworkManagementService, atLeastOnce()).setDnsConfigurationForNetwork( anyInt(), mStringArrayCaptor.capture(), any(), any(), @@ -4039,9 +4064,10 @@ public class ConnectivityServiceTest { mCellNetworkAgent); CallbackInfo cbi = cellNetworkCallback.expectCallback( CallbackState.LINK_PROPERTIES, mCellNetworkAgent); - cellNetworkCallback.assertNoCallback(); assertFalse(((LinkProperties)cbi.arg).isPrivateDnsActive()); assertNull(((LinkProperties)cbi.arg).getPrivateDnsServerName()); + cellNetworkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, mCellNetworkAgent); + cellNetworkCallback.assertNoCallback(); setPrivateDnsSettings(PRIVATE_DNS_MODE_OFF, "ignored.example.com"); verify(mNetworkManagementService, times(1)).setDnsConfigurationForNetwork( @@ -4066,14 +4092,45 @@ public class ConnectivityServiceTest { reset(mNetworkManagementService); cellNetworkCallback.assertNoCallback(); + // Strict mode. + mCellNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults = new InetAddress[] { + InetAddress.getByName("2001:db8::66"), + InetAddress.getByName("192.0.2.44") + }; setPrivateDnsSettings(PRIVATE_DNS_MODE_PROVIDER_HOSTNAME, "strict.example.com"); - // Can't test dns configuration for strict mode without properly mocking - // out the DNS lookups, but can test that LinkProperties is updated. + + // Expect a callback saying that private DNS is now in strict mode. cbi = cellNetworkCallback.expectCallback(CallbackState.LINK_PROPERTIES, mCellNetworkAgent); + LinkProperties lp = (LinkProperties) cbi.arg; + assertTrue(lp.isPrivateDnsActive()); + assertEquals("strict.example.com", lp.getPrivateDnsServerName()); cellNetworkCallback.assertNoCallback(); - assertTrue(((LinkProperties)cbi.arg).isPrivateDnsActive()); - assertEquals("strict.example.com", ((LinkProperties)cbi.arg).getPrivateDnsServerName()); + + // When the validation callback arrives, LinkProperties are updated. + // We need to wait for this callback because the test thread races with the NetworkMonitor + // thread, and if the test thread wins the race, then the times(2) verify call below will + // fail. + mService.mNetdEventCallback.onPrivateDnsValidationEvent( + mCellNetworkAgent.getNetwork().netId, "2001:db8::66", "strict.example.com", true); + cbi = cellNetworkCallback.expectCallback(CallbackState.LINK_PROPERTIES, + mCellNetworkAgent); + lp = (LinkProperties) cbi.arg; + assertTrue(lp.isPrivateDnsActive()); + assertEquals(1, lp.getValidatedPrivateDnsServers().size()); + + // setDnsConfigurationForNetwork is called twice: once when private DNS is set to strict + // mode and once when the hostname resolves. + verify(mNetworkManagementService, times(2)).setDnsConfigurationForNetwork( + anyInt(), mStringArrayCaptor.capture(), any(), any(), + eq("strict.example.com"), tlsServers.capture()); + assertEquals(2, mStringArrayCaptor.getValue().length); + assertTrue(ArrayUtils.containsAll(mStringArrayCaptor.getValue(), + new String[]{"2001:db8::1", "192.0.2.1"})); + assertEquals(2, tlsServers.getValue().length); + assertTrue(ArrayUtils.containsAll(tlsServers.getValue(), + new String[]{"2001:db8::66", "192.0.2.44"})); + reset(mNetworkManagementService); // Send the same LinkProperties and expect getting the same result including private dns. // b/118518971 @@ -4395,6 +4452,54 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(defaultCallback); } + @Test + public void testVpnUnvalidated() throws Exception { + final TestNetworkCallback callback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(callback); + + // Enable private DNS. + setPrivateDnsSettings(PRIVATE_DNS_MODE_PROVIDER_HOSTNAME, "strict.example.com"); + + // Bring up Ethernet. + mEthernetNetworkAgent = new MockNetworkAgent(TRANSPORT_ETHERNET); + mEthernetNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults = + new InetAddress[]{ InetAddress.getByName("2001:db8::1") }; + mEthernetNetworkAgent.connect(true); + callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent); + callback.assertNoCallback(); + + // Bring up a VPN that has the INTERNET capability but does not provide Internet access. + final int uid = Process.myUid(); + final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); + vpnNetworkAgent.getWrappedNetworkMonitor().gen204ProbeResult = 500; + vpnNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults = null; + + final ArraySet ranges = new ArraySet<>(); + ranges.add(new UidRange(uid, uid)); + mMockVpn.setNetworkAgent(vpnNetworkAgent); + mMockVpn.setUids(ranges); + vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */); + mMockVpn.connect(); + + // The VPN validates and becomes the default network for our app. + callback.expectAvailableCallbacksValidated(vpnNetworkAgent); + // TODO: this looks like a spurious callback. + callback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); + callback.assertNoCallback(); + + assertTrue(vpnNetworkAgent.getScore() > mEthernetNetworkAgent.getScore()); + assertEquals(ConnectivityConstants.VPN_DEFAULT_SCORE, vpnNetworkAgent.getScore()); + assertEquals(vpnNetworkAgent.getNetwork(), mCm.getActiveNetwork()); + + NetworkCapabilities nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork()); + assertTrue(nc.hasCapability(NET_CAPABILITY_VALIDATED)); + assertTrue(nc.hasCapability(NET_CAPABILITY_INTERNET)); + + vpnNetworkAgent.disconnect(); + callback.expectCallback(CallbackState.LOST, vpnNetworkAgent); + callback.expectAvailableCallbacksValidated(mEthernetNetworkAgent); + } + @Test public void testVpnSetUnderlyingNetworks() { final int uid = Process.myUid(); From b316f633e5c6c76e794eb71e381321dd449bde86 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 10 May 2019 04:33:43 -0700 Subject: [PATCH 2/7] Support strict mode private DNS on VPNs that provide Internet. Currently, strict mode private DNS does not work on VPNs because NetworkMonitor does not validate VPNs. When a VPN connects, it immediately transitions to ValidatedState, skipping private DNS hostname resolution. This change makes NetworkMonitor perform private DNS hostname resolution and evaluation even on VPNs. In order to ensure that the system always immediately switches to the VPN as soon as it connects, remove the unvalidated penalty for VPN networks. This ensures that the VPN score is always 101 and the VPN always outscores other networks as soon as it connects. Previously, it would only outscore other networks when no-op validation completed. Backport of c4558228465af69d11a88eeda10d43ba48916572. Bug: 122652057 Test: atest FrameworksNetTests Test: manually ran a VPN with private DNS in strict mode Test: atest android.net.cts.ConnectivityManagerTest com.android.cts.net.HostsideVpnTests Change-Id: Iaa78a7edcf23755c89d7b354edbc28d37d74d891 Merged-In: Iaa78a7edcf23755c89d7b354edbc28d37d74d891 --- .../android/server/ConnectivityService.java | 11 +++----- .../server/connectivity/NetworkAgentInfo.java | 4 +-- .../server/ConnectivityServiceTest.java | 28 +++++++++++++++---- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 17cece1a22..b71fabae7f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2349,9 +2349,8 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private boolean networkRequiresValidation(NetworkAgentInfo nai) { - return NetworkMonitor.isValidationRequired( - mDefaultRequest.networkCapabilities, nai.networkCapabilities); + private boolean networkRequiresPrivateDnsValidation(NetworkAgentInfo nai) { + return nai.networkMonitor.isPrivateDnsValidationRequired(); } private void handleFreshlyValidatedNetwork(NetworkAgentInfo nai) { @@ -2369,16 +2368,14 @@ public class ConnectivityService extends IConnectivityManager.Stub for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { handlePerNetworkPrivateDnsConfig(nai, cfg); - if (networkRequiresValidation(nai)) { + if (networkRequiresPrivateDnsValidation(nai)) { handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties)); } } } private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) { - // Private DNS only ever applies to networks that might provide - // Internet access and therefore also require validation. - if (!networkRequiresValidation(nai)) return; + if (!networkRequiresPrivateDnsValidation(nai)) return; // Notify the NetworkMonitor thread in case it needs to cancel or // schedule DNS resolutions. If a DNS resolution is required the diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 505480ea53..5ee572ecb7 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -432,11 +432,11 @@ public class NetworkAgentInfo implements Comparable { // down an explicitly selected network before the user gets a chance to prefer it when // a higher-scoring network (e.g., Ethernet) is available. if (networkMisc.explicitlySelected && (networkMisc.acceptUnvalidated || pretendValidated)) { - return ConnectivityConstants.MAXIMUM_NETWORK_SCORE; + return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE; } int score = currentScore; - if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty()) { + if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) { score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY; } if (score < 0) score = 0; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index d7a64dd38d..6ceed53f4e 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -26,6 +26,7 @@ import static android.net.ConnectivityManager.TYPE_MOBILE; import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA; import static android.net.ConnectivityManager.TYPE_MOBILE_MMS; import static android.net.ConnectivityManager.TYPE_NONE; +import static android.net.ConnectivityManager.TYPE_VPN; import static android.net.ConnectivityManager.TYPE_WIFI; import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL; import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS; @@ -388,7 +389,7 @@ public class ConnectivityServiceTest { MockNetworkAgent(int transport, LinkProperties linkProperties) { final int type = transportToLegacyType(transport); - final String typeName = ConnectivityManager.getNetworkTypeName(transport); + final String typeName = ConnectivityManager.getNetworkTypeName(type); mNetworkInfo = new NetworkInfo(type, 0, typeName, "Mock"); mNetworkCapabilities = new NetworkCapabilities(); mNetworkCapabilities.addTransportType(transport); @@ -1111,6 +1112,8 @@ public class ConnectivityServiceTest { return TYPE_WIFI; case TRANSPORT_CELLULAR: return TYPE_MOBILE; + case TRANSPORT_VPN: + return TYPE_VPN; default: return TYPE_NONE; } @@ -4468,7 +4471,7 @@ public class ConnectivityServiceTest { callback.expectAvailableThenValidatedCallbacks(mEthernetNetworkAgent); callback.assertNoCallback(); - // Bring up a VPN that has the INTERNET capability but does not provide Internet access. + // Bring up a VPN that has the INTERNET capability but does not validate. final int uid = Process.myUid(); final MockNetworkAgent vpnNetworkAgent = new MockNetworkAgent(TRANSPORT_VPN); vpnNetworkAgent.getWrappedNetworkMonitor().gen204ProbeResult = 500; @@ -4481,8 +4484,8 @@ public class ConnectivityServiceTest { vpnNetworkAgent.connect(false /* validated */, true /* hasInternet */); mMockVpn.connect(); - // The VPN validates and becomes the default network for our app. - callback.expectAvailableCallbacksValidated(vpnNetworkAgent); + // Even though the VPN is unvalidated, it becomes the default network for our app. + callback.expectAvailableCallbacksUnvalidated(vpnNetworkAgent); // TODO: this looks like a spurious callback. callback.expectCallback(CallbackState.NETWORK_CAPABILITIES, vpnNetworkAgent); callback.assertNoCallback(); @@ -4492,9 +4495,24 @@ public class ConnectivityServiceTest { assertEquals(vpnNetworkAgent.getNetwork(), mCm.getActiveNetwork()); NetworkCapabilities nc = mCm.getNetworkCapabilities(vpnNetworkAgent.getNetwork()); - assertTrue(nc.hasCapability(NET_CAPABILITY_VALIDATED)); + assertFalse(nc.hasCapability(NET_CAPABILITY_VALIDATED)); assertTrue(nc.hasCapability(NET_CAPABILITY_INTERNET)); + assertFalse(vpnNetworkAgent.getWrappedNetworkMonitor().isValidationRequired()); + assertTrue(vpnNetworkAgent.getWrappedNetworkMonitor().isPrivateDnsValidationRequired()); + + // Pretend that the strict mode private DNS hostname now resolves. Even though the + // connectivity probe still returns 500, the network validates because the connectivity + // probe is not used on VPNs. + vpnNetworkAgent.getWrappedNetworkMonitor().dnsLookupResults = + new InetAddress[]{ InetAddress.getByName("2001:db8::1") }; + mCm.reportNetworkConnectivity(vpnNetworkAgent.getNetwork(), true); + + // Expect to see the validated capability, but no other changes, because the VPN is already + // the default network for the app. + callback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, vpnNetworkAgent); + callback.assertNoCallback(); + vpnNetworkAgent.disconnect(); callback.expectCallback(CallbackState.LOST, vpnNetworkAgent); callback.expectAvailableCallbacksValidated(mEthernetNetworkAgent); From 7307d27ff93469880feb14c05a9644df349521a7 Mon Sep 17 00:00:00 2001 From: paulhu Date: Mon, 16 Dec 2019 18:24:05 +0800 Subject: [PATCH 3/7] Fix security problem on PermissionMonitor#hasPermission PermissionMonitor#hasPermission only checks permssions that app requested but it doesn't check whether the permission can be granted to this app. If requested permission doens't be granted to app, this method still returns that app has this permission. Then PermissionMonitor will pass this info to netd that means this app still can use network even restricted network without granted privileged permission like CONNECTIVITY_INTERNAL or CONNECTIVITY_USE_RESTRICTED_NETWORKS. Bug: 144679405 Test: Build, flash, manual test Change-Id: I5eba4909e4c2e1d9f275f66be90ac36466b93e90 Merged-In: I8a1575dedd6e3b7a8b60ee2ffd475d790aec55c4 Merged-In: Iae9c273af822b18c2e6fce04848a86f8dea6410a --- .../server/connectivity/PermissionMonitor.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index e084ff827c..57dbfd1e15 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java @@ -21,6 +21,7 @@ import static android.Manifest.permission.CONNECTIVITY_INTERNAL; import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS; import static android.content.pm.ApplicationInfo.FLAG_SYSTEM; import static android.content.pm.ApplicationInfo.FLAG_UPDATED_SYSTEM_APP; +import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; import static android.content.pm.PackageManager.GET_PERMISSIONS; import android.content.BroadcastReceiver; @@ -39,6 +40,8 @@ import android.os.UserManager; import android.text.TextUtils; import android.util.Log; +import com.android.internal.util.ArrayUtils; + import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -150,15 +153,13 @@ public class PermissionMonitor { update(mUsers, mApps, true); } - private boolean hasPermission(PackageInfo app, String permission) { - if (app.requestedPermissions != null) { - for (String p : app.requestedPermissions) { - if (permission.equals(p)) { - return true; - } - } + private boolean hasPermission(final PackageInfo app, final String permission) { + if (app.requestedPermissions == null || app.requestedPermissionsFlags == null) { + return false; } - return false; + final int index = ArrayUtils.indexOf(app.requestedPermissions, permission); + if (index < 0 || index >= app.requestedPermissionsFlags.length) return false; + return (app.requestedPermissionsFlags[index] & REQUESTED_PERMISSION_GRANTED) != 0; } private boolean hasNetworkPermission(PackageInfo app) { From 6c93075645c1c022c96a46ecf54e83a3d6abc174 Mon Sep 17 00:00:00 2001 From: paulhu Date: Mon, 16 Dec 2019 18:24:05 +0800 Subject: [PATCH 4/7] Fix security problem on PermissionMonitor#hasPermission PermissionMonitor#hasPermission only checks permssions that app requested but it doesn't check whether the permission can be granted to this app. If requested permission doens't be granted to app, this method still returns that app has this permission. Then PermissionMonitor will pass this info to netd that means this app still can use network even restricted network without granted privileged permission like CONNECTIVITY_INTERNAL or CONNECTIVITY_USE_RESTRICTED_NETWORKS. Bug: 144679405 Test: Build, flash, manual test Change-Id: Iae9c273af822b18c2e6fce04848a86f8dea6410a Merged-In: I8a1575dedd6e3b7a8b60ee2ffd475d790aec55c4 Merged-In: I2da730feda4d7ebed1f158b073167bb3964b3e7d --- .../server/connectivity/PermissionMonitor.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index e471c7d84b..5722f5a72a 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java @@ -22,6 +22,7 @@ import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS; import static android.Manifest.permission.NETWORK_STACK; import static android.content.pm.ApplicationInfo.FLAG_SYSTEM; import static android.content.pm.ApplicationInfo.FLAG_UPDATED_SYSTEM_APP; +import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; import static android.content.pm.PackageManager.GET_PERMISSIONS; import android.content.BroadcastReceiver; @@ -42,6 +43,7 @@ import android.text.TextUtils; import android.util.Log; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.ArrayUtils; import java.util.ArrayList; import java.util.HashMap; @@ -161,15 +163,13 @@ public class PermissionMonitor { } @VisibleForTesting - boolean hasPermission(PackageInfo app, String permission) { - if (app.requestedPermissions != null) { - for (String p : app.requestedPermissions) { - if (permission.equals(p)) { - return true; - } - } + boolean hasPermission(final PackageInfo app, final String permission) { + if (app.requestedPermissions == null || app.requestedPermissionsFlags == null) { + return false; } - return false; + final int index = ArrayUtils.indexOf(app.requestedPermissions, permission); + if (index < 0 || index >= app.requestedPermissionsFlags.length) return false; + return (app.requestedPermissionsFlags[index] & REQUESTED_PERMISSION_GRANTED) != 0; } private boolean hasNetworkPermission(PackageInfo app) { From d73f6baa902c9d77d0706255df66d7d6703a6778 Mon Sep 17 00:00:00 2001 From: Paul Hu Date: Mon, 25 Nov 2019 10:35:55 -0800 Subject: [PATCH 5/7] Fix PermissionMonitor issues PermissionMonitor#hasPermission only checks permssions that app requested but it doesn't check whether the permission can be granted to this app. If requested permission doens't be granted to app, this method still returns that app has this permission. Then PermissionMonitor will pass this info to netd that means this app still can use network even restricted network without granted privileged permission like CONNECTIVITY_INTERNAL or CONNECTIVITY_USE_RESTRICTED_NETWORKS. PermissionMonitor#hasUseBackgroundNetworksPermission only uses the first package name of the uid for checking permission. This is incorrect since each package declared different permissions. So using the mApps which already checked both network and using restricted network permissions. If uid is in the mApps list that means uid has one of permission at least. Bug: 144679405 Test: Build, flash, manual test atest FrameworksNetTests Change-Id: I2da730feda4d7ebed1f158b073167bb3964b3e7d Merged-In: I8b03c9e23ffc9ff46264d6307fb841a7eda76a76 Merged-In: Ib08a940a6e5d3365c392ab7174d8484c197e0947 (cherry picked from commit 2e1da35b3b903f4aa01435c46b7014b88a41328d) --- .../connectivity/PermissionMonitor.java | 53 ++--- .../server/ConnectivityServiceTest.java | 24 +- .../connectivity/PermissionMonitorTest.java | 217 ++++++++++++------ 3 files changed, 185 insertions(+), 109 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/PermissionMonitor.java b/services/core/java/com/android/server/connectivity/PermissionMonitor.java index fbe2589bea..c70012b5e7 100644 --- a/services/core/java/com/android/server/connectivity/PermissionMonitor.java +++ b/services/core/java/com/android/server/connectivity/PermissionMonitor.java @@ -224,22 +224,22 @@ public class PermissionMonitor { } @VisibleForTesting - boolean hasPermission(PackageInfo app, String permission) { - if (app.requestedPermissions != null) { - for (String p : app.requestedPermissions) { - if (permission.equals(p)) { - return true; - } - } + boolean hasPermission(@NonNull final PackageInfo app, @NonNull final String permission) { + if (app.requestedPermissions == null || app.requestedPermissionsFlags == null) { + return false; } - return false; + final int index = ArrayUtils.indexOf(app.requestedPermissions, permission); + if (index < 0 || index >= app.requestedPermissionsFlags.length) return false; + return (app.requestedPermissionsFlags[index] & REQUESTED_PERMISSION_GRANTED) != 0; } - private boolean hasNetworkPermission(PackageInfo app) { + @VisibleForTesting + boolean hasNetworkPermission(@NonNull final PackageInfo app) { return hasPermission(app, CHANGE_NETWORK_STATE); } - private boolean hasRestrictedNetworkPermission(PackageInfo app) { + @VisibleForTesting + boolean hasRestrictedNetworkPermission(@NonNull final PackageInfo app) { // TODO : remove this check in the future(b/31479477). All apps should just // request the appropriate permission for their use case since android Q. if (app.applicationInfo != null) { @@ -255,33 +255,18 @@ public class PermissionMonitor { } } return hasPermission(app, CONNECTIVITY_INTERNAL) + || hasPermission(app, NETWORK_STACK) || hasPermission(app, CONNECTIVITY_USE_RESTRICTED_NETWORKS); } - private boolean hasUseBackgroundNetworksPermission(PackageInfo app) { - // This function defines what it means to hold the permission to use - // background networks. - return hasPermission(app, CHANGE_NETWORK_STATE) - || hasPermission(app, NETWORK_STACK) - || hasRestrictedNetworkPermission(app); - } - - public boolean hasUseBackgroundNetworksPermission(int uid) { - final String[] names = mPackageManager.getPackagesForUid(uid); - if (null == names || names.length == 0) return false; - try { - // Only using the first package name. There may be multiple names if multiple - // apps share the same UID, but in that case they also share permissions so - // querying with any of the names will return the same results. - int userId = UserHandle.getUserId(uid); - final PackageInfo app = mPackageManager.getPackageInfoAsUser( - names[0], GET_PERMISSIONS, userId); - return hasUseBackgroundNetworksPermission(app); - } catch (NameNotFoundException e) { - // App not found. - loge("NameNotFoundException " + names[0], e); - return false; - } + /** Returns whether the given uid has using background network permission. */ + public synchronized boolean hasUseBackgroundNetworksPermission(final int uid) { + // Apps with any of the CHANGE_NETWORK_STATE, NETWORK_STACK, CONNECTIVITY_INTERNAL or + // CONNECTIVITY_USE_RESTRICTED_NETWORKS permission has the permission to use background + // networks. mApps contains the result of checks for both hasNetworkPermission and + // hasRestrictedNetworkPermission. If uid is in the mApps list that means uid has one of + // permissions at least. + return mApps.containsKey(uid); } private int[] toIntArray(Collection list) { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 73ee7f544b..e313e28fbf 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -16,6 +16,9 @@ package com.android.server; +import static android.Manifest.permission.CHANGE_NETWORK_STATE; +import static android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS; +import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; @@ -2332,9 +2335,17 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(trackDefaultCallback); } + private void grantUsingBackgroundNetworksPermissionForUid(final int uid) throws Exception { + final String testPackageName = mContext.getPackageName(); + when(mPackageManager.getPackageInfo(eq(testPackageName), eq(GET_PERMISSIONS))) + .thenReturn(buildPackageInfo(true, uid)); + mService.mPermissionMonitor.onPackageAdded(testPackageName, uid); + } + @Test - public void testNetworkGoesIntoBackgroundAfterLinger() { + public void testNetworkGoesIntoBackgroundAfterLinger() throws Exception { setAlwaysOnNetworks(true); + grantUsingBackgroundNetworksPermissionForUid(Binder.getCallingUid()); NetworkRequest request = new NetworkRequest.Builder() .clearCapabilities() .build(); @@ -3415,6 +3426,7 @@ public class ConnectivityServiceTest { // Create a background request. We can't do this ourselves because ConnectivityService // doesn't have an API for it. So just turn on mobile data always on. setAlwaysOnNetworks(true); + grantUsingBackgroundNetworksPermissionForUid(Binder.getCallingUid()); final NetworkRequest request = new NetworkRequest.Builder().build(); final NetworkRequest fgRequest = new NetworkRequest.Builder() .addCapability(NET_CAPABILITY_FOREGROUND).build(); @@ -3581,6 +3593,7 @@ public class ConnectivityServiceTest { @Test public void testMobileDataAlwaysOn() throws Exception { + grantUsingBackgroundNetworksPermissionForUid(Binder.getCallingUid()); final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback(); final NetworkRequest cellRequest = new NetworkRequest.Builder() .addTransportType(TRANSPORT_CELLULAR).build(); @@ -6528,7 +6541,14 @@ public class ConnectivityServiceTest { private static PackageInfo buildPackageInfo(boolean hasSystemPermission, int uid) { final PackageInfo packageInfo = new PackageInfo(); - packageInfo.requestedPermissions = new String[0]; + if (hasSystemPermission) { + packageInfo.requestedPermissions = new String[] { + CHANGE_NETWORK_STATE, CONNECTIVITY_USE_RESTRICTED_NETWORKS }; + packageInfo.requestedPermissionsFlags = new int[] { + REQUESTED_PERMISSION_GRANTED, REQUESTED_PERMISSION_GRANTED }; + } else { + packageInfo.requestedPermissions = new String[0]; + } packageInfo.applicationInfo = new ApplicationInfo(); packageInfo.applicationInfo.privateFlags = 0; packageInfo.applicationInfo.uid = UserHandle.getUid(UserHandle.USER_SYSTEM, diff --git a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java index cd2bd26ef4..2e892e53fa 100644 --- a/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java +++ b/tests/net/java/com/android/server/connectivity/PermissionMonitorTest.java @@ -27,6 +27,7 @@ import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_OEM; import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_PRODUCT; import static android.content.pm.ApplicationInfo.PRIVATE_FLAG_VENDOR; import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_GRANTED; +import static android.content.pm.PackageInfo.REQUESTED_PERMISSION_REQUIRED; import static android.content.pm.PackageManager.GET_PERMISSIONS; import static android.content.pm.PackageManager.MATCH_ANY_USER; import static android.os.Process.SYSTEM_UID; @@ -36,6 +37,7 @@ import static com.android.server.connectivity.PermissionMonitor.SYSTEM; import static junit.framework.Assert.fail; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -102,7 +104,6 @@ public class PermissionMonitorTest { private static final String MOCK_PACKAGE2 = "appName2"; private static final String SYSTEM_PACKAGE1 = "sysName1"; private static final String SYSTEM_PACKAGE2 = "sysName2"; - private static final String VPN_PACKAGE = "vpnApp"; private static final String PARTITION_SYSTEM = "system"; private static final String PARTITION_OEM = "oem"; private static final String PARTITION_PRODUCT = "product"; @@ -145,28 +146,31 @@ public class PermissionMonitorTest { mObserver = observerCaptor.getValue(); } - private boolean hasBgPermission(String partition, int targetSdkVersion, int uid, - String... permission) throws Exception { - final PackageInfo packageInfo = packageInfoWithPermissions(permission, partition); + private boolean hasRestrictedNetworkPermission(String partition, int targetSdkVersion, int uid, + String... permissions) { + final PackageInfo packageInfo = + packageInfoWithPermissions(REQUESTED_PERMISSION_GRANTED, permissions, partition); packageInfo.applicationInfo.targetSdkVersion = targetSdkVersion; packageInfo.applicationInfo.uid = uid; - when(mPackageManager.getPackageInfoAsUser( - eq(MOCK_PACKAGE1), eq(GET_PERMISSIONS), anyInt())).thenReturn(packageInfo); - when(mPackageManager.getPackagesForUid(anyInt())).thenReturn(new String[] {MOCK_PACKAGE1}); - return mPermissionMonitor.hasUseBackgroundNetworksPermission(uid); + return mPermissionMonitor.hasRestrictedNetworkPermission(packageInfo); } - private static PackageInfo packageInfoWithPermissions(String[] permissions, String partition) { + private static PackageInfo systemPackageInfoWithPermissions(String... permissions) { + return packageInfoWithPermissions( + REQUESTED_PERMISSION_GRANTED, permissions, PARTITION_SYSTEM); + } + + private static PackageInfo vendorPackageInfoWithPermissions(String... permissions) { + return packageInfoWithPermissions( + REQUESTED_PERMISSION_GRANTED, permissions, PARTITION_VENDOR); + } + + private static PackageInfo packageInfoWithPermissions(int permissionsFlags, + String[] permissions, String partition) { int[] requestedPermissionsFlags = new int[permissions.length]; for (int i = 0; i < permissions.length; i++) { - requestedPermissionsFlags[i] = REQUESTED_PERMISSION_GRANTED; + requestedPermissionsFlags[i] = permissionsFlags; } - return packageInfoWithPermissions(permissions, partition, - requestedPermissionsFlags); - } - - private static PackageInfo packageInfoWithPermissions(String[] permissions, String partition, - int[] requestedPermissionsFlags) { final PackageInfo packageInfo = new PackageInfo(); packageInfo.requestedPermissions = permissions; packageInfo.applicationInfo = new ApplicationInfo(); @@ -190,12 +194,10 @@ public class PermissionMonitorTest { private static PackageInfo buildPackageInfo(boolean hasSystemPermission, int uid, int userId) { final PackageInfo pkgInfo; if (hasSystemPermission) { - final String[] systemPermissions = new String[]{ - CHANGE_NETWORK_STATE, NETWORK_STACK, CONNECTIVITY_USE_RESTRICTED_NETWORKS - }; - pkgInfo = packageInfoWithPermissions(systemPermissions, PARTITION_SYSTEM); + pkgInfo = systemPackageInfoWithPermissions( + CHANGE_NETWORK_STATE, NETWORK_STACK, CONNECTIVITY_USE_RESTRICTED_NETWORKS); } else { - pkgInfo = packageInfoWithPermissions(new String[] {}, ""); + pkgInfo = packageInfoWithPermissions(REQUESTED_PERMISSION_GRANTED, new String[] {}, ""); } pkgInfo.applicationInfo.uid = UserHandle.getUid(userId, UserHandle.getAppId(uid)); return pkgInfo; @@ -203,82 +205,151 @@ public class PermissionMonitorTest { @Test public void testHasPermission() { - PackageInfo app = packageInfoWithPermissions(new String[] {}, PARTITION_SYSTEM); + PackageInfo app = systemPackageInfoWithPermissions(); assertFalse(mPermissionMonitor.hasPermission(app, CHANGE_NETWORK_STATE)); assertFalse(mPermissionMonitor.hasPermission(app, NETWORK_STACK)); assertFalse(mPermissionMonitor.hasPermission(app, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); assertFalse(mPermissionMonitor.hasPermission(app, CONNECTIVITY_INTERNAL)); - app = packageInfoWithPermissions(new String[] { - CHANGE_NETWORK_STATE, NETWORK_STACK - }, PARTITION_SYSTEM); + app = systemPackageInfoWithPermissions(CHANGE_NETWORK_STATE, NETWORK_STACK); assertTrue(mPermissionMonitor.hasPermission(app, CHANGE_NETWORK_STATE)); assertTrue(mPermissionMonitor.hasPermission(app, NETWORK_STACK)); assertFalse(mPermissionMonitor.hasPermission(app, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); assertFalse(mPermissionMonitor.hasPermission(app, CONNECTIVITY_INTERNAL)); - app = packageInfoWithPermissions(new String[] { - CONNECTIVITY_USE_RESTRICTED_NETWORKS, CONNECTIVITY_INTERNAL - }, PARTITION_SYSTEM); + app = systemPackageInfoWithPermissions( + CONNECTIVITY_USE_RESTRICTED_NETWORKS, CONNECTIVITY_INTERNAL); assertFalse(mPermissionMonitor.hasPermission(app, CHANGE_NETWORK_STATE)); assertFalse(mPermissionMonitor.hasPermission(app, NETWORK_STACK)); assertTrue(mPermissionMonitor.hasPermission(app, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); assertTrue(mPermissionMonitor.hasPermission(app, CONNECTIVITY_INTERNAL)); + + app = packageInfoWithPermissions(REQUESTED_PERMISSION_REQUIRED, new String[] { + CONNECTIVITY_USE_RESTRICTED_NETWORKS, CONNECTIVITY_INTERNAL, NETWORK_STACK }, + PARTITION_SYSTEM); + assertFalse(mPermissionMonitor.hasPermission(app, CHANGE_NETWORK_STATE)); + assertFalse(mPermissionMonitor.hasPermission(app, NETWORK_STACK)); + assertFalse(mPermissionMonitor.hasPermission(app, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); + assertFalse(mPermissionMonitor.hasPermission(app, CONNECTIVITY_INTERNAL)); + + app = systemPackageInfoWithPermissions(CHANGE_NETWORK_STATE); + app.requestedPermissions = null; + assertFalse(mPermissionMonitor.hasPermission(app, CHANGE_NETWORK_STATE)); + + app = systemPackageInfoWithPermissions(CHANGE_NETWORK_STATE); + app.requestedPermissionsFlags = null; + assertFalse(mPermissionMonitor.hasPermission(app, CHANGE_NETWORK_STATE)); } @Test public void testIsVendorApp() { - PackageInfo app = packageInfoWithPermissions(new String[] {}, PARTITION_SYSTEM); + PackageInfo app = systemPackageInfoWithPermissions(); assertFalse(mPermissionMonitor.isVendorApp(app.applicationInfo)); - app = packageInfoWithPermissions(new String[] {}, PARTITION_OEM); + app = packageInfoWithPermissions(REQUESTED_PERMISSION_GRANTED, + new String[] {}, PARTITION_OEM); assertTrue(mPermissionMonitor.isVendorApp(app.applicationInfo)); - app = packageInfoWithPermissions(new String[] {}, PARTITION_PRODUCT); + app = packageInfoWithPermissions(REQUESTED_PERMISSION_GRANTED, + new String[] {}, PARTITION_PRODUCT); assertTrue(mPermissionMonitor.isVendorApp(app.applicationInfo)); - app = packageInfoWithPermissions(new String[] {}, PARTITION_VENDOR); + app = vendorPackageInfoWithPermissions(); assertTrue(mPermissionMonitor.isVendorApp(app.applicationInfo)); } + @Test + public void testHasNetworkPermission() { + PackageInfo app = systemPackageInfoWithPermissions(); + assertFalse(mPermissionMonitor.hasNetworkPermission(app)); + app = systemPackageInfoWithPermissions(CHANGE_NETWORK_STATE); + assertTrue(mPermissionMonitor.hasNetworkPermission(app)); + app = systemPackageInfoWithPermissions(NETWORK_STACK); + assertFalse(mPermissionMonitor.hasNetworkPermission(app)); + app = systemPackageInfoWithPermissions(CONNECTIVITY_USE_RESTRICTED_NETWORKS); + assertFalse(mPermissionMonitor.hasNetworkPermission(app)); + } + + @Test + public void testHasRestrictedNetworkPermission() { + assertFalse(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1)); + assertFalse(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CHANGE_NETWORK_STATE)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, MOCK_UID1, NETWORK_STACK)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CONNECTIVITY_INTERNAL)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); + assertFalse(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CHANGE_WIFI_STATE)); + + assertFalse(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_Q, MOCK_UID1)); + assertFalse(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_Q, MOCK_UID1, CHANGE_WIFI_STATE)); + } + + @Test + public void testHasRestrictedNetworkPermissionSystemUid() { + doReturn(VERSION_P).when(mPermissionMonitor).getDeviceFirstSdkInt(); + assertTrue(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, SYSTEM_UID, CHANGE_WIFI_STATE)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_P, SYSTEM_UID, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); + + doReturn(VERSION_Q).when(mPermissionMonitor).getDeviceFirstSdkInt(); + assertFalse(hasRestrictedNetworkPermission(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID)); + assertFalse(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID, CHANGE_WIFI_STATE)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); + } + + @Test + public void testHasRestrictedNetworkPermissionVendorApp() { + assertTrue(hasRestrictedNetworkPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_P, MOCK_UID1, CHANGE_NETWORK_STATE)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_P, MOCK_UID1, NETWORK_STACK)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_P, MOCK_UID1, CONNECTIVITY_INTERNAL)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_P, MOCK_UID1, CONNECTIVITY_USE_RESTRICTED_NETWORKS)); + assertTrue(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_P, MOCK_UID1, CHANGE_WIFI_STATE)); + + assertFalse(hasRestrictedNetworkPermission(PARTITION_VENDOR, VERSION_Q, MOCK_UID1)); + assertFalse(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_Q, MOCK_UID1, CHANGE_WIFI_STATE)); + assertFalse(hasRestrictedNetworkPermission( + PARTITION_VENDOR, VERSION_Q, MOCK_UID1, CHANGE_NETWORK_STATE)); + } + + private void assertBackgroundPermission(boolean hasPermission, String name, int uid, + String... permissions) throws Exception { + when(mPackageManager.getPackageInfo(eq(name), anyInt())) + .thenReturn(packageInfoWithPermissions( + REQUESTED_PERMISSION_GRANTED, permissions, PARTITION_SYSTEM)); + mPermissionMonitor.onPackageAdded(name, uid); + assertEquals(hasPermission, mPermissionMonitor.hasUseBackgroundNetworksPermission(uid)); + } + @Test public void testHasUseBackgroundNetworksPermission() throws Exception { - assertFalse(hasBgPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CHANGE_NETWORK_STATE)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1, NETWORK_STACK)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CONNECTIVITY_INTERNAL)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1, - CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - assertFalse(hasBgPermission(PARTITION_SYSTEM, VERSION_P, MOCK_UID1, CHANGE_WIFI_STATE)); + assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(SYSTEM_UID)); + assertBackgroundPermission(false, SYSTEM_PACKAGE1, SYSTEM_UID); + assertBackgroundPermission(false, SYSTEM_PACKAGE1, SYSTEM_UID, CHANGE_WIFI_STATE); + assertBackgroundPermission(true, SYSTEM_PACKAGE1, SYSTEM_UID, CHANGE_NETWORK_STATE); + assertBackgroundPermission(true, SYSTEM_PACKAGE1, SYSTEM_UID, NETWORK_STACK); - assertFalse(hasBgPermission(PARTITION_SYSTEM, VERSION_Q, MOCK_UID1)); - assertFalse(hasBgPermission(PARTITION_SYSTEM, VERSION_Q, MOCK_UID1, CHANGE_WIFI_STATE)); - } + assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(MOCK_UID1)); + assertBackgroundPermission(false, MOCK_PACKAGE1, MOCK_UID1); + assertBackgroundPermission(true, MOCK_PACKAGE1, MOCK_UID1, + CONNECTIVITY_USE_RESTRICTED_NETWORKS); - @Test - public void testHasUseBackgroundNetworksPermissionSystemUid() throws Exception { - doReturn(VERSION_P).when(mPermissionMonitor).getDeviceFirstSdkInt(); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID, CHANGE_WIFI_STATE)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_P, SYSTEM_UID, - CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - - doReturn(VERSION_Q).when(mPermissionMonitor).getDeviceFirstSdkInt(); - assertFalse(hasBgPermission(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID)); - assertFalse(hasBgPermission(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID, CHANGE_WIFI_STATE)); - assertTrue(hasBgPermission(PARTITION_SYSTEM, VERSION_Q, SYSTEM_UID, - CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - } - - @Test - public void testHasUseBackgroundNetworksPermissionVendorApp() throws Exception { - assertTrue(hasBgPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1)); - assertTrue(hasBgPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1, CHANGE_NETWORK_STATE)); - assertTrue(hasBgPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1, NETWORK_STACK)); - assertTrue(hasBgPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1, CONNECTIVITY_INTERNAL)); - assertTrue(hasBgPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1, - CONNECTIVITY_USE_RESTRICTED_NETWORKS)); - assertTrue(hasBgPermission(PARTITION_VENDOR, VERSION_P, MOCK_UID1, CHANGE_WIFI_STATE)); - - assertFalse(hasBgPermission(PARTITION_VENDOR, VERSION_Q, MOCK_UID1)); - assertFalse(hasBgPermission(PARTITION_VENDOR, VERSION_Q, MOCK_UID1, CHANGE_WIFI_STATE)); + assertFalse(mPermissionMonitor.hasUseBackgroundNetworksPermission(MOCK_UID2)); + assertBackgroundPermission(false, MOCK_PACKAGE2, MOCK_UID2); + assertBackgroundPermission(true, MOCK_PACKAGE2, MOCK_UID2, + CONNECTIVITY_INTERNAL); } private class NetdMonitor { @@ -563,7 +634,8 @@ public class PermissionMonitorTest { private PackageInfo addPackage(String packageName, int uid, String[] permissions) throws Exception { - PackageInfo packageInfo = packageInfoWithPermissions(permissions, PARTITION_SYSTEM); + PackageInfo packageInfo = packageInfoWithPermissions( + REQUESTED_PERMISSION_GRANTED, permissions, PARTITION_SYSTEM); when(mPackageManager.getPackageInfo(eq(packageName), anyInt())).thenReturn(packageInfo); when(mPackageManager.getPackagesForUid(eq(uid))).thenReturn(new String[]{packageName}); mObserver.onPackageAdded(packageName, uid); @@ -593,7 +665,7 @@ public class PermissionMonitorTest { // Install another package with the same uid and no permissions should not cause the UID to // lose permissions. - PackageInfo packageInfo2 = packageInfoWithPermissions(new String[]{}, PARTITION_SYSTEM); + PackageInfo packageInfo2 = systemPackageInfoWithPermissions(); when(mPackageManager.getPackageInfo(eq(MOCK_PACKAGE2), anyInt())).thenReturn(packageInfo2); when(mPackageManager.getPackagesForUid(MOCK_UID1)) .thenReturn(new String[]{MOCK_PACKAGE1, MOCK_PACKAGE2}); @@ -641,8 +713,7 @@ public class PermissionMonitorTest { | INetd.PERMISSION_UPDATE_DEVICE_STATS, new int[]{MOCK_UID1}); // Mock another package with the same uid but different permissions. - PackageInfo packageInfo2 = packageInfoWithPermissions(new String[] {INTERNET}, - PARTITION_SYSTEM); + PackageInfo packageInfo2 = systemPackageInfoWithPermissions(INTERNET); when(mPackageManager.getPackageInfo(eq(MOCK_PACKAGE2), anyInt())).thenReturn(packageInfo2); when(mPackageManager.getPackagesForUid(MOCK_UID1)).thenReturn(new String[]{ MOCK_PACKAGE2}); From d47c98c15a47973f822363a64c6afe68102f117a Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Thu, 19 Dec 2019 03:29:50 +0000 Subject: [PATCH 6/7] Fix parceling of LinkProperties Inet6Addr IPv6 addresses parceled for DNS servers, private DNS servers, PCSCF servers were parceled without the scope. This causes issues with link-local DNS servers. Test: atest FrameworksNetTests Bug: 145181158 (cherry picked from commit 091f1d790cffc7c0d3ea8c85f540755584df4077) Merged-In: Ie5b7782d788717dd1cc440e502d6cdf2d1c18eaa Change-Id: I51313f50de8220988c2c1d26981c27d07dfb55f9 --- core/java/android/net/LinkProperties.java | 53 ++++++--- .../java/android/net/LinkPropertiesTest.java | 109 +++++++++++------- 2 files changed, 104 insertions(+), 58 deletions(-) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index d3f48acdd4..94d4eb9762 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -73,6 +73,8 @@ public final class LinkProperties implements Parcelable { private static final int MIN_MTU_V6 = 1280; private static final int MAX_MTU = 10000; + private static final int INET6_ADDR_LENGTH = 16; + // Stores the properties of links that are "stacked" above this link. // Indexed by interface name to allow modification and to prevent duplicates being added. private Hashtable mStackedLinks = new Hashtable<>(); @@ -1590,20 +1592,11 @@ public final class LinkProperties implements Parcelable { dest.writeParcelable(linkAddress, flags); } - dest.writeInt(mDnses.size()); - for (InetAddress d : mDnses) { - dest.writeByteArray(d.getAddress()); - } - dest.writeInt(mValidatedPrivateDnses.size()); - for (InetAddress d : mValidatedPrivateDnses) { - dest.writeByteArray(d.getAddress()); - } + writeAddresses(dest, mDnses); + writeAddresses(dest, mValidatedPrivateDnses); dest.writeBoolean(mUsePrivateDns); dest.writeString(mPrivateDnsServerName); - dest.writeInt(mPcscfs.size()); - for (InetAddress d : mPcscfs) { - dest.writeByteArray(d.getAddress()); - } + writeAddresses(dest, mPcscfs); dest.writeString(mDomains); dest.writeInt(mMtu); dest.writeString(mTcpBufferSizes); @@ -1624,6 +1617,35 @@ public final class LinkProperties implements Parcelable { dest.writeList(stackedLinks); } + private static void writeAddresses(@NonNull Parcel dest, @NonNull List list) { + dest.writeInt(list.size()); + for (InetAddress d : list) { + writeAddress(dest, d); + } + } + + private static void writeAddress(@NonNull Parcel dest, @NonNull InetAddress addr) { + dest.writeByteArray(addr.getAddress()); + if (addr instanceof Inet6Address) { + final Inet6Address v6Addr = (Inet6Address) addr; + final boolean hasScopeId = v6Addr.getScopeId() != 0; + dest.writeBoolean(hasScopeId); + if (hasScopeId) dest.writeInt(v6Addr.getScopeId()); + } + } + + @NonNull + private static InetAddress readAddress(@NonNull Parcel p) throws UnknownHostException { + final byte[] addr = p.createByteArray(); + if (addr.length == INET6_ADDR_LENGTH) { + final boolean hasScopeId = p.readBoolean(); + final int scopeId = hasScopeId ? p.readInt() : 0; + return Inet6Address.getByAddress(null /* host */, addr, scopeId); + } + + return InetAddress.getByAddress(addr); + } + /** * Implement the Parcelable interface. */ @@ -1643,14 +1665,13 @@ public final class LinkProperties implements Parcelable { addressCount = in.readInt(); for (int i = 0; i < addressCount; i++) { try { - netProp.addDnsServer(InetAddress.getByAddress(in.createByteArray())); + netProp.addDnsServer(readAddress(in)); } catch (UnknownHostException e) { } } addressCount = in.readInt(); for (int i = 0; i < addressCount; i++) { try { - netProp.addValidatedPrivateDnsServer( - InetAddress.getByAddress(in.createByteArray())); + netProp.addValidatedPrivateDnsServer(readAddress(in)); } catch (UnknownHostException e) { } } netProp.setUsePrivateDns(in.readBoolean()); @@ -1658,7 +1679,7 @@ public final class LinkProperties implements Parcelable { addressCount = in.readInt(); for (int i = 0; i < addressCount; i++) { try { - netProp.addPcscfServer(InetAddress.getByAddress(in.createByteArray())); + netProp.addPcscfServer(readAddress(in)); } catch (UnknownHostException e) { } } netProp.setDomains(in.readString()); diff --git a/tests/net/common/java/android/net/LinkPropertiesTest.java b/tests/net/common/java/android/net/LinkPropertiesTest.java index e1c4238f12..388e9eb8e9 100644 --- a/tests/net/common/java/android/net/LinkPropertiesTest.java +++ b/tests/net/common/java/android/net/LinkPropertiesTest.java @@ -31,6 +31,7 @@ import android.util.ArraySet; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.util.ParcelableTestUtil; import com.android.internal.util.TestUtils; import org.junit.Test; @@ -47,25 +48,22 @@ import java.util.Set; @RunWith(AndroidJUnit4.class) @SmallTest public class LinkPropertiesTest { - private static final InetAddress ADDRV4 = InetAddresses.parseNumericAddress("75.208.6.1"); - private static final InetAddress ADDRV6 = InetAddresses.parseNumericAddress( - "2001:0db8:85a3:0000:0000:8a2e:0370:7334"); - private static final InetAddress DNS1 = InetAddresses.parseNumericAddress("75.208.7.1"); - private static final InetAddress DNS2 = InetAddresses.parseNumericAddress("69.78.7.1"); - private static final InetAddress DNS6 = InetAddresses.parseNumericAddress( - "2001:4860:4860::8888"); - private static final InetAddress PRIVDNS1 = InetAddresses.parseNumericAddress("1.1.1.1"); - private static final InetAddress PRIVDNS2 = InetAddresses.parseNumericAddress("1.0.0.1"); - private static final InetAddress PRIVDNS6 = InetAddresses.parseNumericAddress( - "2606:4700:4700::1111"); - private static final InetAddress PCSCFV4 = InetAddresses.parseNumericAddress("10.77.25.37"); - private static final InetAddress PCSCFV6 = InetAddresses.parseNumericAddress( - "2001:0db8:85a3:0000:0000:8a2e:0370:1"); - private static final InetAddress GATEWAY1 = InetAddresses.parseNumericAddress("75.208.8.1"); - private static final InetAddress GATEWAY2 = InetAddresses.parseNumericAddress("69.78.8.1"); - private static final InetAddress GATEWAY61 = InetAddresses.parseNumericAddress( - "fe80::6:0000:613"); - private static final InetAddress GATEWAY62 = InetAddresses.parseNumericAddress("fe80::6:2222"); + private static final InetAddress ADDRV4 = address("75.208.6.1"); + private static final InetAddress ADDRV6 = address("2001:0db8:85a3:0000:0000:8a2e:0370:7334"); + private static final InetAddress DNS1 = address("75.208.7.1"); + private static final InetAddress DNS2 = address("69.78.7.1"); + private static final InetAddress DNS6 = address("2001:4860:4860::8888"); + private static final InetAddress PRIVDNS1 = address("1.1.1.1"); + private static final InetAddress PRIVDNS2 = address("1.0.0.1"); + private static final InetAddress PRIVDNS6 = address("2606:4700:4700::1111"); + private static final InetAddress PCSCFV4 = address("10.77.25.37"); + private static final InetAddress PCSCFV6 = address("2001:0db8:85a3:0000:0000:8a2e:0370:1"); + private static final InetAddress GATEWAY1 = address("75.208.8.1"); + private static final InetAddress GATEWAY2 = address("69.78.8.1"); + private static final InetAddress GATEWAY61 = address("fe80::6:0000:613"); + private static final InetAddress GATEWAY62 = address("fe80::6:22%lo"); + private static final InetAddress TESTIPV4ADDR = address("192.168.47.42"); + private static final InetAddress TESTIPV6ADDR = address("fe80::7:33%43"); private static final String NAME = "qmi0"; private static final String DOMAINS = "google.com"; private static final String PRIV_DNS_SERVER_NAME = "private.dns.com"; @@ -75,8 +73,7 @@ public class LinkPropertiesTest { private static final LinkAddress LINKADDRV6 = new LinkAddress(ADDRV6, 128); private static final LinkAddress LINKADDRV6LINKLOCAL = new LinkAddress("fe80::1/64"); - // TODO: replace all calls to NetworkUtils.numericToInetAddress with calls to this method. - private InetAddress Address(String addrString) { + private static InetAddress address(String addrString) { return InetAddresses.parseNumericAddress(addrString); } @@ -223,7 +220,7 @@ public class LinkPropertiesTest { target.clear(); target.setInterfaceName(NAME); // change link addresses - target.addLinkAddress(new LinkAddress(Address("75.208.6.2"), 32)); + target.addLinkAddress(new LinkAddress(address("75.208.6.2"), 32)); target.addLinkAddress(LINKADDRV6); target.addDnsServer(DNS1); target.addDnsServer(DNS2); @@ -238,7 +235,7 @@ public class LinkPropertiesTest { target.addLinkAddress(LINKADDRV4); target.addLinkAddress(LINKADDRV6); // change dnses - target.addDnsServer(Address("75.208.7.2")); + target.addDnsServer(address("75.208.7.2")); target.addDnsServer(DNS2); target.addPcscfServer(PCSCFV6); target.addRoute(new RouteInfo(GATEWAY1)); @@ -250,10 +247,10 @@ public class LinkPropertiesTest { target.setInterfaceName(NAME); target.addLinkAddress(LINKADDRV4); target.addLinkAddress(LINKADDRV6); - target.addDnsServer(Address("75.208.7.2")); + target.addDnsServer(address("75.208.7.2")); target.addDnsServer(DNS2); // change pcscf - target.addPcscfServer(Address("2001::1")); + target.addPcscfServer(address("2001::1")); target.addRoute(new RouteInfo(GATEWAY1)); target.addRoute(new RouteInfo(GATEWAY2)); target.setMtu(MTU); @@ -266,9 +263,9 @@ public class LinkPropertiesTest { target.addDnsServer(DNS1); target.addDnsServer(DNS2); // change gateway - target.addRoute(new RouteInfo(Address("75.208.8.2"))); - target.addRoute(new RouteInfo(GATEWAY2)); + target.addRoute(new RouteInfo(address("75.208.8.2"))); target.setMtu(MTU); + target.addRoute(new RouteInfo(GATEWAY2)); assertFalse(source.equals(target)); target.clear(); @@ -344,7 +341,7 @@ public class LinkPropertiesTest { @Test public void testRouteInterfaces() { - LinkAddress prefix = new LinkAddress(Address("2001:db8::"), 32); + LinkAddress prefix = new LinkAddress(address("2001:db8::"), 32); InetAddress address = ADDRV6; // Add a route with no interface to a LinkProperties with no interface. No errors. @@ -734,8 +731,7 @@ public class LinkPropertiesTest { // Add an on-link route, making the on-link DNS server reachable, // but there is still no IPv4 address. - assertTrue(v4lp.addRoute(new RouteInfo( - new IpPrefix(NetworkUtils.numericToInetAddress("75.208.0.0"), 16)))); + assertTrue(v4lp.addRoute(new RouteInfo(new IpPrefix(address("75.208.0.0"), 16)))); assertFalse(v4lp.isReachable(DNS1)); assertFalse(v4lp.isReachable(DNS2)); @@ -751,9 +747,9 @@ public class LinkPropertiesTest { assertTrue(v4lp.isReachable(DNS2)); final LinkProperties v6lp = new LinkProperties(); - final InetAddress kLinkLocalDns = Address("fe80::6:1"); - final InetAddress kLinkLocalDnsWithScope = Address("fe80::6:2%43"); - final InetAddress kOnLinkDns = Address("2001:db8:85a3::53"); + final InetAddress kLinkLocalDns = address("fe80::6:1"); + final InetAddress kLinkLocalDnsWithScope = address("fe80::6:2%43"); + final InetAddress kOnLinkDns = address("2001:db8:85a3::53"); assertFalse(v6lp.isReachable(kLinkLocalDns)); assertFalse(v6lp.isReachable(kLinkLocalDnsWithScope)); assertFalse(v6lp.isReachable(kOnLinkDns)); @@ -762,7 +758,7 @@ public class LinkPropertiesTest { // Add a link-local route, making the link-local DNS servers reachable. Because // we assume the presence of an IPv6 link-local address, link-local DNS servers // are considered reachable, but only those with a non-zero scope identifier. - assertTrue(v6lp.addRoute(new RouteInfo(new IpPrefix(Address("fe80::"), 64)))); + assertTrue(v6lp.addRoute(new RouteInfo(new IpPrefix(address("fe80::"), 64)))); assertFalse(v6lp.isReachable(kLinkLocalDns)); assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope)); assertFalse(v6lp.isReachable(kOnLinkDns)); @@ -778,7 +774,7 @@ public class LinkPropertiesTest { // Add a global route on link, but no global address yet. DNS servers reachable // via a route that doesn't require a gateway: give them the benefit of the // doubt and hope the link-local source address suffices for communication. - assertTrue(v6lp.addRoute(new RouteInfo(new IpPrefix(Address("2001:db8:85a3::"), 64)))); + assertTrue(v6lp.addRoute(new RouteInfo(new IpPrefix(address("2001:db8:85a3::"), 64)))); assertFalse(v6lp.isReachable(kLinkLocalDns)); assertTrue(v6lp.isReachable(kLinkLocalDnsWithScope)); assertTrue(v6lp.isReachable(kOnLinkDns)); @@ -807,7 +803,7 @@ public class LinkPropertiesTest { stacked.setInterfaceName("v4-test0"); v6lp.addStackedLink(stacked); - InetAddress stackedAddress = Address("192.0.0.4"); + InetAddress stackedAddress = address("192.0.0.4"); LinkAddress stackedLinkAddress = new LinkAddress(stackedAddress, 32); assertFalse(v6lp.isReachable(stackedAddress)); stacked.addLinkAddress(stackedLinkAddress); @@ -840,7 +836,7 @@ public class LinkPropertiesTest { LinkProperties rmnet1 = new LinkProperties(); rmnet1.setInterfaceName("rmnet1"); rmnet1.addLinkAddress(new LinkAddress("10.0.0.3/8")); - RouteInfo defaultRoute1 = new RouteInfo((IpPrefix) null, Address("10.0.0.1"), + RouteInfo defaultRoute1 = new RouteInfo((IpPrefix) null, address("10.0.0.1"), rmnet1.getInterfaceName()); RouteInfo directRoute1 = new RouteInfo(new IpPrefix("10.0.0.0/8"), null, rmnet1.getInterfaceName()); @@ -859,7 +855,7 @@ public class LinkPropertiesTest { rmnet2.setInterfaceName("rmnet2"); rmnet2.addLinkAddress(new LinkAddress("fe80::cafe/64")); rmnet2.addLinkAddress(new LinkAddress("2001:db8::2/64")); - RouteInfo defaultRoute2 = new RouteInfo((IpPrefix) null, Address("2001:db8::1"), + RouteInfo defaultRoute2 = new RouteInfo((IpPrefix) null, address("2001:db8::1"), rmnet2.getInterfaceName()); RouteInfo directRoute2 = new RouteInfo(new IpPrefix("2001:db8::/64"), null, rmnet2.getInterfaceName()); @@ -925,24 +921,53 @@ public class LinkPropertiesTest { public void testLinkPropertiesParcelable() throws Exception { LinkProperties source = new LinkProperties(); source.setInterfaceName(NAME); - // set 2 link addresses + source.addLinkAddress(LINKADDRV4); source.addLinkAddress(LINKADDRV6); - // set 2 dnses + source.addDnsServer(DNS1); source.addDnsServer(DNS2); - // set 2 gateways + source.addDnsServer(GATEWAY62); + + source.addPcscfServer(TESTIPV4ADDR); + source.addPcscfServer(TESTIPV6ADDR); + + source.setUsePrivateDns(true); + source.setPrivateDnsServerName(PRIV_DNS_SERVER_NAME); + + source.setDomains(DOMAINS); + source.addRoute(new RouteInfo(GATEWAY1)); source.addRoute(new RouteInfo(GATEWAY2)); - // set 2 validated private dnses + source.addValidatedPrivateDnsServer(DNS6); source.addValidatedPrivateDnsServer(GATEWAY61); + source.addValidatedPrivateDnsServer(TESTIPV6ADDR); + + source.setHttpProxy(ProxyInfo.buildDirectProxy("test", 8888)); source.setMtu(MTU); + source.setTcpBufferSizes(TCP_BUFFER_SIZES); + source.setNat64Prefix(new IpPrefix("2001:db8:1:2:64:64::/96")); + final LinkProperties stacked = new LinkProperties(); + stacked.setInterfaceName("test-stacked"); + source.addStackedLink(stacked); + TestUtils.assertParcelingIsLossless(source); + ParcelableTestUtil.assertFieldCountEquals(14, LinkProperties.class); + } + + @Test + public void testLinkLocalDnsServerParceling() throws Exception { + final String strAddress = "fe80::1%lo"; + final LinkProperties lp = new LinkProperties(); + lp.addDnsServer(address(strAddress)); + final LinkProperties unparceled = TestUtils.parcelingRoundTrip(lp); + // Inet6Address#equals does not test for the scope id + assertEquals(strAddress, unparceled.getDnsServers().get(0).getHostAddress()); } @Test From bc7e37d0f479aaeca0c3222b10ac2535c32e1b3e Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Thu, 16 Jan 2020 12:17:17 -0800 Subject: [PATCH 7/7] DO NOT MERGE: RELAND: NetworkRequest: Embed requestor uid & packageName Add the requestorUid & requestorPackageName fields to NetworkCapabilities. This is populated by CS when a new network request is received. These 2 requestor fields are also optionally used for network matching. All of the regular app initiated requests will have the requestor uid and package name set by connectivity service. Network agents can optionally set the requestorUid and requestorPackageName to restrict the network created only to the app that requested the network. This will help removing the necessity for the various specifiers to embed the uid & package name info in the specifier for network matching. Note: NetworkSpecifier.assertValidFromUid() is deprecated & removed in favor of setting the uid/package name on the agent to restrict the network to a certain app (useful for wifi peer to peer API & wifi aware). Bug: 144102365 Test: Verified that wifi network request related CTS verifier tests pass. Test: Device boots up and connects to wifi networks Merged-In: I207c446108afdac7ee2c25e6bbcbc37c4e3f6529 Change-Id: I58775e82aa7725aac5aa27ca9d2b5ee8f0be4242 --- .../java/android/net/ConnectivityManager.java | 14 +- .../android/net/IConnectivityManager.aidl | 9 +- .../java/android/net/NetworkCapabilities.java | 161 +++++++++++++++++- core/java/android/net/NetworkRequest.java | 26 +++ .../android/server/ConnectivityService.java | 54 +++--- .../android/net/NetworkCapabilitiesTest.java | 16 +- .../android/net/ConnectivityManagerTest.java | 12 +- .../server/ConnectivityServiceTest.java | 31 +--- 8 files changed, 257 insertions(+), 66 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 6d8565025f..f1b5d3298b 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3747,6 +3747,7 @@ public class ConnectivityManager { checkCallbackNotNull(callback); Preconditions.checkArgument(action == REQUEST || need != null, "null NetworkCapabilities"); final NetworkRequest request; + final String callingPackageName = mContext.getOpPackageName(); try { synchronized(sCallbacks) { if (callback.networkRequest != null @@ -3758,10 +3759,11 @@ public class ConnectivityManager { Messenger messenger = new Messenger(handler); Binder binder = new Binder(); if (action == LISTEN) { - request = mService.listenForNetwork(need, messenger, binder); + request = mService.listenForNetwork( + need, messenger, binder, callingPackageName); } else { request = mService.requestNetwork( - need, messenger, timeoutMs, binder, legacyType); + need, messenger, timeoutMs, binder, legacyType, callingPackageName); } if (request != null) { sCallbacks.put(request, callback); @@ -4034,8 +4036,10 @@ public class ConnectivityManager { @NonNull PendingIntent operation) { printStackTrace(); checkPendingIntentNotNull(operation); + final String callingPackageName = mContext.getOpPackageName(); try { - mService.pendingRequestForNetwork(request.networkCapabilities, operation); + mService.pendingRequestForNetwork( + request.networkCapabilities, operation, callingPackageName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (ServiceSpecificException e) { @@ -4147,8 +4151,10 @@ public class ConnectivityManager { @NonNull PendingIntent operation) { printStackTrace(); checkPendingIntentNotNull(operation); + final String callingPackageName = mContext.getOpPackageName(); try { - mService.pendingListenForNetwork(request.networkCapabilities, operation); + mService.pendingListenForNetwork( + request.networkCapabilities, operation, callingPackageName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } catch (ServiceSpecificException e) { diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index c871c456dc..3a55461a77 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -166,18 +166,19 @@ interface IConnectivityManager in int factorySerialNumber); NetworkRequest requestNetwork(in NetworkCapabilities networkCapabilities, - in Messenger messenger, int timeoutSec, in IBinder binder, int legacy); + in Messenger messenger, int timeoutSec, in IBinder binder, int legacy, + String callingPackageName); NetworkRequest pendingRequestForNetwork(in NetworkCapabilities networkCapabilities, - in PendingIntent operation); + in PendingIntent operation, String callingPackageName); void releasePendingNetworkRequest(in PendingIntent operation); NetworkRequest listenForNetwork(in NetworkCapabilities networkCapabilities, - in Messenger messenger, in IBinder binder); + in Messenger messenger, in IBinder binder, String callingPackageName); void pendingListenForNetwork(in NetworkCapabilities networkCapabilities, - in PendingIntent operation); + in PendingIntent operation, String callingPackageName); void releaseNetworkRequest(in NetworkRequest networkRequest); diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 38f7390abf..ef4a9e5f3b 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -27,6 +27,7 @@ import android.os.Build; import android.os.Parcel; import android.os.Parcelable; import android.os.Process; +import android.text.TextUtils; import android.util.ArraySet; import android.util.proto.ProtoOutputStream; @@ -63,6 +64,16 @@ public final class NetworkCapabilities implements Parcelable { // Set to true when private DNS is broken. private boolean mPrivateDnsBroken; + /** + * Uid of the app making the request. + */ + private int mRequestorUid; + + /** + * Package name of the app making the request. + */ + private String mRequestorPackageName; + public NetworkCapabilities() { clearAll(); mNetworkCapabilities = DEFAULT_CAPABILITIES; @@ -89,6 +100,8 @@ public final class NetworkCapabilities implements Parcelable { mOwnerUid = Process.INVALID_UID; mSSID = null; mPrivateDnsBroken = false; + mRequestorUid = Process.INVALID_UID; + mRequestorPackageName = null; } /** @@ -109,6 +122,8 @@ public final class NetworkCapabilities implements Parcelable { mUnwantedNetworkCapabilities = nc.mUnwantedNetworkCapabilities; mSSID = nc.mSSID; mPrivateDnsBroken = nc.mPrivateDnsBroken; + mRequestorUid = nc.mRequestorUid; + mRequestorPackageName = nc.mRequestorPackageName; } /** @@ -810,7 +825,7 @@ public final class NetworkCapabilities implements Parcelable { } /** - * UID of the app that owns this network, or INVALID_UID if none/unknown. + * UID of the app that owns this network, or Process#INVALID_UID if none/unknown. * *

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

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

+ * This is only legal if either the requestor info of this object is reset, or both info are + * equal. + * nc is assumed nonnull. + */ + private void combineRequestor(@NonNull NetworkCapabilities nc) { + if (mRequestorUid != Process.INVALID_UID && mRequestorUid != nc.mOwnerUid) { + throw new IllegalStateException("Can't combine two uids"); + } + if (mRequestorPackageName != null + && !mRequestorPackageName.equals(nc.mRequestorPackageName)) { + throw new IllegalStateException("Can't combine two package names"); + } + setRequestorUid(nc.mRequestorUid); + setRequestorPackageName(nc.mRequestorPackageName); + } + + private boolean equalsRequestor(NetworkCapabilities nc) { + return mRequestorUid == nc.mRequestorUid + && TextUtils.equals(mRequestorPackageName, nc.mRequestorPackageName); + } } diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index ee4379a85b..b0bf64ecec 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -380,6 +380,7 @@ public class NetworkRequest implements Parcelable { dest.writeInt(requestId); dest.writeString(type.name()); } + public static final @android.annotation.NonNull Creator CREATOR = new Creator() { public NetworkRequest createFromParcel(Parcel in) { @@ -494,6 +495,31 @@ public class NetworkRequest implements Parcelable { return networkCapabilities.getNetworkSpecifier(); } + /** + * @return the uid of the app making the request. + * + * Note: This could return {@link Process#INVALID_UID} if the {@link NetworkRequest} object was + * not obtained from {@link ConnectivityManager}. + * @hide + */ + @SystemApi + public int getRequestorUid() { + return networkCapabilities.getRequestorUid(); + } + + /** + * @return the package name of the app making the request. + * + * Note: This could return {@code null} if the {@link NetworkRequest} object was not obtained + * from {@link ConnectivityManager}. + * @hide + */ + @SystemApi + @Nullable + public String getRequestorPackageName() { + return networkCapabilities.getRequestorPackageName(); + } + public String toString() { return "NetworkRequest [ " + type + " id=" + requestId + (legacyType != ConnectivityManager.TYPE_NONE ? ", legacyType=" + legacyType : "") + diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e91918ac6d..52a2ca974d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -615,7 +615,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private Set mWolSupportedInterfaces; - private TelephonyManager mTelephonyManager; + private final TelephonyManager mTelephonyManager; private final AppOpsManager mAppOpsManager; private final LocationPermissionChecker mLocationPermissionChecker; @@ -966,6 +966,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mDeps = checkNotNull(deps, "missing Dependencies"); mSystemProperties = mDeps.getSystemProperties(); mNetIdManager = mDeps.makeNetIdManager(); + mContext = checkNotNull(context, "missing Context"); mMetricsLog = logger; mDefaultRequest = createDefaultInternetRequestForTransport(-1, NetworkRequest.Type.REQUEST); @@ -995,7 +996,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mLingerDelayMs = mSystemProperties.getInt(LINGER_DELAY_PROPERTY, DEFAULT_LINGER_DELAY_MS); - mContext = checkNotNull(context, "missing Context"); mNMS = checkNotNull(netManager, "missing INetworkManagementService"); mStatsService = checkNotNull(statsService, "missing INetworkStatsService"); mPolicyManager = checkNotNull(policyManager, "missing INetworkPolicyManager"); @@ -1175,6 +1175,7 @@ public class ConnectivityService extends IConnectivityManager.Stub int transportType, NetworkRequest.Type type) { final NetworkCapabilities netCap = new NetworkCapabilities(); netCap.addCapability(NET_CAPABILITY_INTERNET); + netCap.setRequestorUidAndPackageName(Process.myUid(), mContext.getPackageName()); if (transportType > -1) { netCap.addTransportType(transportType); } @@ -1705,10 +1706,12 @@ public class ConnectivityService extends IConnectivityManager.Stub return newLp; } - private void restrictRequestUidsForCaller(NetworkCapabilities nc) { + private void restrictRequestUidsForCallerAndSetRequestorInfo(NetworkCapabilities nc, + int callerUid, String callerPackageName) { if (!checkSettingsPermission()) { - nc.setSingleUid(Binder.getCallingUid()); + nc.setSingleUid(callerUid); } + nc.setRequestorUidAndPackageName(callerUid, callerPackageName); nc.setAdministratorUids(Collections.EMPTY_LIST); // Clear owner UID; this can never come from an app. @@ -5334,7 +5337,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // This checks that the passed capabilities either do not request a // specific SSID/SignalStrength, or the calling app has permission to do so. private void ensureSufficientPermissionsForRequest(NetworkCapabilities nc, - int callerPid, int callerUid) { + int callerPid, int callerUid, String callerPackageName) { if (null != nc.getSSID() && !checkSettingsPermission(callerPid, callerUid)) { throw new SecurityException("Insufficient permissions to request a specific SSID"); } @@ -5344,6 +5347,7 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new SecurityException( "Insufficient permissions to request a specific signal strength"); } + mAppOpsManager.checkPackage(callerUid, callerPackageName); } private ArrayList getSignalStrengthThresholds(NetworkAgentInfo nai) { @@ -5390,7 +5394,6 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } MatchAllNetworkSpecifier.checkNotMatchAllNetworkSpecifier(ns); - ns.assertValidFromUid(Binder.getCallingUid()); } private void ensureValid(NetworkCapabilities nc) { @@ -5402,7 +5405,9 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest requestNetwork(NetworkCapabilities networkCapabilities, - Messenger messenger, int timeoutMs, IBinder binder, int legacyType) { + Messenger messenger, int timeoutMs, IBinder binder, int legacyType, + @NonNull String callingPackageName) { + final int callingUid = Binder.getCallingUid(); final NetworkRequest.Type type = (networkCapabilities == null) ? NetworkRequest.Type.TRACK_DEFAULT : NetworkRequest.Type.REQUEST; @@ -5410,7 +5415,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // the default network request. This allows callers to keep track of // the system default network. if (type == NetworkRequest.Type.TRACK_DEFAULT) { - networkCapabilities = createDefaultNetworkCapabilitiesForUid(Binder.getCallingUid()); + networkCapabilities = createDefaultNetworkCapabilitiesForUid(callingUid); enforceAccessPermission(); } else { networkCapabilities = new NetworkCapabilities(networkCapabilities); @@ -5422,13 +5427,14 @@ public class ConnectivityService extends IConnectivityManager.Stub } ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); + Binder.getCallingPid(), callingUid, callingPackageName); // Set the UID range for this request to the single UID of the requester, or to an empty // set of UIDs if the caller has the appropriate permission and UIDs have not been set. // This will overwrite any allowed UIDs in the requested capabilities. Though there // are no visible methods to set the UIDs, an app could use reflection to try and get // networks for other apps so it's essential that the UIDs are overwritten. - restrictRequestUidsForCaller(networkCapabilities); + restrictRequestUidsForCallerAndSetRequestorInfo(networkCapabilities, + callingUid, callingPackageName); if (timeoutMs < 0) { throw new IllegalArgumentException("Bad timeout specified"); @@ -5503,16 +5509,18 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest pendingRequestForNetwork(NetworkCapabilities networkCapabilities, - PendingIntent operation) { + PendingIntent operation, @NonNull String callingPackageName) { checkNotNull(operation, "PendingIntent cannot be null."); + final int callingUid = Binder.getCallingUid(); networkCapabilities = new NetworkCapabilities(networkCapabilities); enforceNetworkRequestPermissions(networkCapabilities); enforceMeteredApnPolicy(networkCapabilities); ensureRequestableCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); + Binder.getCallingPid(), callingUid, callingPackageName); ensureValidNetworkSpecifier(networkCapabilities); - restrictRequestUidsForCaller(networkCapabilities); + restrictRequestUidsForCallerAndSetRequestorInfo(networkCapabilities, + callingUid, callingPackageName); NetworkRequest networkRequest = new NetworkRequest(networkCapabilities, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.REQUEST); @@ -5560,15 +5568,16 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public NetworkRequest listenForNetwork(NetworkCapabilities networkCapabilities, - Messenger messenger, IBinder binder) { + Messenger messenger, IBinder binder, @NonNull String callingPackageName) { + final int callingUid = Binder.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); - restrictRequestUidsForCaller(nc); + Binder.getCallingPid(), callingUid, callingPackageName); + restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); // Apps without the CHANGE_NETWORK_STATE permission can't use background networks, so // make all their listens include NET_CAPABILITY_FOREGROUND. That way, they will get // onLost and onAvailable callbacks when networks move in and out of the background. @@ -5588,17 +5597,17 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public void pendingListenForNetwork(NetworkCapabilities networkCapabilities, - PendingIntent operation) { + PendingIntent operation, @NonNull String callingPackageName) { checkNotNull(operation, "PendingIntent cannot be null."); + final int callingUid = Binder.getCallingUid(); if (!hasWifiNetworkListenPermission(networkCapabilities)) { enforceAccessPermission(); } ensureValid(networkCapabilities); ensureSufficientPermissionsForRequest(networkCapabilities, - Binder.getCallingPid(), Binder.getCallingUid()); - + Binder.getCallingPid(), callingUid, callingPackageName); final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); - restrictRequestUidsForCaller(nc); + restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); NetworkRequest networkRequest = new NetworkRequest(nc, TYPE_NONE, nextNetworkRequestId(), NetworkRequest.Type.LISTEN); @@ -7897,12 +7906,13 @@ public class ConnectivityService extends IConnectivityManager.Stub throw new IllegalArgumentException("ConnectivityManager.TYPE_* are deprecated." + " Please use NetworkCapabilities instead."); } - mAppOpsManager.checkPackage(Binder.getCallingUid(), callingPackageName); + final int callingUid = Binder.getCallingUid(); + mAppOpsManager.checkPackage(callingUid, callingPackageName); // This NetworkCapabilities is only used for matching to Networks. Clear out its owner uid // and administrator uids to be safe. final NetworkCapabilities nc = new NetworkCapabilities(request.networkCapabilities); - restrictRequestUidsForCaller(nc); + restrictRequestUidsForCallerAndSetRequestorInfo(nc, callingUid, callingPackageName); final NetworkRequest requestWithId = new NetworkRequest( diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java index 3e4f3d8188..efea91ab91 100644 --- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java @@ -272,9 +272,23 @@ public class NetworkCapabilitiesTest { netCap.setOwnerUid(123); assertParcelingIsLossless(netCap); netCap.setSSID(TEST_SSID); - assertParcelSane(netCap, 13); + assertParcelSane(netCap, 15); } + @Test + public void testParcelNetworkCapabilitiesWithRequestorUidAndPackageName() { + final NetworkCapabilities netCap = new NetworkCapabilities() + .addCapability(NET_CAPABILITY_INTERNET) + .setRequestorUid(9304) + .setRequestorPackageName("com.android.test") + .addCapability(NET_CAPABILITY_EIMS) + .addCapability(NET_CAPABILITY_NOT_METERED); + assertParcelingIsLossless(netCap); + netCap.setSSID(TEST_SSID); + assertParcelSane(netCap, 15); + } + + @Test public void testOemPaid() { NetworkCapabilities nc = new NetworkCapabilities(); diff --git a/tests/net/java/android/net/ConnectivityManagerTest.java b/tests/net/java/android/net/ConnectivityManagerTest.java index 7ede14428a..d6bf334ee5 100644 --- a/tests/net/java/android/net/ConnectivityManagerTest.java +++ b/tests/net/java/android/net/ConnectivityManagerTest.java @@ -212,7 +212,8 @@ public class ConnectivityManagerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Messenger.class); // register callback - when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) + when(mService.requestNetwork( + any(), captor.capture(), anyInt(), any(), anyInt(), any())) .thenReturn(request); manager.requestNetwork(request, callback, handler); @@ -240,7 +241,8 @@ public class ConnectivityManagerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(Messenger.class); // register callback - when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) + when(mService.requestNetwork( + any(), captor.capture(), anyInt(), any(), anyInt(), any())) .thenReturn(req1); manager.requestNetwork(req1, callback, handler); @@ -258,7 +260,8 @@ public class ConnectivityManagerTest { verify(callback, timeout(100).times(0)).onLosing(any(), anyInt()); // callback can be registered again - when(mService.requestNetwork(any(), captor.capture(), anyInt(), any(), anyInt())) + when(mService.requestNetwork( + any(), captor.capture(), anyInt(), any(), anyInt(), any())) .thenReturn(req2); manager.requestNetwork(req2, callback, handler); @@ -282,7 +285,8 @@ public class ConnectivityManagerTest { info.targetSdkVersion = VERSION_CODES.N_MR1 + 1; when(mCtx.getApplicationInfo()).thenReturn(info); - when(mService.requestNetwork(any(), any(), anyInt(), any(), anyInt())).thenReturn(request); + when(mService.requestNetwork(any(), any(), anyInt(), any(), anyInt(), any())) + .thenReturn(request); Handler handler = new Handler(Looper.getMainLooper()); manager.requestNetwork(request, callback, handler); diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index a906805d3a..77147c8a35 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -108,6 +108,7 @@ import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -311,6 +312,7 @@ public class ConnectivityServiceTest { private static final String MOBILE_IFNAME = "test_rmnet_data0"; private static final String WIFI_IFNAME = "test_wlan0"; private static final String WIFI_WOL_IFNAME = "test_wlan_wol"; + private static final String TEST_PACKAGE_NAME = "com.android.test.package"; private static final String[] EMPTY_STRING_ARRAY = new String[0]; private MockContext mServiceContext; @@ -666,7 +668,7 @@ public class ConnectivityServiceTest { if (mNmValidationRedirectUrl != null) { mNmCallbacks.showProvisioningNotification( - "test_provisioning_notif_action", "com.android.test.package"); + "test_provisioning_notif_action", TEST_PACKAGE_NAME); mNmProvNotificationRequested = true; } } @@ -3039,7 +3041,7 @@ public class ConnectivityServiceTest { networkCapabilities.addTransportType(TRANSPORT_WIFI) .setNetworkSpecifier(new MatchAllNetworkSpecifier()); mService.requestNetwork(networkCapabilities, null, 0, null, - ConnectivityManager.TYPE_WIFI); + ConnectivityManager.TYPE_WIFI, TEST_PACKAGE_NAME); }); class NonParcelableSpecifier extends NetworkSpecifier { @@ -3078,31 +3080,12 @@ public class ConnectivityServiceTest { } @Test - public void testNetworkSpecifierUidSpoofSecurityException() throws Exception { - class UidAwareNetworkSpecifier extends NetworkSpecifier implements Parcelable { - @Override - public boolean satisfiedBy(NetworkSpecifier other) { - return true; - } - - @Override - public void assertValidFromUid(int requestorUid) { - throw new SecurityException("failure"); - } - - @Override - public int describeContents() { return 0; } - @Override - public void writeToParcel(Parcel dest, int flags) {} - } - + public void testNetworkRequestUidSpoofSecurityException() throws Exception { mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); mWiFiNetworkAgent.connect(false); - - UidAwareNetworkSpecifier networkSpecifier = new UidAwareNetworkSpecifier(); - NetworkRequest networkRequest = newWifiRequestBuilder().setNetworkSpecifier( - networkSpecifier).build(); + NetworkRequest networkRequest = newWifiRequestBuilder().build(); TestNetworkCallback networkCallback = new TestNetworkCallback(); + doThrow(new SecurityException()).when(mAppOpsManager).checkPackage(anyInt(), anyString()); assertThrows(SecurityException.class, () -> { mCm.requestNetwork(networkRequest, networkCallback); });