commit bfcc17cc20ea9a63a06f231453eb8110a1e7df78
author Nathan Harold <nharold@google.com> Tue Feb 27 19:19:40 2018 -0800
committer Nathan Harold <nharold@google.com> Tue Mar 20 18:29:02 2018 -0700
tree fa9df561bc0804994001d24630f8fad90b28d167
parent 76df78ffbfc4c3e94841af0a54a0ad253069e837

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)

tests/net/java/com/android/server/IpSecServiceParameterizedTest.java

diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
index a5c55e8..410f754 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.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;
 
@@ -320,6 +321,30 @@
     }
 
     @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();
         addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig);
@@ -329,7 +354,7 @@
                 mIpSecService.createTransform(ipSecConfig, new Binder());
         mIpSecService.deleteTransform(createTransformResp.resourceId);
 
-        verify(mMockNetd)
+        verify(mMockNetd, times(1))
                 .ipSecDeleteSecurityAssociation(
                         eq(createTransformResp.resourceId),
                         anyString(),
@@ -342,6 +367,21 @@
         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);