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

Commit 4e645f30 authored by Junyu Lai's avatar Junyu Lai Committed by junyulai
Browse files

Revert "Add NATT keepalive resources and methods into IpSecService"

This reverts commit 2445227f.

Reason for revert: Adds dependency between IpSecService and
                   ConnectivityService may lead to future deadlock
		   problems. Uses a simpler approach instead,
		   hence the solution is not needed.
		   See aosp/954040.

Change-Id: Ibff278a6eee666cd85dba81c2bed94d568679b02
parent 17a45f4d
Loading
Loading
Loading
Loading
+0 −4
Original line number Diff line number Diff line
@@ -72,8 +72,4 @@ interface IIpSecService
            int tunnelResourceId, int direction, int transformResourceId, in String callingPackage);

    void removeTransportModeTransforms(in ParcelFileDescriptor socket);

    int lockEncapSocketForNattKeepalive(int encapSocketResourceId, int requesterUid);

    void releaseNattKeepalive(int nattKeepaliveResourceId, int ownerUid);
}
+10 −153
Original line number Diff line number Diff line
@@ -28,7 +28,6 @@ import static android.system.OsConstants.SOCK_DGRAM;
import static com.android.internal.util.Preconditions.checkNotNull;

import android.annotation.NonNull;
import android.annotation.Nullable;
import android.app.AppOpsManager;
import android.content.Context;
import android.content.pm.PackageManager;
@@ -44,7 +43,6 @@ import android.net.IpSecTunnelInterfaceResponse;
import android.net.IpSecUdpEncapResponse;
import android.net.LinkAddress;
import android.net.Network;
import android.net.NetworkStack;
import android.net.NetworkUtils;
import android.net.TrafficStats;
import android.net.util.NetdService;
@@ -77,8 +75,6 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

/**
@@ -179,14 +175,6 @@ public class IpSecService extends IIpSecService.Stub {
        void freeUnderlyingResources() throws RemoteException;
    }

    /**
     * Sentinel value placeholder for a real binder in RefcountedResources.
     *
     * <p>Used for cases where there the allocating party is a system service, and thus is expected
     * to track the resource lifecycles instead of IpSecService.
     */
    private static final Binder DUMMY_BINDER = new Binder();

    /**
     * RefcountedResource manages references and dependencies in an exclusively acyclic graph.
     *
@@ -201,42 +189,24 @@ public class IpSecService extends IIpSecService.Stub {
     */
    @VisibleForTesting
    public class RefcountedResource<T extends IResource> implements IBinder.DeathRecipient {

        @NonNull private final T mResource;
        @NonNull private final List<RefcountedResource> mChildren;
        private final T mResource;
        private final List<RefcountedResource> mChildren;
        int mRefCount = 1; // starts at 1 for user's reference.
        IBinder mBinder;

        /*
         * This field can take one of three states:
         * 1. null, when the object has been released by the user (or the user's binder dies)
         * 2. DUMMY_BINDER, when the refcounted resource is allocated from another system service
         *     and the other system service manages the lifecycle of the resource instead of
         *     IpSecService.
         * 3. an actual binder, to ensure IpSecService can cleanup after users that forget to close
         *     their resources.
         */
        @Nullable IBinder mBinder;

        RefcountedResource(@NonNull T resource, @NonNull RefcountedResource... children) {
            this(resource, DUMMY_BINDER, children);
        }

        RefcountedResource(
                @NonNull T resource,
                @NonNull IBinder binder,
                @NonNull RefcountedResource... children) {
        RefcountedResource(T resource, IBinder binder, RefcountedResource... children) {
            synchronized (IpSecService.this) {
                this.mResource = resource;
                this.mChildren = new ArrayList<>(children.length);
                this.mBinder = binder;
                this.mChildren = Collections.unmodifiableList(Arrays.asList(children));

                for (RefcountedResource child : children) {
                    mChildren.add(child);
                    child.mRefCount++;
                }

                try {
                    if (mBinder != DUMMY_BINDER) {
                    mBinder.linkToDeath(this, 0);
                    }
                } catch (RemoteException e) {
                    binderDied();
                    e.rethrowFromSystemServer();
@@ -283,12 +253,11 @@ public class IpSecService extends IIpSecService.Stub {
                return;
            }

            if (mBinder != DUMMY_BINDER) {
            mBinder.unlinkToDeath(this, 0);
            }
            mBinder = null;

            mResource.invalidate();

            releaseReference();
        }

@@ -413,8 +382,6 @@ public class IpSecService extends IIpSecService.Stub {
                new RefcountedResourceArray<>(EncapSocketRecord.class.getSimpleName());
        final RefcountedResourceArray<TunnelInterfaceRecord> mTunnelInterfaceRecords =
                new RefcountedResourceArray<>(TunnelInterfaceRecord.class.getSimpleName());
        final RefcountedResourceArray<NattKeepaliveRecord> mNattKeepaliveRecords =
                new RefcountedResourceArray<>(NattKeepaliveRecord.class.getSimpleName());

        /**
         * Trackers for quotas for each of the OwnedResource types.
@@ -428,8 +395,6 @@ public class IpSecService extends IIpSecService.Stub {
        final ResourceTracker mSpiQuotaTracker = new ResourceTracker(MAX_NUM_SPIS);
        final ResourceTracker mTransformQuotaTracker = new ResourceTracker(MAX_NUM_TRANSFORMS);
        final ResourceTracker mSocketQuotaTracker = new ResourceTracker(MAX_NUM_ENCAP_SOCKETS);
        final ResourceTracker mNattKeepaliveQuotaTracker =
                new ResourceTracker(MAX_NUM_ENCAP_SOCKETS); // Max 1 NATT keepalive per encap socket
        final ResourceTracker mTunnelQuotaTracker = new ResourceTracker(MAX_NUM_TUNNEL_INTERFACES);

        void removeSpiRecord(int resourceId) {
@@ -448,10 +413,6 @@ public class IpSecService extends IIpSecService.Stub {
            mEncapSocketRecords.remove(resourceId);
        }

        void removeNattKeepaliveRecord(int resourceId) {
            mNattKeepaliveRecords.remove(resourceId);
        }

        @Override
        public String toString() {
            return new StringBuilder()
@@ -461,8 +422,6 @@ public class IpSecService extends IIpSecService.Stub {
                    .append(mTransformQuotaTracker)
                    .append(", mSocketQuotaTracker=")
                    .append(mSocketQuotaTracker)
                    .append(", mNattKeepaliveQuotaTracker=")
                    .append(mNattKeepaliveQuotaTracker)
                    .append(", mTunnelQuotaTracker=")
                    .append(mTunnelQuotaTracker)
                    .append(", mSpiRecords=")
@@ -471,8 +430,6 @@ public class IpSecService extends IIpSecService.Stub {
                    .append(mTransformRecords)
                    .append(", mEncapSocketRecords=")
                    .append(mEncapSocketRecords)
                    .append(", mNattKeepaliveRecords=")
                    .append(mNattKeepaliveRecords)
                    .append(", mTunnelInterfaceRecords=")
                    .append(mTunnelInterfaceRecords)
                    .append("}")
@@ -617,11 +574,6 @@ public class IpSecService extends IIpSecService.Stub {
            mArray.remove(key);
        }

        @VisibleForTesting
        int size() {
            return mArray.size();
        }

        @Override
        public String toString() {
            return mArray.toString();
@@ -1032,50 +984,6 @@ public class IpSecService extends IIpSecService.Stub {
        }
    }

    /**
     * Tracks a NATT-keepalive instance
     *
     * <p>This class ensures that while a NATT-keepalive is active, the UDP encap socket that it is
     * supporting will stay open until the NATT-keepalive is finished. NATT-keepalive offload
     * lifecycles will be managed by ConnectivityService, which will validate that the UDP Encap
     * socket is owned by the requester, and take a reference to it via this NattKeepaliveRecord
     *
     * <p>It shall be the responsibility of the caller to ensure that instances of an EncapSocket do
     * not spawn multiple instances of NATT keepalives (and thereby register duplicate records)
     */
    private final class NattKeepaliveRecord extends OwnedResourceRecord {
        NattKeepaliveRecord(int resourceId) {
            super(resourceId);
        }

        @Override
        @GuardedBy("IpSecService.this")
        public void freeUnderlyingResources() {
            Log.d(TAG, "Natt Keepalive released: " + mResourceId);

            getResourceTracker().give();
        }

        @Override
        protected ResourceTracker getResourceTracker() {
            return getUserRecord().mNattKeepaliveQuotaTracker;
        }

        @Override
        public void invalidate() {
            getUserRecord().removeNattKeepaliveRecord(mResourceId);
        }

        @Override
        public String toString() {
            return new StringBuilder()
                    .append("{super=")
                    .append(super.toString())
                    .append("}")
                    .toString();
        }
    }

    /**
     * Constructs a new IpSecService instance
     *
@@ -1911,57 +1819,6 @@ public class IpSecService extends IIpSecService.Stub {
        }
    }

    private void verifyNetworkStackCaller() {
        if (mContext.checkCallingOrSelfPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
                        != PackageManager.PERMISSION_GRANTED
                && mContext.checkCallingOrSelfPermission(android.Manifest.permission.NETWORK_STACK)
                        != PackageManager.PERMISSION_GRANTED) {
            throw new SecurityException(
                    "Requires permission NETWORK_STACK or MAINLINE_NETWORK_STACK");
        }
    }

    /**
     * Validates that a provided UID owns the encapSocket, and creates a NATT keepalive record
     *
     * <p>For system server use only. Caller must have NETWORK_STACK permission
     *
     * @param encapSocketResourceId resource identifier of the encap socket record
     * @param ownerUid the UID of the caller. Used to verify ownership.
     * @return
     */
    public synchronized int lockEncapSocketForNattKeepalive(
            int encapSocketResourceId, int ownerUid) {
        verifyNetworkStackCaller();

        // Verify ownership. Will throw IllegalArgumentException if the UID specified does not
        // own the specified UDP encapsulation socket
        UserRecord userRecord = mUserResourceTracker.getUserRecord(ownerUid);
        RefcountedResource<EncapSocketRecord> refcountedSocketRecord =
                userRecord.mEncapSocketRecords.getRefcountedResourceOrThrow(encapSocketResourceId);

        // Build NattKeepaliveRecord
        final int resourceId = mNextResourceId++;
        userRecord.mNattKeepaliveRecords.put(
                resourceId,
                new RefcountedResource<NattKeepaliveRecord>(
                        new NattKeepaliveRecord(resourceId), refcountedSocketRecord));

        return resourceId;
    }

    /**
     * Release a previously allocated NattKeepalive that has been registered with the system server
     */
    @Override
    public synchronized void releaseNattKeepalive(int nattKeepaliveResourceId, int ownerUid)
            throws RemoteException {
        verifyNetworkStackCaller();

        UserRecord userRecord = mUserResourceTracker.getUserRecord(ownerUid);
        releaseResource(userRecord.mNattKeepaliveRecords, nattKeepaliveResourceId);
    }

    @Override
    protected synchronized void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        mContext.enforceCallingOrSelfPermission(DUMP, TAG);
+0 −96
Original line number Diff line number Diff line
@@ -118,7 +118,6 @@ public class IpSecServiceTest {
    INetd mMockNetd;
    IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig;
    IpSecService mIpSecService;
    int mUid = Os.getuid();

    @Before
    public void setUp() throws Exception {
@@ -666,99 +665,4 @@ public class IpSecServiceTest {
        mIpSecService.releaseNetId(releasedNetId);
        assertEquals(releasedNetId, mIpSecService.reserveNetId());
    }

    @Test
    public void testLockEncapSocketForNattKeepalive() throws Exception {
        IpSecUdpEncapResponse udpEncapResp =
                mIpSecService.openUdpEncapsulationSocket(0, new Binder());
        assertNotNull(udpEncapResp);
        assertEquals(IpSecManager.Status.OK, udpEncapResp.status);

        // Verify no NATT keepalive records upon startup
        IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid);
        assertEquals(0, userRecord.mNattKeepaliveRecords.size());

        int nattKeepaliveResourceId =
                mIpSecService.lockEncapSocketForNattKeepalive(udpEncapResp.resourceId, mUid);

        // Validate response, and record was added
        assertNotEquals(IpSecManager.INVALID_RESOURCE_ID, nattKeepaliveResourceId);
        assertEquals(1, userRecord.mNattKeepaliveRecords.size());

        // Validate keepalive can be released and removed.
        mIpSecService.releaseNattKeepalive(nattKeepaliveResourceId, mUid);
        assertEquals(0, userRecord.mNattKeepaliveRecords.size());

        mIpSecService.closeUdpEncapsulationSocket(udpEncapResp.resourceId);
    }

    @Test
    public void testLockEncapSocketForNattKeepaliveInvalidUid() throws Exception {
        IpSecUdpEncapResponse udpEncapResp =
                mIpSecService.openUdpEncapsulationSocket(0, new Binder());
        assertNotNull(udpEncapResp);
        assertEquals(IpSecManager.Status.OK, udpEncapResp.status);

        // Verify no NATT keepalive records upon startup
        IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid);
        assertEquals(0, userRecord.mNattKeepaliveRecords.size());

        try {
            int nattKeepaliveResourceId =
                    mIpSecService.lockEncapSocketForNattKeepalive(
                            udpEncapResp.resourceId, mUid + 1);
            fail("Expected SecurityException for invalid user");
        } catch (SecurityException expected) {
        }

        // Validate keepalive was not added to lists
        assertEquals(0, userRecord.mNattKeepaliveRecords.size());
    }

    @Test
    public void testLockEncapSocketForNattKeepaliveInvalidResourceId() throws Exception {
        // Verify no NATT keepalive records upon startup
        IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid);
        assertEquals(0, userRecord.mNattKeepaliveRecords.size());

        try {
            int nattKeepaliveResourceId =
                    mIpSecService.lockEncapSocketForNattKeepalive(12345, mUid);
            fail("Expected IllegalArgumentException for invalid resource ID");
        } catch (IllegalArgumentException expected) {
        }

        // Validate keepalive was not added to lists
        assertEquals(0, userRecord.mNattKeepaliveRecords.size());
    }

    @Test
    public void testEncapSocketReleasedBeforeKeepaliveReleased() throws Exception {
        IpSecUdpEncapResponse udpEncapResp =
                mIpSecService.openUdpEncapsulationSocket(0, new Binder());
        assertNotNull(udpEncapResp);
        assertEquals(IpSecManager.Status.OK, udpEncapResp.status);

        // Get encap socket record, verify initial starting refcount.
        IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(mUid);
        IpSecService.RefcountedResource encapSocketRefcountedRecord =
                userRecord.mEncapSocketRecords.getRefcountedResourceOrThrow(
                        udpEncapResp.resourceId);
        assertEquals(1, encapSocketRefcountedRecord.mRefCount);

        // Verify that the reference was added
        int nattKeepaliveResourceId =
                mIpSecService.lockEncapSocketForNattKeepalive(udpEncapResp.resourceId, mUid);
        assertNotEquals(IpSecManager.INVALID_RESOURCE_ID, nattKeepaliveResourceId);
        assertEquals(2, encapSocketRefcountedRecord.mRefCount);

        // Close UDP encap socket, but expect the refcountedRecord to still have a reference.
        mIpSecService.closeUdpEncapsulationSocket(udpEncapResp.resourceId);
        assertEquals(1, encapSocketRefcountedRecord.mRefCount);

        // Verify UDP encap socket cleaned up once reference is removed. Expect -1 if cleanup
        // was properly completed.
        mIpSecService.releaseNattKeepalive(nattKeepaliveResourceId, mUid);
        assertEquals(-1, encapSocketRefcountedRecord.mRefCount);
    }
}