From 06dc8ccbddd22772c3e15af09773c3ce918293df Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Mon, 7 Aug 2023 15:28:22 -0300 Subject: [PATCH] Fix contact photo upload failure. --- .../securesms/jobs/AttachmentUploadJob.java | 1 + .../securesms/jobs/PushSendJob.java | 3 +- .../api/SignalServiceMessageSender.java | 34 +++++++++++-------- .../api/messages/SignalServiceAttachment.java | 9 ++++- .../SignalServiceAttachmentStream.java | 11 ++++-- .../push/http/DigestingRequestBody.kt | 4 +-- ...talAttachmentCipherOutputStreamFactory.kt} | 21 ++++++------ .../http/IncrementalOutputStreamFactory.kt | 16 +++++++++ ...gacyAttachmentCipherOutputStreamFactory.kt | 26 ++++++++++++++ .../api/crypto/AttachmentCipherTest.java | 16 ++++----- .../push/http/DigestingRequestBodyTest.java | 2 +- 11 files changed, 103 insertions(+), 40 deletions(-) rename libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/{AttachmentCipherOutputStreamFactory.kt => IncrementalAttachmentCipherOutputStreamFactory.kt} (65%) create mode 100644 libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalOutputStreamFactory.kt create mode 100644 libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/LegacyAttachmentCipherOutputStreamFactory.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java index 6e69169385..69e24a5341 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentUploadJob.java @@ -213,6 +213,7 @@ public final class AttachmentUploadJob extends BaseJob { .withCaption(attachment.getCaption()) .withCancelationSignal(this::isCanceled) .withResumableUploadSpec(resumableUploadSpec) + .withIncremental(attachment.getIncrementalDigest() != null) .withListener((total, progress) -> { EventBus.getDefault().postSticky(new PartProgressEvent(attachment, PartProgressEvent.Type.NETWORK, total, progress)); if (notification != null) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushSendJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushSendJob.java index b1643eecbd..e67a91aeb5 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/PushSendJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/PushSendJob.java @@ -366,7 +366,8 @@ public abstract class PushSendJob extends SendJob { .withHeight(thumbnailData.getHeight()) .withLength(thumbnailData.getData().length) .withStream(new ByteArrayInputStream(thumbnailData.getData())) - .withResumableUploadSpec(ApplicationDependencies.getSignalServiceMessageSender().getResumableUploadSpec()); + .withResumableUploadSpec(ApplicationDependencies.getSignalServiceMessageSender().getResumableUploadSpec()) + .withIncremental(attachment.getIncrementalDigest() != null); thumbnail = builder.build(); } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java index 8cf2b2d100..da67c7e95d 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/SignalServiceMessageSender.java @@ -65,8 +65,8 @@ import org.whispersystems.signalservice.api.messages.multidevice.ViewOnceOpenMes import org.whispersystems.signalservice.api.messages.multidevice.ViewedMessage; import org.whispersystems.signalservice.api.messages.shared.SharedContact; import org.whispersystems.signalservice.api.push.DistributionId; -import org.whispersystems.signalservice.api.push.ServiceId.PNI; import org.whispersystems.signalservice.api.push.ServiceId; +import org.whispersystems.signalservice.api.push.ServiceId.PNI; import org.whispersystems.signalservice.api.push.SignalServiceAddress; import org.whispersystems.signalservice.api.push.exceptions.AuthorizationFailedException; import org.whispersystems.signalservice.api.push.exceptions.MalformedResponseException; @@ -123,8 +123,10 @@ import org.whispersystems.signalservice.internal.push.exceptions.GroupStaleDevic import org.whispersystems.signalservice.internal.push.exceptions.InvalidUnidentifiedAccessHeaderException; import org.whispersystems.signalservice.internal.push.exceptions.MismatchedDevicesException; import org.whispersystems.signalservice.internal.push.exceptions.StaleDevicesException; -import org.whispersystems.signalservice.internal.push.http.AttachmentCipherOutputStreamFactory; import org.whispersystems.signalservice.internal.push.http.CancelationSignal; +import org.whispersystems.signalservice.internal.push.http.IncrementalAttachmentCipherOutputStreamFactory; +import org.whispersystems.signalservice.internal.push.http.LegacyAttachmentCipherOutputStreamFactory; +import org.whispersystems.signalservice.internal.push.http.OutputStreamFactory; import org.whispersystems.signalservice.internal.push.http.PartialSendBatchCompleteListener; import org.whispersystems.signalservice.internal.push.http.PartialSendCompleteListener; import org.whispersystems.signalservice.internal.push.http.ResumableUploadSpec; @@ -782,18 +784,20 @@ public class SignalServiceMessageSender { } public SignalServiceAttachmentPointer uploadAttachment(SignalServiceAttachmentStream attachment) throws IOException { - byte[] attachmentKey = attachment.getResumableUploadSpec().map(ResumableUploadSpec::getSecretKey).orElseGet(() -> Util.getSecretBytes(64)); - byte[] attachmentIV = attachment.getResumableUploadSpec().map(ResumableUploadSpec::getIV).orElseGet(() -> Util.getSecretBytes(16)); - long paddedLength = PaddingInputStream.getPaddedSize(attachment.getLength()); - InputStream dataStream = new PaddingInputStream(attachment.getInputStream(), attachment.getLength()); - long ciphertextLength = AttachmentCipherOutputStream.getCiphertextLength(paddedLength); - PushAttachmentData attachmentData = new PushAttachmentData(attachment.getContentType(), - dataStream, - ciphertextLength, - new AttachmentCipherOutputStreamFactory(attachmentKey, attachmentIV), - attachment.getListener(), - attachment.getCancelationSignal(), - attachment.getResumableUploadSpec().orElse(null)); + byte[] attachmentKey = attachment.getResumableUploadSpec().map(ResumableUploadSpec::getSecretKey).orElseGet(() -> Util.getSecretBytes(64)); + byte[] attachmentIV = attachment.getResumableUploadSpec().map(ResumableUploadSpec::getIV).orElseGet(() -> Util.getSecretBytes(16)); + long paddedLength = PaddingInputStream.getPaddedSize(attachment.getLength()); + InputStream dataStream = new PaddingInputStream(attachment.getInputStream(), attachment.getLength()); + long ciphertextLength = AttachmentCipherOutputStream.getCiphertextLength(paddedLength); + OutputStreamFactory outputStreamFactory = attachment.isIncremental() ? new IncrementalAttachmentCipherOutputStreamFactory(attachmentKey, attachmentIV) + : new LegacyAttachmentCipherOutputStreamFactory(attachmentKey, attachmentIV); + PushAttachmentData attachmentData = new PushAttachmentData(attachment.getContentType(), + dataStream, + ciphertextLength, + outputStreamFactory, + attachment.getListener(), + attachment.getCancelationSignal(), + attachment.getResumableUploadSpec().orElse(null)); if (attachment.getResumableUploadSpec().isPresent()) { return uploadAttachmentV3(attachment, attachmentKey, attachmentData); @@ -831,7 +835,7 @@ public class SignalServiceMessageSender { attachment.getPreview(), attachment.getWidth(), attachment.getHeight(), Optional.of(attachmentIdAndDigest.second().getDigest()), - Optional.of(attachmentIdAndDigest.second().getIncrementalDigest()), + Optional.ofNullable(attachmentIdAndDigest.second().getIncrementalDigest()), attachment.getFileName(), attachment.getVoiceNote(), attachment.isBorderless(), diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachment.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachment.java index c7c3692676..fbc406755b 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachment.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachment.java @@ -62,6 +62,7 @@ public abstract class SignalServiceAttachment { private String blurHash; private long uploadTimestamp; private ResumableUploadSpec resumableUploadSpec; + private boolean isIncremental; private Builder() {} @@ -140,6 +141,11 @@ public abstract class SignalServiceAttachment { return this; } + public Builder withIncremental(boolean isIncremental) { + this.isIncremental = isIncremental; + return this; + } + public SignalServiceAttachmentStream build() { if (inputStream == null) throw new IllegalArgumentException("Must specify stream!"); if (contentType == null) throw new IllegalArgumentException("No content type specified!"); @@ -160,7 +166,8 @@ public abstract class SignalServiceAttachment { Optional.ofNullable(blurHash), listener, cancelationSignal, - Optional.ofNullable(resumableUploadSpec)); + Optional.ofNullable(resumableUploadSpec), + isIncremental); } } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java index c35f9a8775..11d15ae938 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/api/messages/SignalServiceAttachmentStream.java @@ -35,6 +35,7 @@ public class SignalServiceAttachmentStream extends SignalServiceAttachment imple private final Optional caption; private final Optional blurHash; private final Optional resumableUploadSpec; + private final boolean isIncremental; public SignalServiceAttachmentStream(InputStream inputStream, String contentType, @@ -46,7 +47,7 @@ public class SignalServiceAttachmentStream extends SignalServiceAttachment imple ProgressListener listener, CancelationSignal cancelationSignal) { - this(inputStream, contentType, length, fileName, voiceNote, borderless, gif, Optional.empty(), 0, 0, System.currentTimeMillis(), Optional.empty(), Optional.empty(), listener, cancelationSignal, Optional.empty()); + this(inputStream, contentType, length, fileName, voiceNote, borderless, gif, Optional.empty(), 0, 0, System.currentTimeMillis(), Optional.empty(), Optional.empty(), listener, cancelationSignal, Optional.empty(), false); } public SignalServiceAttachmentStream(InputStream inputStream, @@ -64,7 +65,8 @@ public class SignalServiceAttachmentStream extends SignalServiceAttachment imple Optional blurHash, ProgressListener listener, CancelationSignal cancelationSignal, - Optional resumableUploadSpec) + Optional resumableUploadSpec, + boolean isIncremental) { super(contentType); this.inputStream = inputStream; @@ -82,6 +84,7 @@ public class SignalServiceAttachmentStream extends SignalServiceAttachment imple this.blurHash = blurHash; this.cancelationSignal = cancelationSignal; this.resumableUploadSpec = resumableUploadSpec; + this.isIncremental = isIncremental; } @Override @@ -154,6 +157,10 @@ public class SignalServiceAttachmentStream extends SignalServiceAttachment imple return resumableUploadSpec; } + public boolean isIncremental() { + return isIncremental; + } + @Override public void close() throws IOException { inputStream.close(); diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBody.kt b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBody.kt index adf5e8b133..069f1f2a2f 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBody.kt +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBody.kt @@ -41,9 +41,9 @@ class DigestingRequestBody( override fun writeTo(sink: BufferedSink) { val digestStream = ByteArrayOutputStream() val inner = SkippingOutputStream(contentStart, sink.outputStream()) - val isIncremental = outputStreamFactory is AttachmentCipherOutputStreamFactory + val isIncremental = outputStreamFactory is IncrementalOutputStreamFactory val outputStream: DigestingOutputStream = if (isIncremental) { - (outputStreamFactory as AttachmentCipherOutputStreamFactory).createIncrementalFor(inner, contentLength, digestStream) + (outputStreamFactory as IncrementalOutputStreamFactory).createIncrementalFor(inner, contentLength, digestStream) } else { outputStreamFactory.createFor(inner) } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/AttachmentCipherOutputStreamFactory.kt b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalAttachmentCipherOutputStreamFactory.kt similarity index 65% rename from libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/AttachmentCipherOutputStreamFactory.kt rename to libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalAttachmentCipherOutputStreamFactory.kt index 25623ad241..37ccb51d49 100644 --- a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/AttachmentCipherOutputStreamFactory.kt +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalAttachmentCipherOutputStreamFactory.kt @@ -1,8 +1,12 @@ +/* + * Copyright 2023 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + package org.whispersystems.signalservice.internal.push.http import org.signal.libsignal.protocol.incrementalmac.ChunkSizeChoice import org.signal.libsignal.protocol.incrementalmac.IncrementalMacOutputStream -import org.whispersystems.signalservice.api.crypto.AttachmentCipherOutputStream import org.whispersystems.signalservice.api.crypto.DigestingOutputStream import java.io.IOException import java.io.OutputStream @@ -10,24 +14,21 @@ import java.io.OutputStream /** * Creates [AttachmentCipherOutputStream] using the provided [key] and [iv]. * - * [createFor] is straightforward, and is the legacy behavior. * [createIncrementalFor] first wraps the stream in an [IncrementalMacOutputStream] to calculate MAC digests on chunks as the stream is written to. * * @property key * @property iv */ -class AttachmentCipherOutputStreamFactory(private val key: ByteArray, private val iv: ByteArray) : OutputStreamFactory { +class IncrementalAttachmentCipherOutputStreamFactory(private val key: ByteArray, private val iv: ByteArray) : IncrementalOutputStreamFactory { + + private val legacyDelegate = LegacyAttachmentCipherOutputStreamFactory(key, iv) + companion object { private const val AES_KEY_LENGTH = 32 } @Throws(IOException::class) - override fun createFor(wrap: OutputStream): DigestingOutputStream { - return AttachmentCipherOutputStream(key, iv, wrap) - } - - @Throws(IOException::class) - fun createIncrementalFor(wrap: OutputStream?, length: Long, incrementalDigestOut: OutputStream?): DigestingOutputStream { + override fun createIncrementalFor(wrap: OutputStream?, length: Long, incrementalDigestOut: OutputStream?): DigestingOutputStream { if (length > Int.MAX_VALUE) { throw IllegalArgumentException("Attachment length overflows int!") } @@ -35,6 +36,6 @@ class AttachmentCipherOutputStreamFactory(private val key: ByteArray, private va val privateKey = key.sliceArray(AES_KEY_LENGTH until key.size) val chunkSizeChoice = ChunkSizeChoice.inferChunkSize(length.toInt().coerceAtLeast(1)) val incrementalStream = IncrementalMacOutputStream(wrap, privateKey, chunkSizeChoice, incrementalDigestOut) - return createFor(incrementalStream) + return legacyDelegate.createFor(incrementalStream) } } diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalOutputStreamFactory.kt b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalOutputStreamFactory.kt new file mode 100644 index 0000000000..6ce4570f2b --- /dev/null +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/IncrementalOutputStreamFactory.kt @@ -0,0 +1,16 @@ +/* + * Copyright 2023 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.signalservice.internal.push.http + +import org.whispersystems.signalservice.api.crypto.DigestingOutputStream +import java.io.OutputStream + +interface IncrementalOutputStreamFactory : OutputStreamFactory { + + override fun createFor(wrap: OutputStream?): DigestingOutputStream = error("Use createIncrementalFor instead.") + + fun createIncrementalFor(wrap: OutputStream?, length: Long, incrementalDigestOut: OutputStream?): DigestingOutputStream +} diff --git a/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/LegacyAttachmentCipherOutputStreamFactory.kt b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/LegacyAttachmentCipherOutputStreamFactory.kt new file mode 100644 index 0000000000..a045127390 --- /dev/null +++ b/libsignal/service/src/main/java/org/whispersystems/signalservice/internal/push/http/LegacyAttachmentCipherOutputStreamFactory.kt @@ -0,0 +1,26 @@ +/* + * Copyright 2023 Signal Messenger, LLC + * SPDX-License-Identifier: AGPL-3.0-only + */ + +package org.whispersystems.signalservice.internal.push.http + +import org.whispersystems.signalservice.api.crypto.AttachmentCipherOutputStream +import org.whispersystems.signalservice.api.crypto.DigestingOutputStream +import java.io.IOException +import java.io.OutputStream + +/** + * Creates [AttachmentCipherOutputStream] using the provided [key] and [iv]. + * + * [createFor] is straightforward, and is the legacy behavior. + * + * @property key + * @property iv + */ +class LegacyAttachmentCipherOutputStreamFactory(private val key: ByteArray, private val iv: ByteArray) : OutputStreamFactory { + @Throws(IOException::class) + override fun createFor(wrap: OutputStream): DigestingOutputStream { + return AttachmentCipherOutputStream(key, iv, wrap) + } +} diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/crypto/AttachmentCipherTest.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/crypto/AttachmentCipherTest.java index b52d753662..a866afb1d4 100644 --- a/libsignal/service/src/test/java/org/whispersystems/signalservice/api/crypto/AttachmentCipherTest.java +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/api/crypto/AttachmentCipherTest.java @@ -6,7 +6,8 @@ import org.signal.libsignal.protocol.InvalidMessageException; import org.signal.libsignal.protocol.incrementalmac.InvalidMacException; import org.signal.libsignal.protocol.kdf.HKDFv3; import org.whispersystems.signalservice.internal.crypto.PaddingInputStream; -import org.whispersystems.signalservice.internal.push.http.AttachmentCipherOutputStreamFactory; +import org.whispersystems.signalservice.internal.push.http.IncrementalAttachmentCipherOutputStreamFactory; +import org.whispersystems.signalservice.internal.push.http.LegacyAttachmentCipherOutputStreamFactory; import org.whispersystems.signalservice.internal.util.Util; import java.io.ByteArrayInputStream; @@ -159,7 +160,7 @@ public final class AttachmentCipherTest { InputStream paddedInputStream = new PaddingInputStream(inputStream, length); ByteArrayOutputStream destinationOutputStream = new ByteArrayOutputStream(); ByteArrayOutputStream incrementalDigestOutputStream = new ByteArrayOutputStream(); - DigestingOutputStream encryptingOutputStream = new AttachmentCipherOutputStreamFactory(key, iv).createIncrementalFor(destinationOutputStream, length, incrementalDigestOutputStream); + DigestingOutputStream encryptingOutputStream = new IncrementalAttachmentCipherOutputStreamFactory(key, iv).createIncrementalFor(destinationOutputStream, length, incrementalDigestOutputStream); Util.copy(paddedInputStream, encryptingOutputStream); @@ -301,16 +302,15 @@ public final class AttachmentCipherTest { } private static EncryptResult encryptData(byte[] data, byte[] keyMaterial, boolean withIncremental) throws IOException { - ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); - ByteArrayOutputStream incrementalDigestOut = new ByteArrayOutputStream(); - byte[] iv = Util.getSecretBytes(16); - AttachmentCipherOutputStreamFactory factory = new AttachmentCipherOutputStreamFactory(keyMaterial, iv); + ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); + ByteArrayOutputStream incrementalDigestOut = new ByteArrayOutputStream(); + byte[] iv = Util.getSecretBytes(16); DigestingOutputStream encryptStream; if (withIncremental) { - encryptStream = factory.createIncrementalFor(outputStream, data.length, incrementalDigestOut); + encryptStream = new IncrementalAttachmentCipherOutputStreamFactory(keyMaterial, iv).createIncrementalFor(outputStream, data.length, incrementalDigestOut); } else { - encryptStream = factory.createFor(outputStream); + encryptStream = new LegacyAttachmentCipherOutputStreamFactory(keyMaterial, iv).createFor(outputStream); } diff --git a/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBodyTest.java b/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBodyTest.java index 30d8e3a335..8037b9fd38 100644 --- a/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBodyTest.java +++ b/libsignal/service/src/test/java/org/whispersystems/signalservice/internal/push/http/DigestingRequestBodyTest.java @@ -20,7 +20,7 @@ public class DigestingRequestBodyTest { private final byte[] attachmentIV = Util.getSecretBytes(16); private final byte[] input = Util.getSecretBytes(CONTENT_LENGTH); - private final OutputStreamFactory outputStreamFactory = new AttachmentCipherOutputStreamFactory(attachmentKey, attachmentIV); + private final OutputStreamFactory outputStreamFactory = new LegacyAttachmentCipherOutputStreamFactory(attachmentKey, attachmentIV); @Test public void givenSameKeyAndIV_whenIWriteToBuffer_thenIExpectSameDigests() throws Exception {