signal-csharp / libsignal-service-dotnet

GNU General Public License v3.0
34 stars 8 forks source link

support sending attachments #3

Closed Trolldemorted closed 6 years ago

mitchcapper commented 6 years ago

Can you clarify this? I see there is some work around attachments. They can be put on messages, the pointers created, and it would seem like it would get serialized?

Trolldemorted commented 6 years ago

We had to deviate from libsignal-service-java, because we should use UWP's background download/upload api and thus don't want to be forced to do the actualy download and upload in here.

When we built support for attachments some months ago we added RetrieveAttachmentDownloadUrl to the receiver's API, and decrypted it completely detached from vanilla libsignal's set of input/output streams. In the recent commits I have repaired a lot of receiving (input) streams, and ensured that the methods using them work fine. I have not tested whether uploading still works after all that refactoring, but if you would like to use it we can make Signal-Windows finally support uploading attachments again soon and fix any upload-related bug that might still linger around here.

mitchcapper commented 6 years ago

OK I did some testing and it seems sending attachments works perfectly fine from the library front. Both standard and voice messages send without issue.

mitchcapper commented 6 years ago

I spoke too soon. I tested this with the old libsignal-service-dotnet but the new protocol libsignal-protocol-dotnet. Unfortunately I just upgraded libsignal-service-dotnet and it no longer works.

I am creating send attachments with:

attachments.Add(new SignalServiceAttachmentStream(stream, "image/png", stream.Length, "file.png", false, null));
var ssdm = new SignalServiceDataMessage { Body = body, Timestamp = Util.CurrentTimeMillis(), Attachments = attachments };

Using the official windows signal client nothing shows up oddly. On the android client it shows an image attachment with the download button but upon hitting download (which normally you don't have to do) nothing happens.

Trolldemorted commented 6 years ago

Do you mean Signal-Desktop with the official windows client?

This is very odd. Is there anything unusual in Signal-Android's log? Also Signal-Android fails to download attachments without any third party software involved sometimes, especially if you receive many attachments in a row.

mitchcapper commented 6 years ago

Correct - https://updates.signal.org/desktop/signal-desktop-win-1.11.0.exe

Android log shows InvalidMessageException "Message shorter than crypt overhead!":

05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob: org.whispersystems.libsignal.InvalidMessageException: Message shorter than crypto overhead!
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.whispersystems.signalservice.api.crypto.AttachmentCipherInputStream.createFor(AttachmentCipherInputStream.java:62)
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.whispersystems.signalservice.api.SignalServiceMessageReceiver.retrieveAttachment(SignalServiceMessageReceiver.java:127)
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.thoughtcrime.securesms.jobs.AttachmentDownloadJob.retrieveAttachment(AttachmentDownloadJob.java:120)
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.thoughtcrime.securesms.jobs.AttachmentDownloadJob.onRun(AttachmentDownloadJob.java:92)
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.thoughtcrime.securesms.jobs.MasterSecretJob.onRun(MasterSecretJob.java:18)
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.whispersystems.jobqueue.JobConsumer.runJob(JobConsumer.java:76)
05-29 10:22:30.657  9004  9027 W AttachmentDownloadJob:     at org.whispersystems.jobqueue.JobConsumer.run(JobConsumer.java:46)

Sending attachments did work before but i have yet to find in the changes to the lib from 2.5 to 2.7 what caused it (a good bit of changes;)). I tried fetching the attachments from https://whispersystems-textsecure-attachments.s3-accelerate.amazonaws.com/ to do a manual compare but just the url from the android log wasn't authorized.

Trolldemorted commented 6 years ago

That is odd.

  public static InputStream createFor(File file, long plaintextLength, byte[] combinedKeyMaterial, byte[] digest)
      throws InvalidMessageException, IOException
  {
    try {
      byte[][] parts = Util.split(combinedKeyMaterial, CIPHER_KEY_SIZE, MAC_KEY_SIZE);
      Mac      mac   = Mac.getInstance("HmacSHA256");
      mac.init(new SecretKeySpec(parts[1], "HmacSHA256"));

      if (file.length() <= BLOCK_SIZE + mac.getMacLength()) {
        throw 

If I am not mistaken, file is the downloaded ciphertext, but how can it be smaller than the block size + the mac length? How big is the file you are trying to send? Maybe we are not properly padding/adding the mac?

mitchcapper commented 6 years ago

517,955B (506K). I will try with some different file sizes as well.

Trolldemorted commented 6 years ago

Sorry, this is indeed caused by our refactoring.

Does https://github.com/signal-csharp/libsignal-service-dotnet/commit/3026619483b795414fffd60c8b79c87c66f57d99 work for you? With that commit my Signal-Android is displaying my test images correctly.