From 5258b1b82f39bf17e0751bcb94479464250aaec5 Mon Sep 17 00:00:00 2001
From: Benedict Wong <benedictwong@google.com>
Date: Thu, 3 Oct 2019 11:09:00 -0700
Subject: [PATCH] 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
---
 .../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 a629b3fbb8faa..3964b399701bb 100644
--- a/services/core/java/com/android/server/IpSecService.java
+++ b/services/core/java/com/android/server/IpSecService.java
@@ -1761,7 +1761,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)
@@ -1794,7 +1794,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 71b72b84de81d..d41c0c737bd64 100644
--- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
@@ -568,6 +568,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();
@@ -689,6 +718,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"}) {