Merge "Force specifying sanitized/not sanitized in caps from agent" am: 1896d713cc
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2001256
Change-Id: Idb21a047bbf6566dcdbb603828e5460aaaab983c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index d3480b9..c8b8a0b 100644
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -3604,10 +3604,9 @@
switch (msg.what) {
case NetworkAgent.EVENT_NETWORK_CAPABILITIES_CHANGED: {
- nai.declaredCapabilitiesUnsanitized =
- new NetworkCapabilities((NetworkCapabilities) arg.second);
- final NetworkCapabilities sanitized = sanitizedCapabilitiesFromAgent(
- mCarrierPrivilegeAuthenticator, nai);
+ nai.setDeclaredCapabilities((NetworkCapabilities) arg.second);
+ final NetworkCapabilities sanitized =
+ nai.getDeclaredCapabilitiesSanitized(mCarrierPrivilegeAuthenticator);
maybeUpdateWifiRoamTimestamp(nai, sanitized);
updateCapabilities(nai.getCurrentScore(), nai, sanitized);
break;
@@ -7334,11 +7333,11 @@
// while the network monitor is starting.
final LinkProperties lp = new LinkProperties(nai.linkProperties);
// Store a copy of the declared capabilities.
- nai.declaredCapabilitiesUnsanitized = new NetworkCapabilities(nai.networkCapabilities);
+ nai.setDeclaredCapabilities(nai.networkCapabilities);
// Make sure the LinkProperties and NetworkCapabilities reflect what the agent info said.
- final NetworkCapabilities nc =
- sanitizedCapabilitiesFromAgent(mCarrierPrivilegeAuthenticator, nai);
- nai.getAndSetNetworkCapabilities(mixInCapabilities(nai, nc));
+ final NetworkCapabilities sanitized =
+ nai.getDeclaredCapabilitiesSanitized(mCarrierPrivilegeAuthenticator);
+ nai.getAndSetNetworkCapabilities(mixInCapabilities(nai, sanitized));
processLinkPropertiesFromAgent(nai, lp);
nai.linkProperties = lp;
@@ -7801,38 +7800,6 @@
}
}
- /**
- * Sanitize capabilities coming from a network agent.
- *
- * Agents have restrictions on what capabilities they can send to Connectivity. For example,
- * they can't change the owner UID from what they declared before, and complex restrictions
- * apply to the accessUids field.
- * They also should not mutate immutable capabilities, although for backward-compatibility
- * this is not enforced and limited to just a log.
- *
- * This method returns a sanitized copy of the passed capabilities to make sure they don't
- * contain stuff they should not, and should generally be called by code that accesses
- * {@link NetworkAgentInfo#declaredCapabilitiesUnsanitized}.
- */
- // TODO : move this to NetworkAgentInfo
- private NetworkCapabilities sanitizedCapabilitiesFromAgent(
- final CarrierPrivilegeAuthenticator carrierPrivilegeAuthenticator,
- @NonNull final NetworkAgentInfo nai) {
- final NetworkCapabilities nc = new NetworkCapabilities(nai.declaredCapabilitiesUnsanitized);
- if (nc.hasConnectivityManagedCapability()) {
- Log.wtf(TAG, "BUG: " + nai + " has CS-managed capability.");
- }
- if (nai.networkCapabilities.getOwnerUid() != nc.getOwnerUid()) {
- Log.e(TAG, nai.toShortString() + ": ignoring attempt to change owner from "
- + nai.networkCapabilities.getOwnerUid() + " to " + nc.getOwnerUid());
- nc.setOwnerUid(nai.networkCapabilities.getOwnerUid());
- }
- NetworkAgentInfo.restrictCapabilitiesFromNetworkAgent(nc, nai.creatorUid,
- mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE),
- carrierPrivilegeAuthenticator);
- return nc;
- }
-
/** Modifies |newNc| based on the capabilities of |underlyingNetworks| and |agentCaps|. */
@VisibleForTesting
void applyUnderlyingCapabilities(@Nullable Network[] underlyingNetworks,
@@ -7958,7 +7925,7 @@
if (nai.propagateUnderlyingCapabilities()) {
applyUnderlyingCapabilities(nai.declaredUnderlyingNetworks,
- sanitizedCapabilitiesFromAgent(mCarrierPrivilegeAuthenticator, nai),
+ nai.getDeclaredCapabilitiesSanitized(mCarrierPrivilegeAuthenticator),
newNc);
}
diff --git a/service/src/com/android/server/connectivity/NetworkAgentInfo.java b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
index 04031af..c863165 100644
--- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java
+++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java
@@ -26,6 +26,7 @@
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.Context;
+import android.content.pm.PackageManager;
import android.net.CaptivePortalData;
import android.net.DscpPolicy;
import android.net.IDnsResolver;
@@ -184,9 +185,8 @@
//
// As the name implies, these capabilities are not sanitized and are not to
// be trusted. Most callers should simply use the {@link networkCapabilities}
- // field instead, and callers who need the declared capabilities should generally
- // pass these to {@link ConnectivityService#sanitizedCapabilitiesFromAgent} before using them.
- public @Nullable NetworkCapabilities declaredCapabilitiesUnsanitized;
+ // field instead.
+ private @Nullable NetworkCapabilities mDeclaredCapabilitiesUnsanitized;
// Indicates if netd has been told to create this Network. From this point on the appropriate
// routing rules are setup and routes are added so packets can begin flowing over the Network.
@@ -240,6 +240,53 @@
// URL, Terms & Conditions URL, and network friendly name.
public CaptivePortalData networkAgentPortalData;
+ /**
+ * Sets the capabilities sent by the agent for later retrieval.
+ *
+ * This method does not sanitize the capabilities ; instead, use
+ * {@link #getDeclaredCapabilitiesSanitized} to retrieve a sanitized
+ * copy of the capabilities as they were passed here.
+ *
+ * This method makes a defensive copy to avoid issues where the passed object is later mutated.
+ *
+ * @param caps the caps sent by the agent
+ */
+ public void setDeclaredCapabilities(@NonNull final NetworkCapabilities caps) {
+ mDeclaredCapabilitiesUnsanitized = new NetworkCapabilities(caps);
+ }
+
+ /**
+ * Get the latest capabilities sent by the network agent, after sanitizing them.
+ *
+ * These are the capabilities as they were sent by the agent (but sanitized to conform to
+ * their restrictions). They are NOT the capabilities currently applying to this agent ;
+ * for that, use {@link #networkCapabilities}.
+ *
+ * Agents have restrictions on what capabilities they can send to Connectivity. For example,
+ * they can't change the owner UID from what they declared before, and complex restrictions
+ * apply to the allowedUids field.
+ * They also should not mutate immutable capabilities, although for backward-compatibility
+ * this is not enforced and limited to just a log.
+ *
+ * @param carrierPrivilegeAuthenticator the authenticator, to check access UIDs.
+ */
+ public NetworkCapabilities getDeclaredCapabilitiesSanitized(
+ final CarrierPrivilegeAuthenticator carrierPrivilegeAuthenticator) {
+ final NetworkCapabilities nc = new NetworkCapabilities(mDeclaredCapabilitiesUnsanitized);
+ if (nc.hasConnectivityManagedCapability()) {
+ Log.wtf(TAG, "BUG: " + this + " has CS-managed capability.");
+ }
+ if (networkCapabilities.getOwnerUid() != nc.getOwnerUid()) {
+ Log.e(TAG, toShortString() + ": ignoring attempt to change owner from "
+ + networkCapabilities.getOwnerUid() + " to " + nc.getOwnerUid());
+ nc.setOwnerUid(networkCapabilities.getOwnerUid());
+ }
+ restrictCapabilitiesFromNetworkAgent(nc, creatorUid,
+ mContext.getPackageManager().hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE),
+ carrierPrivilegeAuthenticator);
+ return nc;
+ }
+
// Networks are lingered when they become unneeded as a result of their NetworkRequests being
// satisfied by a higher-scoring network. so as to allow communication to wrap up before the
// network is taken down. This usually only happens to the default network. Lingering ends with
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index a8a761a..fcba78b 100644
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -242,6 +242,7 @@
import android.net.ConnectivityThread;
import android.net.DataStallReportParcelable;
import android.net.EthernetManager;
+import android.net.EthernetNetworkSpecifier;
import android.net.IConnectivityDiagnosticsCallback;
import android.net.IDnsResolver;
import android.net.INetd;
@@ -15703,6 +15704,45 @@
mCm.unregisterNetworkCallback(cb);
}
+ @Test
+ public void testSanitizedCapabilitiesFromAgentDoesNotMutateArgument()
+ throws Exception {
+ // This NetworkCapabilities builds an usual object to maximize the chance that this requires
+ // sanitization, so we have a high chance to detect any changes to the original.
+ final NetworkCapabilities unsanitized = new NetworkCapabilities.Builder()
+ .withoutDefaultCapabilities()
+ .addTransportType(TRANSPORT_WIFI)
+ .addCapability(NET_CAPABILITY_INTERNET)
+ .setOwnerUid(12345)
+ .setAdministratorUids(new int[] {12345, 23456, 34567})
+ .setLinkUpstreamBandwidthKbps(20)
+ .setLinkDownstreamBandwidthKbps(10)
+ .setNetworkSpecifier(new EthernetNetworkSpecifier("foobar"))
+ .setTransportInfo(new WifiInfo.Builder().setBssid("AA:AA:AA:AA:AA:AA").build())
+ .setSignalStrength(-75)
+ .setSsid("SSID1")
+ .setRequestorUid(98765)
+ .setRequestorPackageName("TestPackage")
+ .setSubscriptionIds(Collections.singleton(Process.myUid()))
+ .setUids(UidRange.toIntRanges(uidRangesForUids(
+ UserHandle.getUid(PRIMARY_USER, 10100),
+ UserHandle.getUid(SECONDARY_USER, 10101),
+ UserHandle.getUid(TERTIARY_USER, 10043))))
+ .setAllowedUids(Set.of(45678, 56789, 65432))
+ .setUnderlyingNetworks(List.of(new Network(99999)))
+ .build();
+ final NetworkCapabilities copyOfUnsanitized = new NetworkCapabilities(unsanitized);
+ final NetworkInfo info = new NetworkInfo(TYPE_MOBILE, TelephonyManager.NETWORK_TYPE_LTE,
+ ConnectivityManager.getNetworkTypeName(TYPE_MOBILE),
+ TelephonyManager.getNetworkTypeName(TelephonyManager.NETWORK_TYPE_LTE));
+ final NetworkAgentInfo agent = fakeNai(unsanitized, info);
+ agent.setDeclaredCapabilities(unsanitized);
+ final NetworkCapabilities sanitized = agent.getDeclaredCapabilitiesSanitized(
+ null /* carrierPrivilegeAuthenticator */);
+ assertEquals(copyOfUnsanitized, unsanitized);
+ assertNotEquals(sanitized, unsanitized);
+ }
+
/**
* Validate request counts are counted accurately on setProfileNetworkPreference on set/replace.
*/