Merge "Fix SocketKeepalive APIs which do not meet API review requirement" am: 8324c3e7e5 am: 08e1787088

am: d24f3fc8b1

Change-Id: I873a82de90efce6f3baa17761c0576dc9c8210be

3 files changed, 90 insertions, 18 deletions

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 529ff0d425b7..2be92cde68ec 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/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,
diff --git a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
index 0e3d82c0a660..ce887eb4f0fe 100644
--- a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java
+++ b/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.TYPE_NATT, null);
+        KeepaliveInfo ki = 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 = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
-                KeepaliveInfo.TYPE_TCP, fd);
+        KeepaliveInfo ki = null;
+        try {
+            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);
     }
 
diff --git a/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java b/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java
index f4d9006a7068..09badec5c197 100644
--- a/services/core/java/com/android/server/connectivity/TcpKeepaliveController.java
+++ b/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.
-                throw new InvalidSocketException(ERROR_HARDWARE_UNSUPPORTED, e);
+                // Treat it as ERROR_UNSUPPORTED.
+                throw new InvalidSocketException(ERROR_UNSUPPORTED, e);
             } else {
                 throw new InvalidSocketException(ERROR_INVALID_SOCKET, e);
             }