From 76df78ffbfc4c3e94841af0a54a0ad253069e837 Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 1 Mar 2018 18:53:07 -0800 Subject: [PATCH 1/2] Added implementation for VTI add/remove address This change adds implementation details for add/remove addresses onto a VTI. Bug: 73675031 Test: New tests added, passing on Walleye Merged-In: Idde9d943a5285d2c13c5c6b0f7b8a9faf718e6a5 Change-Id: Idde9d943a5285d2c13c5c6b0f7b8a9faf718e6a5 (cherry picked from commit ecc9f7cc08804e3fa15fea04ae94ea1bc74edbfe) --- .../java/android/net/IpSecManagerTest.java | 68 +++++++++-- .../server/IpSecServiceParameterizedTest.java | 115 +++++++++++++++++- 2 files changed, 168 insertions(+), 15 deletions(-) diff --git a/tests/net/java/android/net/IpSecManagerTest.java b/tests/net/java/android/net/IpSecManagerTest.java index cc3366fbc8..0ca20dee42 100644 --- a/tests/net/java/android/net/IpSecManagerTest.java +++ b/tests/net/java/android/net/IpSecManagerTest.java @@ -50,13 +50,18 @@ public class IpSecManagerTest { private static final int TEST_UDP_ENCAP_PORT = 34567; private static final int DROID_SPI = 0xD1201D; + private static final int DUMMY_RESOURCE_ID = 0x1234; private static final InetAddress GOOGLE_DNS_4; + private static final String VTI_INTF_NAME = "ipsec_test"; + private static final InetAddress VTI_LOCAL_ADDRESS; + private static final LinkAddress VTI_INNER_ADDRESS = new LinkAddress("10.0.1.1/24"); static { try { // Google Public DNS Addresses; GOOGLE_DNS_4 = InetAddress.getByName("8.8.8.8"); + VTI_LOCAL_ADDRESS = InetAddress.getByName("8.8.4.4"); } catch (UnknownHostException e) { throw new RuntimeException("Could not resolve DNS Addresses", e); } @@ -77,9 +82,8 @@ public class IpSecManagerTest { */ @Test public void testAllocSpi() throws Exception { - int resourceId = 1; IpSecSpiResponse spiResp = - new IpSecSpiResponse(IpSecManager.Status.OK, resourceId, DROID_SPI); + new IpSecSpiResponse(IpSecManager.Status.OK, DUMMY_RESOURCE_ID, DROID_SPI); when(mMockIpSecService.allocateSecurityParameterIndex( eq(GOOGLE_DNS_4.getHostAddress()), eq(DROID_SPI), @@ -92,14 +96,13 @@ public class IpSecManagerTest { droidSpi.close(); - verify(mMockIpSecService).releaseSecurityParameterIndex(resourceId); + verify(mMockIpSecService).releaseSecurityParameterIndex(DUMMY_RESOURCE_ID); } @Test public void testAllocRandomSpi() throws Exception { - int resourceId = 1; IpSecSpiResponse spiResp = - new IpSecSpiResponse(IpSecManager.Status.OK, resourceId, DROID_SPI); + new IpSecSpiResponse(IpSecManager.Status.OK, DUMMY_RESOURCE_ID, DROID_SPI); when(mMockIpSecService.allocateSecurityParameterIndex( eq(GOOGLE_DNS_4.getHostAddress()), eq(IpSecManager.INVALID_SECURITY_PARAMETER_INDEX), @@ -113,7 +116,7 @@ public class IpSecManagerTest { randomSpi.close(); - verify(mMockIpSecService).releaseSecurityParameterIndex(resourceId); + verify(mMockIpSecService).releaseSecurityParameterIndex(DUMMY_RESOURCE_ID); } /* @@ -165,11 +168,10 @@ public class IpSecManagerTest { @Test public void testOpenEncapsulationSocket() throws Exception { - int resourceId = 1; IpSecUdpEncapResponse udpEncapResp = new IpSecUdpEncapResponse( IpSecManager.Status.OK, - resourceId, + DUMMY_RESOURCE_ID, TEST_UDP_ENCAP_PORT, Os.socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP)); when(mMockIpSecService.openUdpEncapsulationSocket(eq(TEST_UDP_ENCAP_PORT), anyObject())) @@ -182,16 +184,15 @@ public class IpSecManagerTest { encapSocket.close(); - verify(mMockIpSecService).closeUdpEncapsulationSocket(resourceId); + verify(mMockIpSecService).closeUdpEncapsulationSocket(DUMMY_RESOURCE_ID); } @Test public void testOpenEncapsulationSocketOnRandomPort() throws Exception { - int resourceId = 1; IpSecUdpEncapResponse udpEncapResp = new IpSecUdpEncapResponse( IpSecManager.Status.OK, - resourceId, + DUMMY_RESOURCE_ID, TEST_UDP_ENCAP_PORT, Os.socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP)); @@ -206,7 +207,7 @@ public class IpSecManagerTest { encapSocket.close(); - verify(mMockIpSecService).closeUdpEncapsulationSocket(resourceId); + verify(mMockIpSecService).closeUdpEncapsulationSocket(DUMMY_RESOURCE_ID); } @Test @@ -219,4 +220,45 @@ public class IpSecManagerTest { } // TODO: add test when applicable transform builder interface is available -} + + private IpSecManager.IpSecTunnelInterface createAndValidateVti(int resourceId, String intfName) + throws Exception { + IpSecTunnelInterfaceResponse dummyResponse = + new IpSecTunnelInterfaceResponse(IpSecManager.Status.OK, resourceId, intfName); + when(mMockIpSecService.createTunnelInterface( + eq(VTI_LOCAL_ADDRESS.getHostAddress()), eq(GOOGLE_DNS_4.getHostAddress()), + anyObject(), anyObject())) + .thenReturn(dummyResponse); + + IpSecManager.IpSecTunnelInterface tunnelIntf = mIpSecManager.createIpSecTunnelInterface( + VTI_LOCAL_ADDRESS, GOOGLE_DNS_4, mock(Network.class)); + + assertNotNull(tunnelIntf); + return tunnelIntf; + } + + @Test + public void testCreateVti() throws Exception { + IpSecManager.IpSecTunnelInterface tunnelIntf = + createAndValidateVti(DUMMY_RESOURCE_ID, VTI_INTF_NAME); + + assertEquals(VTI_INTF_NAME, tunnelIntf.getInterfaceName()); + + tunnelIntf.close(); + verify(mMockIpSecService).deleteTunnelInterface(eq(DUMMY_RESOURCE_ID)); + } + + @Test + public void testAddRemoveAddressesFromVti() throws Exception { + IpSecManager.IpSecTunnelInterface tunnelIntf = + createAndValidateVti(DUMMY_RESOURCE_ID, VTI_INTF_NAME); + + tunnelIntf.addAddress(VTI_INNER_ADDRESS); + verify(mMockIpSecService) + .addAddressToTunnelInterface(eq(DUMMY_RESOURCE_ID), eq(VTI_INNER_ADDRESS)); + + tunnelIntf.removeAddress(VTI_INNER_ADDRESS); + verify(mMockIpSecService) + .addAddressToTunnelInterface(eq(DUMMY_RESOURCE_ID), eq(VTI_INNER_ADDRESS)); + } +} \ No newline at end of file diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index 3e1ff6dd5f..a5c55e8d84 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -17,6 +17,7 @@ package com.android.server; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; @@ -32,6 +33,9 @@ import android.net.IpSecConfig; import android.net.IpSecManager; import android.net.IpSecSpiResponse; import android.net.IpSecTransformResponse; +import android.net.IpSecTunnelInterfaceResponse; +import android.net.LinkAddress; +import android.net.Network; import android.net.NetworkUtils; import android.os.Binder; import android.os.ParcelFileDescriptor; @@ -56,10 +60,15 @@ public class IpSecServiceParameterizedTest { private final String mDestinationAddr; private final String mSourceAddr; + private final LinkAddress mLocalInnerAddress; @Parameterized.Parameters public static Collection ipSecConfigs() { - return Arrays.asList(new Object[][] {{"1.2.3.4", "8.8.4.4"}, {"2601::2", "2601::10"}}); + return Arrays.asList( + new Object[][] { + {"1.2.3.4", "8.8.4.4", "10.0.1.1/24"}, + {"2601::2", "2601::10", "2001:db8::1/64"} + }); } private static final byte[] AEAD_KEY = { @@ -86,6 +95,7 @@ public class IpSecServiceParameterizedTest { INetd mMockNetd; IpSecService.IpSecServiceConfiguration mMockIpSecSrvConfig; IpSecService mIpSecService; + Network fakeNetwork = new Network(0xAB); private static final IpSecAlgorithm AUTH_ALGO = new IpSecAlgorithm(IpSecAlgorithm.AUTH_HMAC_SHA256, AUTH_KEY, AUTH_KEY.length * 4); @@ -94,9 +104,11 @@ public class IpSecServiceParameterizedTest { private static final IpSecAlgorithm AEAD_ALGO = new IpSecAlgorithm(IpSecAlgorithm.AUTH_CRYPT_AES_GCM, AEAD_KEY, 128); - public IpSecServiceParameterizedTest(String sourceAddr, String destAddr) { + public IpSecServiceParameterizedTest( + String sourceAddr, String destAddr, String localInnerAddr) { mSourceAddr = sourceAddr; mDestinationAddr = destAddr; + mLocalInnerAddress = new LinkAddress(localInnerAddr); } @Before @@ -406,4 +418,103 @@ public class IpSecServiceParameterizedTest { verify(mMockNetd).ipSecRemoveTransportModeTransform(pfd.getFileDescriptor()); } + + private IpSecTunnelInterfaceResponse createAndValidateTunnel( + String localAddr, String remoteAddr) { + IpSecTunnelInterfaceResponse createTunnelResp = + mIpSecService.createTunnelInterface( + mSourceAddr, mDestinationAddr, fakeNetwork, new Binder()); + + assertNotNull(createTunnelResp); + assertEquals(IpSecManager.Status.OK, createTunnelResp.status); + return createTunnelResp; + } + + @Test + public void testCreateTunnelInterface() throws Exception { + IpSecTunnelInterfaceResponse createTunnelResp = + createAndValidateTunnel(mSourceAddr, mDestinationAddr); + + // Check that we have stored the tracking object, and retrieve it + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.RefcountedResource refcountedRecord = + userRecord.mTunnelInterfaceRecords.getRefcountedResourceOrThrow( + createTunnelResp.resourceId); + + assertEquals(1, userRecord.mTunnelQuotaTracker.mCurrent); + verify(mMockNetd) + .addVirtualTunnelInterface( + eq(createTunnelResp.interfaceName), + eq(mSourceAddr), + eq(mDestinationAddr), + anyInt(), + anyInt()); + } + + @Test + public void testDeleteTunnelInterface() throws Exception { + IpSecTunnelInterfaceResponse createTunnelResp = + createAndValidateTunnel(mSourceAddr, mDestinationAddr); + + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + + mIpSecService.deleteTunnelInterface(createTunnelResp.resourceId); + + // Verify quota and RefcountedResource objects cleaned up + assertEquals(0, userRecord.mTunnelQuotaTracker.mCurrent); + verify(mMockNetd).removeVirtualTunnelInterface(eq(createTunnelResp.interfaceName)); + try { + userRecord.mTunnelInterfaceRecords.getRefcountedResourceOrThrow( + createTunnelResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void testTunnelInterfaceBinderDeath() throws Exception { + IpSecTunnelInterfaceResponse createTunnelResp = + createAndValidateTunnel(mSourceAddr, mDestinationAddr); + + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + IpSecService.RefcountedResource refcountedRecord = + userRecord.mTunnelInterfaceRecords.getRefcountedResourceOrThrow( + createTunnelResp.resourceId); + + refcountedRecord.binderDied(); + + // Verify quota and RefcountedResource objects cleaned up + assertEquals(0, userRecord.mTunnelQuotaTracker.mCurrent); + verify(mMockNetd).removeVirtualTunnelInterface(eq(createTunnelResp.interfaceName)); + try { + userRecord.mTunnelInterfaceRecords.getRefcountedResourceOrThrow( + createTunnelResp.resourceId); + fail("Expected IllegalArgumentException on attempt to access deleted resource"); + } catch (IllegalArgumentException expected) { + } + } + + @Test + public void testAddRemoveAddressFromTunnelInterface() throws Exception { + IpSecTunnelInterfaceResponse createTunnelResp = + createAndValidateTunnel(mSourceAddr, mDestinationAddr); + + mIpSecService.addAddressToTunnelInterface(createTunnelResp.resourceId, mLocalInnerAddress); + verify(mMockNetd) + .interfaceAddAddress( + eq(createTunnelResp.interfaceName), + eq(mLocalInnerAddress.getAddress().getHostAddress()), + eq(mLocalInnerAddress.getPrefixLength())); + + mIpSecService.removeAddressFromTunnelInterface( + createTunnelResp.resourceId, mLocalInnerAddress); + verify(mMockNetd) + .interfaceDelAddress( + eq(createTunnelResp.interfaceName), + eq(mLocalInnerAddress.getAddress().getHostAddress()), + eq(mLocalInnerAddress.getPrefixLength())); + } } From bfcc17cc20ea9a63a06f231453eb8110a1e7df78 Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Tue, 27 Feb 2018 19:19:40 -0800 Subject: [PATCH 2/2] Check mOwnedByTransform to avoid DELSA on SPI The owned by transform flag prevents the removal of an SPI from accidentally deleting an associated SA in the kernel. That flag wasn't actually being checked, so deleting an SPI would result in the transform being removed. The existing code already guarantees that the SA is deleted when the transform is deleted Bug: 73258845 Test: runtest frameworks-net Merged-In: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06 Change-Id: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06 (cherry picked from commit 22795302be4ec35449908cf566aa7c16945df836) --- .../server/IpSecServiceParameterizedTest.java | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index a5c55e8d84..410f754fb5 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -23,6 +23,7 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -319,6 +320,30 @@ public class IpSecServiceParameterizedTest { } } + @Test + public void testReleaseOwnedSpi() throws Exception { + IpSecConfig ipSecConfig = new IpSecConfig(); + addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); + addAuthAndCryptToIpSecConfig(ipSecConfig); + + IpSecTransformResponse createTransformResp = + mIpSecService.createTransform(ipSecConfig, new Binder()); + IpSecService.UserRecord userRecord = + mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); + assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent); + mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); + verify(mMockNetd, times(0)) + .ipSecDeleteSecurityAssociation( + eq(createTransformResp.resourceId), + anyString(), + anyString(), + eq(TEST_SPI), + anyInt(), + anyInt()); + // quota is not released until the SPI is released by the Transform + assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent); + } + @Test public void testDeleteTransform() throws Exception { IpSecConfig ipSecConfig = new IpSecConfig(); @@ -329,7 +354,7 @@ public class IpSecServiceParameterizedTest { mIpSecService.createTransform(ipSecConfig, new Binder()); mIpSecService.deleteTransform(createTransformResp.resourceId); - verify(mMockNetd) + verify(mMockNetd, times(1)) .ipSecDeleteSecurityAssociation( eq(createTransformResp.resourceId), anyString(), @@ -342,6 +367,21 @@ public class IpSecServiceParameterizedTest { IpSecService.UserRecord userRecord = mIpSecService.mUserResourceTracker.getUserRecord(Os.getuid()); assertEquals(0, userRecord.mTransformQuotaTracker.mCurrent); + assertEquals(1, userRecord.mSpiQuotaTracker.mCurrent); + + mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); + // Verify that ipSecDeleteSa was not called when the SPI was released because the + // ownedByTransform property should prevent it; (note, the called count is cumulative). + verify(mMockNetd, times(1)) + .ipSecDeleteSecurityAssociation( + anyInt(), + anyString(), + anyString(), + anyInt(), + anyInt(), + anyInt()); + assertEquals(0, userRecord.mSpiQuotaTracker.mCurrent); + try { userRecord.mTransformRecords.getRefcountedResourceOrThrow( createTransformResp.resourceId);