From d21105064ef09b72657f2112ebb10e07862776c8 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 16 Nov 2017 15:27:22 -0800 Subject: [PATCH] Convert IpSecService resources to use refcounting This is part 2 of 2 of the refcounting refactor for IpSecService resources. Switched ManagedResources to use RefcountedResource structure for managing reference counts and eventual cleanup. Further, resource arrays and quota management have been aggregated into a UserRecord for better isolation. UID access checking has been similarly moved into the UserRecordTracker, and resourceId checking has been rolled into RefcountedResourceArray's accessor methods. Bug: 63409385 Test: CTS, all unit tests run on aosp_marlin-eng, new tests added Change-Id: Iee52dd1c9d2583bb6bfaf65be87569e9d50a5b63 --- .../server/IpSecServiceParameterizedTest.java | 107 +++++++++++++++++- .../com/android/server/IpSecServiceTest.java | 34 +++++- 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index 5c031eb113..20a48971dd 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -22,7 +22,6 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -39,6 +38,7 @@ import android.net.NetworkUtils; import android.os.Binder; import android.os.ParcelFileDescriptor; import android.support.test.filters.SmallTest; +import android.system.Os; import java.net.Socket; import java.util.Arrays; @@ -154,6 +154,56 @@ public class IpSecServiceParameterizedTest { anyString(), anyString(), eq(TEST_SPI_OUT)); + + // Verify quota and RefcountedResource objects cleaned up + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + assertEquals(0, userRecord.mSpiQuotaTracker.mCurrent); + try { + userRecord.mSpiRecords.getRefcountedResourceOrThrow(spiResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + + } + } + + @Test + public void testSecurityParameterIndexBinderDeath() throws Exception { + when(mMockNetd.ipSecAllocateSpi( + anyInt(), + eq(IpSecTransform.DIRECTION_OUT), + anyString(), + eq(mRemoteAddr), + eq(TEST_SPI_OUT))) + .thenReturn(TEST_SPI_OUT); + + IpSecSpiResponse spiResp = + mIpSecService.reserveSecurityParameterIndex( + IpSecTransform.DIRECTION_OUT, mRemoteAddr, TEST_SPI_OUT, new Binder()); + + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.RefcountedResource refcountedRecord = + userRecord.mSpiRecords.getRefcountedResourceOrThrow(spiResp.resourceId); + + refcountedRecord.binderDied(); + + verify(mMockNetd) + .ipSecDeleteSecurityAssociation( + eq(spiResp.resourceId), + anyInt(), + anyString(), + anyString(), + eq(TEST_SPI_OUT)); + + // Verify quota and RefcountedResource objects cleaned up + assertEquals(0, userRecord.mSpiQuotaTracker.mCurrent); + try { + userRecord.mSpiRecords.getRefcountedResourceOrThrow(spiResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + + } } private int getNewSpiResourceId(int direction, String remoteAddress, int returnSpi) @@ -379,6 +429,61 @@ public class IpSecServiceParameterizedTest { anyString(), anyString(), eq(TEST_SPI_IN)); + + // Verify quota and RefcountedResource objects cleaned up + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + assertEquals(0, userRecord.mTransformQuotaTracker.mCurrent); + try { + userRecord.mTransformRecords.getRefcountedResourceOrThrow( + createTransformResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + + } + } + + @Test + public void testTransportModeTransformBinderDeath() throws Exception { + IpSecConfig ipSecConfig = new IpSecConfig(); + addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); + addAuthAndCryptToIpSecConfig(ipSecConfig); + + IpSecTransformResponse createTransformResp = + mIpSecService.createTransportModeTransform(ipSecConfig, new Binder()); + + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.RefcountedResource refcountedRecord = + userRecord.mTransformRecords.getRefcountedResourceOrThrow( + createTransformResp.resourceId); + + refcountedRecord.binderDied(); + + verify(mMockNetd) + .ipSecDeleteSecurityAssociation( + eq(createTransformResp.resourceId), + eq(IpSecTransform.DIRECTION_OUT), + anyString(), + anyString(), + eq(TEST_SPI_OUT)); + verify(mMockNetd) + .ipSecDeleteSecurityAssociation( + eq(createTransformResp.resourceId), + eq(IpSecTransform.DIRECTION_IN), + anyString(), + anyString(), + eq(TEST_SPI_IN)); + + // Verify quota and RefcountedResource objects cleaned up + assertEquals(0, userRecord.mTransformQuotaTracker.mCurrent); + try { + userRecord.mTransformRecords.getRefcountedResourceOrThrow( + createTransformResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + + } } @Test diff --git a/tests/net/java/com/android/server/IpSecServiceTest.java b/tests/net/java/com/android/server/IpSecServiceTest.java index 0720886f88..354ad2a148 100644 --- a/tests/net/java/com/android/server/IpSecServiceTest.java +++ b/tests/net/java/com/android/server/IpSecServiceTest.java @@ -131,7 +131,39 @@ public class IpSecServiceTest { mIpSecService.closeUdpEncapsulationSocket(udpEncapResp.resourceId); udpEncapResp.fileDescriptor.close(); - // TODO: Added check for the resource tracker + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + assertEquals(0, userRecord.mSocketQuotaTracker.mCurrent); + try { + userRecord.mEncapSocketRecords.getRefcountedResourceOrThrow(udpEncapResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + + } + } + + @Test + public void testUdpEncapsulationSocketBinderDeath() throws Exception { + int localport = findUnusedPort(); + + IpSecUdpEncapResponse udpEncapResp = + mIpSecService.openUdpEncapsulationSocket(localport, new Binder()); + + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.RefcountedResource refcountedRecord = + userRecord.mEncapSocketRecords.getRefcountedResourceOrThrow( + udpEncapResp.resourceId); + + refcountedRecord.binderDied(); + + assertEquals(0, userRecord.mSocketQuotaTracker.mCurrent); + try { + userRecord.mEncapSocketRecords.getRefcountedResourceOrThrow(udpEncapResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + + } } @Test