diff options
3 files changed, 27 insertions, 10 deletions
diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index ae93cf019776..4a64128f146b 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -1934,6 +1934,8 @@ public class ConnectivityManager { @NonNull Callback callback) { ParcelFileDescriptor dup; try { + // Dup is needed here as the pfd inside the socket is owned by the IpSecService, + // which cannot be obtained by the app process. dup = ParcelFileDescriptor.dup(socket.getFileDescriptor()); } catch (IOException ignored) { // Construct an invalid fd, so that if the user later calls start(), it will fail with @@ -1975,6 +1977,7 @@ public class ConnectivityManager { @NonNull Callback callback) { ParcelFileDescriptor dup; try { + // TODO: Consider remove unnecessary dup. dup = pfd.dup(); } catch (IOException ignored) { // Construct an invalid fd, so that if the user later calls start(), it will fail with diff --git a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java index 722c1a0d9a9a..d7a57b992eef 100644 --- a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java +++ b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java @@ -154,12 +154,19 @@ public class KeepaliveTracker { // 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"); + Log.d(TAG, toString() + " calls with null fd"); + if (!mPrivileged) { + throw new SecurityException( + "null fd is not allowed for unprivileged access."); + } + if (mType == TYPE_TCP) { + throw new IllegalArgumentException( + "null fd is not allowed for tcp socket keepalives."); + } mFd = null; } } catch (ErrnoException e) { @@ -531,7 +538,8 @@ public class KeepaliveTracker { try { ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds, KeepaliveInfo.TYPE_NATT, fd); - } catch (InvalidSocketException e) { + } catch (InvalidSocketException | IllegalArgumentException | SecurityException e) { + Log.e(TAG, "Fail to construct keepalive", e); notifyErrorCallback(cb, ERROR_INVALID_SOCKET); return; } @@ -570,7 +578,8 @@ public class KeepaliveTracker { try { ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds, KeepaliveInfo.TYPE_TCP, fd); - } catch (InvalidSocketException e) { + } catch (InvalidSocketException | IllegalArgumentException | SecurityException e) { + Log.e(TAG, "Fail to construct keepalive e=" + e); notifyErrorCallback(cb, ERROR_INVALID_SOCKET); return; } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 92a865a3dc58..8740f079e16a 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4047,8 +4047,6 @@ public class ConnectivityServiceTest { // 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"); final InetAddress myIPv6 = InetAddress.getByName("2001:db8::1"); @@ -4059,7 +4057,8 @@ public class ConnectivityServiceTest { final int invalidKaInterval = 9; final IpSecManager mIpSec = (IpSecManager) mContext.getSystemService(Context.IPSEC_SERVICE); - final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket(srcPort); + final UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket(); + final int srcPort = testSocket.getPort(); LinkProperties lp = new LinkProperties(); lp.setInterfaceName("wlan12"); @@ -4179,6 +4178,7 @@ public class ConnectivityServiceTest { // Check that keepalive slots start from 1 and increment. The first one gets slot 1. mWiFiNetworkAgent.setExpectedKeepaliveSlot(1); + int srcPort2 = 0; try (SocketKeepalive ka = mCm.createSocketKeepalive( myNet, testSocket, myIPv4, dstIPv4, executor, callback)) { ka.start(validKaInterval); @@ -4186,7 +4186,8 @@ public class ConnectivityServiceTest { // The second one gets slot 2. mWiFiNetworkAgent.setExpectedKeepaliveSlot(2); - final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(6789); + final UdpEncapsulationSocket testSocket2 = mIpSec.openUdpEncapsulationSocket(); + srcPort2 = testSocket2.getPort(); TestSocketKeepaliveCallback callback2 = new TestSocketKeepaliveCallback(executor); try (SocketKeepalive ka2 = mCm.createSocketKeepalive( myNet, testSocket2, myIPv4, dstIPv4, executor, callback2)) { @@ -4204,6 +4205,10 @@ public class ConnectivityServiceTest { } } + // Check that there is no port leaked after all keepalives and sockets are closed. + assertFalse(isUdpPortInUse(srcPort)); + assertFalse(isUdpPortInUse(srcPort2)); + mWiFiNetworkAgent.disconnect(); waitFor(mWiFiNetworkAgent.getDisconnectedCV()); mWiFiNetworkAgent = null; @@ -4292,7 +4297,6 @@ public class ConnectivityServiceTest { } 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"); @@ -4311,7 +4315,8 @@ public class ConnectivityServiceTest { // 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 UdpEncapsulationSocket testSocket = mIpSec.openUdpEncapsulationSocket(); + final int srcPort = testSocket.getPort(); final ParcelFileDescriptor testPfd = ParcelFileDescriptor.dup(testSocket.getFileDescriptor()); testSocket.close(); |