From 963502faa33fd144d9e35fa15d2bfdb3b2c2b86b Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Thu, 3 Oct 2019 11:09:00 -0700 Subject: [PATCH 1/2] Use TransformRecord to get SPI instead of SpiRecord IpSecService.applyTunnelModeTransform() currently does not take an SpiRecord instance, yet implicitly requires that the SpiRecord instance is still alive based on the stored SpiRecord resourceId in the TransformRecord's IpSecConfig. This check is unnecessary, as the SpiRecord has been subsumed into the TransformRecord, and the kernel resources are kept alive whether or not the SpiRecord is still held by the user. This allows users of the IpSecManager API to allocate short-lived SPIs during the creation of an IpSecTransform, without having to keep track of both of them (even though the SPI is no longer usable). The TransformRecord.getSpiRecord() call is already used in multiple other places in the same method. Bug: 142072071 Test: New tests added, passing. Change-Id: I1959f3080946267243564459ff4207647922566e Merged-In: I1959f3080946267243564459ff4207647922566e (cherry picked from commit 5258b1b82f39bf17e0751bcb94479464250aaec5) --- .../java/com/android/server/IpSecService.java | 4 +- .../server/IpSecServiceParameterizedTest.java | 68 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java index 905c489e1dcbb..6402e07bddc32 100644 --- a/services/core/java/com/android/server/IpSecService.java +++ b/services/core/java/com/android/server/IpSecService.java @@ -1776,7 +1776,7 @@ public class IpSecService extends IIpSecService.Stub { socketRecord = userRecord.mEncapSocketRecords.getResourceOrThrow(c.getEncapSocketResourceId()); } - SpiRecord spiRecord = userRecord.mSpiRecords.getResourceOrThrow(c.getSpiResourceId()); + SpiRecord spiRecord = transformInfo.getSpiRecord(); int mark = (direction == IpSecManager.DIRECTION_OUT) @@ -1809,7 +1809,7 @@ public class IpSecService extends IIpSecService.Stub { // Set outbound SPI only. We want inbound to use any valid SA (old, new) on rekeys, // but want to guarantee outbound packets are sent over the new SA. - spi = transformInfo.getSpiRecord().getSpi(); + spi = spiRecord.getSpi(); } // Always update the policy with the relevant XFRM_IF_ID diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index 23098ec067d25..84d09e26a8d7d 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -571,6 +571,35 @@ public class IpSecServiceParameterizedTest { eq(TEST_SPI)); } + @Test + public void testApplyTransportModeTransformWithClosedSpi() throws Exception { + IpSecConfig ipSecConfig = new IpSecConfig(); + addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); + addAuthAndCryptToIpSecConfig(ipSecConfig); + + IpSecTransformResponse createTransformResp = + mIpSecService.createTransform(ipSecConfig, new Binder(), "blessedPackage"); + + // Close SPI record + mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); + + Socket socket = new Socket(); + socket.bind(null); + ParcelFileDescriptor pfd = ParcelFileDescriptor.fromSocket(socket); + + int resourceId = createTransformResp.resourceId; + mIpSecService.applyTransportModeTransform(pfd, IpSecManager.DIRECTION_OUT, resourceId); + + verify(mMockNetd) + .ipSecApplyTransportModeTransform( + eq(pfd), + eq(mUid), + eq(IpSecManager.DIRECTION_OUT), + anyString(), + anyString(), + eq(TEST_SPI)); + } + @Test public void testRemoveTransportModeTransform() throws Exception { Socket socket = new Socket(); @@ -693,6 +722,45 @@ public class IpSecServiceParameterizedTest { verifyTransformNetdCalledForCreatingSA(ipSecConfig, createTransformResp); } + + @Test + public void testApplyTunnelModeTransformWithClosedSpi() throws Exception { + IpSecConfig ipSecConfig = new IpSecConfig(); + ipSecConfig.setMode(IpSecTransform.MODE_TUNNEL); + addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); + addAuthAndCryptToIpSecConfig(ipSecConfig); + + IpSecTransformResponse createTransformResp = + mIpSecService.createTransform(ipSecConfig, new Binder(), "blessedPackage"); + IpSecTunnelInterfaceResponse createTunnelResp = + createAndValidateTunnel(mSourceAddr, mDestinationAddr, "blessedPackage"); + + // Close SPI record + mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); + + int transformResourceId = createTransformResp.resourceId; + int tunnelResourceId = createTunnelResp.resourceId; + mIpSecService.applyTunnelModeTransform(tunnelResourceId, IpSecManager.DIRECTION_OUT, + transformResourceId, "blessedPackage"); + + for (int selAddrFamily : ADDRESS_FAMILIES) { + verify(mMockNetd) + .ipSecUpdateSecurityPolicy( + eq(mUid), + eq(selAddrFamily), + eq(IpSecManager.DIRECTION_OUT), + anyString(), + anyString(), + eq(TEST_SPI), + anyInt(), // iKey/oKey + anyInt(), // mask + eq(tunnelResourceId)); + } + + ipSecConfig.setXfrmInterfaceId(tunnelResourceId); + verifyTransformNetdCalledForCreatingSA(ipSecConfig, createTransformResp); + } + @Test public void testAddRemoveAddressFromTunnelInterface() throws Exception { for (String pkgName : new String[]{"blessedPackage", "systemPackage"}) { From e696390feea775e66df6fa48cc122011a51f931e Mon Sep 17 00:00:00 2001 From: Benedict Wong Date: Tue, 10 Mar 2020 23:38:04 -0700 Subject: [PATCH 2/2] Add tests for Tunnel, Transport mode application with released SPIs This change adds tests to validate that both transport and tunnel mode transforms continue to work even after the SPI resource has been released. Specifically, since SPI resources are effectively subsumed by the creation of a Transform, the SPI resource is still "alive", but removed from the user-tracking sparse arrays. Bug: 142072071 Test: Added these new tests. Failing prior to aosp/1133555, passes with. Change-Id: I37dd459d08b57b14f72f3b28ea80fa231b152f10 Merged-In: I37dd459d08b57b14f72f3b28ea80fa231b152f10 (cherry picked from commit 4d3f871a944d24cd7cbe3aa51a789a71020eafb5) --- .../server/IpSecServiceParameterizedTest.java | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java index 84d09e26a8d7d..529d03c520bab 100644 --- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java +++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java @@ -547,6 +547,16 @@ public class IpSecServiceParameterizedTest { @Test public void testApplyTransportModeTransform() throws Exception { + verifyApplyTransportModeTransformCommon(false); + } + + @Test + public void testApplyTransportModeTransformReleasedSpi() throws Exception { + verifyApplyTransportModeTransformCommon(true); + } + + public void verifyApplyTransportModeTransformCommon( + boolean closeSpiBeforeApply) throws Exception { IpSecConfig ipSecConfig = new IpSecConfig(); addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); addAuthAndCryptToIpSecConfig(ipSecConfig); @@ -554,6 +564,10 @@ public class IpSecServiceParameterizedTest { IpSecTransformResponse createTransformResp = mIpSecService.createTransform(ipSecConfig, new Binder(), "blessedPackage"); + if (closeSpiBeforeApply) { + mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); + } + Socket socket = new Socket(); socket.bind(null); ParcelFileDescriptor pfd = ParcelFileDescriptor.fromSocket(socket); @@ -689,6 +703,15 @@ public class IpSecServiceParameterizedTest { @Test public void testApplyTunnelModeTransform() throws Exception { + verifyApplyTunnelModeTransformCommon(false); + } + + @Test + public void testApplyTunnelModeTransformReleasedSpi() throws Exception { + verifyApplyTunnelModeTransformCommon(true); + } + + public void verifyApplyTunnelModeTransformCommon(boolean closeSpiBeforeApply) throws Exception { IpSecConfig ipSecConfig = new IpSecConfig(); ipSecConfig.setMode(IpSecTransform.MODE_TUNNEL); addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig); @@ -699,6 +722,10 @@ public class IpSecServiceParameterizedTest { IpSecTunnelInterfaceResponse createTunnelResp = createAndValidateTunnel(mSourceAddr, mDestinationAddr, "blessedPackage"); + if (closeSpiBeforeApply) { + mIpSecService.releaseSecurityParameterIndex(ipSecConfig.getSpiResourceId()); + } + int transformResourceId = createTransformResp.resourceId; int tunnelResourceId = createTunnelResp.resourceId; mIpSecService.applyTunnelModeTransform(tunnelResourceId, IpSecManager.DIRECTION_OUT,