Force specifying sanitized/not sanitized in caps from agent
To make sure anyone using the capabilities originally from the agent have to know what they're dealing with, make the member private and make accessors that spell out explicitly the important parts. This is the last step towards addressing the leftover comment from aosp/1958906. Bug: 238139913 Test: FrameworksNetTests CtsNetTestCases Change-Id: I9fc1986c59726212acfc2ad921745c8bbc424035
This commit is contained in:
Chalard Jean
Paul Hu
parent
2719dc8682
commit
39b12d4b8f
3 changed files with 98 additions and 44 deletions
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<NetworkAgentInfo>, 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<NetworkAgentInfo>, 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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
*/
|
||||
|
|
|
|||
Loading…
Reference in a new issue