Merge "[KA11] Verify fd ownership and allocate resource for NattKeepalive" am: fb36d44462 (df9591a4) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/ConnectivityService.java

+2 −0

services/core/java/com/android/server/connectivity/KeepaliveTracker.java

+90 −27

Original line number	Original line	Diff line number	Diff line
	@@ -17,6 +17,7 @@
	package com.android.server.connectivity;		package com.android.server.connectivity;

	import static android.content.pm.PackageManager.PERMISSION_GRANTED;		import static android.content.pm.PackageManager.PERMISSION_GRANTED;
			import static android.net.IpSecManager.INVALID_RESOURCE_ID;
	import static android.net.NattSocketKeepalive.NATT_PORT;		import static android.net.NattSocketKeepalive.NATT_PORT;
	import static android.net.NetworkAgent.CMD_ADD_KEEPALIVE_PACKET_FILTER;		import static android.net.NetworkAgent.CMD_ADD_KEEPALIVE_PACKET_FILTER;
	import static android.net.NetworkAgent.CMD_REMOVE_KEEPALIVE_PACKET_FILTER;		import static android.net.NetworkAgent.CMD_REMOVE_KEEPALIVE_PACKET_FILTER;
	@@ -37,6 +38,7 @@ import static android.net.SocketKeepalive.SUCCESS;
	import android.annotation.NonNull;		import android.annotation.NonNull;
	import android.annotation.Nullable;		import android.annotation.Nullable;
	import android.content.Context;		import android.content.Context;
			import android.net.IIpSecService;
	import android.net.ISocketKeepaliveCallback;		import android.net.ISocketKeepaliveCallback;
	import android.net.KeepalivePacketData;		import android.net.KeepalivePacketData;
	import android.net.NattKeepalivePacketData;		import android.net.NattKeepalivePacketData;
	@@ -52,6 +54,7 @@ import android.os.IBinder;
	import android.os.Message;		import android.os.Message;
	import android.os.Process;		import android.os.Process;
	import android.os.RemoteException;		import android.os.RemoteException;
			import android.os.ServiceManager;
	import android.system.ErrnoException;		import android.system.ErrnoException;
	import android.system.Os;		import android.system.Os;
	import android.util.Log;		import android.util.Log;
	@@ -89,11 +92,14 @@ public class KeepaliveTracker {
	private final TcpKeepaliveController mTcpController;		private final TcpKeepaliveController mTcpController;
	@NonNull		@NonNull
	private final Context mContext;		private final Context mContext;
			@NonNull
			private final IIpSecService mIpSec;

	public KeepaliveTracker(Context context, Handler handler) {		public KeepaliveTracker(Context context, Handler handler) {
	mConnectivityServiceHandler = handler;		mConnectivityServiceHandler = handler;
	mTcpController = new TcpKeepaliveController(handler);		mTcpController = new TcpKeepaliveController(handler);
	mContext = context;		mContext = context;
			mIpSec = IIpSecService.Stub.asInterface(ServiceManager.getService(Context.IPSEC_SERVICE));
	}		}

	/**		/**
	@@ -112,6 +118,10 @@ public class KeepaliveTracker {
	private final int mType;		private final int mType;
	private final FileDescriptor mFd;		private final FileDescriptor mFd;

			private final int mEncapSocketResourceId;
			// Stores the NATT keepalive resource ID returned by IpSecService.
			private int mNattIpsecResourceId = INVALID_RESOURCE_ID;

	public static final int TYPE_NATT = 1;		public static final int TYPE_NATT = 1;
	public static final int TYPE_TCP = 2;		public static final int TYPE_TCP = 2;

	@@ -140,7 +150,8 @@ public class KeepaliveTracker {
	@NonNull KeepalivePacketData packet,		@NonNull KeepalivePacketData packet,
	int interval,		int interval,
	int type,		int type,
	@Nullable FileDescriptor fd) throws InvalidSocketException {		@Nullable FileDescriptor fd,
			int encapSocketResourceId) throws InvalidSocketException {
	mCallback = callback;		mCallback = callback;
	mPid = Binder.getCallingPid();		mPid = Binder.getCallingPid();
	mUid = Binder.getCallingUid();		mUid = Binder.getCallingUid();
	@@ -151,6 +162,9 @@ public class KeepaliveTracker {
	mInterval = interval;		mInterval = interval;
	mType = type;		mType = type;

			mEncapSocketResourceId = encapSocketResourceId;
			mNattIpsecResourceId = INVALID_RESOURCE_ID;

	// For SocketKeepalive, a dup of fd is kept in mFd so the source port from which the		// For SocketKeepalive, a dup of fd is kept in mFd so the source port from which the
	// keepalives are sent cannot be reused by another app even if the fd gets closed by		// keepalives are sent cannot be reused by another app even if the fd gets closed by
	// the user. A null is acceptable here for backward compatibility of PacketKeepalive		// the user. A null is acceptable here for backward compatibility of PacketKeepalive
	@@ -206,6 +220,8 @@ public class KeepaliveTracker {
	+ IpUtils.addressAndPortToString(mPacket.dstAddress, mPacket.dstPort)		+ IpUtils.addressAndPortToString(mPacket.dstAddress, mPacket.dstPort)
	+ " interval=" + mInterval		+ " interval=" + mInterval
	+ " uid=" + mUid + " pid=" + mPid + " privileged=" + mPrivileged		+ " uid=" + mUid + " pid=" + mPid + " privileged=" + mPrivileged
			+ " nattIpsecRId=" + mNattIpsecResourceId
			+ " encapSocketRId=" + mEncapSocketResourceId
	+ " packetData=" + HexDump.toHexString(mPacket.getPacket())		+ " packetData=" + HexDump.toHexString(mPacket.getPacket())
	+ " ]";		+ " ]";
	}		}
	@@ -262,6 +278,51 @@ public class KeepaliveTracker {
	return SUCCESS;		return SUCCESS;
	}		}

			private int checkAndLockNattKeepaliveResource() {
			// Check that apps should be either privileged or fill in an ipsec encapsulated socket
			// resource id.
			if (mEncapSocketResourceId == INVALID_RESOURCE_ID) {
			if (mPrivileged) {
			return SUCCESS;
			} else {
			// Invalid access.
			return ERROR_INVALID_SOCKET;
			}
			}

			// Check if the ipsec encapsulated socket resource id is registered.
			final HashMap<Integer, KeepaliveInfo> networkKeepalives = mKeepalives.get(mNai);
			if (networkKeepalives == null) {
			return ERROR_INVALID_NETWORK;
			}
			for (KeepaliveInfo ki : networkKeepalives.values()) {
			if (ki.mEncapSocketResourceId == mEncapSocketResourceId
			&& ki.mNattIpsecResourceId != INVALID_RESOURCE_ID) {
			Log.d(TAG, "Registered resource found on keepalive " + mSlot
			+ " when verify NATT socket with uid=" + mUid + " rid="
			+ mEncapSocketResourceId);
			return ERROR_INVALID_SOCKET;
			}
			}

			// Ensure that the socket is created by IpSecService, and lock the resource that is
			// preserved by IpSecService. If succeed, a resource id is stored to keep tracking
			// the resource preserved by IpSecServce and must be released when stopping keepalive.
			try {
			mNattIpsecResourceId =
			mIpSec.lockEncapSocketForNattKeepalive(mEncapSocketResourceId, mUid);
			return SUCCESS;
			} catch (IllegalArgumentException e) {
			// The UID specified does not own the specified UDP encapsulation socket.
			Log.d(TAG, "Failed to verify NATT socket with uid=" + mUid + " rid="
			+ mEncapSocketResourceId + ": " + e);
			} catch (RemoteException e) {
			Log.wtf(TAG, "Error calling lockEncapSocketForNattKeepalive with "
			+ this.toString(), e);
			}
			return ERROR_INVALID_SOCKET;
			}

	private int isValid() {		private int isValid() {
	synchronized (mNai) {		synchronized (mNai) {
	int error = checkInterval();		int error = checkInterval();
	@@ -275,6 +336,13 @@ public class KeepaliveTracker {
	void start(int slot) {		void start(int slot) {
	mSlot = slot;		mSlot = slot;
	int error = isValid();		int error = isValid();

			// Check and lock ipsec resource needed by natt kepalive. This should be only called
			// once per keepalive.
			if (error == SUCCESS && mType == TYPE_NATT) {
			error = checkAndLockNattKeepaliveResource();
			}

	if (error == SUCCESS) {		if (error == SUCCESS) {
	Log.d(TAG, "Starting keepalive " + mSlot + " on " + mNai.name());		Log.d(TAG, "Starting keepalive " + mSlot + " on " + mNai.name());
	switch (mType) {		switch (mType) {
	@@ -350,6 +418,20 @@ public class KeepaliveTracker {
	}		}
	}		}

			// Release the resource held by keepalive in IpSecService.
			if (mNattIpsecResourceId != INVALID_RESOURCE_ID) {
			if (mType != TYPE_NATT) {
			Log.wtf(TAG, "natt ipsec resource held by incorrect keepalive "
			+ this.toString());
			}
			try {
			mIpSec.releaseNattKeepalive(mNattIpsecResourceId, mUid);
			} catch (RemoteException e) {
			Log.wtf(TAG, "error calling releaseNattKeepalive with " + this.toString(), e);
			}
			mNattIpsecResourceId = INVALID_RESOURCE_ID;
			}

	if (reason == SUCCESS) {		if (reason == SUCCESS) {
	try {		try {
	mCallback.onStopped();		mCallback.onStopped();
	@@ -538,6 +620,7 @@ public class KeepaliveTracker {
	**/		**/
	public void startNattKeepalive(@Nullable NetworkAgentInfo nai,		public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
	@Nullable FileDescriptor fd,		@Nullable FileDescriptor fd,
			int encapSocketResourceId,
	int intervalSeconds,		int intervalSeconds,
	@NonNull ISocketKeepaliveCallback cb,		@NonNull ISocketKeepaliveCallback cb,
	@NonNull String srcAddrString,		@NonNull String srcAddrString,
	@@ -569,7 +652,7 @@ public class KeepaliveTracker {
	KeepaliveInfo ki = null;		KeepaliveInfo ki = null;
	try {		try {
	ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,		ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
	KeepaliveInfo.TYPE_NATT, fd);		KeepaliveInfo.TYPE_NATT, fd, encapSocketResourceId);
	} catch (InvalidSocketException \| IllegalArgumentException \| SecurityException e) {		} catch (InvalidSocketException \| IllegalArgumentException \| SecurityException e) {
	Log.e(TAG, "Fail to construct keepalive", e);		Log.e(TAG, "Fail to construct keepalive", e);
	notifyErrorCallback(cb, ERROR_INVALID_SOCKET);		notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
	@@ -609,7 +692,7 @@ public class KeepaliveTracker {
	KeepaliveInfo ki = null;		KeepaliveInfo ki = null;
	try {		try {
	ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,		ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
	KeepaliveInfo.TYPE_TCP, fd);		KeepaliveInfo.TYPE_TCP, fd, INVALID_RESOURCE_ID /* Unused */);
	} catch (InvalidSocketException \| IllegalArgumentException \| SecurityException e) {		} catch (InvalidSocketException \| IllegalArgumentException \| SecurityException e) {
	Log.e(TAG, "Fail to construct keepalive e=" + e);		Log.e(TAG, "Fail to construct keepalive e=" + e);
	notifyErrorCallback(cb, ERROR_INVALID_SOCKET);		notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
	@@ -628,17 +711,12 @@ public class KeepaliveTracker {
	**/		**/
	public void startNattKeepalive(@Nullable NetworkAgentInfo nai,		public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
	@Nullable FileDescriptor fd,		@Nullable FileDescriptor fd,
	int resourceId,		int encapSocketResourceId,
	int intervalSeconds,		int intervalSeconds,
	@NonNull ISocketKeepaliveCallback cb,		@NonNull ISocketKeepaliveCallback cb,
	@NonNull String srcAddrString,		@NonNull String srcAddrString,
	@NonNull String dstAddrString,		@NonNull String dstAddrString,
	int dstPort) {		int dstPort) {
	// Ensure that the socket is created by IpSecService.
	if (!isNattKeepaliveSocketValid(fd, resourceId)) {
	notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
	}

	// Get src port to adopt old API.		// Get src port to adopt old API.
	int srcPort = 0;		int srcPort = 0;
	try {		try {
	@@ -649,23 +727,8 @@ public class KeepaliveTracker {
	}		}

	// Forward request to old API.		// Forward request to old API.
	startNattKeepalive(nai, fd, intervalSeconds, cb, srcAddrString, srcPort,		startNattKeepalive(nai, fd, encapSocketResourceId, intervalSeconds, cb, srcAddrString,
	dstAddrString, dstPort);		srcPort, dstAddrString, dstPort);
	}

	/**
	* Verify if the IPsec NAT-T file descriptor and resource Id hold for IPsec keepalive is valid.
	**/
	public static boolean isNattKeepaliveSocketValid(@Nullable FileDescriptor fd, int resourceId) {
	// TODO: 1. confirm whether the fd is called from system api or created by IpSecService.
	// 2. If the fd is created from the system api, check that it's bounded. And
	// call dup to keep the fd open.
	// 3. If the fd is created from IpSecService, check if the resource ID is valid. And
	// hold the resource needed in IpSecService.
	if (null == fd) {
	return false;
	}
	return true;
	}		}

	public void dump(IndentingPrintWriter pw) {		public void dump(IndentingPrintWriter pw) {

tests/net/java/com/android/server/ConnectivityServiceTest.java

+23 −0

Original line number	Original line	Diff line number	Diff line
	@@ -4228,6 +4228,25 @@ public class ConnectivityServiceTest {
	callback.expectStarted();		callback.expectStarted();
	ka.stop();		ka.stop();
	callback.expectStopped();		callback.expectStopped();

			// Check that the same NATT socket cannot be used by 2 keepalives.
			try (SocketKeepalive ka2 = mCm.createSocketKeepalive(
			myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
			// Check that second keepalive cannot be started if the first one is running.
			ka.start(validKaInterval);
			callback.expectStarted();
			ka2.start(validKaInterval);
			callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
			ka.stop();
			callback.expectStopped();

			// Check that second keepalive can be started/stopped normally if the first one is
			// stopped.
			ka2.start(validKaInterval);
			callback.expectStarted();
			ka2.stop();
			callback.expectStopped();
			}
	}		}

	// Check that deleting the IP address stops the keepalive.		// Check that deleting the IP address stops the keepalive.
	@@ -4291,6 +4310,10 @@ public class ConnectivityServiceTest {
	testSocket.close();		testSocket.close();
	testSocket2.close();		testSocket2.close();
	}		}

			// Check that the closed socket cannot be used to start keepalive.
			ka.start(validKaInterval);
			callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
	}		}

	// Check that there is no port leaked after all keepalives and sockets are closed.		// Check that there is no port leaked after all keepalives and sockets are closed.