From d3ce899a8056c9b9d307f9993845603b0016c058 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 5 May 2023 12:07:19 +0930 Subject: [PATCH 1/4] Synchronize all Cipher#doFinal --- .../securesms/backup/FullBackupExporter.kt | 35 +++++++++------ .../session/libsession/utilities/AESGCM.kt | 17 ++++--- .../session/libsignal/crypto/CipherUtil.java | 5 +++ .../session/libsignal/crypto/DiffieHellman.kt | 45 ------------------- .../session/libsignal/crypto/kdf/HKDF.java | 8 +--- .../streams/AttachmentCipherOutputStream.java | 15 ++++--- .../streams/ProfileCipherInputStream.java | 30 +++++++------ .../streams/ProfileCipherOutputStream.java | 16 ++++--- 8 files changed, 75 insertions(+), 96 deletions(-) create mode 100644 libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java delete mode 100644 libsignal/src/main/java/org/session/libsignal/crypto/DiffieHellman.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt index 6b5d47a2e6..783468eee7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt +++ b/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 @@ -289,7 +290,7 @@ object FullBackupExporter { private var counter: Int = 0 - constructor(outputStream: OutputStream, passphrase: String) : super() { + private constructor(outputStream: OutputStream, passphrase: String) : super() { 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) diff --git a/libsession/src/main/java/org/session/libsession/utilities/AESGCM.kt b/libsession/src/main/java/org/session/libsession/utilities/AESGCM.kt index 225900b096..4a6a588dc2 100644 --- a/libsession/src/main/java/org/session/libsession/utilities/AESGCM.kt +++ b/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)) + } } /** diff --git a/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java b/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java new file mode 100644 index 0000000000..d3423a0bd4 --- /dev/null +++ b/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java @@ -0,0 +1,5 @@ +package org.session.libsignal.crypto; + +public class CipherUtil { + public static final Object CIPHER_LOCK = new Object(); +} diff --git a/libsignal/src/main/java/org/session/libsignal/crypto/DiffieHellman.kt b/libsignal/src/main/java/org/session/libsignal/crypto/DiffieHellman.kt deleted file mode 100644 index 2b613247bf..0000000000 --- a/libsignal/src/main/java/org/session/libsignal/crypto/DiffieHellman.kt +++ /dev/null @@ -1,45 +0,0 @@ -package org.session.libsignal.crypto - -import org.whispersystems.curve25519.Curve25519 -import org.session.libsignal.utilities.Util -import javax.crypto.Cipher -import javax.crypto.spec.IvParameterSpec -import javax.crypto.spec.SecretKeySpec - -object DiffieHellman { - private val cipher = Cipher.getInstance("AES/CBC/PKCS5Padding") - private val curve = Curve25519.getInstance(Curve25519.BEST) - private val ivSize = 16 - - @JvmStatic @Throws - fun encrypt(plaintext: ByteArray, symmetricKey: ByteArray): ByteArray { - val iv = Util.getSecretBytes(ivSize) - val ivSpec = IvParameterSpec(iv) - val secretKeySpec = SecretKeySpec(symmetricKey, "AES") - cipher.init(Cipher.ENCRYPT_MODE, secretKeySpec, ivSpec) - val ciphertext = cipher.doFinal(plaintext) - return iv + ciphertext - } - - @JvmStatic @Throws - fun encrypt(plaintext: ByteArray, publicKey: ByteArray, privateKey: ByteArray): ByteArray { - val symmetricKey = curve.calculateAgreement(publicKey, privateKey) - return encrypt(plaintext, symmetricKey) - } - - @JvmStatic @Throws - fun decrypt(ivAndCiphertext: ByteArray, symmetricKey: ByteArray): ByteArray { - val iv = ivAndCiphertext.sliceArray(0 until ivSize) - val ciphertext = ivAndCiphertext.sliceArray(ivSize until ivAndCiphertext.size) - val ivSpec = IvParameterSpec(iv) - val secretKeySpec = SecretKeySpec(symmetricKey, "AES") - cipher.init(Cipher.DECRYPT_MODE, secretKeySpec, ivSpec) - return cipher.doFinal(ciphertext) - } - - @JvmStatic @Throws - fun decrypt(ivAndCiphertext: ByteArray, publicKey: ByteArray, privateKey: ByteArray): ByteArray { - val symmetricKey = curve.calculateAgreement(publicKey, privateKey) - return decrypt(ivAndCiphertext, symmetricKey) - } -} diff --git a/libsignal/src/main/java/org/session/libsignal/crypto/kdf/HKDF.java b/libsignal/src/main/java/org/session/libsignal/crypto/kdf/HKDF.java index 3295f06e52..73c87c075d 100644 --- a/libsignal/src/main/java/org/session/libsignal/crypto/kdf/HKDF.java +++ b/libsignal/src/main/java/org/session/libsignal/crypto/kdf/HKDF.java @@ -39,9 +39,7 @@ public abstract class HKDF { 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 @@ public abstract class HKDF { } return results.toByteArray(); - } catch (NoSuchAlgorithmException e) { - throw new AssertionError(e); - } catch (InvalidKeyException e) { + } catch (NoSuchAlgorithmException | InvalidKeyException e) { throw new AssertionError(e); } } diff --git a/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherOutputStream.java b/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherOutputStream.java index 91c3563700..2f58c84c78 100644 --- a/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherOutputStream.java +++ b/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherOutputStream.java @@ -6,6 +6,8 @@ package org.session.libsignal.streams; +import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK; + import org.session.libsignal.utilities.Util; import java.io.IOException; @@ -68,16 +70,17 @@ public class AttachmentCipherOutputStream extends DigestingOutputStream { @Override public void flush() throws IOException { try { - byte[] ciphertext = cipher.doFinal(); + byte[] ciphertext; + synchronized (CIPHER_LOCK) { + ciphertext = cipher.doFinal(); + } byte[] auth = mac.doFinal(ciphertext); super.write(ciphertext); super.write(auth); super.flush(); - } catch (IllegalBlockSizeException e) { - throw new AssertionError(e); - } catch (BadPaddingException e) { + } catch (IllegalBlockSizeException | BadPaddingException e) { throw new AssertionError(e); } } @@ -97,9 +100,7 @@ public class AttachmentCipherOutputStream extends DigestingOutputStream { private Cipher initializeCipher() { try { return Cipher.getInstance("AES/CBC/PKCS5Padding"); - } catch (NoSuchAlgorithmException e) { - throw new AssertionError(e); - } catch (NoSuchPaddingException e) { + } catch (NoSuchAlgorithmException | NoSuchPaddingException e) { throw new AssertionError(e); } } diff --git a/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherInputStream.java b/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherInputStream.java index aa15eb00c6..19996c17cb 100644 --- a/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherInputStream.java +++ b/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherInputStream.java @@ -1,5 +1,7 @@ package org.session.libsignal.streams; +import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK; + import org.session.libsignal.utilities.Util; import java.io.FilterInputStream; @@ -62,23 +64,23 @@ public class ProfileCipherInputStream extends FilterInputStream { byte[] ciphertext = new byte[outputLength / 2]; int read = in.read(ciphertext, 0, ciphertext.length); - if (read == -1) { - if (cipher.getOutputSize(0) > outputLength) { - throw new AssertionError("Need: " + cipher.getOutputSize(0) + " but only have: " + outputLength); - } + synchronized (CIPHER_LOCK) { + if (read == -1) { + if (cipher.getOutputSize(0) > outputLength) { + throw new AssertionError("Need: " + cipher.getOutputSize(0) + " but only have: " + outputLength); + } - finished = true; - return cipher.doFinal(output, outputOffset); - } else { - if (cipher.getOutputSize(read) > outputLength) { - throw new AssertionError("Need: " + cipher.getOutputSize(read) + " but only have: " + outputLength); - } + finished = true; + return cipher.doFinal(output, outputOffset); + } else { + if (cipher.getOutputSize(read) > outputLength) { + throw new AssertionError("Need: " + cipher.getOutputSize(read) + " but only have: " + outputLength); + } - return cipher.update(ciphertext, 0, read, output, outputOffset); + return cipher.update(ciphertext, 0, read, output, outputOffset); + } } - } catch (IllegalBlockSizeException e) { - throw new AssertionError(e); - } catch(ShortBufferException e) { + } catch (IllegalBlockSizeException | ShortBufferException e) { throw new AssertionError(e); } catch (BadPaddingException e) { throw new IOException(e); diff --git a/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherOutputStream.java b/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherOutputStream.java index 9d4e13a0c2..f47a5f72b6 100644 --- a/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherOutputStream.java +++ b/libsignal/src/main/java/org/session/libsignal/streams/ProfileCipherOutputStream.java @@ -1,5 +1,7 @@ package org.session.libsignal.streams; +import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK; + import java.io.IOException; import java.io.OutputStream; import java.security.InvalidAlgorithmParameterException; @@ -54,20 +56,24 @@ public class ProfileCipherOutputStream extends DigestingOutputStream { byte[] input = new byte[1]; input[0] = (byte)b; - byte[] output = cipher.update(input); + byte[] output; + synchronized (CIPHER_LOCK) { + output = cipher.update(input); + } super.write(output); } @Override public void flush() throws IOException { try { - byte[] output = cipher.doFinal(); + byte[] output; + synchronized (CIPHER_LOCK) { + output = cipher.doFinal(); + } super.write(output); super.flush(); - } catch (BadPaddingException e) { - throw new AssertionError(e); - } catch (IllegalBlockSizeException e) { + } catch (BadPaddingException | IllegalBlockSizeException e) { throw new AssertionError(e); } } From a9078c8d08939604087b21c73446ba66bfe3248d Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 5 May 2023 12:32:54 +0930 Subject: [PATCH 2/4] ...and the rest --- .../securesms/backup/FullBackupImporter.kt | 39 ++++++++++++------- .../securesms/crypto/KeyStoreHelper.java | 30 +++++++------- .../securesms/logging/LogFile.java | 27 +++++++------ .../streams/AttachmentCipherInputStream.java | 35 +++++++---------- 4 files changed, 68 insertions(+), 63 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.kt index b40c049bc2..7d2daa910e 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.kt +++ b/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) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/crypto/KeyStoreHelper.java b/app/src/main/java/org/thoughtcrime/securesms/crypto/KeyStoreHelper.java index 43e9865598..c0372cc7f1 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/crypto/KeyStoreHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/crypto/KeyStoreHelper.java @@ -1,6 +1,8 @@ package org.thoughtcrime.securesms.crypto; +import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK; + import android.os.Build; import android.security.keystore.KeyGenParameterSpec; import android.security.keystore.KeyProperties; @@ -45,44 +47,44 @@ public final class KeyStoreHelper { private static final String ANDROID_KEY_STORE = "AndroidKeyStore"; private static final String KEY_ALIAS = "SignalSecret"; - @RequiresApi(Build.VERSION_CODES.M) public static SealedData seal(@NonNull byte[] input) { SecretKey secretKey = getOrCreateKeyStoreEntry(); try { - Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); - cipher.init(Cipher.ENCRYPT_MODE, secretKey); + synchronized (CIPHER_LOCK) { + Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); + cipher.init(Cipher.ENCRYPT_MODE, secretKey); - byte[] iv = cipher.getIV(); - byte[] data = cipher.doFinal(input); + byte[] iv = cipher.getIV(); + byte[] data = cipher.doFinal(input); - return new SealedData(iv, data); + return new SealedData(iv, data); + } } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | IllegalBlockSizeException | BadPaddingException e) { throw new AssertionError(e); } } - @RequiresApi(Build.VERSION_CODES.M) public static byte[] unseal(@NonNull SealedData sealedData) { SecretKey secretKey = getKeyStoreEntry(); try { - Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); - cipher.init(Cipher.DECRYPT_MODE, secretKey, new GCMParameterSpec(128, sealedData.iv)); + synchronized (CIPHER_LOCK) { + Cipher cipher = Cipher.getInstance("AES/GCM/NoPadding"); + cipher.init(Cipher.DECRYPT_MODE, secretKey, new GCMParameterSpec(128, sealedData.iv)); - return cipher.doFinal(sealedData.data); + return cipher.doFinal(sealedData.data); + } } catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException | InvalidAlgorithmParameterException | IllegalBlockSizeException | BadPaddingException e) { throw new AssertionError(e); } } - @RequiresApi(Build.VERSION_CODES.M) private static SecretKey getOrCreateKeyStoreEntry() { if (hasKeyStoreEntry()) return getKeyStoreEntry(); else return createKeyStoreEntry(); } - @RequiresApi(Build.VERSION_CODES.M) private static SecretKey createKeyStoreEntry() { try { KeyGenerator keyGenerator = KeyGenerator.getInstance(KeyProperties.KEY_ALGORITHM_AES, ANDROID_KEY_STORE); @@ -99,7 +101,6 @@ public final class KeyStoreHelper { } } - @RequiresApi(Build.VERSION_CODES.M) private static SecretKey getKeyStoreEntry() { KeyStore keyStore = getKeyStore(); @@ -137,7 +138,6 @@ public final class KeyStoreHelper { } } - @RequiresApi(Build.VERSION_CODES.M) private static boolean hasKeyStoreEntry() { try { KeyStore ks = KeyStore.getInstance(ANDROID_KEY_STORE); @@ -202,7 +202,5 @@ public final class KeyStoreHelper { return Base64.decode(p.getValueAsString(), Base64.NO_WRAP | Base64.NO_PADDING); } } - } - } diff --git a/app/src/main/java/org/thoughtcrime/securesms/logging/LogFile.java b/app/src/main/java/org/thoughtcrime/securesms/logging/LogFile.java index f0c083ca1d..909f19e08c 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/logging/LogFile.java +++ b/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 @@ class LogFile { 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 @@ class LogFile { 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); } diff --git a/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherInputStream.java b/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherInputStream.java index 3158d35f73..fd3c8123df 100644 --- a/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherInputStream.java +++ b/libsignal/src/main/java/org/session/libsignal/streams/AttachmentCipherInputStream.java @@ -6,6 +6,8 @@ package org.session.libsignal.streams; +import static org.session.libsignal.crypto.CipherUtil.CIPHER_LOCK; + import org.session.libsignal.exceptions.InvalidMacException; import org.session.libsignal.exceptions.InvalidMessageException; import org.session.libsignal.utilities.Util; @@ -92,19 +94,15 @@ public class AttachmentCipherInputStream extends FilterInputStream { byte[] iv = new byte[BLOCK_SIZE]; readFully(iv); - this.cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); - this.cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(cipherKey, "AES"), new IvParameterSpec(iv)); + synchronized (CIPHER_LOCK) { + this.cipher = Cipher.getInstance("AES/CBC/PKCS5Padding"); + this.cipher.init(Cipher.DECRYPT_MODE, new SecretKeySpec(cipherKey, "AES"), new IvParameterSpec(iv)); + } this.done = false; this.totalRead = 0; this.totalDataSize = totalDataSize; - } catch (NoSuchAlgorithmException e) { - throw new AssertionError(e); - } catch (InvalidKeyException e) { - throw new AssertionError(e); - } catch (NoSuchPaddingException e) { - throw new AssertionError(e); - } catch (InvalidAlgorithmParameterException e) { + } catch (NoSuchAlgorithmException | InvalidKeyException | NoSuchPaddingException | InvalidAlgorithmParameterException e) { throw new AssertionError(e); } } @@ -141,15 +139,12 @@ public class AttachmentCipherInputStream extends FilterInputStream { private int readFinal(byte[] buffer, int offset, int length) throws IOException { try { - int flourish = cipher.doFinal(buffer, offset); - - done = true; - return flourish; - } catch (IllegalBlockSizeException e) { - throw new IOException(e); - } catch (BadPaddingException e) { - throw new IOException(e); - } catch (ShortBufferException e) { + synchronized (CIPHER_LOCK) { + int flourish = cipher.doFinal(buffer, offset); + done = true; + return flourish; + } + } catch (IllegalBlockSizeException | ShortBufferException | BadPaddingException e) { throw new IOException(e); } } @@ -234,9 +229,7 @@ public class AttachmentCipherInputStream extends FilterInputStream { throw new InvalidMacException("Digest doesn't match!"); } - } catch (IOException e) { - throw new InvalidMacException(e); - } catch (ArithmeticException e) { + } catch (IOException | ArithmeticException e) { throw new InvalidMacException(e); } catch (NoSuchAlgorithmException e) { throw new AssertionError(e); From 9e6d1e27fc4657170e8dd7c87b64f618f3bd16ff Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 5 May 2023 12:33:50 +0930 Subject: [PATCH 3/4] Add comment --- .../src/main/java/org/session/libsignal/crypto/CipherUtil.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java b/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java index d3423a0bd4..a6a3808bb4 100644 --- a/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java +++ b/libsignal/src/main/java/org/session/libsignal/crypto/CipherUtil.java @@ -1,5 +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(); } From 6a5d97a0f063f7385f61d18202352e98ad968cf0 Mon Sep 17 00:00:00 2001 From: andrew Date: Fri, 5 May 2023 12:37:46 +0930 Subject: [PATCH 4/4] Fix something --- .../org/thoughtcrime/securesms/backup/FullBackupExporter.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt index 783468eee7..5eba9b9945 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupExporter.kt @@ -279,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 @@ -290,7 +290,7 @@ object FullBackupExporter { private var counter: Int = 0 - private constructor(outputStream: OutputStream, passphrase: String) : super() { + init { try { val salt = Util.getSecretBytes(32) val key = BackupUtil.computeBackupKey(passphrase, salt)