Daniel Gultsch created
Ever since Android 9+ switched to Conscrypt we can no longer efficiently
encrypt (and decrypt) large files with AES-GCM. We did’t notice this before
because when using 16 byte IVs even modern Androids will fall back to bouncy
castle. However the 'bug'/'feature' in Conscrypt surfaced when we switched over
to 12 byte IVs (which uses Conscrypt on Android 9+)
Switching back entirely to 16 byte IVs is undesirable as this would break
compatibility with Monal. So we end up with a weird compromise where we use
12 byte for normale plain text OMEMO messages and 'small' files where the
inefficiencies aren’t a problem.
The result of this commit is that Monal won’t be able to receive our files
larger than 768KiB. However the alternative is that Conversations would always
OOM when attempting to send larger files (where large depends on the available
RAM.)
fixes #3653build.gradle | 4
src/main/java/eu/siacs/conversations/Config.java | 1
src/main/java/eu/siacs/conversations/crypto/axolotl/AxolotlService.java | 6
src/main/java/eu/siacs/conversations/crypto/axolotl/XmppAxolotlMessage.java | 11
src/main/java/eu/siacs/conversations/entities/DownloadableFile.java | 4
src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java | 13
6 files changed, 25 insertions(+), 14 deletions(-)
@@ -90,8 +90,8 @@ android {
defaultConfig {
minSdkVersion 16
targetSdkVersion 28
- versionCode 364
- versionName "2.7.0"
+ versionCode 365
+ versionName "2.7.1"
archivesBaseName += "-$versionName"
applicationId "eu.siacs.conversations"
resValue "string", "applicationId", applicationId
@@ -100,7 +100,6 @@ public final class Config {
public static final boolean REMOVE_BROKEN_DEVICES = false;
public static final boolean OMEMO_PADDING = false;
public static final boolean PUT_AUTH_TAG_INTO_KEY = true;
- public static final boolean TWELVE_BYTE_IV = false;
public static final boolean USE_BOOKMARKS2 = false;
@@ -1157,7 +1157,7 @@ public class AxolotlService implements OnAdvancedStreamFeaturesLoaded {
@Nullable
public XmppAxolotlMessage encrypt(Message message) {
- final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId());
+ final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId(), true);
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());
+ final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId(), false);
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());
+ final XmppAxolotlMessage axolotlMessage = new XmppAxolotlMessage(account.getJid().asBareJid(), getOwnDeviceId(), true);
axolotlMessage.addDevice(session, true);
try {
final Jid jid = Jid.of(session.getRemoteAddress().getName());
@@ -85,11 +85,11 @@ public class XmppAxolotlMessage {
}
}
- XmppAxolotlMessage(Jid from, int sourceDeviceId) {
+ XmppAxolotlMessage(Jid from, int sourceDeviceId, final boolean twelveByteIv) {
this.from = from;
this.sourceDeviceId = sourceDeviceId;
this.keys = new ArrayList<>();
- this.iv = generateIv();
+ this.iv = generateIv(twelveByteIv);
this.innerKey = generateKey();
}
@@ -115,14 +115,13 @@ public class XmppAxolotlMessage {
generator.init(128);
return generator.generateKey().getEncoded();
} catch (NoSuchAlgorithmException e) {
- Log.e(Config.LOGTAG, e.getMessage());
- return null;
+ throw new IllegalStateException(e);
}
}
- private static byte[] generateIv() {
+ private static byte[] generateIv(final boolean twelveByteIv) {
final SecureRandom random = new SecureRandom();
- byte[] iv = new byte[Config.TWELVE_BYTE_IV ? 12 : 16];
+ byte[] iv = new byte[twelveByteIv ? 12 : 16];
random.nextBytes(iv);
return iv;
}
@@ -1,7 +1,10 @@
package eu.siacs.conversations.entities;
+import android.util.Log;
+
import java.io.File;
+import eu.siacs.conversations.Config;
import eu.siacs.conversations.utils.MimeUtils;
public class DownloadableFile extends File {
@@ -66,6 +69,7 @@ public class DownloadableFile extends File {
this.iv = new byte[]{ 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0xf };
System.arraycopy(keyIvCombo, 0, aeskey, 0, 32);
}
+ Log.d(Config.LOGTAG,"using "+this.iv.length+"-byte IV for file transmission");
}
public void setKey(byte[] key) {
@@ -105,11 +105,20 @@ public class HttpUploadConnection implements Transferable {
} else {
this.mime = this.file.getMimeType();
}
+ final long originalFileSize = file.getSize();
this.delayed = delay;
if (Config.ENCRYPT_ON_HTTP_UPLOADED
|| message.getEncryption() == Message.ENCRYPTION_AXOLOTL
|| message.getEncryption() == Message.ENCRYPTION_OTR) {
- this.key = new byte[Config.TWELVE_BYTE_IV ? 44 : 48];
+ //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 16
+ //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];
mXmppConnectionService.getRNG().nextBytes(this.key);
this.file.setKeyAndIv(this.key);
}
@@ -128,7 +137,7 @@ public class HttpUploadConnection implements Transferable {
md5 = null;
}
- this.file.setExpectedSize(file.getSize() + (file.getKey() != null ? 16 : 0));
+ this.file.setExpectedSize(originalFileSize + (file.getKey() != null ? 16 : 0));
message.resetFileParams();
this.mSlotRequester.request(method, account, file, mime, md5, new SlotRequester.OnSlotRequested() {
@Override