Merge "Fix SocketKeepalive APIs which do not meet API review requirement" am: 66bc22760a

am: 7246f1a563

Change-Id: Iead39e877aec917ff3c409fda1ff0c4dbaa21fcb

core/java/android/net/ConnectivityManager.java

@@ -44,6 +44,7 @@ import android.os.INetworkManagementService;
 import android.os.Looper;
 import android.os.Message;
 import android.os.Messenger;
+import android.os.ParcelFileDescriptor;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.ResultReceiver;
@@ -64,6 +65,8 @@ import com.android.internal.util.Protocol;
 import libcore.net.event.NetworkEventDispatcher;
 
 import java.io.FileDescriptor;
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.net.InetAddress;
@@ -1923,14 +1926,22 @@ public class ConnectivityManager {
      * @return A {@link SocketKeepalive} object that can be used to control the keepalive on the
      *         given socket.
      **/
-    public SocketKeepalive createSocketKeepalive(@NonNull Network network,
+    public @NonNull SocketKeepalive createSocketKeepalive(@NonNull Network network,
             @NonNull UdpEncapsulationSocket socket,
             @NonNull InetAddress source,
             @NonNull InetAddress destination,
             @NonNull @CallbackExecutor Executor executor,
             @NonNull Callback callback) {
-        return new NattSocketKeepalive(mService, network, socket.getFileDescriptor(),
+        ParcelFileDescriptor dup;
-            socket.getResourceId(), source, destination, executor, callback);
+        try {
+            dup = ParcelFileDescriptor.dup(socket.getFileDescriptor());
+        } catch (IOException ignored) {
+            // Construct an invalid fd, so that if the user later calls start(), it will fail with
+            // ERROR_INVALID_SOCKET.
+            dup = new ParcelFileDescriptor(new FileDescriptor());
+        }
+        return new NattSocketKeepalive(mService, network, dup, socket.getResourceId(), source,
+                destination, executor, callback);
     }
 
     /**
@@ -1938,9 +1949,9 @@ public class ConnectivityManager {
      * by system apps which don't use IpSecService to create {@link UdpEncapsulationSocket}.
      *
      * @param network The {@link Network} the socket is on.
-     * @param fd The {@link FileDescriptor} that needs to be kept alive. The provided
+     * @param pfd The {@link ParcelFileDescriptor} that needs to be kept alive. The provided
-     *        {@link FileDescriptor} must be bound to a port and the keepalives will be sent from
+     *        {@link ParcelFileDescriptor} must be bound to a port and the keepalives will be sent
-     *        that port.
+     *        from that port.
      * @param source The source address of the {@link UdpEncapsulationSocket}.
      * @param destination The destination address of the {@link UdpEncapsulationSocket}. The
      *        keepalive packets will always be sent to port 4500 of the given {@code destination}.
@@ -1956,14 +1967,22 @@ public class ConnectivityManager {
      */
     @SystemApi
     @RequiresPermission(android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD)
-    public SocketKeepalive createNattKeepalive(@NonNull Network network,
+    public @NonNull SocketKeepalive createNattKeepalive(@NonNull Network network,
-            @NonNull FileDescriptor fd,
+            @NonNull ParcelFileDescriptor pfd,
             @NonNull InetAddress source,
             @NonNull InetAddress destination,
             @NonNull @CallbackExecutor Executor executor,
             @NonNull Callback callback) {
-        return new NattSocketKeepalive(mService, network, fd, INVALID_RESOURCE_ID /* Unused */,
+        ParcelFileDescriptor dup;
-                source, destination, executor, callback);
+        try {
+            dup = pfd.dup();
+        } catch (IOException ignored) {
+            // Construct an invalid fd, so that if the user later calls start(), it will fail with
+            // ERROR_INVALID_SOCKET.
+            dup = new ParcelFileDescriptor(new FileDescriptor());
+        }
+        return new NattSocketKeepalive(mService, network, dup,
+                INVALID_RESOURCE_ID /* Unused */, source, destination, executor, callback);
     }
 
     /**
@@ -1987,11 +2006,19 @@ public class ConnectivityManager {
      */
     @SystemApi
     @RequiresPermission(android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD)
-    public SocketKeepalive createSocketKeepalive(@NonNull Network network,
+    public @NonNull SocketKeepalive createSocketKeepalive(@NonNull Network network,
             @NonNull Socket socket,
             @NonNull Executor executor,
             @NonNull Callback callback) {
-        return new TcpSocketKeepalive(mService, network, socket, executor, callback);
+        ParcelFileDescriptor dup;
+        try {
+            dup = ParcelFileDescriptor.fromSocket(socket);
+        } catch (UncheckedIOException ignored) {
+            // Construct an invalid fd, so that if the user later calls start(), it will fail with
+            // ERROR_INVALID_SOCKET.
+            dup = new ParcelFileDescriptor(new FileDescriptor());
+        }
+        return new TcpSocketKeepalive(mService, network, dup, executor, callback);
     }
 
     /**
@@ -3323,7 +3350,7 @@ public class ConnectivityManager {
          * @param network The {@link Network} whose blocked status has changed.
          * @param blocked The blocked status of this {@link Network}.
          */
-        public void onBlockedStatusChanged(Network network, boolean blocked) {}
+        public void onBlockedStatusChanged(@NonNull Network network, boolean blocked) {}
 
         private NetworkRequest networkRequest;
     }

core/java/android/net/NattSocketKeepalive.java

@@ -17,10 +17,10 @@
 package android.net;
 
 import android.annotation.NonNull;
+import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.util.Log;
 
-import java.io.FileDescriptor;
 import java.net.InetAddress;
 import java.util.concurrent.Executor;
 
@@ -31,21 +31,19 @@ public final class NattSocketKeepalive extends SocketKeepalive {
 
     @NonNull private final InetAddress mSource;
     @NonNull private final InetAddress mDestination;
-    @NonNull private final FileDescriptor mFd;
     private final int mResourceId;
 
     NattSocketKeepalive(@NonNull IConnectivityManager service,
             @NonNull Network network,
-            @NonNull FileDescriptor fd,
+            @NonNull ParcelFileDescriptor pfd,
             int resourceId,
             @NonNull InetAddress source,
             @NonNull InetAddress destination,
             @NonNull Executor executor,
             @NonNull Callback callback) {
-        super(service, network, executor, callback);
+        super(service, network, pfd, executor, callback);
         mSource = source;
         mDestination = destination;
-        mFd = fd;
         mResourceId = resourceId;
     }
 
@@ -53,8 +51,8 @@ public final class NattSocketKeepalive extends SocketKeepalive {
     void startImpl(int intervalSec) {
         mExecutor.execute(() -> {
             try {
-                mService.startNattKeepaliveWithFd(mNetwork, mFd, mResourceId, intervalSec,
+                mService.startNattKeepaliveWithFd(mNetwork, mPfd.getFileDescriptor(), mResourceId,
-                        mCallback,
+                        intervalSec, mCallback,
                         mSource.getHostAddress(), mDestination.getHostAddress());
             } catch (RemoteException e) {
                 Log.e(TAG, "Error starting socket keepalive: ", e);
@@ -75,6 +73,5 @@ public final class NattSocketKeepalive extends SocketKeepalive {
                 throw e.rethrowFromSystemServer();
             }
         });
-
     }
 }

core/java/android/net/NetworkAgent.java

@@ -488,14 +488,14 @@ public abstract class NetworkAgent extends Handler {
      * Requests that the network hardware send the specified packet at the specified interval.
      */
     protected void startSocketKeepalive(Message msg) {
-        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_UNSUPPORTED);
     }
 
     /**
      * Requests that the network hardware stops sending keepalive packets.
      */
     protected void stopSocketKeepalive(Message msg) {
-        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_UNSUPPORTED);
     }
 
     /**
@@ -511,7 +511,7 @@ public abstract class NetworkAgent extends Handler {
      * override this method.
      */
     protected void addKeepalivePacketFilter(Message msg) {
-        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_UNSUPPORTED);
     }
 
     /**
@@ -520,7 +520,7 @@ public abstract class NetworkAgent extends Handler {
      * must override this method.
      */
     protected void removeKeepalivePacketFilter(Message msg) {
-        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+        onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_UNSUPPORTED);
     }
 
     /**

core/java/android/net/SocketKeepalive.java

@@ -21,8 +21,10 @@ import android.annotation.IntRange;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
 import android.os.Binder;
+import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 
+import java.io.IOException;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.concurrent.Executor;
@@ -73,10 +75,15 @@ public abstract class SocketKeepalive implements AutoCloseable {
     /** The target socket is not idle. */
     public static final int ERROR_SOCKET_NOT_IDLE = -26;
 
-    /** The hardware does not support this request. */
+    /** The device does not support this request. */
-    public static final int ERROR_HARDWARE_UNSUPPORTED = -30;
+    public static final int ERROR_UNSUPPORTED = -30;
+    /** @hide TODO: delete when telephony code has been updated. */
+    public static final int ERROR_HARDWARE_UNSUPPORTED = ERROR_UNSUPPORTED;
     /** The hardware returned an error. */
     public static final int ERROR_HARDWARE_ERROR = -31;
+    /** The limitation of resource is reached. */
+    public static final int ERROR_INSUFFICIENT_RESOURCES = -32;
+
 
     /** @hide */
     @Retention(RetentionPolicy.SOURCE)
@@ -147,15 +154,18 @@ public abstract class SocketKeepalive implements AutoCloseable {
 
     @NonNull final IConnectivityManager mService;
     @NonNull final Network mNetwork;
+    @NonNull final ParcelFileDescriptor mPfd;
     @NonNull final Executor mExecutor;
     @NonNull final ISocketKeepaliveCallback mCallback;
     // TODO: remove slot since mCallback could be used to identify which keepalive to stop.
     @Nullable Integer mSlot;
 
     SocketKeepalive(@NonNull IConnectivityManager service, @NonNull Network network,
+            @NonNull ParcelFileDescriptor pfd,
             @NonNull Executor executor, @NonNull Callback callback) {
         mService = service;
         mNetwork = network;
+        mPfd = pfd;
         mExecutor = executor;
         mCallback = new ISocketKeepaliveCallback.Stub() {
             @Override
@@ -233,6 +243,11 @@ public abstract class SocketKeepalive implements AutoCloseable {
     @Override
     public final void close() {
         stop();
+        try {
+            mPfd.close();
+        } catch (IOException e) {
+            // Nothing much can be done.
+        }
     }
 
     /**

core/java/android/net/TcpSocketKeepalive.java

@@ -17,25 +17,22 @@
 package android.net;
 
 import android.annotation.NonNull;
+import android.os.ParcelFileDescriptor;
 import android.os.RemoteException;
 import android.util.Log;
 
 import java.io.FileDescriptor;
-import java.net.Socket;
 import java.util.concurrent.Executor;
 
 /** @hide */
 final class TcpSocketKeepalive extends SocketKeepalive {
 
-    private final Socket mSocket;
-
     TcpSocketKeepalive(@NonNull IConnectivityManager service,
             @NonNull Network network,
-            @NonNull Socket socket,
+            @NonNull ParcelFileDescriptor pfd,
             @NonNull Executor executor,
             @NonNull Callback callback) {
-        super(service, network, executor, callback);
+        super(service, network, pfd, executor, callback);
-        mSocket = socket;
     }
 
     /**
@@ -57,7 +54,7 @@ final class TcpSocketKeepalive extends SocketKeepalive {
     void startImpl(int intervalSec) {
         mExecutor.execute(() -> {
             try {
-                final FileDescriptor fd = mSocket.getFileDescriptor$();
+                final FileDescriptor fd = mPfd.getFileDescriptor();
                 mService.startTcpKeepalive(mNetwork, fd, intervalSec, mCallback);
             } catch (RemoteException e) {
                 Log.e(TAG, "Error starting packet keepalive: ", e);

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

@@ -988,7 +988,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
 
         mUserManager = (UserManager) context.getSystemService(Context.USER_SERVICE);
 
-        mKeepaliveTracker = new KeepaliveTracker(mHandler);
+        mKeepaliveTracker = new KeepaliveTracker(mContext, mHandler);
         mNotifier = new NetworkNotificationManager(mContext, mTelephonyManager,
                 mContext.getSystemService(NotificationManager.class));
 
@@ -6702,7 +6702,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
             ISocketKeepaliveCallback cb, String srcAddr, int srcPort, String dstAddr) {
         enforceKeepalivePermission();
         mKeepaliveTracker.startNattKeepalive(
-                getNetworkAgentInfoForNetwork(network),
+                getNetworkAgentInfoForNetwork(network), null /* fd */,
                 intervalSeconds, cb,
                 srcAddr, srcPort, dstAddr, NattSocketKeepalive.NATT_PORT);
     }
@@ -6711,7 +6711,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
     public void startNattKeepaliveWithFd(Network network, FileDescriptor fd, int resourceId,
             int intervalSeconds, ISocketKeepaliveCallback cb, String srcAddr,
             String dstAddr) {
-        enforceKeepalivePermission();
         mKeepaliveTracker.startNattKeepalive(
                 getNetworkAgentInfoForNetwork(network), fd, resourceId,
                 intervalSeconds, cb,

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

@@ -16,6 +16,7 @@
 
 package com.android.server.connectivity;
 
+import static android.content.pm.PackageManager.PERMISSION_GRANTED;
 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;
@@ -23,6 +24,7 @@ import static android.net.NetworkAgent.CMD_START_SOCKET_KEEPALIVE;
 import static android.net.NetworkAgent.CMD_STOP_SOCKET_KEEPALIVE;
 import static android.net.SocketKeepalive.BINDER_DIED;
 import static android.net.SocketKeepalive.DATA_RECEIVED;
+import static android.net.SocketKeepalive.ERROR_INSUFFICIENT_RESOURCES;
 import static android.net.SocketKeepalive.ERROR_INVALID_INTERVAL;
 import static android.net.SocketKeepalive.ERROR_INVALID_IP_ADDRESS;
 import static android.net.SocketKeepalive.ERROR_INVALID_NETWORK;
@@ -34,6 +36,7 @@ import static android.net.SocketKeepalive.SUCCESS;
 
 import android.annotation.NonNull;
 import android.annotation.Nullable;
+import android.content.Context;
 import android.net.ISocketKeepaliveCallback;
 import android.net.KeepalivePacketData;
 import android.net.NattKeepalivePacketData;
@@ -84,10 +87,13 @@ public class KeepaliveTracker {
     private final Handler mConnectivityServiceHandler;
     @NonNull
     private final TcpKeepaliveController mTcpController;
+    @NonNull
+    private final Context mContext;
 
-    public KeepaliveTracker(Handler handler) {
+    public KeepaliveTracker(Context context, Handler handler) {
         mConnectivityServiceHandler = handler;
         mTcpController = new TcpKeepaliveController(handler);
+        mContext = context;
     }
 
     /**
@@ -101,6 +107,7 @@ public class KeepaliveTracker {
         private final ISocketKeepaliveCallback mCallback;
         private final int mUid;
         private final int mPid;
+        private final boolean mPrivileged;
         private final NetworkAgentInfo mNai;
         private final int mType;
         private final FileDescriptor mFd;
@@ -108,6 +115,11 @@ public class KeepaliveTracker {
         public static final int TYPE_NATT = 1;
         public static final int TYPE_TCP = 2;
 
+        // Max allowed unprivileged keepalive slots per network. Caller's permission will be
+        // enforced if number of existing keepalives reach this limit.
+        // TODO: consider making this limit configurable via resources.
+        private static final int MAX_UNPRIVILEGED_SLOTS = 3;
+
         // Keepalive slot. A small integer that identifies this keepalive among the ones handled
         // by this network.
         private int mSlot = NO_KEEPALIVE;
@@ -127,16 +139,33 @@ public class KeepaliveTracker {
                 @NonNull KeepalivePacketData packet,
                 int interval,
                 int type,
-                @NonNull FileDescriptor fd) {
+                @Nullable FileDescriptor fd) throws InvalidSocketException {
             mCallback = callback;
             mPid = Binder.getCallingPid();
             mUid = Binder.getCallingUid();
+            mPrivileged = (PERMISSION_GRANTED == mContext.checkPermission(PERMISSION, mPid, mUid));
 
             mNai = nai;
             mPacket = packet;
             mInterval = interval;
             mType = type;
-            mFd = fd;
+
+            // 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
+            // API.
+            // TODO: don't accept null fd after legacy packetKeepalive API is removed.
+            try {
+                if (fd != null) {
+                    mFd = Os.dup(fd);
+                }  else {
+                    Log.d(TAG, "uid/pid " + mUid + "/" + mPid + " calls with null fd");
+                    mFd = null;
+                }
+            } catch (ErrnoException e) {
+                Log.e(TAG, "Cannot dup fd: ", e);
+                throw new InvalidSocketException(ERROR_INVALID_SOCKET, e);
+            }
 
             try {
                 mCallback.asBinder().linkToDeath(this, 0);
@@ -167,7 +196,7 @@ public class KeepaliveTracker {
                     + "->"
                     + IpUtils.addressAndPortToString(mPacket.dstAddress, mPacket.dstPort)
                     + " interval=" + mInterval
-                    + " uid=" + mUid + " pid=" + mPid
+                    + " uid=" + mUid + " pid=" + mPid + " privileged=" + mPrivileged
                     + " packetData=" + HexDump.toHexString(mPacket.getPacket())
                     + " ]";
         }
@@ -207,9 +236,27 @@ public class KeepaliveTracker {
             return SUCCESS;
         }
 
+        private int checkPermission() {
+            final HashMap<Integer, KeepaliveInfo> networkKeepalives = mKeepalives.get(mNai);
+            int unprivilegedCount = 0;
+            if (networkKeepalives == null) {
+                return ERROR_INVALID_NETWORK;
+            }
+            for (KeepaliveInfo ki : networkKeepalives.values()) {
+                if (!ki.mPrivileged) {
+                    unprivilegedCount++;
+                }
+                if (unprivilegedCount >= MAX_UNPRIVILEGED_SLOTS) {
+                    return mPrivileged ? SUCCESS : ERROR_INSUFFICIENT_RESOURCES;
+                }
+            }
+            return SUCCESS;
+        }
+
         private int isValid() {
             synchronized (mNai) {
                 int error = checkInterval();
+                if (error == SUCCESS) error = checkPermission();
                 if (error == SUCCESS) error = checkNetworkConnected();
                 if (error == SUCCESS) error = checkSourceAddress();
                 return error;
@@ -272,6 +319,18 @@ public class KeepaliveTracker {
                 }
             }
 
+            // Close the duplicated fd that maintains the lifecycle of socket whenever
+            // keepalive is running.
+            if (mFd != null) {
+                try {
+                    Os.close(mFd);
+                } catch (ErrnoException e) {
+                    // This should not happen since system server controls the lifecycle of fd when
+                    // keepalive offload is running.
+                    Log.wtf(TAG, "Error closing fd for keepalive " + mSlot + ": " + e);
+                }
+            }
+
             if (reason == SUCCESS) {
                 try {
                     mCallback.onStopped();
@@ -355,8 +414,9 @@ public class KeepaliveTracker {
             return;
         }
         ki.stop(reason);
-        Log.d(TAG, "Stopped keepalive " + slot + " on " + networkName);
         networkKeepalives.remove(slot);
+        Log.d(TAG, "Stopped keepalive " + slot + " on " + networkName + ", "
+                + networkKeepalives.size() + " remains.");
         if (networkKeepalives.isEmpty()) {
             mKeepalives.remove(nai);
         }
@@ -389,7 +449,8 @@ public class KeepaliveTracker {
             ki = mKeepalives.get(nai).get(slot);
         } catch(NullPointerException e) {}
         if (ki == null) {
-            Log.e(TAG, "Event for unknown keepalive " + slot + " on " + nai.name());
+            Log.e(TAG, "Event " + message.what + " for unknown keepalive " + slot + " on "
+                    + nai.name());
             return;
         }
 
@@ -437,6 +498,7 @@ public class KeepaliveTracker {
      * {@link android.net.SocketKeepalive}.
      **/
     public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
+            @Nullable FileDescriptor fd,
             int intervalSeconds,
             @NonNull ISocketKeepaliveCallback cb,
             @NonNull String srcAddrString,
@@ -465,8 +527,14 @@ public class KeepaliveTracker {
             notifyErrorCallback(cb, e.error);
             return;
         }
-        KeepaliveInfo ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
+        KeepaliveInfo ki = null;
-                KeepaliveInfo.TYPE_NATT, null);
+        try {
+            ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
+                    KeepaliveInfo.TYPE_NATT, fd);
+        } catch (InvalidSocketException e) {
+            notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
+            return;
+        }
         Log.d(TAG, "Created keepalive: " + ki.toString());
         mConnectivityServiceHandler.obtainMessage(
                 NetworkAgent.CMD_START_SOCKET_KEEPALIVE, ki).sendToTarget();
@@ -498,9 +566,14 @@ public class KeepaliveTracker {
             notifyErrorCallback(cb, e.error);
             return;
         }
-
+        KeepaliveInfo ki = null;
-        KeepaliveInfo ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
+        try {
-                KeepaliveInfo.TYPE_TCP, fd);
+            ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
+                    KeepaliveInfo.TYPE_TCP, fd);
+        } catch (InvalidSocketException e) {
+            notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
+            return;
+        }
         Log.d(TAG, "Created keepalive: " + ki.toString());
         mConnectivityServiceHandler.obtainMessage(CMD_START_SOCKET_KEEPALIVE, ki).sendToTarget();
     }
@@ -535,7 +608,7 @@ public class KeepaliveTracker {
         }
 
         // Forward request to old API.
-        startNattKeepalive(nai, intervalSeconds, cb, srcAddrString, srcPort,
+        startNattKeepalive(nai, fd, intervalSeconds, cb, srcAddrString, srcPort,
                 dstAddrString, dstPort);
     }
 

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

@@ -16,9 +16,9 @@
 package com.android.server.connectivity;
 
 import static android.net.SocketKeepalive.DATA_RECEIVED;
-import static android.net.SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED;
 import static android.net.SocketKeepalive.ERROR_INVALID_SOCKET;
 import static android.net.SocketKeepalive.ERROR_SOCKET_NOT_IDLE;
+import static android.net.SocketKeepalive.ERROR_UNSUPPORTED;
 import static android.os.MessageQueue.OnFileDescriptorEventListener.EVENT_ERROR;
 import static android.os.MessageQueue.OnFileDescriptorEventListener.EVENT_INPUT;
 import static android.system.OsConstants.ENOPROTOOPT;
@@ -197,8 +197,8 @@ public class TcpKeepaliveController {
             Log.e(TAG, "Exception reading TCP state from socket", e);
             if (e.errno == ENOPROTOOPT) {
                 // ENOPROTOOPT may happen in kernel version lower than 4.8.
-                // Treat it as ERROR_HARDWARE_UNSUPPORTED.
+                // Treat it as ERROR_UNSUPPORTED.
-                throw new InvalidSocketException(ERROR_HARDWARE_UNSUPPORTED, e);
+                throw new InvalidSocketException(ERROR_UNSUPPORTED, e);
             } else {
                 throw new InvalidSocketException(ERROR_INVALID_SOCKET, e);
             }

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

@@ -144,12 +144,14 @@ import android.os.INetworkManagementService;
 import android.os.Looper;
 import android.os.Message;
 import android.os.Parcel;
+import android.os.ParcelFileDescriptor;
 import android.os.Parcelable;
 import android.os.Process;
 import android.os.RemoteException;
 import android.os.SystemClock;
 import android.os.UserHandle;
 import android.provider.Settings;
+import android.system.Os;
 import android.test.mock.MockContentResolver;
 import android.text.TextUtils;
 import android.util.ArraySet;
@@ -188,6 +190,8 @@ import org.mockito.MockitoAnnotations;
 import org.mockito.Spy;
 import org.mockito.stubbing.Answer;
 
+import java.io.IOException;
+import java.net.DatagramSocket;
 import java.net.Inet4Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -421,7 +425,7 @@ public class ConnectivityServiceTest {
         private final ConditionVariable mPreventReconnectReceived = new ConditionVariable();
         private int mScore;
         private NetworkAgent mNetworkAgent;
-        private int mStartKeepaliveError = SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED;
+        private int mStartKeepaliveError = SocketKeepalive.ERROR_UNSUPPORTED;
         private int mStopKeepaliveError = SocketKeepalive.NO_KEEPALIVE;
         private Integer mExpectedKeepaliveSlot = null;
         // Contains the redirectUrl from networkStatus(). Before reading, wait for
@@ -4032,6 +4036,7 @@ public class ConnectivityServiceTest {
         runTestWithSerialExecutors(executor -> {
             try {
                 doTestNattSocketKeepalivesWithExecutor(executor);
+                doTestNattSocketKeepalivesFdWithExecutor(executor);
             } catch (Exception e) {
                 fail(e.getMessage());
             }
@@ -4041,6 +4046,8 @@ public class ConnectivityServiceTest {
     private void doTestNattSocketKeepalivesWithExecutor(Executor executor) throws Exception {
         // TODO: 1. Move this outside of ConnectivityServiceTest.
         //       2. Make test to verify that Nat-T keepalive socket is created by IpSecService.
+        //       3. Mock ipsec service.
+        //       4. Find a free port instead of a fixed port.
         final int srcPort = 12345;
         final InetAddress myIPv4 = InetAddress.getByName("192.0.2.129");
         final InetAddress notMyIPv4 = InetAddress.getByName("192.0.2.35");
@@ -4065,89 +4072,106 @@ public class ConnectivityServiceTest {
         Network myNet = connectKeepaliveNetwork(lp);
 
         TestSocketKeepaliveCallback callback = new TestSocketKeepaliveCallback(executor);
-        SocketKeepalive ka;
 
         // Attempt to start keepalives with invalid parameters and check for errors.
         // Invalid network.
-        ka = mCm.createSocketKeepalive(notMyNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                notMyNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+        }
 
         // Invalid interval.
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(invalidKaInterval);
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_INTERVAL);
+            ka.start(invalidKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_INTERVAL);
+        }
 
         // Invalid destination.
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv6, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv4, dstIPv6, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+        }
 
         // Invalid source;
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv6, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv6, dstIPv4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+        }
 
         // NAT-T is only supported for IPv4.
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv6, dstIPv6, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv6, dstIPv6, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+        }
 
         // Sanity check before testing started keepalive.
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_UNSUPPORTED);
+        }
 
         // Check that a started keepalive can be stopped.
         mWiFiNetworkAgent.setStartKeepaliveError(SocketKeepalive.SUCCESS);
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectStarted();
+            ka.start(validKaInterval);
-        mWiFiNetworkAgent.setStopKeepaliveError(SocketKeepalive.SUCCESS);
+            callback.expectStarted();
-        ka.stop();
+            mWiFiNetworkAgent.setStopKeepaliveError(SocketKeepalive.SUCCESS);
-        callback.expectStopped();
+            ka.stop();
+            callback.expectStopped();
 
-        // Check that keepalive could be restarted.
+            // Check that keepalive could be restarted.
-        ka.start(validKaInterval);
+            ka.start(validKaInterval);
-        callback.expectStarted();
+            callback.expectStarted();
-        ka.stop();
+            ka.stop();
-        callback.expectStopped();
+            callback.expectStopped();
 
-        // Check that keepalive can be restarted without waiting for callback.
+            // Check that keepalive can be restarted without waiting for callback.
-        ka.start(validKaInterval);
+            ka.start(validKaInterval);
-        callback.expectStarted();
+            callback.expectStarted();
-        ka.stop();
+            ka.stop();
-        ka.start(validKaInterval);
+            ka.start(validKaInterval);
-        callback.expectStopped();
+            callback.expectStopped();
-        callback.expectStarted();
+            callback.expectStarted();
-        ka.stop();
+            ka.stop();
-        callback.expectStopped();
+            callback.expectStopped();
+        }
 
         // Check that deleting the IP address stops the keepalive.
         LinkProperties bogusLp = new LinkProperties(lp);
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectStarted();
+            ka.start(validKaInterval);
-        bogusLp.removeLinkAddress(new LinkAddress(myIPv4, 25));
+            callback.expectStarted();
-        bogusLp.addLinkAddress(new LinkAddress(notMyIPv4, 25));
+            bogusLp.removeLinkAddress(new LinkAddress(myIPv4, 25));
-        mWiFiNetworkAgent.sendLinkProperties(bogusLp);
+            bogusLp.addLinkAddress(new LinkAddress(notMyIPv4, 25));
-        callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+            mWiFiNetworkAgent.sendLinkProperties(bogusLp);
-        mWiFiNetworkAgent.sendLinkProperties(lp);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+            mWiFiNetworkAgent.sendLinkProperties(lp);
+        }
 
         // Check that a started keepalive is stopped correctly when the network disconnects.
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectStarted();
+            ka.start(validKaInterval);
-        mWiFiNetworkAgent.disconnect();
+            callback.expectStarted();
-        waitFor(mWiFiNetworkAgent.getDisconnectedCV());
+            mWiFiNetworkAgent.disconnect();
-        callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+            waitFor(mWiFiNetworkAgent.getDisconnectedCV());
+            callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
 
-        // ... and that stopping it after that has no adverse effects.
+            // ... and that stopping it after that has no adverse effects.
-        waitForIdle();
+            waitForIdle();
-        final Network myNetAlias = myNet;
+            final Network myNetAlias = myNet;
-        assertNull(mCm.getNetworkCapabilities(myNetAlias));
+            assertNull(mCm.getNetworkCapabilities(myNetAlias));
-        ka.stop();
+            ka.stop();
-        callback.assertNoCallback();
+            callback.assertNoCallback();
+        }
 
         // Reconnect.
         myNet = connectKeepaliveNetwork(lp);
@@ -4155,27 +4179,30 @@ public class ConnectivityServiceTest {
 
         // Check that keepalive slots start from 1 and increment. The first one gets slot 1.
         mWiFiNetworkAgent.setExpectedKeepaliveSlot(1);
-        ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+                myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
-        callback.expectStarted();
+            ka.start(validKaInterval);
+            callback.expectStarted();
 
-        // The second one gets slot 2.
+            // The second one gets slot 2.
-        mWiFiNetworkAgent.setExpectedKeepaliveSlot(2);
+            mWiFiNetworkAgent.setExpectedKeepaliveSlot(2);
-        final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(6789);
+            final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(6789);
-        TestSocketKeepaliveCallback callback2 = new TestSocketKeepaliveCallback(executor);
+            TestSocketKeepaliveCallback callback2 = new TestSocketKeepaliveCallback(executor);
-        SocketKeepalive ka2 =
+            try (SocketKeepalive ka2 = mCm.createSocketKeepalive(
-                mCm.createSocketKeepalive(myNet, testSocket2, myIPv4, dstIPv4, executor, callback2);
+                    myNet, testSocket2, myIPv4, dstIPv4, executor, callback2)) {
-        ka2.start(validKaInterval);
+                ka2.start(validKaInterval);
-        callback2.expectStarted();
+                callback2.expectStarted();
 
-        ka.stop();
+                ka.stop();
-        callback.expectStopped();
+                callback.expectStopped();
 
-        ka2.stop();
+                ka2.stop();
-        callback2.expectStopped();
+                callback2.expectStopped();
 
-        testSocket.close();
+                testSocket.close();
-        testSocket2.close();
+                testSocket2.close();
+            }
+        }
 
         mWiFiNetworkAgent.disconnect();
         waitFor(mWiFiNetworkAgent.getDisconnectedCV());
@@ -4200,7 +4227,6 @@ public class ConnectivityServiceTest {
         final InetAddress myIPv6 = InetAddress.getByName("::1");
 
         final int validKaInterval = 15;
-        final int invalidKaInterval = 9;
 
         final LinkProperties lp = new LinkProperties();
         lp.setInterfaceName("wlan12");
@@ -4216,37 +4242,46 @@ public class ConnectivityServiceTest {
         final Socket testSocketV6 = new Socket();
 
         TestSocketKeepaliveCallback callback = new TestSocketKeepaliveCallback(executor);
-        SocketKeepalive ka;
 
         // Attempt to start Tcp keepalives with invalid parameters and check for errors.
         // Invalid network.
-        ka = mCm.createSocketKeepalive(notMyNet, testSocketV4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+            notMyNet, testSocketV4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+        }
 
         // Invalid Socket (socket is not bound with IPv4 address).
-        ka = mCm.createSocketKeepalive(myNet, testSocketV4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+            myNet, testSocketV4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+        }
 
         // Invalid Socket (socket is not bound with IPv6 address).
-        ka = mCm.createSocketKeepalive(myNet, testSocketV6, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+            myNet, testSocketV6, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+        }
 
         // Bind the socket address
         testSocketV4.bind(new InetSocketAddress(myIPv4, srcPortV4));
         testSocketV6.bind(new InetSocketAddress(myIPv6, srcPortV6));
 
         // Invalid Socket (socket is bound with IPv4 address).
-        ka = mCm.createSocketKeepalive(myNet, testSocketV4, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+            myNet, testSocketV4, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+        }
 
         // Invalid Socket (socket is bound with IPv6 address).
-        ka = mCm.createSocketKeepalive(myNet, testSocketV6, executor, callback);
+        try (SocketKeepalive ka = mCm.createSocketKeepalive(
-        ka.start(validKaInterval);
+            myNet, testSocketV6, executor, callback)) {
-        callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+            ka.start(validKaInterval);
+            callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+        }
 
         testSocketV4.close();
         testSocketV6.close();
@@ -4256,6 +4291,66 @@ public class ConnectivityServiceTest {
         mWiFiNetworkAgent = null;
     }
 
+    private void doTestNattSocketKeepalivesFdWithExecutor(Executor executor) throws Exception {
+        final int srcPort = 12345;
+        final InetAddress myIPv4 = InetAddress.getByName("192.0.2.129");
+        final InetAddress anyIPv4 = InetAddress.getByName("0.0.0.0");
+        final InetAddress dstIPv4 = InetAddress.getByName("8.8.8.8");
+        final int validKaInterval = 15;
+
+        // Prepare the target network.
+        LinkProperties lp = new LinkProperties();
+        lp.setInterfaceName("wlan12");
+        lp.addLinkAddress(new LinkAddress(myIPv4, 25));
+        lp.addRoute(new RouteInfo(InetAddress.getByName("192.0.2.254")));
+        Network myNet = connectKeepaliveNetwork(lp);
+        mWiFiNetworkAgent.setStartKeepaliveError(SocketKeepalive.SUCCESS);
+        mWiFiNetworkAgent.setStopKeepaliveError(SocketKeepalive.SUCCESS);
+
+        TestSocketKeepaliveCallback callback = new TestSocketKeepaliveCallback(executor);
+
+        // Prepare the target file descriptor, keep only one instance.
+        final IpSecManager mIpSec = (IpSecManager) mContext.getSystemService(Context.IPSEC_SERVICE);
+        final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket(srcPort);
+        final ParcelFileDescriptor testPfd =
+                ParcelFileDescriptor.dup(testSocket.getFileDescriptor());
+        testSocket.close();
+        assertTrue(isUdpPortInUse(srcPort));
+
+        // Start keepalive and explicit make the variable goes out of scope with try-with-resources
+        // block.
+        try (SocketKeepalive ka = mCm.createNattKeepalive(
+                myNet, testPfd, myIPv4, dstIPv4, executor, callback)) {
+            ka.start(validKaInterval);
+            callback.expectStarted();
+            ka.stop();
+            callback.expectStopped();
+        }
+
+        // Check that the ParcelFileDescriptor is still valid after keepalive stopped,
+        // ErrnoException with EBADF will be thrown if the socket is closed when checking local
+        // address.
+        assertTrue(isUdpPortInUse(srcPort));
+        final InetSocketAddress sa =
+                (InetSocketAddress) Os.getsockname(testPfd.getFileDescriptor());
+        assertEquals(anyIPv4, sa.getAddress());
+
+        testPfd.close();
+        assertFalse(isUdpPortInUse(srcPort));
+
+        mWiFiNetworkAgent.disconnect();
+        waitFor(mWiFiNetworkAgent.getDisconnectedCV());
+        mWiFiNetworkAgent = null;
+    }
+
+    private static boolean isUdpPortInUse(int port) {
+        try (DatagramSocket ignored = new DatagramSocket(port)) {
+            return false;
+        } catch (IOException ignored) {
+            return true;
+        }
+    }
+
     @Test
     public void testGetCaptivePortalServerUrl() throws Exception {
         String url = mCm.getCaptivePortalServerUrl();