Fix SocketKeepalive APIs which do not meet API review requirement
Per API review, change the use of FileDescriptor to
ParcelFileDescriptor.
This change also fix nullability according to API review
feedbacks.
Fix: 126698610
Fix: 126699425
Fix: 126699232
Fix: 126700278
Test: 1. m -j
2. atest FrameworksNetTests --generate-new-metrics 50
3. m -j doc-comment-check-docs
Change-Id: I19476c50dd1ca290bf3f41973829da2bd229796a
diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java
index 428c2e7..c26191f 100644
--- a/core/java/android/net/ConnectivityManager.java
+++ b/core/java/android/net/ConnectivityManager.java
@@ -44,6 +44,7 @@
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 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;
@@ -1920,14 +1923,22 @@
* @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(),
- socket.getResourceId(), source, destination, executor, callback);
+ ParcelFileDescriptor dup;
+ 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);
}
/**
@@ -1935,9 +1946,9 @@
* 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
- * {@link FileDescriptor} must be bound to a port and the keepalives will be sent from
- * that port.
+ * @param pfd The {@link ParcelFileDescriptor} that needs to be kept alive. The provided
+ * {@link ParcelFileDescriptor} must be bound to a port and the keepalives will be sent
+ * 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}.
@@ -1953,14 +1964,22 @@
*/
@SystemApi
@RequiresPermission(android.Manifest.permission.PACKET_KEEPALIVE_OFFLOAD)
- public SocketKeepalive createNattKeepalive(@NonNull Network network,
- @NonNull FileDescriptor fd,
+ public @NonNull SocketKeepalive createNattKeepalive(@NonNull Network network,
+ @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 */,
- source, destination, executor, callback);
+ ParcelFileDescriptor dup;
+ 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);
}
/**
@@ -1984,11 +2003,19 @@
*/
@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);
}
/**
@@ -3320,7 +3347,7 @@
* @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;
}
diff --git a/core/java/android/net/NattSocketKeepalive.java b/core/java/android/net/NattSocketKeepalive.java
index 84da294..b0ce0c7 100644
--- a/core/java/android/net/NattSocketKeepalive.java
+++ b/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 @@
@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 @@
void startImpl(int intervalSec) {
mExecutor.execute(() -> {
try {
- mService.startNattKeepaliveWithFd(mNetwork, mFd, mResourceId, intervalSec,
- mCallback,
+ mService.startNattKeepaliveWithFd(mNetwork, mPfd.getFileDescriptor(), mResourceId,
+ intervalSec, mCallback,
mSource.getHostAddress(), mDestination.getHostAddress());
} catch (RemoteException e) {
Log.e(TAG, "Error starting socket keepalive: ", e);
@@ -75,6 +73,5 @@
throw e.rethrowFromSystemServer();
}
});
-
}
}
diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java
index b55f6ba..1edea55 100644
--- a/core/java/android/net/NetworkAgent.java
+++ b/core/java/android/net/NetworkAgent.java
@@ -488,14 +488,14 @@
* 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 @@
* 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 @@
* must override this method.
*/
protected void removeKeepalivePacketFilter(Message msg) {
- onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+ onSocketKeepaliveEvent(msg.arg1, SocketKeepalive.ERROR_UNSUPPORTED);
}
/**
diff --git a/core/java/android/net/SocketKeepalive.java b/core/java/android/net/SocketKeepalive.java
index 0e768df..9d91620 100644
--- a/core/java/android/net/SocketKeepalive.java
+++ b/core/java/android/net/SocketKeepalive.java
@@ -21,8 +21,10 @@
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 @@
/** The target socket is not idle. */
public static final int ERROR_SOCKET_NOT_IDLE = -26;
- /** The hardware does not support this request. */
- public static final int ERROR_HARDWARE_UNSUPPORTED = -30;
+ /** The device does not support this request. */
+ 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 @@
@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 @@
@Override
public final void close() {
stop();
+ try {
+ mPfd.close();
+ } catch (IOException e) {
+ // Nothing much can be done.
+ }
}
/**
diff --git a/core/java/android/net/TcpSocketKeepalive.java b/core/java/android/net/TcpSocketKeepalive.java
index 26cc8ff..436397e 100644
--- a/core/java/android/net/TcpSocketKeepalive.java
+++ b/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);
- mSocket = socket;
+ super(service, network, pfd, executor, callback);
}
/**
@@ -57,7 +54,7 @@
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);
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 9487faf..a2c5655 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -988,7 +988,7 @@
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));
@@ -6700,7 +6700,7 @@
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);
}
@@ -6709,7 +6709,6 @@
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 0e3d82c..ce887eb 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_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 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 @@
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 @@
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 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 @@
@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 @@
+ "->"
+ 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 @@
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 @@
}
}
+ // 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 @@
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 @@
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 @@
* {@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 @@
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 @@
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 @@
}
// 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 f4d9006..09badec 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 @@
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);
}
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index d3616aa..92a865a 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java
@@ -144,12 +144,14 @@
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.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 @@
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 @@
runTestWithSerialExecutors(executor -> {
try {
doTestNattSocketKeepalivesWithExecutor(executor);
+ doTestNattSocketKeepalivesFdWithExecutor(executor);
} catch (Exception e) {
fail(e.getMessage());
}
@@ -4041,6 +4046,8 @@
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 @@
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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ notMyNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+ }
// Invalid interval.
- ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv4, executor, callback);
- ka.start(invalidKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_INTERVAL);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ ka.start(invalidKaInterval);
+ callback.expectError(SocketKeepalive.ERROR_INVALID_INTERVAL);
+ }
// Invalid destination.
- ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv4, dstIPv6, executor, callback);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv6, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+ }
// Invalid source;
- ka = mCm.createSocketKeepalive(myNet, testSocket, myIPv6, dstIPv4, executor, callback);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv6, dstIPv4, executor, callback)) {
+ 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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv6, dstIPv6, executor, callback)) {
+ 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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_HARDWARE_UNSUPPORTED);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ 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);
- ka.start(validKaInterval);
- callback.expectStarted();
- mWiFiNetworkAgent.setStopKeepaliveError(SocketKeepalive.SUCCESS);
- ka.stop();
- callback.expectStopped();
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectStarted();
+ mWiFiNetworkAgent.setStopKeepaliveError(SocketKeepalive.SUCCESS);
+ ka.stop();
+ callback.expectStopped();
- // Check that keepalive could be restarted.
- ka.start(validKaInterval);
- callback.expectStarted();
- ka.stop();
- callback.expectStopped();
+ // Check that keepalive could be restarted.
+ ka.start(validKaInterval);
+ callback.expectStarted();
+ ka.stop();
+ callback.expectStopped();
- // Check that keepalive can be restarted without waiting for callback.
- ka.start(validKaInterval);
- callback.expectStarted();
- ka.stop();
- ka.start(validKaInterval);
- callback.expectStopped();
- callback.expectStarted();
- ka.stop();
- callback.expectStopped();
+ // Check that keepalive can be restarted without waiting for callback.
+ ka.start(validKaInterval);
+ callback.expectStarted();
+ ka.stop();
+ ka.start(validKaInterval);
+ callback.expectStopped();
+ callback.expectStarted();
+ ka.stop();
+ 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);
- ka.start(validKaInterval);
- callback.expectStarted();
- bogusLp.removeLinkAddress(new LinkAddress(myIPv4, 25));
- bogusLp.addLinkAddress(new LinkAddress(notMyIPv4, 25));
- mWiFiNetworkAgent.sendLinkProperties(bogusLp);
- callback.expectError(SocketKeepalive.ERROR_INVALID_IP_ADDRESS);
- mWiFiNetworkAgent.sendLinkProperties(lp);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectStarted();
+ bogusLp.removeLinkAddress(new LinkAddress(myIPv4, 25));
+ bogusLp.addLinkAddress(new LinkAddress(notMyIPv4, 25));
+ mWiFiNetworkAgent.sendLinkProperties(bogusLp);
+ 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);
- ka.start(validKaInterval);
- callback.expectStarted();
- mWiFiNetworkAgent.disconnect();
- waitFor(mWiFiNetworkAgent.getDisconnectedCV());
- callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectStarted();
+ mWiFiNetworkAgent.disconnect();
+ waitFor(mWiFiNetworkAgent.getDisconnectedCV());
+ callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
- // ... and that stopping it after that has no adverse effects.
- waitForIdle();
- final Network myNetAlias = myNet;
- assertNull(mCm.getNetworkCapabilities(myNetAlias));
- ka.stop();
- callback.assertNoCallback();
+ // ... and that stopping it after that has no adverse effects.
+ waitForIdle();
+ final Network myNetAlias = myNet;
+ assertNull(mCm.getNetworkCapabilities(myNetAlias));
+ ka.stop();
+ callback.assertNoCallback();
+ }
// Reconnect.
myNet = connectKeepaliveNetwork(lp);
@@ -4155,27 +4179,30 @@
// 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);
- ka.start(validKaInterval);
- callback.expectStarted();
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectStarted();
- // The second one gets slot 2.
- mWiFiNetworkAgent.setExpectedKeepaliveSlot(2);
- final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(6789);
- TestSocketKeepaliveCallback callback2 = new TestSocketKeepaliveCallback(executor);
- SocketKeepalive ka2 =
- mCm.createSocketKeepalive(myNet, testSocket2, myIPv4, dstIPv4, executor, callback2);
- ka2.start(validKaInterval);
- callback2.expectStarted();
+ // The second one gets slot 2.
+ mWiFiNetworkAgent.setExpectedKeepaliveSlot(2);
+ final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(6789);
+ TestSocketKeepaliveCallback callback2 = new TestSocketKeepaliveCallback(executor);
+ try (SocketKeepalive ka2 = mCm.createSocketKeepalive(
+ myNet, testSocket2, myIPv4, dstIPv4, executor, callback2)) {
+ ka2.start(validKaInterval);
+ callback2.expectStarted();
- ka.stop();
- callback.expectStopped();
+ ka.stop();
+ callback.expectStopped();
- ka2.stop();
- callback2.expectStopped();
+ ka2.stop();
+ callback2.expectStopped();
- testSocket.close();
- testSocket2.close();
+ testSocket.close();
+ testSocket2.close();
+ }
+ }
mWiFiNetworkAgent.disconnect();
waitFor(mWiFiNetworkAgent.getDisconnectedCV());
@@ -4200,7 +4227,6 @@
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 @@
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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_NETWORK);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ notMyNet, testSocketV4, executor, callback)) {
+ 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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocketV4, executor, callback)) {
+ 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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocketV6, executor, callback)) {
+ 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);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocketV4, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+ }
// Invalid Socket (socket is bound with IPv6 address).
- ka = mCm.createSocketKeepalive(myNet, testSocketV6, executor, callback);
- ka.start(validKaInterval);
- callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+ try (SocketKeepalive ka = mCm.createSocketKeepalive(
+ myNet, testSocketV6, executor, callback)) {
+ ka.start(validKaInterval);
+ callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
+ }
testSocketV4.close();
testSocketV6.close();
@@ -4256,6 +4291,66 @@
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();