Minor Scrypt tidy-ups.

Cherry-picked from upstream PR #1055.

Update native method signatures to match ScryptKeySpec types, factor
constants out of tests and test all aliases at least once.

No functional change in this CL, more tests and implementation to
come in a future one.

Bug: 331613998
Bug: 303794624
Test: MTS
Change-Id: Icb696e5c1f8957e1984ccf8fa9dc62adc3fecdce

9 files changed, 101 insertions, 89 deletions

diff --git a/common/src/jni/main/cpp/conscrypt/native_crypto.cc b/common/src/jni/main/cpp/conscrypt/native_crypto.cc
index b5403822..784b7069 100644
--- a/common/src/jni/main/cpp/conscrypt/native_crypto.cc
+++ b/common/src/jni/main/cpp/conscrypt/native_crypto.cc
@@ -10849,9 +10849,9 @@ static jboolean NativeCrypto_usesBoringSsl_FIPS_mode() {
  */
 
 static jbyteArray NativeCrypto_Scrypt_generate_key(JNIEnv* env, jclass, jbyteArray password, jbyteArray salt,
-                                                   jlong n, jlong r, jlong p, jint key_len) {
+                                                   jint n, jint r, jint p, jint key_len) {
     CHECK_ERROR_QUEUE_ON_RETURN;
-    JNI_TRACE("Scrypt_generate_key(%p, %p, %ld, %ld, %ld, %d)", password, salt, n, r, p, key_len);
+    JNI_TRACE("Scrypt_generate_key(%p, %p, %d, %d, %d, %d)", password, salt, n, r, p, key_len);
 
     if (password == nullptr) {
         conscrypt::jniutil::throwNullPointerException(env, "password == null");
@@ -11365,7 +11365,7 @@ static JNINativeMethod sNativeCryptoMethods[] = {
         CONSCRYPT_NATIVE_METHOD(ENGINE_SSL_force_read, "(J" REF_SSL SSL_CALLBACKS ")V"),
         CONSCRYPT_NATIVE_METHOD(ENGINE_SSL_shutdown, "(J" REF_SSL SSL_CALLBACKS ")V"),
         CONSCRYPT_NATIVE_METHOD(usesBoringSsl_FIPS_mode, "()Z"),
-        CONSCRYPT_NATIVE_METHOD(Scrypt_generate_key, "([B[BJJJI)[B"),
+        CONSCRYPT_NATIVE_METHOD(Scrypt_generate_key, "([B[BIIII)[B"),
 
         // Used for testing only.
         CONSCRYPT_NATIVE_METHOD(BIO_read, "(J[B)I"),
diff --git a/common/src/main/java/org/conscrypt/NativeCrypto.java b/common/src/main/java/org/conscrypt/NativeCrypto.java
index 68e41979..d105e22a 100644
--- a/common/src/main/java/org/conscrypt/NativeCrypto.java
+++ b/common/src/main/java/org/conscrypt/NativeCrypto.java
@@ -1513,7 +1513,7 @@ public final class NativeCrypto {
     /**
      * Generates a key from a password and salt using Scrypt.
      */
-    static native byte[] Scrypt_generate_key(byte[] password, byte[] salt, long n, long r, long p, int key_len);
+    static native byte[] Scrypt_generate_key(byte[] password, byte[] salt, int n, int r, int p, int key_len);
 
     /**
      * Return {@code true} if BoringSSL has been built in FIPS mode.
diff --git a/common/src/main/java/org/conscrypt/OpenSSLProvider.java b/common/src/main/java/org/conscrypt/OpenSSLProvider.java
index 9c6f0560..6fc30ff9 100644
--- a/common/src/main/java/org/conscrypt/OpenSSLProvider.java
+++ b/common/src/main/java/org/conscrypt/OpenSSLProvider.java
@@ -217,9 +217,9 @@ public final class OpenSSLProvider extends Provider {
         /* == SecretKeyFactory == */
         put("SecretKeyFactory.DESEDE", PREFIX + "DESEDESecretKeyFactory");
         put("Alg.Alias.SecretKeyFactory.TDEA", "DESEDE");
-        put("SecretKeyFactory.Scrypt", PREFIX + "ScryptSecretKeyFactory");
-        put("SecretKeyFactory.1.3.6.1.4.1.11591.4.11", PREFIX + "ScryptSecretKeyFactory");
-        put("SecretKeyFactory.OID.1.3.6.1.4.1.11591.4.11", PREFIX + "ScryptSecretKeyFactory");
+        put("SecretKeyFactory.SCRYPT", PREFIX + "ScryptSecretKeyFactory");
+        put("Alg.Alias.SecretKeyFactory.1.3.6.1.4.1.11591.4.11", "SCRYPT");
+        put("Alg.Alias.SecretKeyFactory.OID.1.3.6.1.4.1.11591.4.11", "SCRYPT");
 
         /* == KeyAgreement == */
         putECDHKeyAgreementImplClass("OpenSSLECDHKeyAgreement");
diff --git a/common/src/main/java/org/conscrypt/ScryptSecretKeyFactory.java b/common/src/main/java/org/conscrypt/ScryptSecretKeyFactory.java
index 8b7afe34..9714bcf3 100644
--- a/common/src/main/java/org/conscrypt/ScryptSecretKeyFactory.java
+++ b/common/src/main/java/org/conscrypt/ScryptSecretKeyFactory.java
@@ -44,8 +44,8 @@ public class ScryptSecretKeyFactory extends SecretKeyFactorySpi {
             p = spec.getParallelizationParameter();
             keyOutputBits = spec.getKeyLength();
         } else {
-            // Extract parameters from any `KeySpec` that has getters with the correct name. This allows,
-            // for example, code to use BouncyCastle's KeySpec with the conscrypt provider.
+            // Extract parameters from any `KeySpec` that has getters with the correct name. This
+            // allows, for example, code to use BouncyCastle's KeySpec with the Conscrypt provider.
             try {
                 password = (char[]) getValue(inKeySpec, "getPassword");
                 salt = (byte[]) getValue(inKeySpec, "getSalt");
@@ -65,10 +65,11 @@ public class ScryptSecretKeyFactory extends SecretKeyFactorySpi {
         try {
         return new ScryptKey(
                 NativeCrypto.Scrypt_generate_key(
-                        new String(password).getBytes("UTF-8"), salt, n, r, p, keyOutputBits / 8));
+                        new String(password).getBytes("UTF-8"),
+                        salt, n, r, p, keyOutputBits / 8));
         } catch (UnsupportedEncodingException e) {
                 // Impossible according to the Java docs: UTF-8 is always supported.
-                throw new RuntimeException(e);
+                throw new IllegalStateException(e);
         }
     }
 
@@ -106,7 +107,7 @@ public class ScryptSecretKeyFactory extends SecretKeyFactorySpi {
 
         @Override
         public String getAlgorithm() {
-            // capitalised because BouncyCastle does it.
+            // Capitalised because BouncyCastle does it.
             return "SCRYPT";
         }
 
diff --git a/common/src/test/java/org/conscrypt/javax/crypto/ScryptTest.java b/common/src/test/java/org/conscrypt/javax/crypto/ScryptTest.java
index f0183a81..133306cc 100644
--- a/common/src/test/java/org/conscrypt/javax/crypto/ScryptTest.java
+++ b/common/src/test/java/org/conscrypt/javax/crypto/ScryptTest.java
@@ -35,10 +35,36 @@ import org.conscrypt.TestUtils;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public final class ScryptTest {
+    @Parameters(name = "{0}")
+    public static String[] params() {
+        return new String[] {
+                "SCRYPT",
+                "1.3.6.1.4.1.11591.4.11",
+                "OID.1.3.6.1.4.1.11591.4.11"
+        };
+    }
+
+    @Parameter
+    public String alias;
+
+    // One of the test vectors from RFC 7914
+    private static final char[] TEST_PASSWORD = "password".toCharArray();
+    private static final byte[] TEST_SALT = "NaCl".getBytes(StandardCharsets.UTF_8);
+    private static final int TEST_COST = 1024;
+    private static final int TEST_BLOCKSIZE = 8;
+    private static final int TEST_PARALLELIZATION = 16;
+    private static final int TEST_KEY_SIZE = 512;
+    private static final byte[] TEST_KEY = decodeHex(
+            "fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b373162"
+                    + "2eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640");
+
+
     private final List<String[]> testVectors = readTestVectors();
 
     // Column indices in test vector CSV file
@@ -56,24 +82,13 @@ public final class ScryptTest {
 
     @Test
     public void smokeTest() throws Exception {
-        SecretKeyFactory factory = SecretKeyFactory.getInstance("Scrypt");
-        assertNotNull(factory);
-
-        // One of the test vectors from RFC 7914
-        ScryptKeySpec spec =
-                new ScryptKeySpec(
-                        "password".toCharArray(),
-                        "NaCl".getBytes(StandardCharsets.UTF_8),
-                        1024,
-                        8,
-                        16,
-                        512);
-        SecretKey key = factory.generateSecret(spec);
-        assertNotNull(key);
+        SecretKeyFactory factory = SecretKeyFactory.getInstance(alias);
+        assertEquals(alias, factory.getAlgorithm());
 
-        assertArrayEquals(
-                decodeHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640"),
-                key.getEncoded());
+        ScryptKeySpec spec = new ScryptKeySpec(TEST_PASSWORD, TEST_SALT,
+                TEST_COST, TEST_BLOCKSIZE, TEST_PARALLELIZATION, TEST_KEY_SIZE);
+        SecretKey key = factory.generateSecret(spec);
+        assertArrayEquals(TEST_KEY, key.getEncoded());
 
         // Convert for use with AES
         SecretKeySpec aesKey = makeAesKeySpec(key);
@@ -84,19 +99,13 @@ public final class ScryptTest {
 
     @Test
     public void duckTypingTest() throws Exception {
-        SecretKeyFactory factory = SecretKeyFactory.getInstance("Scrypt");
-        assertNotNull(factory);
+        SecretKeyFactory factory = SecretKeyFactory.getInstance(alias);
 
-        // One of the test vectors from RFC 7914
-        KeySpec spec =
-                new MyPrivateKeySpec(
-                        "password".toCharArray(), "NaCl".getBytes(StandardCharsets.UTF_8), 1024, 8, 16, 512);
-        SecretKey key = factory.generateSecret(spec);
-        assertNotNull(key);
+        KeySpec spec = new MyPrivateKeySpec(TEST_PASSWORD, TEST_SALT,
+                TEST_COST, TEST_BLOCKSIZE, TEST_PARALLELIZATION, TEST_KEY_SIZE);
 
-        assertArrayEquals(
-                decodeHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640"),
-                key.getEncoded());
+        SecretKey key = factory.generateSecret(spec);
+        assertArrayEquals(TEST_KEY, key.getEncoded());
     }
 
     private SecretKeySpec makeAesKeySpec(SecretKey key) {
@@ -108,7 +117,7 @@ public final class ScryptTest {
     }
 
     private void checkKeyIsUsableWithAes(SecretKeySpec spec) throws Exception {
-        // Terrible encryption mode but saves messing with IVs and padding which don't matter here.
+        // Terrible encryption mode but saves messing with IVs which don't matter here.
         Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding");
 
         cipher.init(Cipher.ENCRYPT_MODE, spec);
@@ -132,7 +141,7 @@ public final class ScryptTest {
             byte[] expectedBytes = decodeHex(entry[KEY_INDEX]);
 
             ScryptKeySpec spec = new ScryptKeySpec(password, salt, n, r, p, expectedBytes.length * 8);
-            SecretKeyFactory factory = SecretKeyFactory.getInstance("Scrypt");
+            SecretKeyFactory factory = SecretKeyFactory.getInstance(alias);
             SecretKey key = factory.generateSecret(spec);
             assertNotNull(key);
             assertArrayEquals(expectedBytes, key.getEncoded());
diff --git a/repackaged/common/src/main/java/com/android/org/conscrypt/NativeCrypto.java b/repackaged/common/src/main/java/com/android/org/conscrypt/NativeCrypto.java
index 80f3053c..714b9612 100644
--- a/repackaged/common/src/main/java/com/android/org/conscrypt/NativeCrypto.java
+++ b/repackaged/common/src/main/java/com/android/org/conscrypt/NativeCrypto.java
@@ -1549,7 +1549,7 @@ public final class NativeCrypto {
      * Generates a key from a password and salt using Scrypt.
      */
     static native byte[] Scrypt_generate_key(
-            byte[] password, byte[] salt, long n, long r, long p, int key_len);
+            byte[] password, byte[] salt, int n, int r, int p, int key_len);
 
     /**
      * Return {@code true} if BoringSSL has been built in FIPS mode.
diff --git a/repackaged/common/src/main/java/com/android/org/conscrypt/OpenSSLProvider.java b/repackaged/common/src/main/java/com/android/org/conscrypt/OpenSSLProvider.java
index fda60e88..5654d815 100644
--- a/repackaged/common/src/main/java/com/android/org/conscrypt/OpenSSLProvider.java
+++ b/repackaged/common/src/main/java/com/android/org/conscrypt/OpenSSLProvider.java
@@ -224,9 +224,9 @@ public final class OpenSSLProvider extends Provider {
         /* == SecretKeyFactory == */
         put("SecretKeyFactory.DESEDE", PREFIX + "DESEDESecretKeyFactory");
         put("Alg.Alias.SecretKeyFactory.TDEA", "DESEDE");
-        put("SecretKeyFactory.Scrypt", PREFIX + "ScryptSecretKeyFactory");
-        put("SecretKeyFactory.1.3.6.1.4.1.11591.4.11", PREFIX + "ScryptSecretKeyFactory");
-        put("SecretKeyFactory.OID.1.3.6.1.4.1.11591.4.11", PREFIX + "ScryptSecretKeyFactory");
+        put("SecretKeyFactory.SCRYPT", PREFIX + "ScryptSecretKeyFactory");
+        put("Alg.Alias.SecretKeyFactory.1.3.6.1.4.1.11591.4.11", "SCRYPT");
+        put("Alg.Alias.SecretKeyFactory.OID.1.3.6.1.4.1.11591.4.11", "SCRYPT");
 
         /* == KeyAgreement == */
         putECDHKeyAgreementImplClass("OpenSSLECDHKeyAgreement");
diff --git a/repackaged/common/src/main/java/com/android/org/conscrypt/ScryptSecretKeyFactory.java b/repackaged/common/src/main/java/com/android/org/conscrypt/ScryptSecretKeyFactory.java
index bd1c0543..ac6b67f6 100644
--- a/repackaged/common/src/main/java/com/android/org/conscrypt/ScryptSecretKeyFactory.java
+++ b/repackaged/common/src/main/java/com/android/org/conscrypt/ScryptSecretKeyFactory.java
@@ -48,8 +48,8 @@ public class ScryptSecretKeyFactory extends SecretKeyFactorySpi {
             p = spec.getParallelizationParameter();
             keyOutputBits = spec.getKeyLength();
         } else {
-            // Extract parameters from any `KeySpec` that has getters with the correct name. This allows,
-            // for example, code to use BouncyCastle's KeySpec with the conscrypt provider.
+            // Extract parameters from any `KeySpec` that has getters with the correct name. This
+            // allows, for example, code to use BouncyCastle's KeySpec with the Conscrypt provider.
             try {
                 password = (char[]) getValue(inKeySpec, "getPassword");
                 salt = (byte[]) getValue(inKeySpec, "getSalt");
@@ -67,12 +67,11 @@ public class ScryptSecretKeyFactory extends SecretKeyFactorySpi {
         }
 
         try {
-        return new ScryptKey(
-                NativeCrypto.Scrypt_generate_key(
-                        new String(password).getBytes("UTF-8"), salt, n, r, p, keyOutputBits / 8));
+            return new ScryptKey(NativeCrypto.Scrypt_generate_key(
+                    new String(password).getBytes("UTF-8"), salt, n, r, p, keyOutputBits / 8));
         } catch (UnsupportedEncodingException e) {
                 // Impossible according to the Java docs: UTF-8 is always supported.
-                throw new RuntimeException(e);
+                throw new IllegalStateException(e);
         }
     }
 
@@ -110,7 +109,7 @@ public class ScryptSecretKeyFactory extends SecretKeyFactorySpi {
 
         @Override
         public String getAlgorithm() {
-            // capitalised because BouncyCastle does it.
+            // Capitalised because BouncyCastle does it.
             return "SCRYPT";
         }
 
diff --git a/repackaged/common/src/test/java/com/android/org/conscrypt/javax/crypto/ScryptTest.java b/repackaged/common/src/test/java/com/android/org/conscrypt/javax/crypto/ScryptTest.java
index e5721673..b20c79d6 100644
--- a/repackaged/common/src/test/java/com/android/org/conscrypt/javax/crypto/ScryptTest.java
+++ b/repackaged/common/src/test/java/com/android/org/conscrypt/javax/crypto/ScryptTest.java
@@ -23,6 +23,8 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 
+import com.android.org.conscrypt.ScryptKeySpec;
+import com.android.org.conscrypt.TestUtils;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.security.spec.KeySpec;
@@ -31,18 +33,36 @@ import javax.crypto.Cipher;
 import javax.crypto.SecretKey;
 import javax.crypto.SecretKeyFactory;
 import javax.crypto.spec.SecretKeySpec;
-import com.android.org.conscrypt.ScryptKeySpec;
-import com.android.org.conscrypt.TestUtils;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 /**
  * @hide This class is not part of the Android public SDK API
  */
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public final class ScryptTest {
+    @Parameters(name = "{0}")
+    public static String[] params() {
+        return new String[] {"SCRYPT", "1.3.6.1.4.1.11591.4.11", "OID.1.3.6.1.4.1.11591.4.11"};
+    }
+
+    @Parameter public String alias;
+
+    // One of the test vectors from RFC 7914
+    private static final char[] TEST_PASSWORD = "password".toCharArray();
+    private static final byte[] TEST_SALT = "NaCl".getBytes(StandardCharsets.UTF_8);
+    private static final int TEST_COST = 1024;
+    private static final int TEST_BLOCKSIZE = 8;
+    private static final int TEST_PARALLELIZATION = 16;
+    private static final int TEST_KEY_SIZE = 512;
+    private static final byte[] TEST_KEY =
+            decodeHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b373162"
+                    + "2eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640");
+
     private final List<String[]> testVectors = readTestVectors();
 
     // Column indices in test vector CSV file
@@ -60,24 +80,13 @@ public final class ScryptTest {
 
     @Test
     public void smokeTest() throws Exception {
-        SecretKeyFactory factory = SecretKeyFactory.getInstance("Scrypt");
-        assertNotNull(factory);
-
-        // One of the test vectors from RFC 7914
-        ScryptKeySpec spec =
-                new ScryptKeySpec(
-                        "password".toCharArray(),
-                        "NaCl".getBytes(StandardCharsets.UTF_8),
-                        1024,
-                        8,
-                        16,
-                        512);
-        SecretKey key = factory.generateSecret(spec);
-        assertNotNull(key);
+        SecretKeyFactory factory = SecretKeyFactory.getInstance(alias);
+        assertEquals(alias, factory.getAlgorithm());
 
-        assertArrayEquals(
-                decodeHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640"),
-                key.getEncoded());
+        ScryptKeySpec spec = new ScryptKeySpec(TEST_PASSWORD, TEST_SALT, TEST_COST, TEST_BLOCKSIZE,
+                TEST_PARALLELIZATION, TEST_KEY_SIZE);
+        SecretKey key = factory.generateSecret(spec);
+        assertArrayEquals(TEST_KEY, key.getEncoded());
 
         // Convert for use with AES
         SecretKeySpec aesKey = makeAesKeySpec(key);
@@ -88,19 +97,13 @@ public final class ScryptTest {
 
     @Test
     public void duckTypingTest() throws Exception {
-        SecretKeyFactory factory = SecretKeyFactory.getInstance("Scrypt");
-        assertNotNull(factory);
+        SecretKeyFactory factory = SecretKeyFactory.getInstance(alias);
 
-        // One of the test vectors from RFC 7914
-        KeySpec spec =
-                new MyPrivateKeySpec(
-                        "password".toCharArray(), "NaCl".getBytes(StandardCharsets.UTF_8), 1024, 8, 16, 512);
-        SecretKey key = factory.generateSecret(spec);
-        assertNotNull(key);
+        KeySpec spec = new MyPrivateKeySpec(TEST_PASSWORD, TEST_SALT, TEST_COST, TEST_BLOCKSIZE,
+                TEST_PARALLELIZATION, TEST_KEY_SIZE);
 
-        assertArrayEquals(
-                decodeHex("fdbabe1c9d3472007856e7190d01e9fe7c6ad7cbc8237830e77376634b3731622eaf30d92e22a3886ff109279d9830dac727afb94a83ee6d8360cbdfa2cc0640"),
-                key.getEncoded());
+        SecretKey key = factory.generateSecret(spec);
+        assertArrayEquals(TEST_KEY, key.getEncoded());
     }
 
     private SecretKeySpec makeAesKeySpec(SecretKey key) {
@@ -112,7 +115,7 @@ public final class ScryptTest {
     }
 
     private void checkKeyIsUsableWithAes(SecretKeySpec spec) throws Exception {
-        // Terrible encryption mode but saves messing with IVs and padding which don't matter here.
+        // Terrible encryption mode but saves messing with IVs which don't matter here.
         Cipher cipher = Cipher.getInstance("AES/ECB/PKCS5Padding");
 
         cipher.init(Cipher.ENCRYPT_MODE, spec);
@@ -136,7 +139,7 @@ public final class ScryptTest {
             byte[] expectedBytes = decodeHex(entry[KEY_INDEX]);
 
             ScryptKeySpec spec = new ScryptKeySpec(password, salt, n, r, p, expectedBytes.length * 8);
-            SecretKeyFactory factory = SecretKeyFactory.getInstance("Scrypt");
+            SecretKeyFactory factory = SecretKeyFactory.getInstance(alias);
             SecretKey key = factory.generateSecret(spec);
             assertNotNull(key);
             assertArrayEquals(expectedBytes, key.getEncoded());