Block address families with routes, not NetworkAgent side channel
Now that we support unreachable routes, use those to block
address families on VPNs. This is a much more elegant solution.
Also update LinkProperties when IP addresses are added and
removed, fixing a TODO.
Bug: 17462989
Change-Id: Ib749d84710dca70d672350b9f129bb91419ec77e
diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java
index b83198d..74d4ac2 100644
--- a/core/java/android/net/NetworkAgent.java
+++ b/core/java/android/net/NetworkAgent.java
@@ -107,27 +107,13 @@
public static final int EVENT_UID_RANGES_REMOVED = BASE + 6;
/**
- * Sent by the NetworkAgent to ConnectivityService to block all routes for a certain address
- * family (AF_INET or AF_INET6) on this Network. For VPNs only.
- * obj = Integer representing the family (AF_INET or AF_INET6)
- */
- public static final int EVENT_BLOCK_ADDRESS_FAMILY = BASE + 7;
-
- /**
- * Sent by the NetworkAgent to ConnectivityService to unblock routes for a certain address
- * family (AF_INET or AF_INET6) on this Network. For VPNs only.
- * obj = Integer representing the family (AF_INET or AF_INET6)
- */
- public static final int EVENT_UNBLOCK_ADDRESS_FAMILY = BASE + 8;
-
- /**
* Sent by ConnectivitySerice to the NetworkAgent to inform the agent of the
* networks status - whether we could use the network or could not, due to
* either a bad network configuration (no internet link) or captive portal.
*
* arg1 = either {@code VALID_NETWORK} or {@code INVALID_NETWORK}
*/
- public static final int CMD_REPORT_NETWORK_STATUS = BASE + 9;
+ public static final int CMD_REPORT_NETWORK_STATUS = BASE + 7;
public static final int VALID_NETWORK = 1;
public static final int INVALID_NETWORK = 2;
@@ -137,7 +123,7 @@
* explicitly selected. This should be sent before the NetworkInfo is marked
* CONNECTED so it can be given special treatment at that time.
*/
- public static final int EVENT_SET_EXPLICITLY_SELECTED = BASE + 10;
+ public static final int EVENT_SET_EXPLICITLY_SELECTED = BASE + 8;
public NetworkAgent(Looper looper, Context context, String logTag, NetworkInfo ni,
NetworkCapabilities nc, LinkProperties lp, int score) {
@@ -273,21 +259,6 @@
}
/**
- * Called by the VPN code when it wants to block an address family from being routed, typically
- * because the VPN network doesn't support that family.
- */
- public void blockAddressFamily(int family) {
- queueOrSendMessage(EVENT_BLOCK_ADDRESS_FAMILY, family);
- }
-
- /**
- * Called by the VPN code when it wants to unblock an address family from being routed.
- */
- public void unblockAddressFamily(int family) {
- queueOrSendMessage(EVENT_UNBLOCK_ADDRESS_FAMILY, family);
- }
-
- /**
* Called by the bearer to indicate this network was manually selected by the user.
* This should be called before the NetworkInfo is marked CONNECTED so that this
* Network can be given special treatment at that time.
diff --git a/core/java/android/os/INetworkManagementService.aidl b/core/java/android/os/INetworkManagementService.aidl
index b3e28ea..fca15ac 100644
--- a/core/java/android/os/INetworkManagementService.aidl
+++ b/core/java/android/os/INetworkManagementService.aidl
@@ -410,7 +410,4 @@
void addInterfaceToLocalNetwork(String iface, in List<RouteInfo> routes);
void removeInterfaceFromLocalNetwork(String iface);
-
- void blockAddressFamily(int family, int netId, String iface);
- void unblockAddressFamily(int family, int netId, String iface);
}
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 967681b..f579d6f 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -1888,36 +1888,6 @@
}
break;
}
- case NetworkAgent.EVENT_BLOCK_ADDRESS_FAMILY: {
- NetworkAgentInfo nai = mNetworkAgentInfos.get(msg.replyTo);
- if (nai == null) {
- loge("EVENT_BLOCK_ADDRESS_FAMILY from unknown NetworkAgent");
- break;
- }
- try {
- mNetd.blockAddressFamily((Integer) msg.obj, nai.network.netId,
- nai.linkProperties.getInterfaceName());
- } catch (Exception e) {
- // Never crash!
- loge("Exception in blockAddressFamily: " + e);
- }
- break;
- }
- case NetworkAgent.EVENT_UNBLOCK_ADDRESS_FAMILY: {
- NetworkAgentInfo nai = mNetworkAgentInfos.get(msg.replyTo);
- if (nai == null) {
- loge("EVENT_UNBLOCK_ADDRESS_FAMILY from unknown NetworkAgent");
- break;
- }
- try {
- mNetd.unblockAddressFamily((Integer) msg.obj, nai.network.netId,
- nai.linkProperties.getInterfaceName());
- } catch (Exception e) {
- // Never crash!
- loge("Exception in blockAddressFamily: " + e);
- }
- break;
- }
case NetworkAgent.EVENT_SET_EXPLICITLY_SELECTED: {
NetworkAgentInfo nai = mNetworkAgentInfos.get(msg.replyTo);
if (nai == null) {
diff --git a/services/core/java/com/android/server/NetworkManagementService.java b/services/core/java/com/android/server/NetworkManagementService.java
index 6144078..822007a 100644
--- a/services/core/java/com/android/server/NetworkManagementService.java
+++ b/services/core/java/com/android/server/NetworkManagementService.java
@@ -28,8 +28,6 @@
import static android.net.RouteInfo.RTN_THROW;
import static android.net.RouteInfo.RTN_UNICAST;
import static android.net.RouteInfo.RTN_UNREACHABLE;
-import static android.system.OsConstants.AF_INET;
-import static android.system.OsConstants.AF_INET6;
import static com.android.server.NetworkManagementService.NetdResponseCode.ClatdStatusResult;
import static com.android.server.NetworkManagementService.NetdResponseCode.InterfaceGetCfgResult;
import static com.android.server.NetworkManagementService.NetdResponseCode.InterfaceListResult;
@@ -2131,37 +2129,4 @@
public void removeInterfaceFromLocalNetwork(String iface) {
modifyInterfaceInNetwork("remove", "local", iface);
}
-
- @Override
- public void blockAddressFamily(int family, int netId, String iface) {
- modifyAddressFamily("add", family, netId, iface);
- }
-
- @Override
- public void unblockAddressFamily(int family, int netId, String iface) {
- modifyAddressFamily("remove", family, netId, iface);
- }
-
- // TODO: get rid of this and add RTN_UNREACHABLE routes instead.
- private void modifyAddressFamily(String action, int family, int netId, String iface) {
- mContext.enforceCallingOrSelfPermission(CONNECTIVITY_INTERNAL, TAG);
-
- final Command cmd = new Command("network", "route", action, netId, iface);
-
- if (family == AF_INET) {
- cmd.appendArg(Inet4Address.ANY.getHostAddress() + "/0");
- } else if (family == AF_INET6) {
- cmd.appendArg(Inet6Address.ANY.getHostAddress() + "/0");
- } else {
- throw new IllegalStateException(family + " is neither " + AF_INET + " nor " + AF_INET6);
- }
-
- cmd.appendArg("unreachable");
-
- try {
- mConnector.execute(cmd);
- } catch (NativeDaemonConnectorException e) {
- throw e.rethrowAsParcelableException();
- }
- }
}
diff --git a/services/core/java/com/android/server/connectivity/Vpn.java b/services/core/java/com/android/server/connectivity/Vpn.java
index 4100ae9..3f6b71a 100644
--- a/services/core/java/com/android/server/connectivity/Vpn.java
+++ b/services/core/java/com/android/server/connectivity/Vpn.java
@@ -19,6 +19,7 @@
import static android.Manifest.permission.BIND_VPN_SERVICE;
import static android.os.UserHandle.PER_USER_RANGE;
import static android.net.RouteInfo.RTN_THROW;
+import static android.net.RouteInfo.RTN_UNREACHABLE;
import static android.system.OsConstants.AF_INET;
import static android.system.OsConstants.AF_INET6;
@@ -107,8 +108,6 @@
private String mPackage;
private int mOwnerUID;
private String mInterface;
- private boolean mAllowIPv4;
- private boolean mAllowIPv6;
private Connection mConnection;
private LegacyVpnRunner mLegacyVpnRunner;
private PendingIntent mStatusIntent;
@@ -344,33 +343,47 @@
return mNetworkInfo;
}
- private void agentConnect() {
+ private LinkProperties makeLinkProperties() {
+ boolean allowIPv4 = mConfig.allowIPv4;
+ boolean allowIPv6 = mConfig.allowIPv6;
+
LinkProperties lp = new LinkProperties();
+
lp.setInterfaceName(mInterface);
- boolean hasDefaultRoute = false;
- for (RouteInfo route : mConfig.routes) {
- lp.addRoute(route);
- if (route.isDefaultRoute()) hasDefaultRoute = true;
+ if (mConfig.addresses != null) {
+ for (LinkAddress address : mConfig.addresses) {
+ lp.addLinkAddress(address);
+ allowIPv4 |= address.getAddress() instanceof Inet4Address;
+ allowIPv6 |= address.getAddress() instanceof Inet6Address;
+ }
}
- if (hasDefaultRoute) {
- mNetworkCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
- } else {
- mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
+
+ if (mConfig.routes != null) {
+ for (RouteInfo route : mConfig.routes) {
+ lp.addRoute(route);
+ InetAddress address = route.getDestination().getAddress();
+ allowIPv4 |= address instanceof Inet4Address;
+ allowIPv6 |= address instanceof Inet6Address;
+ }
}
if (mConfig.dnsServers != null) {
for (String dnsServer : mConfig.dnsServers) {
InetAddress address = InetAddress.parseNumericAddress(dnsServer);
lp.addDnsServer(address);
- if (address instanceof Inet4Address) {
- mAllowIPv4 = true;
- } else {
- mAllowIPv6 = true;
- }
+ allowIPv4 |= address instanceof Inet4Address;
+ allowIPv6 |= address instanceof Inet6Address;
}
}
+ if (!allowIPv4) {
+ lp.addRoute(new RouteInfo(new IpPrefix(Inet4Address.ANY, 0), RTN_UNREACHABLE));
+ }
+ if (!allowIPv6) {
+ lp.addRoute(new RouteInfo(new IpPrefix(Inet6Address.ANY, 0), RTN_UNREACHABLE));
+ }
+
// Concatenate search domains into a string.
StringBuilder buffer = new StringBuilder();
if (mConfig.searchDomains != null) {
@@ -380,6 +393,20 @@
}
lp.setDomains(buffer.toString().trim());
+ // TODO: Stop setting the MTU in jniCreate and set it here.
+
+ return lp;
+ }
+
+ private void agentConnect() {
+ LinkProperties lp = makeLinkProperties();
+
+ if (lp.hasIPv4DefaultRoute() || lp.hasIPv6DefaultRoute()) {
+ mNetworkCapabilities.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
+ } else {
+ mNetworkCapabilities.removeCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET);
+ }
+
mNetworkInfo.setIsAvailable(true);
mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, null);
@@ -399,13 +426,6 @@
Binder.restoreCallingIdentity(token);
}
- if (!mAllowIPv4) {
- mNetworkAgent.blockAddressFamily(AF_INET);
- }
- if (!mAllowIPv6) {
- mNetworkAgent.blockAddressFamily(AF_INET6);
- }
-
addVpnUserLocked(mUserHandle);
// If we are owner assign all Restricted Users to this VPN
if (mUserHandle == UserHandle.USER_OWNER) {
@@ -491,8 +511,6 @@
NetworkAgent oldNetworkAgent = mNetworkAgent;
mNetworkAgent = null;
List<UidRange> oldUsers = mVpnUsers;
- boolean oldAllowIPv4 = mAllowIPv4;
- boolean oldAllowIPv6 = mAllowIPv6;
// Configure the interface. Abort if any of these steps fails.
ParcelFileDescriptor tun = ParcelFileDescriptor.adoptFd(jniCreate(config.mtu));
@@ -525,8 +543,6 @@
// Set up forwarding and DNS rules.
mVpnUsers = new ArrayList<UidRange>();
- mAllowIPv4 = mConfig.allowIPv4;
- mAllowIPv6 = mConfig.allowIPv6;
agentConnect();
@@ -556,8 +572,6 @@
mVpnUsers = oldUsers;
mNetworkAgent = oldNetworkAgent;
mInterface = oldInterface;
- mAllowIPv4 = oldAllowIPv4;
- mAllowIPv6 = oldAllowIPv6;
throw e;
}
Log.i(TAG, "Established by " + config.user + " on " + mInterface);
@@ -780,25 +794,9 @@
return false;
}
boolean success = jniAddAddress(mInterface, address, prefixLength);
- if (success && (!mAllowIPv4 || !mAllowIPv6)) {
- try {
- InetAddress inetAddress = InetAddress.parseNumericAddress(address);
- if ((inetAddress instanceof Inet4Address) && !mAllowIPv4) {
- mAllowIPv4 = true;
- mNetworkAgent.unblockAddressFamily(AF_INET);
- } else if ((inetAddress instanceof Inet6Address) && !mAllowIPv6) {
- mAllowIPv6 = true;
- mNetworkAgent.unblockAddressFamily(AF_INET6);
- }
- } catch (IllegalArgumentException e) {
- // ignore
- }
+ if (mNetworkAgent != null) {
+ mNetworkAgent.sendLinkProperties(makeLinkProperties());
}
- // Ideally, we'd call mNetworkAgent.sendLinkProperties() here to notify ConnectivityService
- // that the LinkAddress has changed. But we don't do so for two reasons: (1) We don't set
- // LinkAddresses on the LinkProperties we establish in the first place (see agentConnect())
- // and (2) CS doesn't do anything with LinkAddresses anyway (see updateLinkProperties()).
- // TODO: Maybe fix this.
return success;
}
@@ -807,11 +805,9 @@
return false;
}
boolean success = jniDelAddress(mInterface, address, prefixLength);
- // Ideally, we'd call mNetworkAgent.sendLinkProperties() here to notify ConnectivityService
- // that the LinkAddress has changed. But we don't do so for two reasons: (1) We don't set
- // LinkAddresses on the LinkProperties we establish in the first place (see agentConnect())
- // and (2) CS doesn't do anything with LinkAddresses anyway (see updateLinkProperties()).
- // TODO: Maybe fix this.
+ if (mNetworkAgent != null) {
+ mNetworkAgent.sendLinkProperties(makeLinkProperties());
+ }
return success;
}
@@ -1284,8 +1280,6 @@
// Now INetworkManagementEventObserver is watching our back.
mInterface = mConfig.interfaze;
mVpnUsers = new ArrayList<UidRange>();
- mAllowIPv4 = mConfig.allowIPv4;
- mAllowIPv6 = mConfig.allowIPv6;
agentConnect();