Replace weak crypto implementation (#158)

YkSix · web-flow · commit aefcdaa5cf60 · 2023-12-06T10:23:38.000+08:00
Replace crypto implementation with stronger key generation method.
diff --git a/line-sdk/build.gradle b/line-sdk/build.gradle
@@ -7,15 +7,15 @@ apply plugin: 'maven-publish'
 apply plugin: 'signing'
 
 group = "com.linecorp.linesdk"
-version = "5.9.1"
+version = "5.10.0"
 
 android {
     compileSdk 33
 
     defaultConfig {
         minSdkVersion 24
         targetSdkVersion 33
-        versionCode 5_09_00
+        versionCode 5_10_00
         versionName version
 
         consumerProguardFiles 'consumer-proguard-rules.pro'
diff --git a/line-sdk/src/main/java/com/linecorp/android/security/encryption/CipherData.kt b/line-sdk/src/main/java/com/linecorp/android/security/encryption/CipherData.kt
@@ -0,0 +1,35 @@
+package com.linecorp.android.security.encryption
+
+import android.util.Base64
+
+class CipherData(
+    val encryptedData: ByteArray,
+    val initialVector: ByteArray,
+    val hmacValue: ByteArray,
+) {
+    fun encodeToBase64String(): String =
+        listOf(encryptedData, initialVector, hmacValue)
+            .joinToString(SEPARATOR) { it.encodeBase64() }
+
+    companion object {
+        private const val SEPARATOR = ";"
+        private const val SIZE_DATA_TYPES = 3
+
+        fun decodeFromBase64String(cipherDataBase64String: String): CipherData {
+            val parts = cipherDataBase64String.split(SEPARATOR)
+            require(parts.size == SIZE_DATA_TYPES) {
+                "Failed to split encrypted text `$cipherDataBase64String`"
+            }
+
+            return CipherData(
+                encryptedData = parts[0].decodeBase64(),
+                initialVector = parts[1].decodeBase64(),
+                hmacValue = parts[2].decodeBase64()
+            )
+        }
+    }
+}
+
+private fun ByteArray.encodeBase64(): String = Base64.encodeToString(this, Base64.NO_WRAP)
+
+private fun String.decodeBase64(): ByteArray = Base64.decode(this, Base64.NO_WRAP)
diff --git a/line-sdk/src/main/java/com/linecorp/android/security/encryption/StringAesCipher.kt b/line-sdk/src/main/java/com/linecorp/android/security/encryption/StringAesCipher.kt
@@ -0,0 +1,187 @@
+package com.linecorp.android.security.encryption
+
+import android.content.Context
+import android.security.keystore.KeyGenParameterSpec
+import android.security.keystore.KeyProperties
+import android.security.keystore.KeyProperties.PURPOSE_DECRYPT
+import android.security.keystore.KeyProperties.PURPOSE_ENCRYPT
+import android.security.keystore.KeyProperties.PURPOSE_SIGN
+import android.security.keystore.KeyProperties.PURPOSE_VERIFY
+import java.security.KeyStore
+import java.security.MessageDigest
+import javax.crypto.Cipher
+import javax.crypto.KeyGenerator
+import javax.crypto.Mac
+import javax.crypto.SecretKey
+import javax.crypto.spec.IvParameterSpec
+
+/**
+ * AES cipher by AndroidKeyStore
+ */
+class StringAesCipher : StringCipher {
+    private val keyStore: KeyStore by lazy {
+        KeyStore.getInstance(ANDROID_KEY_STORE).also {
+            it.load(null)
+        }
+    }
+
+    private lateinit var hmac: Mac
+
+    override fun initialize(context: Context) {
+        if (::hmac.isInitialized) {
+            return
+        }
+
+        synchronized(this) {
+            getAesSecretKey()
+            val integrityKey = getIntegrityKey()
+
+            hmac = Mac.getInstance(KeyProperties.KEY_ALGORITHM_HMAC_SHA256).apply {
+                init(integrityKey)
+            }
+        }
+    }
+
+    override fun encrypt(context: Context, plainText: String): String {
+        synchronized(this) {
+            initialize(context)
+
+            try {
+                val secretKey = getAesSecretKey()
+
+                val cipher = Cipher.getInstance(TRANSFORMATION_FORMAT).apply {
+                    init(Cipher.ENCRYPT_MODE, secretKey)
+                }
+                val encryptedData: ByteArray = cipher.doFinal(plainText.toByteArray())
+
+                return CipherData(
+                    encryptedData = encryptedData,
+                    initialVector = cipher.iv,
+                    hmacValue = hmac.calculateHmacValue(encryptedData, cipher.iv)
+                ).encodeToBase64String()
+            } catch (e: Exception) {
+                throw EncryptionException("Failed to encrypt", e)
+            }
+        }
+    }
+
+    override fun decrypt(context: Context, cipherText: String): String {
+        synchronized(this) {
+            try {
+                val secretKey = getAesSecretKey()
+
+                val cipherData = CipherData.decodeFromBase64String(cipherText)
+
+                cipherData.verifyHmacValue(hmac)
+
+                val ivSpec = IvParameterSpec(cipherData.initialVector)
+
+                return Cipher.getInstance(TRANSFORMATION_FORMAT)
+                    .apply { init(Cipher.DECRYPT_MODE, secretKey, ivSpec) }
+                    .run { doFinal(cipherData.encryptedData) }
+                    .let {
+                        String(it)
+                    }
+            } catch (e: Exception) {
+                throw EncryptionException("Failed to decrypt", e)
+            }
+        }
+    }
+
+    private fun getAesSecretKey(): SecretKey {
+        return if (keyStore.containsAlias(AES_KEY_ALIAS)) {
+            val secretKeyEntry =
+                keyStore.getEntry(AES_KEY_ALIAS, null) as KeyStore.SecretKeyEntry
+
+            secretKeyEntry.secretKey
+        } else {
+            createAesKey()
+        }
+    }
+
+    private fun getIntegrityKey(): SecretKey {
+        return if (keyStore.containsAlias(INTEGRITY_KEY_ALIAS)) {
+            val secretKeyEntry =
+                keyStore.getEntry(INTEGRITY_KEY_ALIAS, null) as KeyStore.SecretKeyEntry
+
+            secretKeyEntry.secretKey
+        } else {
+            createIntegrityKey()
+        }
+    }
+
+    /**
+     * Create a new AES key in the Android KeyStore. This key will be used for
+     * encrypting and decrypting data. The key is generated with a size of 256 bits,
+     * using the CBC block mode and PKCS7 padding.
+     */
+    private fun createAesKey(): SecretKey {
+        val keyGenerator = KeyGenerator
+            .getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEY_STORE)
+        val keyGenParameterSpec = KeyGenParameterSpec.Builder(
+            AES_KEY_ALIAS,
+            PURPOSE_ENCRYPT or PURPOSE_DECRYPT
+        )
+            .setKeySize(KEY_SIZE_IN_BIT)
+            .setBlockModes(KeyProperties.BLOCK_MODE_CBC)
+            .setEncryptionPaddings(KeyProperties.ENCRYPTION_PADDING_PKCS7)
+            .build()
+
+        keyGenerator.run {
+            init(keyGenParameterSpec)
+            return generateKey()
+        }
+    }
+
+    private fun createIntegrityKey(): SecretKey {
+        val keyGenerator = KeyGenerator
+            .getInstance(KeyProperties.KEY_ALGORITHM_HMAC_SHA256, ANDROID_KEY_STORE)
+        val keyGenParameterSpec = KeyGenParameterSpec.Builder(
+            INTEGRITY_KEY_ALIAS,
+            PURPOSE_SIGN or PURPOSE_VERIFY
+        )
+            .build()
+
+        keyGenerator.run {
+            init(keyGenParameterSpec)
+            return generateKey()
+        }
+    }
+
+    private fun Mac.calculateHmacValue(
+        encryptedData: ByteArray,
+        initialVector: ByteArray
+    ): ByteArray = doFinal(encryptedData + initialVector)
+
+    /**
+     * Validate the HMAC value
+     *
+     * @throws SecurityException if the HMAC value doesn't match with [encryptedData]
+     */
+    private fun CipherData.verifyHmacValue(mac: Mac) {
+        val expectedHmacValue: ByteArray = mac.calculateHmacValue(
+            encryptedData = encryptedData,
+            initialVector = initialVector
+        )
+
+        if (!MessageDigest.isEqual(expectedHmacValue, hmacValue)) {
+            throw SecurityException("Cipher text has been tampered with.")
+        }
+    }
+
+    companion object {
+        private const val AES_KEY_ALIAS =
+            "com.linecorp.android.security.encryption.StringAesCipher"
+
+        private const val INTEGRITY_KEY_ALIAS =
+            "com.linecorp.android.security.encryption.StringAesCipher.INTEGRITY_KEY"
+
+        private const val ANDROID_KEY_STORE = "AndroidKeyStore"
+        private const val KEY_SIZE_IN_BIT = 256
+
+        private const val TRANSFORMATION_FORMAT =
+            KeyProperties.KEY_ALGORITHM_AES +
+                "/${KeyProperties.BLOCK_MODE_CBC}" +
+                "/${KeyProperties.ENCRYPTION_PADDING_PKCS7}"
+    }
+}
diff --git a/line-sdk/src/main/java/com/linecorp/android/security/encryption/StringCipher.kt b/line-sdk/src/main/java/com/linecorp/android/security/encryption/StringCipher.kt
@@ -0,0 +1,12 @@
+package com.linecorp.android.security.encryption
+
+import android.content.Context
+
+interface StringCipher {
+
+    fun initialize(context: Context)
+
+    fun encrypt(context: Context, plainText: String): String
+
+    fun decrypt(context: Context, cipherText: String): String
+}
diff --git a/line-sdk/src/main/java/com/linecorp/android/security/encryption/StringCipherDeprecated.java b/line-sdk/src/main/java/com/linecorp/android/security/encryption/StringCipherDeprecated.java
@@ -5,11 +5,12 @@
 import android.content.SharedPreferences;
 import android.os.Build;
 import android.provider.Settings;
+import android.text.TextUtils;
+import android.util.Base64;
+
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.WorkerThread;
-import android.text.TextUtils;
-import android.util.Base64;
 
 import java.io.UnsupportedEncodingException;
 import java.security.InvalidAlgorithmParameterException;
@@ -45,9 +46,12 @@
  * Either first access of {@link #encrypt(Context, String)}, {@link #decrypt(Context, String)} or
  * {@link #initialize(Context)} is very slow because there is secret key generation with PBKDF2.
  * We recommend that you initialize an instance of this class beforehand and cache it.
+ *
+ * @deprecated This class is obsolete. Use {@link StringAesCipher} as its replacement.
  */
+@Deprecated
 @WorkerThread
-public class StringCipher {
+public class StringCipherDeprecated implements StringCipher {
     // for PBKDF
     private static final int DEFAULT_ITERATIONS = 10000;
 
@@ -79,7 +83,7 @@ public class StringCipher {
     @Nullable
     private SecretKeys secretKeys;
 
-    public StringCipher(@NonNull String sharedPreferenceName) {
+    public StringCipherDeprecated(@NonNull String sharedPreferenceName) {
         this(sharedPreferenceName, DEFAULT_ITERATIONS, false);
     }
 
@@ -92,7 +96,7 @@ public StringCipher(@NonNull String sharedPreferenceName) {
                      Note : This field should always be false as it is deprecated and
                      returns UNKNOWN in some cases from Android SDK >= 27
      */
-    public StringCipher(
+    public StringCipherDeprecated(
             @NonNull String sharedPreferenceName,
             int pbkdf2IterationCount,
             boolean isSerialIncludedInDevicePackageSpecificId) {
@@ -111,6 +115,7 @@ public StringCipher(
         }
     }
 
+    @Override
     public void initialize(@NonNull Context context) {
         synchronized (syncObject) {
             if (secretKeys == null) {
@@ -119,6 +124,7 @@ public void initialize(@NonNull Context context) {
         }
     }
 
+    @Override
     @NonNull
     public String encrypt(@NonNull Context context, @NonNull String plainText) {
         synchronized (syncObject) {
@@ -160,12 +166,13 @@ public String encrypt(@NonNull Context context, @NonNull String plainText) {
         }
     }
 
+    @Override
     @NonNull
-    public String decrypt(@NonNull Context context, @NonNull String b64CipherText) {
+    public String decrypt(@NonNull Context context, @NonNull String cipherText) {
         synchronized (syncObject) {
             initialize(context);
             try {
-                byte[] cipherTextAndMac = Base64.decode(b64CipherText, Base64.DEFAULT);
+                byte[] cipherTextAndMac = Base64.decode(cipherText, Base64.DEFAULT);
                 // get mac, last 32 bytes
                 int idx = cipherTextAndMac.length - HMAC_SIZE_IN_BYTE;
                 byte[] mac = Arrays.copyOfRange(cipherTextAndMac, idx, cipherTextAndMac.length);
@@ -219,7 +226,7 @@ private SecretKeys getSecretKeys(@NonNull Context context) {
         SecretKey encryptionKey = new SecretKeySpec(
                 Arrays.copyOfRange(keyBytes, 0, AES_KEY_SIZE_IN_BIT / 8), "AES");
         SecretKey integrityKey = new SecretKeySpec(
-                Arrays.copyOfRange(keyBytes, HMAC_KEY_SIZE_IN_BIT / 8, keyBytes.length), "HmacSHA256");
+                Arrays.copyOfRange(keyBytes, AES_KEY_SIZE_IN_BIT / 8, keyBytes.length), "HmacSHA256");
         return new SecretKeys(encryptionKey, integrityKey);
     }
 
diff --git a/line-sdk/src/main/java/com/linecorp/linesdk/api/internal/LineApiClientImpl.java b/line-sdk/src/main/java/com/linecorp/linesdk/api/internal/LineApiClientImpl.java
@@ -2,6 +2,9 @@
 
 import android.text.TextUtils;
 
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+
 import com.linecorp.linesdk.FriendSortField;
 import com.linecorp.linesdk.GetFriendsResponse;
 import com.linecorp.linesdk.GetGroupsResponse;
@@ -29,9 +32,6 @@
 
 import java.util.List;
 
-import androidx.annotation.NonNull;
-import androidx.annotation.Nullable;
-
 /**
  * Implementation of {@link LineApiClient}.
  */
diff --git a/line-sdk/src/main/java/com/linecorp/linesdk/internal/AccessTokenCache.java b/line-sdk/src/main/java/com/linecorp/linesdk/internal/AccessTokenCache.java
@@ -4,14 +4,14 @@
 import android.content.SharedPreferences;
 import android.text.TextUtils;
 
-import com.linecorp.android.security.encryption.EncryptionException;
-import com.linecorp.android.security.encryption.StringCipher;
-import com.linecorp.linesdk.utils.ObjectUtils;
-
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
+import com.linecorp.android.security.encryption.EncryptionException;
+import com.linecorp.android.security.encryption.StringCipher;
+import com.linecorp.linesdk.utils.ObjectUtils;
+
 /**
  * Class to cache {@link InternalAccessToken}.
  */
diff --git a/line-sdk/src/main/java/com/linecorp/linesdk/internal/EncryptorHolder.java b/line-sdk/src/main/java/com/linecorp/linesdk/internal/EncryptorHolder.java
diff --git a/line-sdk/src/test/java/com/linecorp/linesdk/TestStringCipher.java b/line-sdk/src/test/java/com/linecorp/linesdk/TestStringCipher.java
diff --git a/line-sdk/src/test/java/com/linecorp/linesdk/internal/AccessTokenCacheTest.java b/line-sdk/src/test/java/com/linecorp/linesdk/internal/AccessTokenCacheTest.java
