explicitly use BouncyCastle for file crypto

Daniel Gultsch created

Change summary

build.gradle                                                                 |  4 
src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java      |  6 
src/main/java/eu/siacs/conversations/crypto/axolotl/XmppAxolotlMessage.java  |  8 
src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java          | 10 
src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java | 33 
5 files changed, 24 insertions(+), 37 deletions(-)

Detailed changes

build.gradle 🔗

@@ -54,7 +54,7 @@ dependencies {
     compatImplementation "com.android.support:support-emoji-appcompat:$supportLibVersion"
     conversationsFreeCompatImplementation "com.android.support:support-emoji-bundled:$supportLibVersion"
     quicksyFreeCompatImplementation "com.android.support:support-emoji-bundled:$supportLibVersion"
-    implementation 'org.bouncycastle:bcmail-jdk15on:1.58'
+    implementation 'org.bouncycastle:bcmail-jdk15on:1.64'
     //zxing stopped supporting Java 7 so we have to stick with 3.3.3
     //https://github.com/zxing/zxing/issues/1170
     implementation 'com.google.zxing:core:3.3.3'
@@ -90,7 +90,7 @@ android {
     defaultConfig {
         minSdkVersion 16
         targetSdkVersion 28
-        versionCode 365
+        versionCode 366
         versionName "2.7.1"
         archivesBaseName += "-$versionName"
         applicationId "eu.siacs.conversations"

src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java 🔗

@@ -1157,7 +1157,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
 
     @Nullable
     public XmppAxolotlMessage encrypt(Message message) {
-        final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId(), true);
+        final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId());
         final String content;
         if (message.hasFileOnRemoteHost()) {
             content = message.getFileParams().url.toString();
@@ -1201,7 +1201,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
         executor.execute(new Runnable() {
             @Override
             public void run() {
-                final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId(), false);
+                final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId());
                 if (buildHeader(axolotlMessage, conversation)) {
                     onMessageCreatedCallback.run(axolotlMessage);
                 } else {
@@ -1362,7 +1362,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
     }
 
     private void completeSession(XmppAxolotlSession session) {
-        final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId(), true);
+        final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId());
         axolotlMessage.addDevice(session, true);
         try {
             final Jid jid = Jid.of(session.getRemoteAddress().getName());

src/main/java/eu/siacs/conversations/crypto/axolotl/XmppAxolotlMessage.java 🔗

@@ -85,11 +85,11 @@ public class XmppAxolotlMessage {
         }
     }
 
-    XmppAxolotlMessage(Jid from, int sourceDeviceId, final boolean twelveByteIv) {
+    XmppAxolotlMessage(Jid from, int sourceDeviceId) {
         this.from = from;
         this.sourceDeviceId = sourceDeviceId;
         this.keys = new ArrayList<>();
-        this.iv = generateIv(twelveByteIv);
+        this.iv = generateIv();
         this.innerKey = generateKey();
     }
 
@@ -119,9 +119,9 @@ public class XmppAxolotlMessage {
         }
     }
 
-    private static byte[] generateIv(final boolean twelveByteIv) {
+    private static byte[] generateIv() {
         final SecureRandom random = new SecureRandom();
-        byte[] iv = new byte[twelveByteIv ? 12 : 16];
+        final byte[] iv = new byte[12];
         random.nextBytes(iv);
         return iv;
     }

src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java 🔗

@@ -110,15 +110,7 @@ public class HttpUploadConnection implements Transferable {
 		if (Config.ENCRYPT_ON_HTTP_UPLOADED
 				|| message.getEncryption() == Message.ENCRYPTION_AXOLOTL
 				|| message.getEncryption() == Message.ENCRYPTION_OTR) {
-			//ok, this is going to sound super crazy but on Android 9+ a 12 byte IV will use the
-			//internal conscrypt library (provided by the OS) instead of bounce castle, while 16 bytes
-			//will still 'fallback' to bounce castle even on Android 9+ because conscrypt doesnt
-			//have support for anything but 12.
-			//For large files conscrypt has extremely bad performance; so why not always use 16 you ask?
-			//well the ecosystem was moving and some clients like Monal *only* support 12
-			//so the result of this code is that we can only send 'small' files to Monal.
-			//'small' was relatively arbitrarily choose and correlates to roughly 'small' compressed images
-			this.key = new byte[originalFileSize <= 786432 ? 44 : 48];
+			this.key = new byte[44];
 			mXmppConnectionService.getRNG().nextBytes(this.key);
 			this.file.setKeyAndIv(this.key);
 		}

src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java 🔗

@@ -5,6 +5,14 @@ import android.os.PowerManager;
 import android.os.SystemClock;
 import android.util.Log;
 
+import org.bouncycastle.crypto.engines.AESEngine;
+import org.bouncycastle.crypto.io.CipherInputStream;
+import org.bouncycastle.crypto.io.CipherOutputStream;
+import org.bouncycastle.crypto.modes.AEADBlockCipher;
+import org.bouncycastle.crypto.modes.GCMBlockCipher;
+import org.bouncycastle.crypto.params.AEADParameters;
+import org.bouncycastle.crypto.params.KeyParameter;
+
 import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.InputStream;
@@ -15,24 +23,15 @@ import java.security.NoSuchAlgorithmException;
 import java.security.NoSuchProviderException;
 import java.util.concurrent.atomic.AtomicLong;
 
-import javax.crypto.Cipher;
-import javax.crypto.CipherInputStream;
-import javax.crypto.CipherOutputStream;
 import javax.crypto.NoSuchPaddingException;
-import javax.crypto.spec.IvParameterSpec;
-import javax.crypto.spec.SecretKeySpec;
 
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.R;
 import eu.siacs.conversations.entities.DownloadableFile;
 import eu.siacs.conversations.utils.Compatibility;
-import eu.siacs.conversations.utils.CryptoHelper;
 
 public class AbstractConnectionManager {
 
-    private static final String KEYTYPE = "AES";
-    private static final String CIPHERMODE = "AES/GCM/NoPadding";
-    private static final String PROVIDER = "BC";
     private static final int UI_REFRESH_THRESHOLD = 250;
     private static final AtomicLong LAST_UI_UPDATE_CALL = new AtomicLong(0);
     protected XmppConnectionService mXmppConnectionService;
@@ -43,10 +42,8 @@ public class AbstractConnectionManager {
 
     public static InputStream upgrade(DownloadableFile file, InputStream is) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, InvalidKeyException, NoSuchPaddingException, NoSuchProviderException {
         if (file.getKey() != null && file.getIv() != null) {
-            final Cipher cipher = Compatibility.twentyEight() ? Cipher.getInstance(CIPHERMODE) : Cipher.getInstance(CIPHERMODE, PROVIDER);
-            SecretKeySpec keySpec = new SecretKeySpec(file.getKey(), KEYTYPE);
-            IvParameterSpec ivSpec = new IvParameterSpec(file.getIv());
-            cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec);
+            AEADBlockCipher cipher = new GCMBlockCipher(new AESEngine());
+            cipher.init(true, new AEADParameters(new KeyParameter(file.getKey()), 128, file.getIv()));
             return new CipherInputStream(is, cipher);
         } else {
             return is;
@@ -61,17 +58,15 @@ public class AbstractConnectionManager {
                 return os;
             }
         } catch (FileNotFoundException e) {
-            Log.d(Config.LOGTAG,"unable to create output stream", e);
+            Log.d(Config.LOGTAG, "unable to create output stream", e);
             return null;
         }
         try {
-            final Cipher cipher = Compatibility.twentyEight() ? Cipher.getInstance(CIPHERMODE) : Cipher.getInstance(CIPHERMODE, PROVIDER);
-            SecretKeySpec keySpec = new SecretKeySpec(file.getKey(), KEYTYPE);
-            IvParameterSpec ivSpec = new IvParameterSpec(file.getIv());
-            cipher.init(Cipher.DECRYPT_MODE, keySpec, ivSpec);
+            AEADBlockCipher cipher = new GCMBlockCipher(new AESEngine());
+            cipher.init(false, new AEADParameters(new KeyParameter(file.getKey()), 128, file.getIv()));
             return new CipherOutputStream(os, cipher);
         } catch (Exception e) {
-            Log.d(Config.LOGTAG,"unable to create cipher output stream", e);
+            Log.d(Config.LOGTAG, "unable to create cipher output stream", e);
             return null;
         }
     }