Merge "Mark several NetworkAgentInfo members @NonNull and address TODO."
diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 8742ecb..ce0ed5b 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -72,7 +72,6 @@
import libcore.net.event.NetworkEventDispatcher;
-import java.io.FileDescriptor;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.annotation.Retention;
@@ -1958,6 +1957,12 @@
return k;
}
+ // Construct an invalid fd.
+ private ParcelFileDescriptor createInvalidFd() {
+ final int invalidFd = -1;
+ return ParcelFileDescriptor.adoptFd(invalidFd);
+ }
+
/**
* Request that keepalives be started on a IPsec NAT-T socket.
*
@@ -1988,7 +1993,7 @@
} catch (IOException ignored) {
// Construct an invalid fd, so that if the user later calls start(), it will fail with
// ERROR_INVALID_SOCKET.
- dup = new ParcelFileDescriptor(new FileDescriptor());
+ dup = createInvalidFd();
}
return new NattSocketKeepalive(mService, network, dup, socket.getResourceId(), source,
destination, executor, callback);
@@ -2030,7 +2035,7 @@
} catch (IOException ignored) {
// Construct an invalid fd, so that if the user later calls start(), it will fail with
// ERROR_INVALID_SOCKET.
- dup = new ParcelFileDescriptor(new FileDescriptor());
+ dup = createInvalidFd();
}
return new NattSocketKeepalive(mService, network, dup,
INVALID_RESOURCE_ID /* Unused */, source, destination, executor, callback);
@@ -2067,7 +2072,7 @@
} catch (UncheckedIOException ignored) {
// Construct an invalid fd, so that if the user later calls start(), it will fail with
// ERROR_INVALID_SOCKET.
- dup = new ParcelFileDescriptor(new FileDescriptor());
+ dup = createInvalidFd();
}
return new TcpSocketKeepalive(mService, network, dup, executor, callback);
}
diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java
index 4166c2c..4f46736 100644
--- a/core/java/android/net/NetworkAgent.java
+++ b/core/java/android/net/NetworkAgent.java
@@ -408,7 +408,8 @@
throw new IllegalArgumentException();
}
- mInitialConfiguration = new InitialConfiguration(context, new NetworkCapabilities(nc),
+ mInitialConfiguration = new InitialConfiguration(context,
+ new NetworkCapabilities(nc, /* parcelLocationSensitiveFields */ true),
new LinkProperties(lp), score, config, ni);
}
@@ -818,7 +819,9 @@
Objects.requireNonNull(networkCapabilities);
mBandwidthUpdatePending.set(false);
mLastBwRefreshTime = System.currentTimeMillis();
- final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities);
+ final NetworkCapabilities nc =
+ new NetworkCapabilities(networkCapabilities,
+ /* parcelLocationSensitiveFields */ true);
queueOrSendMessage(reg -> reg.sendNetworkCapabilities(nc));
}
diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java
index 48c4832..2d9f6d8 100644
--- a/core/java/android/net/NetworkCapabilities.java
+++ b/core/java/android/net/NetworkCapabilities.java
@@ -76,12 +76,33 @@
*/
private String mRequestorPackageName;
+ /**
+ * Indicates whether parceling should preserve fields that are set based on permissions of
+ * the process receiving the {@link NetworkCapabilities}.
+ */
+ private final boolean mParcelLocationSensitiveFields;
+
public NetworkCapabilities() {
+ mParcelLocationSensitiveFields = false;
clearAll();
mNetworkCapabilities = DEFAULT_CAPABILITIES;
}
public NetworkCapabilities(NetworkCapabilities nc) {
+ this(nc, false /* parcelLocationSensitiveFields */);
+ }
+
+ /**
+ * Make a copy of NetworkCapabilities.
+ *
+ * @param nc Original NetworkCapabilities
+ * @param parcelLocationSensitiveFields Whether to parcel location sensitive data or not.
+ * @hide
+ */
+ @SystemApi
+ public NetworkCapabilities(
+ @Nullable NetworkCapabilities nc, boolean parcelLocationSensitiveFields) {
+ mParcelLocationSensitiveFields = parcelLocationSensitiveFields;
if (nc != null) {
set(nc);
}
@@ -93,6 +114,12 @@
* @hide
*/
public void clearAll() {
+ // Ensures that the internal copies maintained by the connectivity stack does not set
+ // this bit.
+ if (mParcelLocationSensitiveFields) {
+ throw new UnsupportedOperationException(
+ "Cannot clear NetworkCapabilities when parcelLocationSensitiveFields is set");
+ }
mNetworkCapabilities = mTransportTypes = mUnwantedNetworkCapabilities = 0;
mLinkUpBandwidthKbps = mLinkDownBandwidthKbps = LINK_BANDWIDTH_UNSPECIFIED;
mNetworkSpecifier = null;
@@ -109,6 +136,8 @@
/**
* Set all contents of this object to the contents of a NetworkCapabilities.
+ *
+ * @param nc Original NetworkCapabilities
* @hide
*/
public void set(@NonNull NetworkCapabilities nc) {
@@ -117,7 +146,11 @@
mLinkUpBandwidthKbps = nc.mLinkUpBandwidthKbps;
mLinkDownBandwidthKbps = nc.mLinkDownBandwidthKbps;
mNetworkSpecifier = nc.mNetworkSpecifier;
- mTransportInfo = nc.mTransportInfo;
+ if (nc.getTransportInfo() != null) {
+ setTransportInfo(nc.getTransportInfo().makeCopy(mParcelLocationSensitiveFields));
+ } else {
+ setTransportInfo(null);
+ }
mSignalStrength = nc.mSignalStrength;
setUids(nc.mUids); // Will make the defensive copy
setAdministratorUids(nc.getAdministratorUids());
diff --git a/core/java/android/net/TransportInfo.java b/core/java/android/net/TransportInfo.java
index b78d3fe..aa4bbb0 100644
--- a/core/java/android/net/TransportInfo.java
+++ b/core/java/android/net/TransportInfo.java
@@ -16,10 +16,48 @@
package android.net;
+import android.annotation.NonNull;
+import android.annotation.SystemApi;
+
/**
* A container for transport-specific capabilities which is returned by
* {@link NetworkCapabilities#getTransportInfo()}. Specific networks
* may provide concrete implementations of this interface.
+ * @see android.net.wifi.aware.WifiAwareNetworkInfo
+ * @see android.net.wifi.WifiInfo
*/
public interface TransportInfo {
+
+ /**
+ * Create a copy of a {@link TransportInfo} that will preserve location sensitive fields that
+ * were set based on the permissions of the process that originally received it.
+ *
+ * <p>By default {@link TransportInfo} does not preserve such fields during parceling, as
+ * they should not be shared outside of the process that receives them without appropriate
+ * checks.
+ *
+ * @param parcelLocationSensitiveFields Whether the location sensitive fields should be kept
+ * when parceling
+ * @return Copy of this instance.
+ * @hide
+ */
+ @SystemApi
+ @NonNull
+ default TransportInfo makeCopy(boolean parcelLocationSensitiveFields) {
+ return this;
+ }
+
+ /**
+ * Returns whether this TransportInfo type has location sensitive fields or not (helps
+ * to determine whether to perform a location permission check or not before sending to
+ * apps).
+ *
+ * @return {@code true} if this instance contains location sensitive info, {@code false}
+ * otherwise.
+ * @hide
+ */
+ @SystemApi
+ default boolean hasLocationSensitiveFields() {
+ return false;
+ }
}
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 55774c4..53a8597 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -194,7 +194,6 @@
import com.android.server.connectivity.DataConnectionStats;
import com.android.server.connectivity.DnsManager;
import com.android.server.connectivity.DnsManager.PrivateDnsValidationUpdate;
-import com.android.server.connectivity.IpConnectivityMetrics;
import com.android.server.connectivity.KeepaliveTracker;
import com.android.server.connectivity.LingerMonitor;
import com.android.server.connectivity.MockableSystemProperties;
@@ -918,14 +917,6 @@
return new MultinetworkPolicyTracker(c, h, r);
}
- /**
- * @see IpConnectivityMetrics.Logger
- */
- public IpConnectivityMetrics.Logger getMetricsLogger() {
- return Objects.requireNonNull(LocalServices.getService(IpConnectivityMetrics.Logger.class),
- "no IpConnectivityMetrics service");
- }
-
public IBatteryStats getBatteryStatsService() {
return BatteryStatsService.getService();
}
@@ -1569,7 +1560,7 @@
if (nc != null) {
result.put(
nai.network,
- maybeSanitizeLocationInfoForCaller(
+ createWithLocationInfoSanitizedIfNecessaryWhenParceled(
nc, mDeps.getCallingUid(), callingPackageName));
}
@@ -1579,7 +1570,9 @@
for (Network network : networks) {
nc = getNetworkCapabilitiesInternal(network);
if (nc != null) {
- result.put(network, maybeSanitizeLocationInfoForCaller(
+ result.put(
+ network,
+ createWithLocationInfoSanitizedIfNecessaryWhenParceled(
nc, mDeps.getCallingUid(), callingPackageName));
}
}
@@ -1661,7 +1654,7 @@
public NetworkCapabilities getNetworkCapabilities(Network network, String callingPackageName) {
mAppOpsManager.checkPackage(mDeps.getCallingUid(), callingPackageName);
enforceAccessPermission();
- return maybeSanitizeLocationInfoForCaller(
+ return createWithLocationInfoSanitizedIfNecessaryWhenParceled(
getNetworkCapabilitiesInternal(network),
mDeps.getCallingUid(), callingPackageName);
}
@@ -1682,37 +1675,51 @@
return newNc;
}
+ private boolean hasLocationPermission(int callerUid, @NonNull String callerPkgName) {
+ final long token = Binder.clearCallingIdentity();
+ try {
+ return mLocationPermissionChecker.checkLocationPermission(
+ callerPkgName, null /* featureId */, callerUid, null /* message */);
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
+ }
+
@VisibleForTesting
@Nullable
- NetworkCapabilities maybeSanitizeLocationInfoForCaller(
+ NetworkCapabilities createWithLocationInfoSanitizedIfNecessaryWhenParceled(
@Nullable NetworkCapabilities nc, int callerUid, @NonNull String callerPkgName) {
if (nc == null) {
return null;
}
- final NetworkCapabilities newNc = new NetworkCapabilities(nc);
- if (callerUid != newNc.getOwnerUid()) {
+ Boolean hasLocationPermission = null;
+ final NetworkCapabilities newNc;
+ // Avoid doing location permission check if the transport info has no location sensitive
+ // data.
+ if (nc.getTransportInfo() != null && nc.getTransportInfo().hasLocationSensitiveFields()) {
+ hasLocationPermission = hasLocationPermission(callerUid, callerPkgName);
+ newNc = new NetworkCapabilities(nc, hasLocationPermission);
+ } else {
+ newNc = new NetworkCapabilities(nc, false /* parcelLocationSensitiveFields */);
+ }
+ // Reset owner uid if not destined for the owner app.
+ if (callerUid != nc.getOwnerUid()) {
newNc.setOwnerUid(INVALID_UID);
return newNc;
}
-
// Allow VPNs to see ownership of their own VPN networks - not location sensitive.
if (nc.hasTransport(TRANSPORT_VPN)) {
// Owner UIDs already checked above. No need to re-check.
return newNc;
}
-
- final long token = Binder.clearCallingIdentity();
- try {
- if (!mLocationPermissionChecker.checkLocationPermission(
- callerPkgName, null /* featureId */, callerUid, null /* message */)) {
- // Caller does not have the requisite location permissions. Reset the
- // owner's UID in the NetworkCapabilities.
- newNc.setOwnerUid(INVALID_UID);
- }
- } finally {
- Binder.restoreCallingIdentity(token);
+ if (hasLocationPermission == null) {
+ // Location permission not checked yet, check now for masking owner UID.
+ hasLocationPermission = hasLocationPermission(callerUid, callerPkgName);
}
-
+ // Reset owner uid if the app has no location permission.
+ if (!hasLocationPermission) {
+ newNc.setOwnerUid(INVALID_UID);
+ }
return newNc;
}
@@ -6971,7 +6978,7 @@
networkAgent.networkCapabilities, nri.mPid, nri.mUid);
putParcelable(
bundle,
- maybeSanitizeLocationInfoForCaller(
+ createWithLocationInfoSanitizedIfNecessaryWhenParceled(
nc, nri.mUid, nri.request.getRequestorPackageName()));
putParcelable(bundle, linkPropertiesRestrictedForCallerPermissions(
networkAgent.linkProperties, nri.mPid, nri.mUid));
@@ -6990,7 +6997,7 @@
networkAgent.networkCapabilities, nri.mPid, nri.mUid);
putParcelable(
bundle,
- maybeSanitizeLocationInfoForCaller(
+ createWithLocationInfoSanitizedIfNecessaryWhenParceled(
netCap, nri.mUid, nri.request.getRequestorPackageName()));
break;
}
diff --git a/services/core/java/com/android/server/connectivity/ProxyTracker.java b/services/core/java/com/android/server/connectivity/ProxyTracker.java
index d83ff83..b618d2b 100644
--- a/services/core/java/com/android/server/connectivity/ProxyTracker.java
+++ b/services/core/java/com/android/server/connectivity/ProxyTracker.java
@@ -226,9 +226,9 @@
final ProxyInfo defaultProxy = getDefaultProxy();
final ProxyInfo proxyInfo = null != defaultProxy ?
defaultProxy : ProxyInfo.buildDirectProxy("", 0, Collections.emptyList());
+ mPacProxyInstaller.setCurrentProxyScriptUrl(proxyInfo);
- if (mPacProxyInstaller.setCurrentProxyScriptUrl(proxyInfo)
- == PacProxyInstaller.DONT_SEND_BROADCAST) {
+ if (!shouldSendBroadcast(proxyInfo)) {
return;
}
if (DBG) Log.d(TAG, "sending Proxy Broadcast for " + proxyInfo);
@@ -244,6 +244,13 @@
}
}
+ private boolean shouldSendBroadcast(ProxyInfo proxy) {
+ if (Uri.EMPTY.equals(proxy.getPacFileUrl())) return false;
+ if (proxy.getPacFileUrl().equals(proxy.getPacFileUrl())
+ && (proxy.getPort() > 0)) return true;
+ return true;
+ }
+
/**
* Sets the global proxy in memory. Also writes the values to the global settings of the device.
*
diff --git a/tests/net/common/Android.bp b/tests/net/common/Android.bp
index 373aac6..c271f49 100644
--- a/tests/net/common/Android.bp
+++ b/tests/net/common/Android.bp
@@ -24,6 +24,7 @@
"androidx.test.rules",
"junit",
"mockito-target-minus-junit4",
+ "modules-utils-build",
"net-tests-utils",
"net-utils-framework-common",
"platform-test-annotations",
diff --git a/tests/net/common/java/android/net/CaptivePortalDataTest.kt b/tests/net/common/java/android/net/CaptivePortalDataTest.kt
index 8710d23..2cb16d3 100644
--- a/tests/net/common/java/android/net/CaptivePortalDataTest.kt
+++ b/tests/net/common/java/android/net/CaptivePortalDataTest.kt
@@ -18,12 +18,15 @@
import android.os.Build
import androidx.test.filters.SmallTest
+import com.android.modules.utils.build.SdkLevel
import com.android.testutils.assertParcelSane
import com.android.testutils.assertParcelingIsLossless
+import com.android.testutils.DevSdkIgnoreRule
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
import com.android.testutils.DevSdkIgnoreRunner
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
+import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import kotlin.test.assertEquals
@@ -33,6 +36,9 @@
@RunWith(DevSdkIgnoreRunner::class)
@IgnoreUpTo(Build.VERSION_CODES.Q)
class CaptivePortalDataTest {
+ @Rule @JvmField
+ val ignoreRule = DevSdkIgnoreRule()
+
private val data = CaptivePortalData.Builder()
.setRefreshTime(123L)
.setUserPortalUrl(Uri.parse("https://portal.example.com/test"))
@@ -41,7 +47,11 @@
.setBytesRemaining(456L)
.setExpiryTime(789L)
.setCaptive(true)
- .setVenueFriendlyName("venue friendly name")
+ .apply {
+ if (SdkLevel.isAtLeastS()) {
+ setVenueFriendlyName("venue friendly name")
+ }
+ }
.build()
private fun makeBuilder() = CaptivePortalData.Builder(data)
@@ -67,8 +77,11 @@
assertNotEqualsAfterChange { it.setBytesRemaining(789L) }
assertNotEqualsAfterChange { it.setExpiryTime(12L) }
assertNotEqualsAfterChange { it.setCaptive(false) }
- assertNotEqualsAfterChange { it.setVenueFriendlyName("another friendly name") }
- assertNotEqualsAfterChange { it.setVenueFriendlyName(null) }
+
+ if (SdkLevel.isAtLeastS()) {
+ assertNotEqualsAfterChange { it.setVenueFriendlyName("another friendly name") }
+ assertNotEqualsAfterChange { it.setVenueFriendlyName(null) }
+ }
}
@Test
@@ -111,7 +124,7 @@
assertFalse(makeBuilder().setCaptive(false).build().isCaptive)
}
- @Test
+ @Test @IgnoreUpTo(Build.VERSION_CODES.R)
fun testVenueFriendlyName() {
assertEquals("venue friendly name", data.venueFriendlyName)
}
diff --git a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java
index 6b7ea66..5d0e016 100644
--- a/tests/net/common/java/android/net/NetworkCapabilitiesTest.java
+++ b/tests/net/common/java/android/net/NetworkCapabilitiesTest.java
@@ -42,9 +42,11 @@
import static android.net.NetworkCapabilities.TRANSPORT_WIFI;
import static android.net.NetworkCapabilities.TRANSPORT_WIFI_AWARE;
import static android.net.NetworkCapabilities.UNRESTRICTED_CAPABILITIES;
+import static android.os.Process.INVALID_UID;
import static com.android.testutils.ParcelUtils.assertParcelSane;
import static com.android.testutils.ParcelUtils.assertParcelingIsLossless;
+import static com.android.testutils.ParcelUtils.parcelingRoundTrip;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
@@ -53,18 +55,19 @@
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.junit.Assume.assumeTrue;
+import android.net.wifi.WifiInfo;
import android.net.wifi.aware.DiscoverySession;
import android.net.wifi.aware.PeerHandle;
import android.net.wifi.aware.WifiAwareNetworkSpecifier;
import android.os.Build;
-import android.os.Process;
import android.test.suitebuilder.annotation.SmallTest;
import android.util.ArraySet;
-import androidx.core.os.BuildCompat;
import androidx.test.runner.AndroidJUnit4;
+import com.android.modules.utils.build.SdkLevel;
import com.android.testutils.DevSdkIgnoreRule;
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
@@ -89,10 +92,11 @@
private PeerHandle mPeerHandle = Mockito.mock(PeerHandle.class);
private boolean isAtLeastR() {
- // BuildCompat.isAtLeastR() is used to check the Android version before releasing Android R.
- // Build.VERSION.SDK_INT > Build.VERSION_CODES.Q is used to check the Android version after
- // releasing Android R.
- return BuildCompat.isAtLeastR() || Build.VERSION.SDK_INT > Build.VERSION_CODES.Q;
+ return SdkLevel.isAtLeastR();
+ }
+
+ private boolean isAtLeastS() {
+ return SdkLevel.isAtLeastS();
}
@Test
@@ -324,8 +328,59 @@
testParcelSane(netCap);
}
+ private NetworkCapabilities createNetworkCapabilitiesWithWifiInfo() {
+ // uses a real WifiInfo to test parceling of sensitive data.
+ final WifiInfo wifiInfo = new WifiInfo.Builder()
+ .setSsid("sssid1234".getBytes())
+ .setBssid("00:11:22:33:44:55")
+ .build();
+ return new NetworkCapabilities()
+ .addCapability(NET_CAPABILITY_INTERNET)
+ .addCapability(NET_CAPABILITY_EIMS)
+ .addCapability(NET_CAPABILITY_NOT_METERED)
+ .setSSID(TEST_SSID)
+ .setTransportInfo(wifiInfo)
+ .setRequestorPackageName("com.android.test")
+ .setRequestorUid(9304);
+ }
+
+ @Test
+ public void testParcelNetworkCapabilitiesWithLocationSensitiveFields() {
+ assumeTrue(isAtLeastS());
+
+ final NetworkCapabilities netCap = createNetworkCapabilitiesWithWifiInfo();
+ final NetworkCapabilities netCapWithLocationSensitiveFields =
+ new NetworkCapabilities(netCap, true);
+
+ assertParcelingIsLossless(netCapWithLocationSensitiveFields);
+ testParcelSane(netCapWithLocationSensitiveFields);
+
+ assertEquals(netCapWithLocationSensitiveFields,
+ parcelingRoundTrip(netCapWithLocationSensitiveFields));
+ }
+
+ @Test
+ public void testParcelNetworkCapabilitiesWithoutLocationSensitiveFields() {
+ assumeTrue(isAtLeastS());
+
+ final NetworkCapabilities netCap = createNetworkCapabilitiesWithWifiInfo();
+ final NetworkCapabilities netCapWithoutLocationSensitiveFields =
+ new NetworkCapabilities(netCap, false);
+
+ final NetworkCapabilities sanitizedNetCap =
+ new NetworkCapabilities(netCapWithoutLocationSensitiveFields);
+ final WifiInfo sanitizedWifiInfo = new WifiInfo.Builder()
+ .setSsid(new byte[0])
+ .setBssid(WifiInfo.DEFAULT_MAC_ADDRESS)
+ .build();
+ sanitizedNetCap.setTransportInfo(sanitizedWifiInfo);
+ assertEquals(sanitizedNetCap, parcelingRoundTrip(netCapWithoutLocationSensitiveFields));
+ }
+
private void testParcelSane(NetworkCapabilities cap) {
- if (isAtLeastR()) {
+ if (isAtLeastS()) {
+ assertParcelSane(cap, 16);
+ } else if (isAtLeastR()) {
assertParcelSane(cap, 15);
} else {
assertParcelSane(cap, 11);
@@ -639,26 +694,23 @@
// Sequence 1: Transport + Transport + TransportInfo
NetworkCapabilities nc1 = new NetworkCapabilities();
nc1.addTransportType(TRANSPORT_CELLULAR).addTransportType(TRANSPORT_WIFI)
- .setTransportInfo(new TransportInfo() {});
+ .setTransportInfo(new TestTransportInfo());
// Sequence 2: Transport + NetworkSpecifier + Transport
NetworkCapabilities nc2 = new NetworkCapabilities();
- nc2.addTransportType(TRANSPORT_CELLULAR).setTransportInfo(new TransportInfo() {})
+ nc2.addTransportType(TRANSPORT_CELLULAR).setTransportInfo(new TestTransportInfo())
.addTransportType(TRANSPORT_WIFI);
}
@Test
public void testCombineTransportInfo() {
NetworkCapabilities nc1 = new NetworkCapabilities();
- nc1.setTransportInfo(new TransportInfo() {
- // empty
- });
+ nc1.setTransportInfo(new TestTransportInfo());
+
NetworkCapabilities nc2 = new NetworkCapabilities();
// new TransportInfo so that object is not #equals to nc1's TransportInfo (that's where
// combine fails)
- nc2.setTransportInfo(new TransportInfo() {
- // empty
- });
+ nc2.setTransportInfo(new TestTransportInfo());
try {
nc1.combineCapabilities(nc2);
@@ -761,7 +813,7 @@
// Test default owner uid.
// If the owner uid is not set, the default value should be Process.INVALID_UID.
final NetworkCapabilities nc1 = new NetworkCapabilities.Builder().build();
- assertEquals(Process.INVALID_UID, nc1.getOwnerUid());
+ assertEquals(INVALID_UID, nc1.getOwnerUid());
// Test setAdministratorUids and getAdministratorUids.
final int[] administratorUids = {1001, 10001};
final NetworkCapabilities nc2 = new NetworkCapabilities.Builder()
@@ -906,6 +958,16 @@
private class TestTransportInfo implements TransportInfo {
TestTransportInfo() {
}
+
+ @Override
+ public TransportInfo makeCopy(boolean parcelLocationSensitiveFields) {
+ return this;
+ }
+
+ @Override
+ public boolean hasLocationSensitiveFields() {
+ return false;
+ }
}
@Test @IgnoreUpTo(Build.VERSION_CODES.Q)
diff --git a/tests/net/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/net/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
index 8e18751..16c4865 100644
--- a/tests/net/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
+++ b/tests/net/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
@@ -46,8 +46,6 @@
import com.android.server.LocalServices
import com.android.server.NetworkAgentWrapper
import com.android.server.TestNetIdManager
-import com.android.server.connectivity.DefaultNetworkMetrics
-import com.android.server.connectivity.IpConnectivityMetrics
import com.android.server.connectivity.MockableSystemProperties
import com.android.server.connectivity.ProxyTracker
import com.android.server.net.NetworkPolicyManagerInternal
@@ -92,10 +90,6 @@
private lateinit var netd: INetd
@Mock
private lateinit var dnsResolver: IDnsResolver
- @Mock
- private lateinit var metricsLogger: IpConnectivityMetrics.Logger
- @Mock
- private lateinit var defaultMetrics: DefaultNetworkMetrics
@Spy
private var context = TestableContext(realContext)
@@ -149,7 +143,6 @@
@Before
fun setUp() {
MockitoAnnotations.initMocks(this)
- doReturn(defaultMetrics).`when`(metricsLogger).defaultNetworkMetrics()
doNothing().`when`(context).sendStickyBroadcastAsUser(any(), any(), any())
networkStackClient = TestNetworkStackClient(realContext)
@@ -173,7 +166,6 @@
private fun makeDependencies(): ConnectivityService.Dependencies {
val deps = spy(ConnectivityService.Dependencies())
doReturn(networkStackClient).`when`(deps).networkStack
- doReturn(metricsLogger).`when`(deps).metricsLogger
doReturn(mock(ProxyTracker::class.java)).`when`(deps).makeProxyTracker(any(), any())
doReturn(mock(MockableSystemProperties::class.java)).`when`(deps).systemProperties
doReturn(TestNetIdManager()).`when`(deps).makeNetIdManager()
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index 2a0414f..abf16f2 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -201,6 +201,7 @@
import android.net.shared.NetworkMonitorUtils;
import android.net.shared.PrivateDnsConfig;
import android.net.util.MultinetworkPolicyTracker;
+import android.net.wifi.WifiInfo;
import android.os.BadParcelableException;
import android.os.Binder;
import android.os.Build;
@@ -242,8 +243,6 @@
import com.android.internal.util.test.FakeSettingsProvider;
import com.android.server.ConnectivityService.ConnectivityDiagnosticsCallbackInfo;
import com.android.server.connectivity.ConnectivityConstants;
-import com.android.server.connectivity.DefaultNetworkMetrics;
-import com.android.server.connectivity.IpConnectivityMetrics;
import com.android.server.connectivity.MockableSystemProperties;
import com.android.server.connectivity.Nat464Xlat;
import com.android.server.connectivity.NetworkAgentInfo;
@@ -366,8 +365,6 @@
private HandlerThread mAlarmManagerThread;
private TestNetIdManager mNetIdManager;
- @Mock IpConnectivityMetrics.Logger mMetricsService;
- @Mock DefaultNetworkMetrics mDefaultNetworkMetrics;
@Mock DeviceIdleInternal mDeviceIdleInternal;
@Mock INetworkManagementService mNetworkManagementService;
@Mock INetworkStatsService mStatsService;
@@ -1286,7 +1283,6 @@
mContext = InstrumentationRegistry.getContext();
MockitoAnnotations.initMocks(this);
- when(mMetricsService.defaultNetworkMetrics()).thenReturn(mDefaultNetworkMetrics);
when(mUserManager.getAliveUsers()).thenReturn(
Arrays.asList(new UserInfo[] {
@@ -1361,7 +1357,6 @@
doReturn(mNetworkStack).when(deps).getNetworkStack();
doReturn(mSystemProperties).when(deps).getSystemProperties();
doReturn(mock(ProxyTracker.class)).when(deps).makeProxyTracker(any(), any());
- doReturn(mMetricsService).when(deps).getMetricsLogger();
doReturn(true).when(deps).queryUserAccess(anyInt(), anyInt());
doReturn(mBatteryStatsService).when(deps).getBatteryStatsService();
doAnswer(inv -> {
@@ -2073,10 +2068,6 @@
@Test
public void testOwnerUidCannotChange() throws Exception {
- // Owner UIDs are not visible without location permission.
- setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
- Manifest.permission.ACCESS_FINE_LOCATION);
-
final NetworkCapabilities ncTemplate = new NetworkCapabilities();
final int originalOwnerUid = Process.myUid();
ncTemplate.setOwnerUid(originalOwnerUid);
@@ -2096,6 +2087,10 @@
mWiFiNetworkAgent.setNetworkCapabilities(agentCapabilities, true);
waitForIdle();
+ // Owner UIDs are not visible without location permission.
+ setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
+ Manifest.permission.ACCESS_FINE_LOCATION);
+
// Check that the capability change has been applied but the owner UID is not modified.
NetworkCapabilities nc = mCm.getNetworkCapabilities(mWiFiNetworkAgent.getNetwork());
assertEquals(originalOwnerUid, nc.getOwnerUid());
@@ -7780,8 +7775,22 @@
naExtraInfo.unregister();
}
+ // To avoid granting location permission bypass.
+ private void denyAllLocationPrivilegedPermissions() {
+ mServiceContext.setPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK,
+ PERMISSION_DENIED);
+ mServiceContext.setPermission(Manifest.permission.NETWORK_SETTINGS,
+ PERMISSION_DENIED);
+ mServiceContext.setPermission(Manifest.permission.NETWORK_STACK,
+ PERMISSION_DENIED);
+ mServiceContext.setPermission(Manifest.permission.NETWORK_SETUP_WIZARD,
+ PERMISSION_DENIED);
+ }
+
private void setupLocationPermissions(
int targetSdk, boolean locationToggle, String op, String perm) throws Exception {
+ denyAllLocationPrivilegedPermissions();
+
final ApplicationInfo applicationInfo = new ApplicationInfo();
applicationInfo.targetSdkVersion = targetSdk;
when(mPackageManager.getApplicationInfoAsUser(anyString(), anyInt(), any()))
@@ -7802,51 +7811,76 @@
private int getOwnerUidNetCapsForCallerPermission(int ownerUid, int callerUid) {
final NetworkCapabilities netCap = new NetworkCapabilities().setOwnerUid(ownerUid);
- return mService
- .maybeSanitizeLocationInfoForCaller(netCap, callerUid, mContext.getPackageName())
- .getOwnerUid();
+ return mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled(
+ netCap, callerUid, mContext.getPackageName()).getOwnerUid();
+ }
+
+ private void verifyWifiInfoCopyNetCapsForCallerPermission(
+ int callerUid, boolean shouldMakeCopyWithLocationSensitiveFieldsParcelable) {
+ final WifiInfo wifiInfo = mock(WifiInfo.class);
+ when(wifiInfo.hasLocationSensitiveFields()).thenReturn(true);
+ final NetworkCapabilities netCap = new NetworkCapabilities().setTransportInfo(wifiInfo);
+
+ mService.createWithLocationInfoSanitizedIfNecessaryWhenParceled(
+ netCap, callerUid, mContext.getPackageName());
+ verify(wifiInfo).makeCopy(eq(shouldMakeCopyWithLocationSensitiveFieldsParcelable));
}
@Test
- public void testMaybeSanitizeLocationInfoForCallerWithFineLocationAfterQ() throws Exception {
+ public void testCreateForCallerWithLocationInfoSanitizedWithFineLocationAfterQ()
+ throws Exception {
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION);
final int myUid = Process.myUid();
assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+
+ verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
+ true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
- public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationPreQ() throws Exception {
+ public void testCreateForCallerWithLocationInfoSanitizedWithCoarseLocationPreQ()
+ throws Exception {
setupLocationPermissions(Build.VERSION_CODES.P, true, AppOpsManager.OPSTR_COARSE_LOCATION,
Manifest.permission.ACCESS_COARSE_LOCATION);
final int myUid = Process.myUid();
assertEquals(myUid, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+
+ verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
+ true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
- public void testMaybeSanitizeLocationInfoForCallerLocationOff() throws Exception {
+ public void testCreateForCallerWithLocationInfoSanitizedLocationOff() throws Exception {
// Test that even with fine location permission, and UIDs matching, the UID is sanitized.
setupLocationPermissions(Build.VERSION_CODES.Q, false, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION);
final int myUid = Process.myUid();
assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+
+ verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
+ false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
- public void testMaybeSanitizeLocationInfoForCallerWrongUid() throws Exception {
+ public void testCreateForCallerWithLocationInfoSanitizedWrongUid() throws Exception {
// Test that even with fine location permission, not being the owner leads to sanitization.
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION);
final int myUid = Process.myUid();
assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid + 1, myUid));
+
+ verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
+ true /* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
- public void testMaybeSanitizeLocationInfoForCallerWithCoarseLocationAfterQ() throws Exception {
+ public void testCreateForCallerWithLocationInfoSanitizedWithCoarseLocationAfterQ()
+ throws Exception {
// Test that not having fine location permission leads to sanitization.
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_COARSE_LOCATION,
Manifest.permission.ACCESS_COARSE_LOCATION);
@@ -7854,15 +7888,22 @@
// Test that without the location permission, the owner field is sanitized.
final int myUid = Process.myUid();
assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+
+ verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
+ false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
@Test
- public void testMaybeSanitizeLocationInfoForCallerWithoutLocationPermission() throws Exception {
+ public void testCreateForCallerWithLocationInfoSanitizedWithoutLocationPermission()
+ throws Exception {
setupLocationPermissions(Build.VERSION_CODES.Q, true, null /* op */, null /* perm */);
// Test that without the location permission, the owner field is sanitized.
final int myUid = Process.myUid();
assertEquals(Process.INVALID_UID, getOwnerUidNetCapsForCallerPermission(myUid, myUid));
+
+ verifyWifiInfoCopyNetCapsForCallerPermission(myUid,
+ false/* shouldMakeCopyWithLocationSensitiveFieldsParcelable */);
}
private void setupConnectionOwnerUid(int vpnOwnerUid, @VpnManager.VpnType int vpnType)
@@ -8123,15 +8164,13 @@
new NetworkAgentInfo(null, network, null, null, new NetworkCapabilities(), 0,
mServiceContext, null, null, mService, null, null, null, 0, INVALID_UID);
- setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
- Manifest.permission.ACCESS_FINE_LOCATION);
-
mMockVpn.establishForMyUid();
assertUidRangesUpdatedForMyUid(true);
// Wait for networks to connect and broadcasts to be sent before removing permissions.
waitForIdle();
- mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);
+ setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
+ Manifest.permission.ACCESS_FINE_LOCATION);
assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {network}));
waitForIdle();
@@ -8396,6 +8435,7 @@
mCm.registerNetworkCallback(genericRequest, genericNetworkCallback);
mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback);
mCm.registerNetworkCallback(cellRequest, cellNetworkCallback);
+ waitForIdle();
final ConnectivityService.NetworkRequestInfo[] nriOutput = mService.requestsSortedById();
@@ -8457,4 +8497,20 @@
assertVpnUidRangesUpdated(true, newRanges, VPN_UID);
assertVpnUidRangesUpdated(false, vpnRanges, VPN_UID);
}
+
+ @Test
+ public void testInvalidRequestTypes() {
+ final int[] invalidReqTypeInts = new int[] {-1, NetworkRequest.Type.NONE.ordinal(),
+ NetworkRequest.Type.LISTEN.ordinal(), NetworkRequest.Type.values().length};
+ final NetworkCapabilities nc = new NetworkCapabilities().addTransportType(TRANSPORT_WIFI);
+
+ for (int reqTypeInt : invalidReqTypeInts) {
+ assertThrows("Expect throws for invalid request type " + reqTypeInt,
+ IllegalArgumentException.class,
+ () -> mService.requestNetwork(nc, reqTypeInt, null, 0, null,
+ ConnectivityManager.TYPE_NONE, mContext.getPackageName(),
+ getAttributionTag())
+ );
+ }
+ }
}