From 17a45f4d6e846631b46e4e14555c45ad896c24d2 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Fri, 26 Apr 2019 01:22:29 +0000 Subject: [PATCH 1/2] Revert "[KA11] Verify fd ownership and allocate resource for NattKeepalive" This reverts commit cecd4a3d64a0fc6006c133ab2a35be44f5d6fb4b. 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: If6d537a39595cf132d3ed81d4eaac6700f5f0ab3 --- .../android/server/ConnectivityService.java | 2 - .../server/connectivity/KeepaliveTracker.java | 117 ++++-------------- .../server/ConnectivityServiceTest.java | 23 ---- 3 files changed, 27 insertions(+), 115 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 63fd2fda1ce6f..d22a5d27cc898 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -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); } diff --git a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java index bde430cc297f2..77a18e2b3ddc5 100644 --- a/services/core/java/com/android/server/connectivity/KeepaliveTracker.java +++ b/services/core/java/com/android/server/connectivity/KeepaliveTracker.java @@ -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 @@ -172,7 +158,7 @@ public class KeepaliveTracker { try { if (fd != null) { mFd = Os.dup(fd); - } else { + } else { Log.d(TAG, toString() + " calls with null fd"); if (!mPrivileged) { throw new SecurityException( @@ -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 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(); @@ -620,7 +538,6 @@ public class KeepaliveTracker { **/ public void startNattKeepalive(@Nullable NetworkAgentInfo nai, @Nullable FileDescriptor fd, - int encapSocketResourceId, int intervalSeconds, @NonNull ISocketKeepaliveCallback cb, @NonNull String srcAddrString, @@ -652,7 +569,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); @@ -692,7 +609,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); @@ -711,12 +628,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 { @@ -727,8 +649,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) { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 37af461b13056..c5bfc4b0ff0c2 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -4266,25 +4266,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. @@ -4348,10 +4329,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. From 4e645f30a9103487e4807f5eb36db9d1098fcf60 Mon Sep 17 00:00:00 2001 From: Junyu Lai Date: Fri, 26 Apr 2019 01:38:04 +0000 Subject: [PATCH 2/2] Revert "Add NATT keepalive resources and methods into IpSecService" This reverts commit 2445227fa803e66ad39693057625aae72db454ed. 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 --- core/java/android/net/IIpSecService.aidl | 4 - .../java/com/android/server/IpSecService.java | 163 ++---------------- .../com/android/server/IpSecServiceTest.java | 96 ----------- 3 files changed, 10 insertions(+), 253 deletions(-) diff --git a/core/java/android/net/IIpSecService.aidl b/core/java/android/net/IIpSecService.aidl index b5a2a16c7ff53..d6774d47b49e8 100644 --- a/core/java/android/net/IIpSecService.aidl +++ b/core/java/android/net/IIpSecService.aidl @@ -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); } diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index c1f52552eaf73..b4e1c32f25359 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -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. - * - *

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 implements IBinder.DeathRecipient { - - @NonNull private final T mResource; - @NonNull private final List mChildren; + private final T mResource; + private final List 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); - } + 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.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 mTunnelInterfaceRecords = new RefcountedResourceArray<>(TunnelInterfaceRecord.class.getSimpleName()); - final RefcountedResourceArray 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 - * - *

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 - * - *

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 - * - *

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 refcountedSocketRecord = - userRecord.mEncapSocketRecords.getRefcountedResourceOrThrow(encapSocketResourceId); - - // Build NattKeepaliveRecord - final int resourceId = mNextResourceId++; - userRecord.mNattKeepaliveRecords.put( - resourceId, - new RefcountedResource( - 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); diff --git a/tests/net/java/com/android/server/IpSecServiceTest.java b/tests/net/java/com/android/server/IpSecServiceTest.java index 6b5a2203ce74d..4a35015044ff2 100644 --- a/tests/net/java/com/android/server/IpSecServiceTest.java +++ b/tests/net/java/com/android/server/IpSecServiceTest.java @@ -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); - } }