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
Change-Id: I4c26aea7af817a5d9e54da5db1cdf4f943bcae06
diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java
index 45e9481..89f599b 100644
--- a/services/core/java/com/android/server/IpSecService.java
+++ b/services/core/java/com/android/server/IpSecService.java
@@ -676,10 +676,12 @@
@Override
public void freeUnderlyingResources() {
try {
- mSrvConfig
- .getNetdInstance()
- .ipSecDeleteSecurityAssociation(
- mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0);
+ if (!mOwnedByTransform) {
+ mSrvConfig
+ .getNetdInstance()
+ .ipSecDeleteSecurityAssociation(
+ mResourceId, mSourceAddress, mDestinationAddress, mSpi, 0, 0);
+ }
} catch (ServiceSpecificException | RemoteException e) {
Log.e(TAG, "Failed to delete SPI reservation with ID: " + mResourceId, e);
}
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);