Improve IKEv2/IPsec VPN by proposing more IPsec algorithms

This commit allows IKEv2/IPsec VPN to propose more algorithms that
newly added in IpSecAlgorithm. Those new algorithms have stronger
security guarantees and better performances.

This commit also removes algorithm name validation because all
algorithms are URL encoded to ensure no special characters create
problems due to their use by VpnProfile for list or field delimiting
(e.g. rfc7539esp(chacha20,poly1305))

Bug: 185265778
Test: atest FrameworksNetTests, CtsNetTestCases
Test: All new algorithms are manually verified
Change-Id: I1de322c95aacc8924e95bcdbcfdbd1ec441de99c
Merged-In: I1de322c95aacc8924e95bcdbcfdbd1ec441de99c

5 files changed, 107 insertions, 75 deletions

diff --git a/core/java/android/net/Ikev2VpnProfile.java b/core/java/android/net/Ikev2VpnProfile.java
index cc1312bac180..b18e9be28eb5 100644
--- a/core/java/android/net/Ikev2VpnProfile.java
+++ b/core/java/android/net/Ikev2VpnProfile.java
@@ -16,6 +16,16 @@
 
 package android.net;
 
+import static android.net.IpSecAlgorithm.AUTH_AES_CMAC;
+import static android.net.IpSecAlgorithm.AUTH_AES_XCBC;
+import static android.net.IpSecAlgorithm.AUTH_CRYPT_AES_GCM;
+import static android.net.IpSecAlgorithm.AUTH_CRYPT_CHACHA20_POLY1305;
+import static android.net.IpSecAlgorithm.AUTH_HMAC_SHA256;
+import static android.net.IpSecAlgorithm.AUTH_HMAC_SHA384;
+import static android.net.IpSecAlgorithm.AUTH_HMAC_SHA512;
+import static android.net.IpSecAlgorithm.CRYPT_AES_CBC;
+import static android.net.IpSecAlgorithm.CRYPT_AES_CTR;
+
 import static com.android.internal.annotations.VisibleForTesting.Visibility;
 import static com.android.internal.util.Preconditions.checkStringNotEmpty;
 import static com.android.net.module.util.NetworkStackConstants.IPV6_MIN_MTU;
@@ -70,13 +80,28 @@ public final class Ikev2VpnProfile extends PlatformVpnProfile {
     private static final String EMPTY_CERT = "";
 
     /** @hide */
-    public static final List<String> DEFAULT_ALGORITHMS =
-            Collections.unmodifiableList(Arrays.asList(
-                    IpSecAlgorithm.CRYPT_AES_CBC,
-                    IpSecAlgorithm.AUTH_HMAC_SHA256,
-                    IpSecAlgorithm.AUTH_HMAC_SHA384,
-                    IpSecAlgorithm.AUTH_HMAC_SHA512,
-                    IpSecAlgorithm.AUTH_CRYPT_AES_GCM));
+    public static final List<String> DEFAULT_ALGORITHMS;
+
+    private static void addAlgorithmIfSupported(List<String> algorithms, String ipSecAlgoName) {
+        if (IpSecAlgorithm.getSupportedAlgorithms().contains(ipSecAlgoName)) {
+            algorithms.add(ipSecAlgoName);
+        }
+    }
+
+    static {
+        final List<String> algorithms = new ArrayList<>();
+        addAlgorithmIfSupported(algorithms, CRYPT_AES_CBC);
+        addAlgorithmIfSupported(algorithms, CRYPT_AES_CTR);
+        addAlgorithmIfSupported(algorithms, AUTH_HMAC_SHA256);
+        addAlgorithmIfSupported(algorithms, AUTH_HMAC_SHA384);
+        addAlgorithmIfSupported(algorithms, AUTH_HMAC_SHA512);
+        addAlgorithmIfSupported(algorithms, AUTH_AES_XCBC);
+        addAlgorithmIfSupported(algorithms, AUTH_AES_CMAC);
+        addAlgorithmIfSupported(algorithms, AUTH_CRYPT_AES_GCM);
+        addAlgorithmIfSupported(algorithms, AUTH_CRYPT_CHACHA20_POLY1305);
+
+        DEFAULT_ALGORITHMS = Collections.unmodifiableList(algorithms);
+    }
 
     @NonNull private final String mServerAddr;
     @NonNull private final String mUserIdentity;
@@ -195,8 +220,6 @@ public final class Ikev2VpnProfile extends PlatformVpnProfile {
      * @param allowedAlgorithms The list to be validated
      */
     private static void validateAllowedAlgorithms(@NonNull List<String> algorithmNames) {
-        VpnProfile.validateAllowedAlgorithms(algorithmNames);
-
         // First, make sure no insecure algorithms were proposed.
         if (algorithmNames.contains(IpSecAlgorithm.AUTH_HMAC_MD5)
                 || algorithmNames.contains(IpSecAlgorithm.AUTH_HMAC_SHA1)) {
diff --git a/core/java/com/android/internal/net/VpnProfile.java b/core/java/com/android/internal/net/VpnProfile.java
index b7170d857da9..6e1d3ce9a297 100644
--- a/core/java/com/android/internal/net/VpnProfile.java
+++ b/core/java/com/android/internal/net/VpnProfile.java
@@ -30,7 +30,10 @@ import android.text.TextUtils;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.net.module.util.ProxyUtils;
 
+import java.io.UnsupportedEncodingException;
 import java.net.InetAddress;
+import java.net.URLDecoder;
+import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -74,6 +77,9 @@ public final class VpnProfile implements Cloneable, Parcelable {
 
     private static final String ENCODED_NULL_PROXY_INFO = "\0\0\0\0";
 
+    /** Default URL encoding. */
+    private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name();
+
     // Entity fields.
     @UnsupportedAppUsage
     public final String key;                                   // -1
@@ -129,9 +135,6 @@ public final class VpnProfile implements Cloneable, Parcelable {
 
     /**
      * The list of allowable algorithms.
-     *
-     * <p>This list is validated in the setter to ensure that encoding characters (list, value
-     * delimiters) are not present in the algorithm names. See {@link #validateAllowedAlgorithms()}
      */
     private List<String> mAllowedAlgorithms = new ArrayList<>(); // 19
     public boolean isBypassable = false;                         // 20
@@ -196,11 +199,8 @@ public final class VpnProfile implements Cloneable, Parcelable {
      *
      * @param allowedAlgorithms the list of allowable algorithms, as listed in {@link
      *     IpSecAlgorithm}.
-     * @throws IllegalArgumentException if any delimiters are used in algorithm names. See {@link
-     *     #VALUE_DELIMITER} and {@link LIST_DELIMITER}.
      */
     public void setAllowedAlgorithms(List<String> allowedAlgorithms) {
-        validateAllowedAlgorithms(allowedAlgorithms);
         mAllowedAlgorithms = allowedAlgorithms;
     }
 
@@ -297,7 +297,11 @@ public final class VpnProfile implements Cloneable, Parcelable {
 
             // Either all must be present, or none must be.
             if (values.length >= 24) {
-                profile.mAllowedAlgorithms = Arrays.asList(values[19].split(LIST_DELIMITER));
+                profile.mAllowedAlgorithms = new ArrayList<>();
+                for (String algo : Arrays.asList(values[19].split(LIST_DELIMITER))) {
+                    profile.mAllowedAlgorithms.add(URLDecoder.decode(algo, DEFAULT_ENCODING));
+                }
+
                 profile.isBypassable = Boolean.parseBoolean(values[20]);
                 profile.isMetered = Boolean.parseBoolean(values[21]);
                 profile.maxMtu = Integer.parseInt(values[22]);
@@ -348,7 +352,19 @@ public final class VpnProfile implements Cloneable, Parcelable {
             builder.append(ENCODED_NULL_PROXY_INFO);
         }
 
-        builder.append(VALUE_DELIMITER).append(String.join(LIST_DELIMITER, mAllowedAlgorithms));
+        final List<String> encodedAlgoNames = new ArrayList<>();
+
+        try {
+            for (String algo : mAllowedAlgorithms) {
+                encodedAlgoNames.add(URLEncoder.encode(algo, DEFAULT_ENCODING));
+            }
+        } catch (UnsupportedEncodingException e) {
+            // Unexpected error
+            throw new IllegalStateException("Failed to encode algorithms.", e);
+        }
+
+        builder.append(VALUE_DELIMITER).append(String.join(LIST_DELIMITER, encodedAlgoNames));
+
         builder.append(VALUE_DELIMITER).append(isBypassable);
         builder.append(VALUE_DELIMITER).append(isMetered);
         builder.append(VALUE_DELIMITER).append(maxMtu);
@@ -425,20 +441,6 @@ public final class VpnProfile implements Cloneable, Parcelable {
         return true;
     }
 
-    /**
-     * Validates that the provided list of algorithms does not contain illegal characters.
-     *
-     * @param allowedAlgorithms The list to be validated
-     */
-    public static void validateAllowedAlgorithms(List<String> allowedAlgorithms) {
-        for (final String alg : allowedAlgorithms) {
-            if (alg.contains(VALUE_DELIMITER) || alg.contains(LIST_DELIMITER)) {
-                throw new IllegalArgumentException(
-                        "Algorithm contained illegal ('\0' or ',') character");
-            }
-        }
-    }
-
     /** Generates a hashcode over the VpnProfile. */
     @Override
     public int hashCode() {
diff --git a/packages/Connectivity/tests/unit/java/android/net/Ikev2VpnProfileTest.java b/packages/Connectivity/tests/unit/java/android/net/Ikev2VpnProfileTest.java
index 1abd39a32bdf..0707ef3ed1d6 100644
--- a/packages/Connectivity/tests/unit/java/android/net/Ikev2VpnProfileTest.java
+++ b/packages/Connectivity/tests/unit/java/android/net/Ikev2VpnProfileTest.java
@@ -29,8 +29,8 @@ import androidx.test.filters.SmallTest;
 import androidx.test.runner.AndroidJUnit4;
 
 import com.android.internal.net.VpnProfile;
-import com.android.net.module.util.ProxyUtils;
 import com.android.internal.org.bouncycastle.x509.X509V1CertificateGenerator;
+import com.android.net.module.util.ProxyUtils;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -170,7 +170,10 @@ public class Ikev2VpnProfileTest {
         final Ikev2VpnProfile.Builder builder = getBuilderWithDefaultOptions();
         builder.setAuthPsk(PSK_BYTES);
 
-        List<String> allowedAlgorithms = Arrays.asList(IpSecAlgorithm.AUTH_CRYPT_AES_GCM);
+        List<String> allowedAlgorithms =
+                Arrays.asList(
+                        IpSecAlgorithm.AUTH_CRYPT_AES_GCM,
+                        IpSecAlgorithm.AUTH_CRYPT_CHACHA20_POLY1305);
         builder.setAllowedAlgorithms(allowedAlgorithms);
 
         final Ikev2VpnProfile profile = builder.build();
@@ -183,7 +186,12 @@ public class Ikev2VpnProfileTest {
         builder.setAuthPsk(PSK_BYTES);
 
         List<String> allowedAlgorithms =
-                Arrays.asList(IpSecAlgorithm.AUTH_HMAC_SHA512, IpSecAlgorithm.CRYPT_AES_CBC);
+                Arrays.asList(
+                        IpSecAlgorithm.AUTH_HMAC_SHA512,
+                        IpSecAlgorithm.AUTH_AES_XCBC,
+                        IpSecAlgorithm.AUTH_AES_CMAC,
+                        IpSecAlgorithm.CRYPT_AES_CBC,
+                        IpSecAlgorithm.CRYPT_AES_CTR);
         builder.setAllowedAlgorithms(allowedAlgorithms);
 
         final Ikev2VpnProfile profile = builder.build();
diff --git a/packages/Connectivity/tests/unit/java/com/android/internal/net/VpnProfileTest.java b/packages/Connectivity/tests/unit/java/com/android/internal/net/VpnProfileTest.java
index 46597d19ef1b..cb0f0710d6e7 100644
--- a/packages/Connectivity/tests/unit/java/com/android/internal/net/VpnProfileTest.java
+++ b/packages/Connectivity/tests/unit/java/com/android/internal/net/VpnProfileTest.java
@@ -23,7 +23,6 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 import android.net.IpSecAlgorithm;
 
@@ -97,6 +96,7 @@ public class VpnProfileTest {
         p.setAllowedAlgorithms(
                 Arrays.asList(
                         IpSecAlgorithm.AUTH_CRYPT_AES_GCM,
+                        IpSecAlgorithm.AUTH_CRYPT_CHACHA20_POLY1305,
                         IpSecAlgorithm.AUTH_HMAC_SHA512,
                         IpSecAlgorithm.CRYPT_AES_CBC));
         p.isBypassable = true;
@@ -126,30 +126,6 @@ public class VpnProfileTest {
     }
 
     @Test
-    public void testSetInvalidAlgorithmValueDelimiter() {
-        final VpnProfile profile = getSampleIkev2Profile(DUMMY_PROFILE_KEY);
-
-        try {
-            profile.setAllowedAlgorithms(
-                    Arrays.asList("test" + VpnProfile.VALUE_DELIMITER + "test"));
-            fail("Expected failure due to value separator in algorithm name");
-        } catch (IllegalArgumentException expected) {
-        }
-    }
-
-    @Test
-    public void testSetInvalidAlgorithmListDelimiter() {
-        final VpnProfile profile = getSampleIkev2Profile(DUMMY_PROFILE_KEY);
-
-        try {
-            profile.setAllowedAlgorithms(
-                    Arrays.asList("test" + VpnProfile.LIST_DELIMITER + "test"));
-            fail("Expected failure due to value separator in algorithm name");
-        } catch (IllegalArgumentException expected) {
-        }
-    }
-
-    @Test
     public void testEncodeDecode() {
         final VpnProfile profile = getSampleIkev2Profile(DUMMY_PROFILE_KEY);
         final VpnProfile decoded = VpnProfile.decode(DUMMY_PROFILE_KEY, profile.encode());
diff --git a/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java b/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java
index 887665cfff2a..a0a596d998bf 100644
--- a/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java
+++ b/services/core/java/com/android/server/connectivity/VpnIkev2Utils.java
@@ -213,19 +213,25 @@ public class VpnIkev2Utils {
 
     /** Builds a child SA proposal based on the allowed IPsec algorithms */
     private static List<ChildSaProposal> getChildSaProposals(List<String> allowedAlgorithms) {
-        // TODO: Add new IpSecAlgorithm in the followup commit
-
         final List<ChildSaProposal> proposals = new ArrayList<>();
 
+        final List<Integer> aesKeyLenOptions =
+                Arrays.asList(KEY_LEN_AES_256, KEY_LEN_AES_192, KEY_LEN_AES_128);
+
         // Add non-AEAD options
         if (Ikev2VpnProfile.hasNormalModeAlgorithms(allowedAlgorithms)) {
             final ChildSaProposal.Builder normalModeBuilder = new ChildSaProposal.Builder();
 
             // Encryption Algorithms:
-            // AES-CBC is currently the only supported encryption algorithm.
-            normalModeBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_CBC, KEY_LEN_AES_256);
-            normalModeBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_CBC, KEY_LEN_AES_192);
-            normalModeBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_CBC, KEY_LEN_AES_128);
+            // AES-CBC and AES_CTR are currently the only supported encryption algorithms.
+            for (int len : aesKeyLenOptions) {
+                if (allowedAlgorithms.contains(IpSecAlgorithm.CRYPT_AES_CTR)) {
+                    normalModeBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_CTR, len);
+                }
+                if (allowedAlgorithms.contains(IpSecAlgorithm.CRYPT_AES_CBC)) {
+                    normalModeBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_CBC, len);
+                }
+            }
 
             // Authentication/Integrity Algorithms:
             // Guaranteed by Ikev2VpnProfile constructor to contain at least one of these.
@@ -238,6 +244,12 @@ public class VpnIkev2Utils {
             if (allowedAlgorithms.contains(IpSecAlgorithm.AUTH_HMAC_SHA256)) {
                 normalModeBuilder.addIntegrityAlgorithm(INTEGRITY_ALGORITHM_HMAC_SHA2_256_128);
             }
+            if (allowedAlgorithms.contains(IpSecAlgorithm.AUTH_AES_XCBC)) {
+                normalModeBuilder.addIntegrityAlgorithm(INTEGRITY_ALGORITHM_AES_XCBC_96);
+            }
+            if (allowedAlgorithms.contains(IpSecAlgorithm.AUTH_AES_CMAC)) {
+                normalModeBuilder.addIntegrityAlgorithm(INTEGRITY_ALGORITHM_AES_CMAC_96);
+            }
 
             ChildSaProposal proposal = normalModeBuilder.build();
             if (proposal.getIntegrityAlgorithms().isEmpty()) {
@@ -252,16 +264,27 @@ public class VpnIkev2Utils {
         if (Ikev2VpnProfile.hasAeadAlgorithms(allowedAlgorithms)) {
             final ChildSaProposal.Builder aeadBuilder = new ChildSaProposal.Builder();
 
-            // AES-GCM is currently the only supported AEAD algorithm
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_16, KEY_LEN_AES_256);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_12, KEY_LEN_AES_256);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_8, KEY_LEN_AES_256);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_16, KEY_LEN_AES_192);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_12, KEY_LEN_AES_192);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_8, KEY_LEN_AES_192);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_16, KEY_LEN_AES_128);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_12, KEY_LEN_AES_128);
-            aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_8, KEY_LEN_AES_128);
+            if (allowedAlgorithms.contains(IpSecAlgorithm.AUTH_CRYPT_CHACHA20_POLY1305)) {
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_CHACHA20_POLY1305, KEY_LEN_UNUSED);
+            }
+            if (allowedAlgorithms.contains(IpSecAlgorithm.AUTH_CRYPT_AES_GCM)) {
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_AES_GCM_16, KEY_LEN_AES_256);
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_AES_GCM_12, KEY_LEN_AES_256);
+                aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_8, KEY_LEN_AES_256);
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_AES_GCM_16, KEY_LEN_AES_192);
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_AES_GCM_12, KEY_LEN_AES_192);
+                aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_8, KEY_LEN_AES_192);
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_AES_GCM_16, KEY_LEN_AES_128);
+                aeadBuilder.addEncryptionAlgorithm(
+                        ENCRYPTION_ALGORITHM_AES_GCM_12, KEY_LEN_AES_128);
+                aeadBuilder.addEncryptionAlgorithm(ENCRYPTION_ALGORITHM_AES_GCM_8, KEY_LEN_AES_128);
+            }
 
             proposals.add(aeadBuilder.build());
         }