From 00191e2b6055292a871bb3b525d79fdc8e38cb30 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 9 Mar 2020 19:12:30 +0100 Subject: [PATCH] explicitly use BouncyCastle for file crypto --- build.gradle | 4 +-- .../crypto/axolotl/AxolotlService.java | 6 ++-- .../crypto/axolotl/XmppAxolotlMessage.java | 8 ++--- .../http/HttpUploadConnection.java | 10 +----- .../services/AbstractConnectionManager.java | 33 ++++++++----------- 5 files changed, 24 insertions(+), 37 deletions(-) diff --git a/build.gradle b/build.gradle index f9f73f18f..39bf82793 100644 --- a/build.gradle +++ b/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" diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java index 906080d06..e67755586 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java +++ b/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()); diff --git a/src/main/java/eu/siacs/conversations/crypto/axolotl/XmppAxolotlMessage.java b/src/main/java/eu/siacs/conversations/crypto/axolotl/XmppAxolotlMessage.java index e2a9a2da2..bec0ff9e2 100644 --- a/src/main/java/eu/siacs/conversations/crypto/axolotl/XmppAxolotlMessage.java +++ b/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; } diff --git a/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java b/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java index 3a21b4bad..430c7532b 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java +++ b/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); } diff --git a/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java b/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java index 81f3f80bc..98721aaf4 100644 --- a/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java +++ b/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; } }