From f608fb349a3c777d65c8723d957feff89dd246f3 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Wed, 3 Oct 2018 18:14:41 +0200 Subject: [PATCH] refactored file encryption to give access to inner stream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Conscrypt on some plattforms doesn’t like when we close the CipherInputStream. Therefor we refactor the api to give us access to the inner stream so we can close that independently. --- .../http/HttpUploadConnection.java | 27 +++++++------------ .../services/AbstractConnectionManager.java | 25 +++++++++-------- .../xmpp/jingle/JingleConnection.java | 26 +++++++++--------- .../xmpp/jingle/JingleInbandTransport.java | 25 ++++++----------- .../xmpp/jingle/JingleSocks5Transport.java | 4 ++- 5 files changed, 44 insertions(+), 63 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java b/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java index b635ab92334a6a5cc607318da9458084ef669412..2a6bdec3a98726c299c459f9ea7843ed852ffbbf 100644 --- a/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java +++ b/src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java @@ -4,6 +4,7 @@ import android.os.PowerManager; import android.util.Log; import android.util.Pair; +import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -31,7 +32,7 @@ import eu.siacs.conversations.utils.WakeLockHelper; public class HttpUploadConnection implements Transferable { - public static final List WHITE_LISTED_HEADERS = Arrays.asList( + static final List WHITE_LISTED_HEADERS = Arrays.asList( "Authorization", "Cookie", "Expires" @@ -52,8 +53,6 @@ public class HttpUploadConnection implements Transferable { private long transmitted = 0; - private InputStream mFileInputStream; - public HttpUploadConnection(Method method, HttpConnectionManager httpConnectionManager) { this.method = method; this.mHttpConnectionManager = httpConnectionManager; @@ -94,7 +93,6 @@ public class HttpUploadConnection implements Transferable { mHttpConnectionManager.finishUploadConnection(this); message.setTransferable(null); mXmppConnectionService.markMessage(message, Message.STATUS_SEND_FAILED, errorMessage); - FileBackend.close(mFileInputStream); } public void init(Message message, boolean delay) { @@ -119,7 +117,7 @@ public class HttpUploadConnection implements Transferable { if (method == Method.P1_S3) { try { - md5 = Checksum.md5(AbstractConnectionManager.createInputStream(file).first); + md5 = Checksum.md5(AbstractConnectionManager.upgrade(file, new FileInputStream(file))); } catch (Exception e) { Log.d(Config.LOGTAG, account.getJid().asBareJid()+": unable to calculate md5()", e); fail(e.getMessage()); @@ -129,17 +127,8 @@ public class HttpUploadConnection implements Transferable { md5 = null; } - Pair pair; - try { - pair = AbstractConnectionManager.createInputStream(file); - } catch (FileNotFoundException e) { - Log.d(Config.LOGTAG, account.getJid().asBareJid()+": could not find file to upload - "+e.getMessage()); - fail(e.getMessage()); - return; - } - this.file.setExpectedSize(pair.second); + this.file.setExpectedSize(file.getSize() + (file.getKey() != null ? 16 : 0)); message.resetFileParams(); - this.mFileInputStream = pair.first; this.mSlotRequester.request(method, account, file, mime, md5, new SlotRequester.OnSlotRequested() { @Override public void success(SlotRequester.Slot slot) { @@ -160,9 +149,11 @@ public class HttpUploadConnection implements Transferable { private void upload() { OutputStream os = null; + InputStream fileInputStream = null; HttpURLConnection connection = null; PowerManager.WakeLock wakeLock = mHttpConnectionManager.createWakeLock("http_upload_"+message.getUuid()); try { + fileInputStream = new FileInputStream(file); final int expectedFileSize = (int) file.getExpectedSize(); final int readTimeout = (expectedFileSize / 2048) + Config.SOCKET_TIMEOUT; //assuming a minimum transfer speed of 16kbit/s wakeLock.acquire(readTimeout); @@ -189,18 +180,18 @@ public class HttpUploadConnection implements Transferable { connection.setConnectTimeout(Config.SOCKET_TIMEOUT * 1000); connection.setReadTimeout(readTimeout * 1000); connection.connect(); + final InputStream innerInputStream = AbstractConnectionManager.upgrade(file, fileInputStream); os = connection.getOutputStream(); transmitted = 0; int count; byte[] buffer = new byte[4096]; - while (((count = mFileInputStream.read(buffer)) != -1) && !canceled) { + while (((count = innerInputStream.read(buffer)) != -1) && !canceled) { transmitted += count; os.write(buffer, 0, count); mHttpConnectionManager.updateConversationUi(false); } os.flush(); os.close(); - mFileInputStream.close(); int code = connection.getResponseCode(); InputStream is = connection.getErrorStream(); if (is != null) { @@ -235,7 +226,7 @@ public class HttpUploadConnection implements Transferable { Log.d(Config.LOGTAG,"http upload failed "+e.getMessage()); fail(e.getMessage()); } finally { - FileBackend.close(mFileInputStream); + FileBackend.close(fileInputStream); FileBackend.close(os); if (connection != null) { connection.disconnect(); diff --git a/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java b/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java index 860e91ab729a4fe3872aaa37788aae80b7f3d4e9..692c6ecfa5ce8f7746a3d589916dbc32b1466452 100644 --- a/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java +++ b/src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java @@ -13,11 +13,16 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.InputStream; import java.io.OutputStream; +import java.security.InvalidAlgorithmParameterException; +import java.security.InvalidKeyException; +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; @@ -39,25 +44,19 @@ public class AbstractConnectionManager { this.mXmppConnectionService = service; } - public static Pair createInputStream(DownloadableFile file) throws FileNotFoundException { - FileInputStream is; - int size; - is = new FileInputStream(file); - size = (int) file.getSize(); - if (file.getKey() == null) { - return new Pair<>(is, size); - } - try { - Cipher cipher = Cipher.getInstance(CIPHERMODE); + public static InputStream upgrade(DownloadableFile file, InputStream is) throws InvalidAlgorithmParameterException, NoSuchAlgorithmException, InvalidKeyException, NoSuchPaddingException, NoSuchProviderException { + if (file.getKey() != null && file.getIv() != null) { + Cipher cipher = Compatibility.twentyTwo() ? 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); - return new Pair<>(new CipherInputStream(is, cipher), cipher.getOutputSize(size)); - } catch (Exception e) { - throw new AssertionError(e); + return new CipherInputStream(is, cipher); + } else { + return is; } } + public static OutputStream createAppendedOutputStream(DownloadableFile file) { return createOutputStream(file, true); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java index 7fb1ee4e4497383beddc4bb7d4b5eeefee94d5ed..96d558dc20e7a15a0dd0c0cb0a62d295f0889ab1 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java @@ -4,6 +4,7 @@ import android.util.Base64; import android.util.Log; import android.util.Pair; +import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; @@ -471,25 +472,22 @@ public class JingleConnection implements Transferable { if (message.getType() == Message.TYPE_IMAGE || message.getType() == Message.TYPE_FILE) { content.setTransportId(this.transportId); this.file = this.mXmppConnectionService.getFileBackend().getFile(message, false); - Pair pair; + if (message.getEncryption() == Message.ENCRYPTION_AXOLOTL) { + this.file.setKey(mXmppAxolotlMessage.getInnerKey()); + this.file.setIv(mXmppAxolotlMessage.getIV()); + this.file.setExpectedSize(file.getSize() + 16); + content.setFileOffer(this.file, false, this.ftVersion).addChild(mXmppAxolotlMessage.toElement()); + } else { + this.file.setExpectedSize(file.getSize()); + content.setFileOffer(this.file, false, this.ftVersion); + } + message.resetFileParams(); try { - if (message.getEncryption() == Message.ENCRYPTION_AXOLOTL) { - this.file.setKey(mXmppAxolotlMessage.getInnerKey()); - this.file.setIv(mXmppAxolotlMessage.getIV()); - pair = AbstractConnectionManager.createInputStream(this.file); - this.file.setExpectedSize(pair.second); - content.setFileOffer(this.file, false, this.ftVersion).addChild(mXmppAxolotlMessage.toElement()); - } else { - pair = AbstractConnectionManager.createInputStream(this.file); - this.file.setExpectedSize(pair.second); - content.setFileOffer(this.file, false, this.ftVersion); - } + this.mFileInputStream = new FileInputStream(file); } catch (FileNotFoundException e) { cancel(); return; } - message.resetFileParams(); - this.mFileInputStream = pair.first; content.setTransportId(this.transportId); content.socks5transport().setChildren(getCandidatesAsElements()); packet.setContent(content); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleInbandTransport.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleInbandTransport.java index dfa3850cf92a4429f8845995485d2ec52a67b31e..54679631fa143f7f945933bd9224ee4363b9705b 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleInbandTransport.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleInbandTransport.java @@ -14,6 +14,7 @@ import eu.siacs.conversations.Config; import eu.siacs.conversations.entities.Account; import eu.siacs.conversations.entities.DownloadableFile; import eu.siacs.conversations.persistance.FileBackend; +import eu.siacs.conversations.services.AbstractConnectionManager; import eu.siacs.conversations.utils.CryptoHelper; import eu.siacs.conversations.xml.Element; import eu.siacs.conversations.xmpp.OnIqPacketReceived; @@ -36,6 +37,7 @@ public class JingleInbandTransport extends JingleTransport { private JingleConnection connection; private InputStream fileInputStream = null; + private InputStream innerInputStream = null; private OutputStream fileOutputStream = null; private long remainingSize = 0; private long fileSize = 0; @@ -128,10 +130,11 @@ public class JingleInbandTransport extends JingleTransport { callback.onFileTransferAborted(); return; } + innerInputStream = AbstractConnectionManager.upgrade(file, fileInputStream); if (this.connected) { this.sendNextBlock(); } - } catch (NoSuchAlgorithmException e) { + } catch (Exception e) { callback.onFileTransferAborted(); Log.d(Config.LOGTAG,account.getJid().asBareJid()+": "+e.getMessage()); } @@ -140,26 +143,14 @@ public class JingleInbandTransport extends JingleTransport { @Override public void disconnect() { this.connected = false; - if (this.fileOutputStream != null) { - try { - this.fileOutputStream.close(); - } catch (IOException e) { - - } - } - if (this.fileInputStream != null) { - try { - this.fileInputStream.close(); - } catch (IOException e) { - - } - } + FileBackend.close(fileOutputStream); + FileBackend.close(fileInputStream); } private void sendNextBlock() { byte[] buffer = new byte[this.blockSize]; try { - int count = fileInputStream.read(buffer); + int count = innerInputStream.read(buffer); if (count == -1) { sendClose(); file.setSha1Sum(digest.digest()); @@ -167,7 +158,7 @@ public class JingleInbandTransport extends JingleTransport { fileInputStream.close(); return; } else if (count != buffer.length) { - int rem = fileInputStream.read(buffer,count,buffer.length-count); + int rem = innerInputStream.read(buffer,count,buffer.length-count); if (rem > 0) { count += rem; } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleSocks5Transport.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleSocks5Transport.java index 15aebc72481fa106e8579997c9cb8dd48ff8cd99..99c12ba3aa0bb537d1a88c6b2ecc853066d944b8 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleSocks5Transport.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleSocks5Transport.java @@ -15,6 +15,7 @@ import java.security.NoSuchAlgorithmException; import eu.siacs.conversations.Config; import eu.siacs.conversations.entities.DownloadableFile; import eu.siacs.conversations.persistance.FileBackend; +import eu.siacs.conversations.services.AbstractConnectionManager; import eu.siacs.conversations.utils.CryptoHelper; import eu.siacs.conversations.utils.SocksSocketFactory; import eu.siacs.conversations.utils.WakeLockHelper; @@ -94,11 +95,12 @@ public class JingleSocks5Transport extends JingleTransport { callback.onFileTransferAborted(); return; } + final InputStream innerInputStream = AbstractConnectionManager.upgrade(file, fileInputStream); long size = file.getExpectedSize(); long transmitted = 0; int count; byte[] buffer = new byte[8192]; - while ((count = fileInputStream.read(buffer)) > 0) { + while ((count = innerInputStream.read(buffer)) > 0) { outputStream.write(buffer, 0, count); digest.update(buffer, 0, count); transmitted += count;