Use tunnel local/remote addresses in security policies

This patch changes tunnel mode security policies to use the actual
tunnel's local and remote addresses to select the SA.
This prevents the kernel from calling xfrm_get_saddr(), which does a
route lookup, potentially resolving an incorrect saddr.

Bug: 79384676
Test: CTS, IpSecService* tests passing
Change-Id: I8223225e2363a79591a0bb0040aa8619cf84c184
This commit is contained in:
Benedict Wong
2018-05-07 20:06:44 -07:00
parent c9fc125bb4
commit 38e52973d2

View File

@@ -27,7 +27,6 @@ import static com.android.internal.util.Preconditions.checkNotNull;
import android.annotation.NonNull; import android.annotation.NonNull;
import android.app.AppOpsManager; import android.app.AppOpsManager;
import android.content.Context; import android.content.Context;
import android.net.ConnectivityManager;
import android.net.IIpSecService; import android.net.IIpSecService;
import android.net.INetd; import android.net.INetd;
import android.net.IpSecAlgorithm; import android.net.IpSecAlgorithm;
@@ -44,7 +43,6 @@ import android.net.NetworkUtils;
import android.net.TrafficStats; import android.net.TrafficStats;
import android.net.util.NetdService; import android.net.util.NetdService;
import android.os.Binder; import android.os.Binder;
import android.os.DeadSystemException;
import android.os.IBinder; import android.os.IBinder;
import android.os.ParcelFileDescriptor; import android.os.ParcelFileDescriptor;
import android.os.RemoteException; import android.os.RemoteException;
@@ -89,9 +87,8 @@ public class IpSecService extends IIpSecService.Stub {
private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG); private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG);
private static final String NETD_SERVICE_NAME = "netd"; private static final String NETD_SERVICE_NAME = "netd";
private static final int[] DIRECTIONS = private static final int[] ADDRESS_FAMILIES =
new int[] {IpSecManager.DIRECTION_OUT, IpSecManager.DIRECTION_IN}; new int[] {OsConstants.AF_INET, OsConstants.AF_INET6};
private static final String[] WILDCARD_ADDRESSES = new String[]{"0.0.0.0", "::"};
private static final int NETD_FETCH_TIMEOUT_MS = 5000; // ms private static final int NETD_FETCH_TIMEOUT_MS = 5000; // ms
private static final int MAX_PORT_BIND_ATTEMPTS = 10; private static final int MAX_PORT_BIND_ATTEMPTS = 10;
@@ -819,16 +816,22 @@ public class IpSecService extends IIpSecService.Stub {
// Teardown VTI // Teardown VTI
// Delete global policies // Delete global policies
try { try {
mSrvConfig.getNetdInstance().removeVirtualTunnelInterface(mInterfaceName); final INetd netd = mSrvConfig.getNetdInstance();
netd.removeVirtualTunnelInterface(mInterfaceName);
for(String wildcardAddr : WILDCARD_ADDRESSES) { for (int selAddrFamily : ADDRESS_FAMILIES) {
for (int direction : DIRECTIONS) { netd.ipSecDeleteSecurityPolicy(
int mark = (direction == IpSecManager.DIRECTION_IN) ? mIkey : mOkey; 0,
mSrvConfig selAddrFamily,
.getNetdInstance() IpSecManager.DIRECTION_OUT,
.ipSecDeleteSecurityPolicy( mOkey,
0, direction, wildcardAddr, wildcardAddr, mark, 0xffffffff); 0xffffffff);
} netd.ipSecDeleteSecurityPolicy(
0,
selAddrFamily,
IpSecManager.DIRECTION_IN,
mIkey,
0xffffffff);
} }
} catch (ServiceSpecificException | RemoteException e) { } catch (ServiceSpecificException | RemoteException e) {
Log.e( Log.e(
@@ -1276,25 +1279,29 @@ public class IpSecService extends IIpSecService.Stub {
// Create VTI // Create VTI
// Add inbound/outbound global policies // Add inbound/outbound global policies
// (use reqid = 0) // (use reqid = 0)
mSrvConfig final INetd netd = mSrvConfig.getNetdInstance();
.getNetdInstance() netd.addVirtualTunnelInterface(intfName, localAddr, remoteAddr, ikey, okey);
.addVirtualTunnelInterface(intfName, localAddr, remoteAddr, ikey, okey);
for(String wildcardAddr : WILDCARD_ADDRESSES) { for (int selAddrFamily : ADDRESS_FAMILIES) {
for (int direction : DIRECTIONS) { // Always send down correct local/remote addresses for template.
int mark = (direction == IpSecManager.DIRECTION_OUT) ? okey : ikey; netd.ipSecAddSecurityPolicy(
mSrvConfig
.getNetdInstance()
.ipSecAddSecurityPolicy(
0, // Use 0 for reqId 0, // Use 0 for reqId
direction, selAddrFamily,
wildcardAddr, IpSecManager.DIRECTION_OUT,
wildcardAddr, localAddr,
remoteAddr,
0, 0,
mark, okey,
0xffffffff);
netd.ipSecAddSecurityPolicy(
0, // Use 0 for reqId
selAddrFamily,
IpSecManager.DIRECTION_IN,
remoteAddr,
localAddr,
0,
ikey,
0xffffffff); 0xffffffff);
}
} }
userRecord.mTunnelInterfaceRecords.put( userRecord.mTunnelInterfaceRecords.put(
@@ -1693,9 +1700,9 @@ public class IpSecService extends IIpSecService.Stub {
SpiRecord spiRecord = userRecord.mSpiRecords.getResourceOrThrow(c.getSpiResourceId()); SpiRecord spiRecord = userRecord.mSpiRecords.getResourceOrThrow(c.getSpiResourceId());
int mark = int mark =
(direction == IpSecManager.DIRECTION_IN) (direction == IpSecManager.DIRECTION_OUT)
? tunnelInterfaceInfo.getIkey() ? tunnelInterfaceInfo.getOkey()
: tunnelInterfaceInfo.getOkey(); : tunnelInterfaceInfo.getIkey();
try { try {
c.setMarkValue(mark); c.setMarkValue(mark);
@@ -1706,14 +1713,15 @@ public class IpSecService extends IIpSecService.Stub {
c.setNetwork(tunnelInterfaceInfo.getUnderlyingNetwork()); c.setNetwork(tunnelInterfaceInfo.getUnderlyingNetwork());
// If outbound, also add SPI to the policy. // If outbound, also add SPI to the policy.
for(String wildcardAddr : WILDCARD_ADDRESSES) { for (int selAddrFamily : ADDRESS_FAMILIES) {
mSrvConfig mSrvConfig
.getNetdInstance() .getNetdInstance()
.ipSecUpdateSecurityPolicy( .ipSecUpdateSecurityPolicy(
0, // Use 0 for reqId 0, // Use 0 for reqId
selAddrFamily,
direction, direction,
wildcardAddr, tunnelInterfaceInfo.getLocalAddress(),
wildcardAddr, tunnelInterfaceInfo.getRemoteAddress(),
transformInfo.getSpiRecord().getSpi(), transformInfo.getSpiRecord().getSpi(),
mark, mark,
0xffffffff); 0xffffffff);