Allow resp proposal to omit transform if transform ID NONE is proposed

This commit fixes the interoperability issue in SA negotiation to allow
the following two cases:
- When the request proposal has a NONE value transform, response
proposal is still valid if it does not have this transform
- When the request proposal does not have this transform, the response
is still valid if it has a NONE value of this transform.

Bug: 156757683
Test: CtsIkeTestCases, FrameworksIkeTests(new tests added)
Change-Id: I1cc17b92903912a03bdb27729ab0b9db093c94b7

2 files changed, 85 insertions, 3 deletions

diff --git a/src/java/android/net/ipsec/ike/SaProposal.java b/src/java/android/net/ipsec/ike/SaProposal.java
index fa6fbf17..d5eb8c81 100644
--- a/src/java/android/net/ipsec/ike/SaProposal.java
+++ b/src/java/android/net/ipsec/ike/SaProposal.java
@@ -228,6 +228,11 @@ public abstract class SaProposal {
      * Check if the current SaProposal from the SA responder is consistent with the selected
      * reqProposal from the SA initiator.
      *
+     * <p>As per RFC 7296, The accepted cryptographic suite MUST contain exactly one transform of
+     * each type included in the proposal. But for interoperability reason, IKE library allows
+     * exceptions when the accepted suite or the request proposal has a NONE value transform.
+     * Currently only IntegrityTransform and DhGroupTransform have NONE value transform ID defined.
+     *
      * @param reqProposal selected SaProposal from SA initiator
      * @return if current SaProposal from SA responder is consistent with the selected reqProposal
      *     from SA initiator.
@@ -236,11 +241,16 @@ public abstract class SaProposal {
     public boolean isNegotiatedFrom(SaProposal reqProposal) {
         return this.mProtocolId == reqProposal.mProtocolId
                 && isTransformSelectedFrom(mEncryptionAlgorithms, reqProposal.mEncryptionAlgorithms)
-                && isTransformSelectedFrom(mIntegrityAlgorithms, reqProposal.mIntegrityAlgorithms)
-                && isTransformSelectedFrom(mDhGroups, reqProposal.mDhGroups);
+                && isIntegrityTransformSelectedFrom(
+                        mIntegrityAlgorithms, reqProposal.mIntegrityAlgorithms)
+                && isDhGroupTransformSelectedFrom(mDhGroups, reqProposal.mDhGroups);
     }
 
-    /** Package private */
+    /**
+     * Check if the response transform can be selected from the request transforms
+     *
+     * <p>Package private
+     */
     static boolean isTransformSelectedFrom(Transform[] selected, Transform[] selectFrom) {
         // If the selected proposal has multiple transforms with the same type, the responder MUST
         // choose a single one.
@@ -253,6 +263,45 @@ public abstract class SaProposal {
         return Arrays.asList(selectFrom).contains(selected[0]);
     }
 
+    /**
+     * Check if the response integrity transform can be selected from the request integrity
+     * transforms.
+     *
+     * <p>For interoperability reason, it is allowed to do not include integrity transform in the
+     * response proposal when the request proposal has a NONE value integrity transform; and it is
+     * also allowed to have a NONE value integrity transform when the request proposal does not have
+     * integrity transforms.
+     */
+    private static boolean isIntegrityTransformSelectedFrom(
+            IntegrityTransform[] selected, IntegrityTransform[] selectFrom) {
+        if (selected.length == 0) {
+            selected = new IntegrityTransform[] {new IntegrityTransform(INTEGRITY_ALGORITHM_NONE)};
+        }
+        if (selectFrom.length == 0) {
+            selectFrom =
+                    new IntegrityTransform[] {new IntegrityTransform(INTEGRITY_ALGORITHM_NONE)};
+        }
+        return isTransformSelectedFrom(selected, selectFrom);
+    }
+
+    /**
+     * Check if the response DH group can be selected from the request DH groups
+     *
+     * <p>For interoperability reason, it is allowed to do not include DH group in the response
+     * proposal when the request proposal has a NONE value DH group; and it is also allowed to have
+     * a NONE value DH group when the request proposal does not have DH groups.
+     */
+    private static boolean isDhGroupTransformSelectedFrom(
+            DhGroupTransform[] selected, DhGroupTransform[] selectFrom) {
+        if (selected.length == 0) {
+            selected = new DhGroupTransform[] {new DhGroupTransform(DH_GROUP_NONE)};
+        }
+        if (selectFrom.length == 0) {
+            selectFrom = new DhGroupTransform[] {new DhGroupTransform(DH_GROUP_NONE)};
+        }
+        return isTransformSelectedFrom(selected, selectFrom);
+    }
+
     /** @hide */
     @IkePayload.ProtocolId
     public int getProtocolId() {
diff --git a/tests/iketests/src/java/android/net/ipsec/ike/SaProposalTest.java b/tests/iketests/src/java/android/net/ipsec/ike/SaProposalTest.java
index d4efb0c3..0c98bfa0 100644
--- a/tests/iketests/src/java/android/net/ipsec/ike/SaProposalTest.java
+++ b/tests/iketests/src/java/android/net/ipsec/ike/SaProposalTest.java
@@ -16,8 +16,15 @@
 
 package android.net.ipsec.ike;
 
+import static android.net.ipsec.ike.SaProposal.DH_GROUP_1024_BIT_MODP;
+import static android.net.ipsec.ike.SaProposal.DH_GROUP_2048_BIT_MODP;
+import static android.net.ipsec.ike.SaProposal.ENCRYPTION_ALGORITHM_AES_GCM_12;
+import static android.net.ipsec.ike.SaProposal.ENCRYPTION_ALGORITHM_AES_GCM_8;
+import static android.net.ipsec.ike.SaProposal.INTEGRITY_ALGORITHM_NONE;
 import static android.net.ipsec.ike.SaProposal.KEY_LEN_AES_128;
 import static android.net.ipsec.ike.SaProposal.KEY_LEN_UNUSED;
+import static android.net.ipsec.ike.SaProposal.PSEUDORANDOM_FUNCTION_AES128_XCBC;
+import static android.net.ipsec.ike.SaProposal.PSEUDORANDOM_FUNCTION_SHA2_256;
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
@@ -318,4 +325,30 @@ public final class SaProposalTest {
                         new Transform[] {mIntegrityNoneTransform},
                         new Transform[] {mIntegrityHmacSha1Transform}));
     }
+
+    @Test
+    public void testIsNegotiatedFromProposalWithIntegrityNone() throws Exception {
+        SaProposal respProposal =
+                new IkeSaProposal.Builder()
+                        .addEncryptionAlgorithm(
+                                ENCRYPTION_ALGORITHM_AES_GCM_12, SaProposal.KEY_LEN_AES_128)
+                        .addDhGroup(DH_GROUP_2048_BIT_MODP)
+                        .addPseudorandomFunction(PSEUDORANDOM_FUNCTION_AES128_XCBC)
+                        .build();
+
+        SaProposal reqProposal =
+                new IkeSaProposal.Builder()
+                        .addEncryptionAlgorithm(
+                                ENCRYPTION_ALGORITHM_AES_GCM_12, SaProposal.KEY_LEN_AES_128)
+                        .addEncryptionAlgorithm(
+                                ENCRYPTION_ALGORITHM_AES_GCM_8, SaProposal.KEY_LEN_AES_128)
+                        .addIntegrityAlgorithm(INTEGRITY_ALGORITHM_NONE)
+                        .addDhGroup(DH_GROUP_1024_BIT_MODP)
+                        .addDhGroup(DH_GROUP_2048_BIT_MODP)
+                        .addPseudorandomFunction(PSEUDORANDOM_FUNCTION_AES128_XCBC)
+                        .addPseudorandomFunction(PSEUDORANDOM_FUNCTION_SHA2_256)
+                        .build();
+
+        assertTrue(respProposal.isNegotiatedFrom(reqProposal));
+    }
 }