Merge pull request #1176 from bemusementpark/sync-everything

Synchronize usage of Cipher

app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt

@@ -14,6 +14,7 @@ import org.session.libsession.avatars.AvatarHelper
 import org.session.libsession.messaging.sending_receiving.attachments.AttachmentId
 import org.session.libsession.utilities.Conversions
 import org.session.libsession.utilities.Util
+import org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK
 import org.session.libsignal.crypto.kdf.HKDFv3
 import org.session.libsignal.utilities.ByteUtil
 import org.session.libsignal.utilities.Log
@@ -278,7 +279,7 @@ object FullBackupExporter {
         return false
     }
 
-    private class BackupFrameOutputStream : Closeable, Flushable {
+    private class BackupFrameOutputStream(outputStream: OutputStream, passphrase: String) : Closeable, Flushable {
 
         private val outputStream: OutputStream
         private var cipher: Cipher
@@ -289,7 +290,7 @@ object FullBackupExporter {
 
         private var counter: Int = 0
 
-        constructor(outputStream: OutputStream, passphrase: String) : super() {
+        init {
             try {
                 val salt = Util.getSecretBytes(32)
                 val key = BackupUtil.computeBackupKey(passphrase, salt)
@@ -381,18 +382,24 @@ object FullBackupExporter {
         private fun writeStream(inputStream: InputStream) {
             try {
                 Conversions.intToByteArray(iv, 0, counter++)
-                cipher.init(Cipher.ENCRYPT_MODE, SecretKeySpec(cipherKey, "AES"), IvParameterSpec(iv))
-                mac.update(iv)
-                val buffer = ByteArray(8192)
-                var read: Int
-                while (inputStream.read(buffer).also { read = it } != -1) {
-                    val ciphertext = cipher.update(buffer, 0, read)
-                    if (ciphertext != null) {
-                        outputStream.write(ciphertext)
-                        mac.update(ciphertext)
+                val remainder = synchronized(CIPHER_LOCK) {
+                    cipher.init(
+                        Cipher.ENCRYPT_MODE,
+                        SecretKeySpec(cipherKey, "AES"),
+                        IvParameterSpec(iv)
+                    )
+                    mac.update(iv)
+                    val buffer = ByteArray(8192)
+                    var read: Int
+                    while (inputStream.read(buffer).also { read = it } != -1) {
+                        val ciphertext = cipher.update(buffer, 0, read)
+                        if (ciphertext != null) {
+                            outputStream.write(ciphertext)
+                            mac.update(ciphertext)
+                        }
                     }
+                    cipher.doFinal()
                 }
-                val remainder = cipher.doFinal()
                 outputStream.write(remainder)
                 mac.update(remainder)
                 val attachmentDigest = mac.doFinal()
@@ -414,8 +421,10 @@ object FullBackupExporter {
         private fun write(out: OutputStream, frame: BackupFrame) {
             try {
                 Conversions.intToByteArray(iv, 0, counter++)
-                cipher.init(Cipher.ENCRYPT_MODE, SecretKeySpec(cipherKey, "AES"), IvParameterSpec(iv))
-                val frameCiphertext = cipher.doFinal(frame.toByteArray())
+                val frameCiphertext = synchronized(CIPHER_LOCK) {
+                    cipher.init(Cipher.ENCRYPT_MODE, SecretKeySpec(cipherKey, "AES"), IvParameterSpec(iv))
+                    cipher.doFinal(frame.toByteArray())
+                }
                 val frameMac = mac.doFinal(frameCiphertext)
                 val length = Conversions.intToByteArray(frameCiphertext.size + 10)
                 out.write(length)

app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.kt

@@ -12,6 +12,7 @@ import org.session.libsession.messaging.sending_receiving.attachments.Attachment
 import org.session.libsession.utilities.Address
 import org.session.libsession.utilities.Conversions
 import org.session.libsession.utilities.Util
+import org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK
 import org.session.libsignal.crypto.kdf.HKDFv3
 import org.session.libsignal.utilities.ByteUtil
 import org.session.libsignal.utilities.Log
@@ -243,7 +244,7 @@ object FullBackupImporter {
                 val split = ByteUtil.split(derived, 32, 32)
                 cipherKey = split[0]
                 macKey = split[1]
-                cipher = Cipher.getInstance("AES/CTR/NoPadding")
+                cipher = synchronized(CIPHER_LOCK) { Cipher.getInstance("AES/CTR/NoPadding") }
                 mac = Mac.getInstance("HmacSHA256")
                 mac.init(SecretKeySpec(macKey, "HmacSHA256"))
                 counter = Conversions.byteArrayToInt(iv)
@@ -269,20 +270,26 @@ object FullBackupImporter {
             var length = length
             try {
                 Conversions.intToByteArray(iv, 0, counter++)
-                cipher.init(Cipher.DECRYPT_MODE, SecretKeySpec(cipherKey, "AES"), IvParameterSpec(iv))
-                mac.update(iv)
-                val buffer = ByteArray(8192)
-                while (length > 0) {
-                    val read = inputStream.read(buffer, 0, Math.min(buffer.size, length))
-                    if (read == -1) throw IOException("File ended early!")
-                    mac.update(buffer, 0, read)
-                    val plaintext = cipher.update(buffer, 0, read)
-                    if (plaintext != null) {
-                        out.write(plaintext, 0, plaintext.size)
+                val plaintext = synchronized(CIPHER_LOCK) {
+                    cipher.init(
+                        Cipher.DECRYPT_MODE,
+                        SecretKeySpec(cipherKey, "AES"),
+                        IvParameterSpec(iv)
+                    )
+                    mac.update(iv)
+                    val buffer = ByteArray(8192)
+                    while (length > 0) {
+                        val read = inputStream.read(buffer, 0, Math.min(buffer.size, length))
+                        if (read == -1) throw IOException("File ended early!")
+                        mac.update(buffer, 0, read)
+                        val plaintext = cipher.update(buffer, 0, read)
+                        if (plaintext != null) {
+                            out.write(plaintext, 0, plaintext.size)
+                        }
+                        length -= read
                     }
-                    length -= read
+                    cipher.doFinal()
                 }
-                val plaintext = cipher.doFinal()
                 if (plaintext != null) {
                     out.write(plaintext, 0, plaintext.size)
                 }
@@ -325,8 +332,10 @@ object FullBackupImporter {
                     throw IOException("Bad MAC")
                 }
                 Conversions.intToByteArray(iv, 0, counter++)
-                cipher.init(Cipher.DECRYPT_MODE, SecretKeySpec(cipherKey, "AES"), IvParameterSpec(iv))
-                val plaintext = cipher.doFinal(frame, 0, frame.size - 10)
+                val plaintext = synchronized(CIPHER_LOCK) {
+                    cipher.init(Cipher.DECRYPT_MODE, SecretKeySpec(cipherKey, "AES"), IvParameterSpec(iv))
+                    cipher.doFinal(frame, 0, frame.size - 10)
+                }
                 BackupFrame.parseFrom(plaintext)
             } catch (e: Exception) {
                 when (e) {

app/src/main/java/org/thoughtcrime/securesms/crypto/KeyStoreHelper.java

@@ -1,13 +1,13 @@
 package org.thoughtcrime.securesms.crypto;
 
 
-import android.os.Build;
+import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK;
+
 import android.security.keystore.KeyGenParameterSpec;
 import android.security.keystore.KeyProperties;
 import android.util.Base64;
 
 import androidx.annotation.NonNull;
-import androidx.annotation.RequiresApi;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.core.JsonGenerator;
@@ -45,16 +45,14 @@ public final class KeyStoreHelper {
   private static final String ANDROID_KEY_STORE = "AndroidKeyStore";
   private static final String KEY_ALIAS         = "SignalSecret";
 
-  private static final Object lock = new Object();
-
   public static SealedData seal(@NonNull byte[] input) {
     SecretKey secretKey = getOrCreateKeyStoreEntry();
 
     try {
       // Cipher operations are not thread-safe so we synchronize over them through doFinal to
       // prevent crashes with quickly repeated encrypt/decrypt operations
       // https://github.com/mozilla-mobile/android-components/issues/5342
-      synchronized (lock) {
+      synchronized (CIPHER_LOCK) {
         Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
         cipher.init(Cipher.ENCRYPT_MODE, secretKey);
 
@@ -75,7 +73,7 @@ public static byte[] unseal(@NonNull SealedData sealedData) {
       // Cipher operations are not thread-safe so we synchronize over them through doFinal to
       // prevent crashes with quickly repeated encrypt/decrypt operations
       // https://github.com/mozilla-mobile/android-components/issues/5342
-      synchronized (lock) {
+      synchronized (CIPHER_LOCK) {
         Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding");
         cipher.init(Cipher.DECRYPT_MODE, secretKey, new GCMParameterSpec(128, sealedData.iv));
 
@@ -208,7 +206,5 @@ public byte[] deserialize(JsonParser p, DeserializationContext ctxt) throws IOEx
         return Base64.decode(p.getValueAsString(), Base64.NO_WRAP | Base64.NO_PADDING);
       }
     }
-
   }
-
 }

app/src/main/java/org/thoughtcrime/securesms/logging/LogFile.java

@@ -1,5 +1,7 @@
 package org.thoughtcrime.securesms.logging;
 
+import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK;
+
 import androidx.annotation.NonNull;
 
 import org.session.libsession.utilities.Conversions;
@@ -66,15 +68,17 @@ void writeEntry(@NonNull String entry) throws IOException {
 
       byte[] plaintext = entry.getBytes();
       try {
-        cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(secret, "AES"), new IvParameterSpec(ivBuffer));
+        synchronized (CIPHER_LOCK) {
+          cipher.init(Cipher.ENCRYPT_MODE, new SecretKeySpec(secret, "AES"), new IvParameterSpec(ivBuffer));
 
-        int    cipherLength = cipher.getOutputSize(plaintext.length);
-        byte[] ciphertext   = ciphertextBuffer.get(cipherLength);
-        cipherLength = cipher.doFinal(plaintext, 0, plaintext.length, ciphertext);
+          int cipherLength = cipher.getOutputSize(plaintext.length);
+          byte[] ciphertext = ciphertextBuffer.get(cipherLength);
+          cipherLength = cipher.doFinal(plaintext, 0, plaintext.length, ciphertext);
 
-        outputStream.write(ivBuffer);
-        outputStream.write(Conversions.intToByteArray(cipherLength));
-        outputStream.write(ciphertext, 0, cipherLength);
+          outputStream.write(ivBuffer);
+          outputStream.write(Conversions.intToByteArray(cipherLength));
+          outputStream.write(ciphertext, 0, cipherLength);
+        }
 
         outputStream.flush();
       } catch (ShortBufferException | InvalidAlgorithmParameterException | InvalidKeyException | BadPaddingException | IllegalBlockSizeException e) {
@@ -134,10 +138,11 @@ String readEntry() throws IOException {
         Util.readFully(inputStream, ciphertext, length);
 
         try {
-          cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(secret, "AES"), new IvParameterSpec(ivBuffer));
-          byte[] plaintext = cipher.doFinal(ciphertext, 0, length);
-
-          return new String(plaintext);
+          synchronized (CIPHER_LOCK) {
+            cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(secret, "AES"), new IvParameterSpec(ivBuffer));
+            byte[] plaintext = cipher.doFinal(ciphertext, 0, length);
+            return new String(plaintext);
+          }
         } catch (InvalidKeyException | InvalidAlgorithmParameterException | IllegalBlockSizeException | BadPaddingException e) {
           throw new AssertionError(e);
         }

libsession/src/main/java/org/session/libsession/utilities/AESGCM.kt

@@ -1,6 +1,7 @@
 package org.session.libsession.utilities
 
 import androidx.annotation.WorkerThread
+import org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK
 import org.session.libsignal.utilities.ByteUtil
 import org.session.libsignal.utilities.Util
 import org.session.libsignal.utilities.Hex
@@ -27,9 +28,11 @@ internal object AESGCM {
     internal fun decrypt(ivAndCiphertext: ByteArray, symmetricKey: ByteArray): ByteArray {
         val iv = ivAndCiphertext.sliceArray(0 until ivSize)
         val ciphertext = ivAndCiphertext.sliceArray(ivSize until ivAndCiphertext.count())
-        val cipher = Cipher.getInstance("AES/GCM/NoPadding")
-        cipher.init(Cipher.DECRYPT_MODE, SecretKeySpec(symmetricKey, "AES"), GCMParameterSpec(gcmTagSize, iv))
-        return cipher.doFinal(ciphertext)
+        synchronized(CIPHER_LOCK) {
+            val cipher = Cipher.getInstance("AES/GCM/NoPadding")
+            cipher.init(Cipher.DECRYPT_MODE, SecretKeySpec(symmetricKey, "AES"), GCMParameterSpec(gcmTagSize, iv))
+            return cipher.doFinal(ciphertext)
+        }
     }
 
     /**
@@ -47,9 +50,11 @@ internal object AESGCM {
      */
     internal fun encrypt(plaintext: ByteArray, symmetricKey: ByteArray): ByteArray {
         val iv = Util.getSecretBytes(ivSize)
-        val cipher = Cipher.getInstance("AES/GCM/NoPadding")
-        cipher.init(Cipher.ENCRYPT_MODE, SecretKeySpec(symmetricKey, "AES"), GCMParameterSpec(gcmTagSize, iv))
-        return ByteUtil.combine(iv, cipher.doFinal(plaintext))
+        synchronized(CIPHER_LOCK) {
+            val cipher = Cipher.getInstance("AES/GCM/NoPadding")
+            cipher.init(Cipher.ENCRYPT_MODE, SecretKeySpec(symmetricKey, "AES"), GCMParameterSpec(gcmTagSize, iv))
+            return ByteUtil.combine(iv, cipher.doFinal(plaintext))
+        }
     }
 
     /**

libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java

@@ -0,0 +1,8 @@
+package org.session.libsignal.crypto;
+
+public class CipherUtil {
+    // Cipher operations are not thread-safe so we synchronize over them through doFinal to
+    // prevent crashes with quickly repeated encrypt/decrypt operations
+    // https://github.com/mozilla-mobile/android-components/issues/5342
+    public static final Object CIPHER_LOCK = new Object();
+}

libsignal/src/main/java/org/session/libsignal/crypto/kdf/HKDF.java

@@ -39,9 +39,7 @@ private byte[] extract(byte[] salt, byte[] inputKeyMaterial) {
       Mac mac = Mac.getInstance("HmacSHA256");
       mac.init(new SecretKeySpec(salt, "HmacSHA256"));
       return mac.doFinal(inputKeyMaterial);
-    } catch (NoSuchAlgorithmException e) {
-      throw new AssertionError(e);
-    } catch (InvalidKeyException e) {
+    } catch (NoSuchAlgorithmException | InvalidKeyException e) {
       throw new AssertionError(e);
     }
   }
@@ -73,9 +71,7 @@ private byte[] expand(byte[] prk, byte[] info, int outputSize) {
       }
 
       return results.toByteArray();
-    } catch (NoSuchAlgorithmException e) {
-      throw new AssertionError(e);
-    } catch (InvalidKeyException e) {
+    } catch (NoSuchAlgorithmException | InvalidKeyException e) {
       throw new AssertionError(e);
     }
   }