Enforce that NetworkAgentInfo fields are never null.

These fields have been recently audited, confirmed never to be
null, and annotated @NonNull. Ensure that they can never become
null by throwing exceptions in the codepaths that set them.
Also remove some null checks.

Test: atest FrameworksNetTests
Change-Id: I6ce5bb4d69a990f1c857c599b7e50e372352eb87
This commit is contained in:
Lorenzo Colitti
2021-01-18 14:15:17 +09:00
parent ac155fc61e
commit bb4d45fb70
5 changed files with 38 additions and 37 deletions

View File

@@ -1644,7 +1644,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
private NetworkCapabilities getNetworkCapabilitiesInternal(NetworkAgentInfo nai) { private NetworkCapabilities getNetworkCapabilitiesInternal(NetworkAgentInfo nai) {
if (nai == null) return null; if (nai == null) return null;
synchronized (nai) { synchronized (nai) {
if (nai.networkCapabilities == null) return null;
return networkCapabilitiesRestrictedForCallerPermissions( return networkCapabilitiesRestrictedForCallerPermissions(
nai.networkCapabilities, Binder.getCallingPid(), mDeps.getCallingUid()); nai.networkCapabilities, Binder.getCallingPid(), mDeps.getCallingUid());
} }
@@ -2768,7 +2767,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
} }
private boolean isLiveNetworkAgent(NetworkAgentInfo nai, int what) { private boolean isLiveNetworkAgent(NetworkAgentInfo nai, int what) {
if (nai.network == null) return false;
final NetworkAgentInfo officialNai = getNetworkAgentInfoForNetwork(nai.network); final NetworkAgentInfo officialNai = getNetworkAgentInfoForNetwork(nai.network);
if (officialNai != null && officialNai.equals(nai)) return true; if (officialNai != null && officialNai.equals(nai)) return true;
if (officialNai != null || VDBG) { if (officialNai != null || VDBG) {
@@ -6059,6 +6057,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
public Network registerNetworkAgent(INetworkAgent na, NetworkInfo networkInfo, public Network registerNetworkAgent(INetworkAgent na, NetworkInfo networkInfo,
LinkProperties linkProperties, NetworkCapabilities networkCapabilities, LinkProperties linkProperties, NetworkCapabilities networkCapabilities,
int currentScore, NetworkAgentConfig networkAgentConfig, int providerId) { int currentScore, NetworkAgentConfig networkAgentConfig, int providerId) {
Objects.requireNonNull(networkInfo, "networkInfo must not be null");
Objects.requireNonNull(linkProperties, "linkProperties must not be null");
Objects.requireNonNull(networkCapabilities, "networkCapabilities must not be null");
Objects.requireNonNull(networkAgentConfig, "networkAgentConfig must not be null");
if (networkCapabilities.hasTransport(TRANSPORT_TEST)) { if (networkCapabilities.hasTransport(TRANSPORT_TEST)) {
enforceAnyPermissionOf(Manifest.permission.MANAGE_TEST_NETWORKS); enforceAnyPermissionOf(Manifest.permission.MANAGE_TEST_NETWORKS);
} else { } else {
@@ -7574,10 +7576,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
if (!networkAgent.everConnected && state == NetworkInfo.State.CONNECTED) { if (!networkAgent.everConnected && state == NetworkInfo.State.CONNECTED) {
networkAgent.everConnected = true; networkAgent.everConnected = true;
if (networkAgent.linkProperties == null) {
Log.wtf(TAG, networkAgent.toShortString() + " connected with null LinkProperties");
}
// NetworkCapabilities need to be set before sending the private DNS config to // NetworkCapabilities need to be set before sending the private DNS config to
// NetworkMonitor, otherwise NetworkMonitor cannot determine if validation is required. // NetworkMonitor, otherwise NetworkMonitor cannot determine if validation is required.
networkAgent.getAndSetNetworkCapabilities(networkAgent.networkCapabilities); networkAgent.getAndSetNetworkCapabilities(networkAgent.networkCapabilities);

View File

@@ -246,11 +246,6 @@ public class Nat464Xlat extends BaseNetworkObserver {
return; return;
} }
if (mNetwork.linkProperties == null) {
Log.e(TAG, "startClat: Can't start clat with null LinkProperties");
return;
}
String baseIface = mNetwork.linkProperties.getInterfaceName(); String baseIface = mNetwork.linkProperties.getInterfaceName();
if (baseIface == null) { if (baseIface == null) {
Log.e(TAG, "startClat: Can't start clat on null interface"); Log.e(TAG, "startClat: Can't start clat on null interface");

View File

@@ -329,6 +329,12 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
Handler handler, NetworkAgentConfig config, ConnectivityService connService, INetd netd, Handler handler, NetworkAgentConfig config, ConnectivityService connService, INetd netd,
IDnsResolver dnsResolver, INetworkManagementService nms, int factorySerialNumber, IDnsResolver dnsResolver, INetworkManagementService nms, int factorySerialNumber,
int creatorUid) { int creatorUid) {
Objects.requireNonNull(net);
Objects.requireNonNull(info);
Objects.requireNonNull(lp);
Objects.requireNonNull(nc);
Objects.requireNonNull(context);
Objects.requireNonNull(config);
networkAgent = na; networkAgent = na;
network = net; network = net;
networkInfo = info; networkInfo = info;
@@ -536,19 +542,22 @@ public class NetworkAgentInfo implements Comparable<NetworkAgentInfo> {
} }
@Override @Override
public void sendNetworkCapabilities(NetworkCapabilities nc) { public void sendNetworkCapabilities(@NonNull NetworkCapabilities nc) {
Objects.requireNonNull(nc);
mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_CAPABILITIES_CHANGED, mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_CAPABILITIES_CHANGED,
new Pair<>(NetworkAgentInfo.this, nc)).sendToTarget(); new Pair<>(NetworkAgentInfo.this, nc)).sendToTarget();
} }
@Override @Override
public void sendLinkProperties(LinkProperties lp) { public void sendLinkProperties(@NonNull LinkProperties lp) {
Objects.requireNonNull(lp);
mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED,
new Pair<>(NetworkAgentInfo.this, lp)).sendToTarget(); new Pair<>(NetworkAgentInfo.this, lp)).sendToTarget();
} }
@Override @Override
public void sendNetworkInfo(NetworkInfo info) { public void sendNetworkInfo(@NonNull NetworkInfo info) {
Objects.requireNonNull(info);
mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_INFO_CHANGED, mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_INFO_CHANGED,
new Pair<>(NetworkAgentInfo.this, info)).sendToTarget(); new Pair<>(NetworkAgentInfo.this, info)).sendToTarget();
} }

View File

@@ -8112,11 +8112,18 @@ public class ConnectivityServiceTest {
assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder)); assertTrue(mService.mConnectivityDiagnosticsCallbacks.containsKey(mIBinder));
} }
public NetworkAgentInfo fakeMobileNai(NetworkCapabilities nc) {
final NetworkInfo info = new NetworkInfo(TYPE_MOBILE, TelephonyManager.NETWORK_TYPE_LTE,
ConnectivityManager.getNetworkTypeName(TYPE_MOBILE),
TelephonyManager.getNetworkTypeName(TelephonyManager.NETWORK_TYPE_LTE));
return new NetworkAgentInfo(null, new Network(NET_ID), info, new LinkProperties(),
nc, 0, mServiceContext, null, new NetworkAgentConfig(), mService, null, null, null,
0, INVALID_UID);
}
@Test @Test
public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception { public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception {
final NetworkAgentInfo naiWithoutUid = final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities());
new NetworkAgentInfo(null, null, null, null, new NetworkCapabilities(), 0,
mServiceContext, null, null, mService, null, null, null, 0, INVALID_UID);
mServiceContext.setPermission( mServiceContext.setPermission(
android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED); android.Manifest.permission.NETWORK_STACK, PERMISSION_GRANTED);
@@ -8129,9 +8136,7 @@ public class ConnectivityServiceTest {
@Test @Test
public void testCheckConnectivityDiagnosticsPermissionsWrongUidPackageName() throws Exception { public void testCheckConnectivityDiagnosticsPermissionsWrongUidPackageName() throws Exception {
final NetworkAgentInfo naiWithoutUid = final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities());
new NetworkAgentInfo(null, null, null, null, new NetworkCapabilities(), 0,
mServiceContext, null, null, mService, null, null, null, 0, INVALID_UID);
mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);
@@ -8144,9 +8149,7 @@ public class ConnectivityServiceTest {
@Test @Test
public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception {
final NetworkAgentInfo naiWithoutUid = final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities());
new NetworkAgentInfo(null, null, null, null, new NetworkCapabilities(), 0,
mServiceContext, null, null, mService, null, null, null, 0, INVALID_UID);
mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED);
@@ -8159,10 +8162,7 @@ public class ConnectivityServiceTest {
@Test @Test
public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception { public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception {
final Network network = new Network(NET_ID); final NetworkAgentInfo naiWithoutUid = fakeMobileNai(new NetworkCapabilities());
final NetworkAgentInfo naiWithoutUid =
new NetworkAgentInfo(null, network, null, null, new NetworkCapabilities(), 0,
mServiceContext, null, null, mService, null, null, null, 0, INVALID_UID);
mMockVpn.establishForMyUid(); mMockVpn.establishForMyUid();
assertUidRangesUpdatedForMyUid(true); assertUidRangesUpdatedForMyUid(true);
@@ -8172,7 +8172,7 @@ public class ConnectivityServiceTest {
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION); Manifest.permission.ACCESS_FINE_LOCATION);
assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {network})); assertTrue(mService.setUnderlyingNetworksForVpn(new Network[] {naiWithoutUid.network}));
waitForIdle(); waitForIdle();
assertTrue( assertTrue(
"Active VPN permission not applied", "Active VPN permission not applied",
@@ -8193,9 +8193,7 @@ public class ConnectivityServiceTest {
public void testCheckConnectivityDiagnosticsPermissionsNetworkAdministrator() throws Exception { public void testCheckConnectivityDiagnosticsPermissionsNetworkAdministrator() throws Exception {
final NetworkCapabilities nc = new NetworkCapabilities(); final NetworkCapabilities nc = new NetworkCapabilities();
nc.setAdministratorUids(new int[] {Process.myUid()}); nc.setAdministratorUids(new int[] {Process.myUid()});
final NetworkAgentInfo naiWithUid = final NetworkAgentInfo naiWithUid = fakeMobileNai(nc);
new NetworkAgentInfo(null, null, null, null, nc, 0, mServiceContext, null, null,
mService, null, null, null, 0, INVALID_UID);
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION); Manifest.permission.ACCESS_FINE_LOCATION);
@@ -8212,9 +8210,7 @@ public class ConnectivityServiceTest {
final NetworkCapabilities nc = new NetworkCapabilities(); final NetworkCapabilities nc = new NetworkCapabilities();
nc.setOwnerUid(Process.myUid()); nc.setOwnerUid(Process.myUid());
nc.setAdministratorUids(new int[] {Process.myUid()}); nc.setAdministratorUids(new int[] {Process.myUid()});
final NetworkAgentInfo naiWithUid = final NetworkAgentInfo naiWithUid = fakeMobileNai(nc);
new NetworkAgentInfo(null, null, null, null, nc, 0, mServiceContext, null, null,
mService, null, null, null, 0, INVALID_UID);
setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION,
Manifest.permission.ACCESS_FINE_LOCATION); Manifest.permission.ACCESS_FINE_LOCATION);

View File

@@ -34,7 +34,9 @@ import android.content.res.Resources;
import android.net.ConnectivityManager; import android.net.ConnectivityManager;
import android.net.IDnsResolver; import android.net.IDnsResolver;
import android.net.INetd; import android.net.INetd;
import android.net.LinkProperties;
import android.net.Network; import android.net.Network;
import android.net.NetworkAgentConfig;
import android.net.NetworkCapabilities; import android.net.NetworkCapabilities;
import android.net.NetworkInfo; import android.net.NetworkInfo;
import android.net.NetworkProvider; import android.net.NetworkProvider;
@@ -353,9 +355,10 @@ public class LingerMonitorTest {
NetworkCapabilities caps = new NetworkCapabilities(); NetworkCapabilities caps = new NetworkCapabilities();
caps.addCapability(0); caps.addCapability(0);
caps.addTransportType(transport); caps.addTransportType(transport);
NetworkAgentInfo nai = new NetworkAgentInfo(null, new Network(netId), info, null, NetworkAgentInfo nai = new NetworkAgentInfo(null, new Network(netId), info,
caps, 50, mCtx, null, null /* config */, mConnService, mNetd, mDnsResolver, mNMS, new LinkProperties(), caps, 50, mCtx, null, new NetworkAgentConfig() /* config */,
NetworkProvider.ID_NONE, Binder.getCallingUid()); mConnService, mNetd, mDnsResolver, mNMS, NetworkProvider.ID_NONE,
Binder.getCallingUid());
nai.everValidated = true; nai.everValidated = true;
return nai; return nai;
} }