refactored file encryption to give access to inner stream

Daniel Gultsch created

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.

Change summary

src/main/java/eu/siacs/conversations/http/HttpUploadConnection.java          | 27 
src/main/java/eu/siacs/conversations/services/AbstractConnectionManager.java | 25 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleConnection.java       | 26 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleInbandTransport.java  | 25 
src/main/java/eu/siacs/conversations/xmpp/jingle/JingleSocks5Transport.java  |  4 
5 files changed, 44 insertions(+), 63 deletions(-)

Detailed changes

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<String> WHITE_LISTED_HEADERS = Arrays.asList(
+	static final List<String> 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<InputStream,Integer> 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();

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<InputStream, Integer> 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);
     }

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<InputStream,Integer> 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);

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;
 				}

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;