Merge "Add checks to ensure SPIs are not reused" am: 0a1dd194aa

am: 1a2e3f3b54

Change-Id: I8c7243bc600e5f2e9abec03b5a357ed6376aa9bf

2 files changed, 36 insertions, 10 deletions

diff --git a/services/core/java/com/android/server/IpSecService.java b/services/core/java/com/android/server/IpSecService.java
index ef6bc437cb6c..24d493e3013e 100644
--- a/services/core/java/com/android/server/IpSecService.java
+++ b/services/core/java/com/android/server/IpSecService.java
@@ -571,6 +571,8 @@ public class IpSecService extends IIpSecService.Stub {
             mConfig = config;
             mSpi = spi;
             mSocket = socket;
+
+            spi.setOwnedByTransform();
         }
 
         public IpSecConfig getConfig() {
@@ -651,16 +653,6 @@ public class IpSecService extends IIpSecService.Stub {
         /** always guarded by IpSecService#this */
         @Override
         public void freeUnderlyingResources() {
-            if (mOwnedByTransform) {
-                Log.d(TAG, "Cannot release Spi " + mSpi + ": Currently locked by a Transform");
-                // Because SPIs are "handed off" to transform, objects, they should never be
-                // freed from the SpiRecord once used in a transform. (They refer to the same SA,
-                // thus ownership and responsibility for freeing these resources passes to the
-                // Transform object). Thus, we should let the user free them without penalty once
-                // they are applied in a Transform object.
-                return;
-            }
-
             try {
                 mSrvConfig
                         .getNetdInstance()
@@ -694,6 +686,10 @@ public class IpSecService extends IIpSecService.Stub {
             mOwnedByTransform = true;
         }
 
+        public boolean getOwnedByTransform() {
+            return mOwnedByTransform;
+        }
+
         @Override
         public void invalidate() throws RemoteException {
             getUserRecord().removeSpiRecord(mResourceId);
@@ -1107,6 +1103,11 @@ public class IpSecService extends IIpSecService.Stub {
         // Retrieve SPI record; will throw IllegalArgumentException if not found
         SpiRecord s = userRecord.mSpiRecords.getResourceOrThrow(config.getSpiResourceId());
 
+        // Check to ensure that SPI has not already been used.
+        if (s.getOwnedByTransform()) {
+            throw new IllegalStateException("SPI already in use; cannot be used in new Transforms");
+        }
+
         // If no remote address is supplied, then use one from the SPI.
         if (TextUtils.isEmpty(config.getDestinationAddress())) {
             config.setDestinationAddress(s.getDestinationAddress());
diff --git a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
index d9d4eeba900f..1618e07a79c0 100644
--- a/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
+++ b/tests/net/java/com/android/server/IpSecServiceParameterizedTest.java
@@ -268,6 +268,31 @@ public class IpSecServiceParameterizedTest {
                         anyInt());
     }
 
+    public void testCreateTwoTransformsWithSameSpis() throws Exception {
+        IpSecConfig ipSecConfig = new IpSecConfig();
+        addDefaultSpisAndRemoteAddrToIpSecConfig(ipSecConfig);
+        addAuthAndCryptToIpSecConfig(ipSecConfig);
+
+        IpSecTransformResponse createTransformResp =
+                mIpSecService.createTransform(ipSecConfig, new Binder());
+        assertEquals(IpSecManager.Status.OK, createTransformResp.status);
+
+        // Attempting to create transform a second time with the same SPIs should throw an error...
+        try {
+                mIpSecService.createTransform(ipSecConfig, new Binder());
+                fail("IpSecService should have thrown an error for reuse of SPI");
+        } catch (IllegalStateException expected) {
+        }
+
+        // ... even if the transform is deleted
+        mIpSecService.deleteTransform(createTransformResp.resourceId);
+        try {
+                mIpSecService.createTransform(ipSecConfig, new Binder());
+                fail("IpSecService should have thrown an error for reuse of SPI");
+        } catch (IllegalStateException expected) {
+        }
+    }
+
     @Test
     public void testDeleteTransform() throws Exception {
         IpSecConfig ipSecConfig = new IpSecConfig();