diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 7b5026d455..960059bf5c 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -3604,10 +3604,9 @@ public class ConnectivityService extends IConnectivityManager.Stub 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; @@ -7328,11 +7327,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // 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; @@ -7795,38 +7794,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - /** - * 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, @@ -7952,7 +7919,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 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 04031af632..c863165476 100644 --- a/service/src/com/android/server/connectivity/NetworkAgentInfo.java +++ b/service/src/com/android/server/connectivity/NetworkAgentInfo.java @@ -26,6 +26,7 @@ import static android.net.NetworkCapabilities.transportNamesOf; 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 @@ public class NetworkAgentInfo implements Comparable, NetworkRa // // 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 @@ public class NetworkAgentInfo implements Comparable, NetworkRa // 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 28119d8e9c..0196f48569 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -241,6 +241,7 @@ import android.net.ConnectivitySettingsManager; 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; @@ -15723,6 +15724,45 @@ public class ConnectivityServiceTest { 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. */