Merge "[KA11] Verify fd ownership and allocate resource for NattKeepalive" am: 91cc5cc467

am: a1ed113552

Change-Id: I410acc19ceb13e32c65cbdf3095da9db0802470c

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

@@ -27,6 +27,7 @@ import static android.net.ConnectivityManager.getNetworkTypeName;
 import static android.net.ConnectivityManager.isNetworkTypeValid;
 import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY;
 import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_VALID;
+import static android.net.IpSecManager.INVALID_RESOURCE_ID;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_FOREGROUND;
 import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
@@ -6835,6 +6836,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         enforceKeepalivePermission();
         mKeepaliveTracker.startNattKeepalive(
                 getNetworkAgentInfoForNetwork(network), null /* fd */,
+                INVALID_RESOURCE_ID /* Unused */,
                 intervalSeconds, cb,
                 srcAddr, srcPort, dstAddr, NattSocketKeepalive.NATT_PORT);
     }

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

@@ -17,6 +17,7 @@
 package com.android.server.connectivity;
 
 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.NetworkAgent.CMD_ADD_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.Nullable;
 import android.content.Context;
+import android.net.IIpSecService;
 import android.net.ISocketKeepaliveCallback;
 import android.net.KeepalivePacketData;
 import android.net.NattKeepalivePacketData;
@@ -52,6 +54,7 @@ import android.os.IBinder;
 import android.os.Message;
 import android.os.Process;
 import android.os.RemoteException;
+import android.os.ServiceManager;
 import android.system.ErrnoException;
 import android.system.Os;
 import android.util.Log;
@@ -89,11 +92,14 @@ public class KeepaliveTracker {
     private final TcpKeepaliveController mTcpController;
     @NonNull
     private final Context mContext;
+    @NonNull
+    private final IIpSecService mIpSec;
 
     public KeepaliveTracker(Context context, Handler handler) {
         mConnectivityServiceHandler = handler;
         mTcpController = new TcpKeepaliveController(handler);
         mContext = context;
+        mIpSec = IIpSecService.Stub.asInterface(ServiceManager.getService(Context.IPSEC_SERVICE));
     }
 
     /**
@@ -112,6 +118,10 @@ public class KeepaliveTracker {
         private final int mType;
         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_TCP = 2;
 
@@ -140,7 +150,8 @@ public class KeepaliveTracker {
                 @NonNull KeepalivePacketData packet,
                 int interval,
                 int type,
-                @Nullable FileDescriptor fd) throws InvalidSocketException {
+                @Nullable FileDescriptor fd,
+                int encapSocketResourceId) throws InvalidSocketException {
             mCallback = callback;
             mPid = Binder.getCallingPid();
             mUid = Binder.getCallingUid();
@@ -151,6 +162,9 @@ public class KeepaliveTracker {
             mInterval = interval;
             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
             // 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
@@ -158,7 +172,7 @@ public class KeepaliveTracker {
             try {
                 if (fd != null) {
                     mFd = Os.dup(fd);
-                }  else {
+                } else {
                     Log.d(TAG, toString() + " calls with null fd");
                     if (!mPrivileged) {
                         throw new SecurityException(
@@ -206,6 +220,8 @@ public class KeepaliveTracker {
                     + IpUtils.addressAndPortToString(mPacket.dstAddress, mPacket.dstPort)
                     + " interval=" + mInterval
                     + " uid=" + mUid + " pid=" + mPid + " privileged=" + mPrivileged
+                    + " nattIpsecRId=" + mNattIpsecResourceId
+                    + " encapSocketRId=" + mEncapSocketResourceId
                     + " packetData=" + HexDump.toHexString(mPacket.getPacket())
                     + " ]";
         }
@@ -262,6 +278,51 @@ public class KeepaliveTracker {
             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() {
             synchronized (mNai) {
                 int error = checkInterval();
@@ -275,6 +336,13 @@ public class KeepaliveTracker {
         void start(int slot) {
             mSlot = slot;
             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) {
                 Log.d(TAG, "Starting keepalive " + mSlot + " on " + mNai.name());
                 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) {
                 try {
                     mCallback.onStopped();
@@ -538,6 +620,7 @@ public class KeepaliveTracker {
      **/
     public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
             @Nullable FileDescriptor fd,
+            int encapSocketResourceId,
             int intervalSeconds,
             @NonNull ISocketKeepaliveCallback cb,
             @NonNull String srcAddrString,
@@ -569,7 +652,7 @@ public class KeepaliveTracker {
         KeepaliveInfo ki = null;
         try {
             ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
-                    KeepaliveInfo.TYPE_NATT, fd);
+                    KeepaliveInfo.TYPE_NATT, fd, encapSocketResourceId);
         } catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
             Log.e(TAG, "Fail to construct keepalive", e);
             notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
@@ -609,7 +692,7 @@ public class KeepaliveTracker {
         KeepaliveInfo ki = null;
         try {
             ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
-                    KeepaliveInfo.TYPE_TCP, fd);
+                    KeepaliveInfo.TYPE_TCP, fd, INVALID_RESOURCE_ID /* Unused */);
         } catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
             Log.e(TAG, "Fail to construct keepalive e=" + e);
             notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
@@ -628,17 +711,12 @@ public class KeepaliveTracker {
     **/
     public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
             @Nullable FileDescriptor fd,
-            int resourceId,
+            int encapSocketResourceId,
             int intervalSeconds,
             @NonNull ISocketKeepaliveCallback cb,
             @NonNull String srcAddrString,
             @NonNull String dstAddrString,
             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.
         int srcPort = 0;
         try {
@@ -649,23 +727,8 @@ public class KeepaliveTracker {
         }
 
         // 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) {

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

@@ -4228,6 +4228,25 @@ public class ConnectivityServiceTest {
             callback.expectStarted();
             ka.stop();
             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.
@@ -4291,6 +4310,10 @@ public class ConnectivityServiceTest {
                 testSocket.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.