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

Commit ecc6fad6 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge changes Ibff278a6,If6d537a3

* changes:
  Revert "Add NATT keepalive resources and methods into IpSecService"
  Revert "[KA11] Verify fd ownership and allocate resource for NattKeepalive"
parents be49b65d 4e645f30
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);
}
+0 −2
Original line number Diff line number Diff line
@@ -27,7 +27,6 @@ import static android.net.ConnectivityManager.getNetworkTypeName;
import static android.net.ConnectivityManager.isNetworkTypeValid;
import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_PARTIAL_CONNECTIVITY;
import static android.net.INetworkMonitor.NETWORK_TEST_RESULT_VALID;
import static android.net.IpSecManager.INVALID_RESOURCE_ID;
import static android.net.NetworkCapabilities.NET_CAPABILITY_CAPTIVE_PORTAL;
import static android.net.NetworkCapabilities.NET_CAPABILITY_FOREGROUND;
import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET;
@@ -6859,7 +6858,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
        enforceKeepalivePermission();
        mKeepaliveTracker.startNattKeepalive(
                getNetworkAgentInfoForNetwork(network), null /* fd */,
                INVALID_RESOURCE_ID /* Unused */,
                intervalSeconds, cb,
                srcAddr, srcPort, dstAddr, NattSocketKeepalive.NATT_PORT);
    }
+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);
+27 −90
Original line number Diff line number Diff line
@@ -17,7 +17,6 @@
package com.android.server.connectivity;

import static android.content.pm.PackageManager.PERMISSION_GRANTED;
import static android.net.IpSecManager.INVALID_RESOURCE_ID;
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;
@@ -38,7 +37,6 @@ import static android.net.SocketKeepalive.SUCCESS;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.content.Context;
import android.net.IIpSecService;
import android.net.ISocketKeepaliveCallback;
import android.net.KeepalivePacketData;
import android.net.NattKeepalivePacketData;
@@ -54,7 +52,6 @@ import android.os.IBinder;
import android.os.Message;
import android.os.Process;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.system.ErrnoException;
import android.system.Os;
import android.util.Log;
@@ -92,14 +89,11 @@ public class KeepaliveTracker {
    private final TcpKeepaliveController mTcpController;
    @NonNull
    private final Context mContext;
    @NonNull
    private final IIpSecService mIpSec;

    public KeepaliveTracker(Context context, Handler handler) {
        mConnectivityServiceHandler = handler;
        mTcpController = new TcpKeepaliveController(handler);
        mContext = context;
        mIpSec = IIpSecService.Stub.asInterface(ServiceManager.getService(Context.IPSEC_SERVICE));
    }

    /**
@@ -118,10 +112,6 @@ public class KeepaliveTracker {
        private final int mType;
        private final FileDescriptor mFd;

        private final int mEncapSocketResourceId;
        // Stores the NATT keepalive resource ID returned by IpSecService.
        private int mNattIpsecResourceId = INVALID_RESOURCE_ID;

        public static final int TYPE_NATT = 1;
        public static final int TYPE_TCP = 2;

@@ -150,8 +140,7 @@ public class KeepaliveTracker {
                @NonNull KeepalivePacketData packet,
                int interval,
                int type,
                @Nullable FileDescriptor fd,
                int encapSocketResourceId) throws InvalidSocketException {
                @Nullable FileDescriptor fd) throws InvalidSocketException {
            mCallback = callback;
            mPid = Binder.getCallingPid();
            mUid = Binder.getCallingUid();
@@ -162,9 +151,6 @@ public class KeepaliveTracker {
            mInterval = interval;
            mType = type;

            mEncapSocketResourceId = encapSocketResourceId;
            mNattIpsecResourceId = INVALID_RESOURCE_ID;

            // 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
@@ -220,8 +206,6 @@ public class KeepaliveTracker {
                    + IpUtils.addressAndPortToString(mPacket.dstAddress, mPacket.dstPort)
                    + " interval=" + mInterval
                    + " uid=" + mUid + " pid=" + mPid + " privileged=" + mPrivileged
                    + " nattIpsecRId=" + mNattIpsecResourceId
                    + " encapSocketRId=" + mEncapSocketResourceId
                    + " packetData=" + HexDump.toHexString(mPacket.getPacket())
                    + " ]";
        }
@@ -278,51 +262,6 @@ public class KeepaliveTracker {
            return SUCCESS;
        }

        private int checkAndLockNattKeepaliveResource() {
            // Check that apps should be either privileged or fill in an ipsec encapsulated socket
            // resource id.
            if (mEncapSocketResourceId == INVALID_RESOURCE_ID) {
                if (mPrivileged) {
                    return SUCCESS;
                } else {
                    // Invalid access.
                    return ERROR_INVALID_SOCKET;
                }
            }

            // Check if the ipsec encapsulated socket resource id is registered.
            final HashMap<Integer, KeepaliveInfo> networkKeepalives = mKeepalives.get(mNai);
            if (networkKeepalives == null) {
                return ERROR_INVALID_NETWORK;
            }
            for (KeepaliveInfo ki : networkKeepalives.values()) {
                if (ki.mEncapSocketResourceId == mEncapSocketResourceId
                        && ki.mNattIpsecResourceId != INVALID_RESOURCE_ID) {
                    Log.d(TAG, "Registered resource found on keepalive " + mSlot
                            + " when verify NATT socket with uid=" + mUid + " rid="
                            + mEncapSocketResourceId);
                    return ERROR_INVALID_SOCKET;
                }
            }

            // Ensure that the socket is created by IpSecService, and lock the resource that is
            // preserved by IpSecService. If succeed, a resource id is stored to keep tracking
            // the resource preserved by IpSecServce and must be released when stopping keepalive.
            try {
                mNattIpsecResourceId =
                        mIpSec.lockEncapSocketForNattKeepalive(mEncapSocketResourceId, mUid);
                return SUCCESS;
            } catch (IllegalArgumentException e) {
                // The UID specified does not own the specified UDP encapsulation socket.
                Log.d(TAG, "Failed to verify NATT socket with uid=" + mUid + " rid="
                        + mEncapSocketResourceId + ": " + e);
            } catch (RemoteException e) {
                Log.wtf(TAG, "Error calling lockEncapSocketForNattKeepalive with "
                        + this.toString(), e);
            }
            return ERROR_INVALID_SOCKET;
        }

        private int isValid() {
            synchronized (mNai) {
                int error = checkInterval();
@@ -336,13 +275,6 @@ public class KeepaliveTracker {
        void start(int slot) {
            mSlot = slot;
            int error = isValid();

            // Check and lock ipsec resource needed by natt kepalive. This should be only called
            // once per keepalive.
            if (error == SUCCESS && mType == TYPE_NATT) {
                error = checkAndLockNattKeepaliveResource();
            }

            if (error == SUCCESS) {
                Log.d(TAG, "Starting keepalive " + mSlot + " on " + mNai.name());
                switch (mType) {
@@ -418,20 +350,6 @@ public class KeepaliveTracker {
                }
            }

            // Release the resource held by keepalive in IpSecService.
            if (mNattIpsecResourceId != INVALID_RESOURCE_ID) {
                if (mType != TYPE_NATT) {
                    Log.wtf(TAG, "natt ipsec resource held by incorrect keepalive "
                            + this.toString());
                }
                try {
                    mIpSec.releaseNattKeepalive(mNattIpsecResourceId, mUid);
                } catch (RemoteException e) {
                    Log.wtf(TAG, "error calling releaseNattKeepalive with " + this.toString(), e);
                }
                mNattIpsecResourceId = INVALID_RESOURCE_ID;
            }

            if (reason == SUCCESS) {
                try {
                    mCallback.onStopped();
@@ -621,7 +539,6 @@ public class KeepaliveTracker {
     **/
    public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
            @Nullable FileDescriptor fd,
            int encapSocketResourceId,
            int intervalSeconds,
            @NonNull ISocketKeepaliveCallback cb,
            @NonNull String srcAddrString,
@@ -653,7 +570,7 @@ public class KeepaliveTracker {
        KeepaliveInfo ki = null;
        try {
            ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
                    KeepaliveInfo.TYPE_NATT, fd, encapSocketResourceId);
                    KeepaliveInfo.TYPE_NATT, fd);
        } catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
            Log.e(TAG, "Fail to construct keepalive", e);
            notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
@@ -693,7 +610,7 @@ public class KeepaliveTracker {
        KeepaliveInfo ki = null;
        try {
            ki = new KeepaliveInfo(cb, nai, packet, intervalSeconds,
                    KeepaliveInfo.TYPE_TCP, fd, INVALID_RESOURCE_ID /* Unused */);
                    KeepaliveInfo.TYPE_TCP, fd);
        } catch (InvalidSocketException | IllegalArgumentException | SecurityException e) {
            Log.e(TAG, "Fail to construct keepalive e=" + e);
            notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
@@ -712,12 +629,17 @@ public class KeepaliveTracker {
    **/
    public void startNattKeepalive(@Nullable NetworkAgentInfo nai,
            @Nullable FileDescriptor fd,
            int encapSocketResourceId,
            int resourceId,
            int intervalSeconds,
            @NonNull ISocketKeepaliveCallback cb,
            @NonNull String srcAddrString,
            @NonNull String dstAddrString,
            int dstPort) {
        // Ensure that the socket is created by IpSecService.
        if (!isNattKeepaliveSocketValid(fd, resourceId)) {
            notifyErrorCallback(cb, ERROR_INVALID_SOCKET);
        }

        // Get src port to adopt old API.
        int srcPort = 0;
        try {
@@ -728,8 +650,23 @@ public class KeepaliveTracker {
        }

        // Forward request to old API.
        startNattKeepalive(nai, fd, encapSocketResourceId, intervalSeconds, cb, srcAddrString,
                srcPort, dstAddrString, dstPort);
        startNattKeepalive(nai, fd, intervalSeconds, cb, srcAddrString, srcPort,
                dstAddrString, dstPort);
    }

    /**
     * Verify if the IPsec NAT-T file descriptor and resource Id hold for IPsec keepalive is valid.
     **/
    public static boolean isNattKeepaliveSocketValid(@Nullable FileDescriptor fd, int resourceId) {
        // TODO: 1. confirm whether the fd is called from system api or created by IpSecService.
        //       2. If the fd is created from the system api, check that it's bounded. And
        //          call dup to keep the fd open.
        //       3. If the fd is created from IpSecService, check if the resource ID is valid. And
        //          hold the resource needed in IpSecService.
        if (null == fd) {
            return false;
        }
        return true;
    }

    public void dump(IndentingPrintWriter pw) {
+0 −23
Original line number Diff line number Diff line
@@ -4269,25 +4269,6 @@ public class ConnectivityServiceTest {
            callback.expectStarted();
            ka.stop();
            callback.expectStopped();

            // Check that the same NATT socket cannot be used by 2 keepalives.
            try (SocketKeepalive ka2 = mCm.createSocketKeepalive(
                    myNet, testSocket, myIPv4, dstIPv4, executor, callback)) {
                // Check that second keepalive cannot be started if the first one is running.
                ka.start(validKaInterval);
                callback.expectStarted();
                ka2.start(validKaInterval);
                callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
                ka.stop();
                callback.expectStopped();

                // Check that second keepalive can be started/stopped normally if the first one is
                // stopped.
                ka2.start(validKaInterval);
                callback.expectStarted();
                ka2.stop();
                callback.expectStopped();
            }
        }

        // Check that deleting the IP address stops the keepalive.
@@ -4351,10 +4332,6 @@ public class ConnectivityServiceTest {
                testSocket.close();
                testSocket2.close();
            }

            // Check that the closed socket cannot be used to start keepalive.
            ka.start(validKaInterval);
            callback.expectError(SocketKeepalive.ERROR_INVALID_SOCKET);
        }

        // Check that there is no port leaked after all keepalives and sockets are closed.
Loading