Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e5ee6b9b authored by Junyu Lai's avatar Junyu Lai Committed by Gerrit Code Review
Browse files

Merge changes I0baf582f,I4bba01ba 

* changes:
  Block unpriviledged apps which create keepalives with null fd
  Fix keepalive don't get removed when lower layer error
parents 6a753ba7 15178155

core/java/android/net/ConnectivityManager.java

+3 −0
Original line number Diff line number Diff line
@@ -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

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

+14 −5
Original line number Diff line number Diff line
@@ -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) {
@@ -480,7 +487,6 @@ public class KeepaliveTracker { 
            }

        } else {

            // Keepalive successfully stopped, or error.

            ki.mStartedState = KeepaliveInfo.NOT_STARTED;

            if (reason == SUCCESS) {

                // The message indicated success stopping : don't call handleStopKeepalive.

                if (DBG) Log.d(TAG, "Successfully stopped keepalive " + slot + " on " + nai.name());
@@ -490,6 +496,7 @@ public class KeepaliveTracker { 
                handleStopKeepalive(nai, slot, reason);

                if (DBG) Log.d(TAG, "Keepalive " + slot + " on " + nai.name() + " error " + reason);

            }

            ki.mStartedState = KeepaliveInfo.NOT_STARTED;

        }

    }
@@ -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;

        }

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

+11 −6
Original line number Diff line number Diff line
@@ -4066,8 +4066,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");
@@ -4078,7 +4076,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");
@@ -4198,6 +4197,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);
@@ -4205,7 +4205,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)) {
@@ -4223,6 +4224,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;
@@ -4305,7 +4310,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");
@@ -4324,7 +4328,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();