Clear incrementalMac if we discover it's bad during playback.

2025-12-27 22:43:52 +00:00 · 2025-08-25 12:35:17 -04:00
parent 45c64f825d
commit 2046b44fce
9 changed files with 124 additions and 24 deletions
--- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentTable.kt
@@ -966,6 +966,26 @@ class AttachmentTable(
      }
  }

+  /**
+   * Clears out the incrementalMac for the specified [attachmentId], as well as any other attachments that share the same ([remoteKey], [plaintextHash]) pair (if present).
+   */
+  fun clearIncrementalMacsForAttachmentAndAnyDuplicates(attachmentId: AttachmentId, remoteKey: String?, plaintextHash: String?) {
+    val query = if (remoteKey != null && plaintextHash != null) {
+      SqlUtil.buildQuery("$ID = ? OR ($REMOTE_KEY = ?  AND $DATA_HASH_END = ?)", attachmentId, remoteKey, plaintextHash)
+    } else {
+      SqlUtil.buildQuery("$ID = ?", attachmentId)
+    }
+
+    writableDatabase
+      .update(TABLE_NAME)
+      .values(
+        REMOTE_INCREMENTAL_DIGEST to null,
+        REMOTE_INCREMENTAL_DIGEST_CHUNK_SIZE to 0
+      )
+      .where(query.where, query.whereArgs)
+      .run()
+  }
+
  fun deleteAttachmentsForMessage(mmsId: Long): Boolean {
    Log.d(TAG, "[deleteAttachmentsForMessage] mmsId: $mmsId")

--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/AttachmentDownloadJob.kt
@@ -280,7 +280,7 @@ class AttachmentDownloadJob private constructor(

    try {
      if (attachment.size > maxReceiveSize) {
-        throw MmsException("Attachment too large, failing download")
+        throw MmsException("[$attachmentId] Attachment too large, failing download")
      }

      val pointer = createAttachmentPointer(attachment)
@@ -296,7 +296,7 @@ class AttachmentDownloadJob private constructor(
      }

      if (attachment.remoteDigest == null && attachment.dataHash == null) {
-        Log.w(TAG, "Attachment has no integrity check!")
+        Log.w(TAG, "[$attachmentId] Attachment has no integrity check!")
        throw InvalidAttachmentException("Attachment has no integrity check!")
      }

@@ -314,39 +314,43 @@ class AttachmentDownloadJob private constructor(
        SignalDatabase.attachments.finalizeAttachmentAfterDownload(messageId, attachmentId, input)
      }
    } catch (e: RangeException) {
-      Log.w(TAG, "Range exception, file size " + attachmentFile.length(), e)
+      Log.w(TAG, "[$attachmentId] Range exception, file size " + attachmentFile.length(), e)
      if (attachmentFile.delete()) {
-        Log.i(TAG, "Deleted temp download file to recover")
+        Log.i(TAG, "[$attachmentId] Deleted temp download file to recover")
        throw RetryLaterException(e)
      } else {
-        throw IOException("Failed to delete temp download file following range exception")
+        throw IOException("[$attachmentId] Failed to delete temp download file following range exception")
      }
    } catch (e: InvalidAttachmentException) {
-      Log.w(TAG, "Experienced exception while trying to download an attachment.", e)
+      Log.w(TAG, "[$attachmentId] Experienced exception while trying to download an attachment.", e)
      markFailed(messageId, attachmentId)
    } catch (e: NonSuccessfulResponseCodeException) {
      if (SignalStore.backup.backsUpMedia && e.code == 404 && attachment.archiveTransferState === AttachmentTable.ArchiveTransferState.FINISHED) {
-        Log.i(TAG, "Retrying download from archive CDN")
+        Log.i(TAG, "[$attachmentId] Retrying download from archive CDN")
        RestoreAttachmentJob.forManualRestore(attachment)
        return
      }

-      Log.w(TAG, "Experienced exception while trying to download an attachment.", e)
+      Log.w(TAG, "[$attachmentId] Experienced exception while trying to download an attachment.", e)
      markFailed(messageId, attachmentId)
    } catch (e: MmsException) {
-      Log.w(TAG, "Experienced exception while trying to download an attachment.", e)
+      Log.w(TAG, "[$attachmentId] Experienced exception while trying to download an attachment.", e)
      markFailed(messageId, attachmentId)
    } catch (e: MissingConfigurationException) {
-      Log.w(TAG, "Experienced exception while trying to download an attachment.", e)
+      Log.w(TAG, "[$attachmentId] Experienced exception while trying to download an attachment.", e)
      markFailed(messageId, attachmentId)
    } catch (e: InvalidMessageException) {
-      Log.w(TAG, "Experienced an InvalidMessageException while trying to download an attachment.", e)
+      Log.w(TAG, "[$attachmentId] Experienced an InvalidMessageException while trying to download an attachment.", e)
      if (e.cause is InvalidMacException) {
-        Log.w(TAG, "Detected an invalid mac. Treating as a permanent failure.")
+        Log.w(TAG, "[$attachmentId] Detected an invalid mac. Treating as a permanent failure.")
        markPermanentlyFailed(messageId, attachmentId)
      } else {
        markFailed(messageId, attachmentId)
      }
+    } catch (e: org.signal.libsignal.protocol.incrementalmac.InvalidMacException) {
+      Log.w(TAG, "[$attachmentId] Detected an invalid incremental mac. Clearing and marking as a temporary failure, requiring the user to manually try again.")
+      SignalDatabase.attachments.clearIncrementalMacsForAttachmentAndAnyDuplicates(attachmentId, attachment.remoteKey, attachment.dataHash)
+      markFailed(messageId, attachmentId)
    }
  }

--- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentJob.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RestoreAttachmentJob.kt
@@ -420,6 +420,10 @@ class RestoreAttachmentJob private constructor(
      } else {
        markFailed(attachmentId)
      }
+    } catch (e: org.signal.libsignal.protocol.incrementalmac.InvalidMacException) {
+      Log.w(TAG, "[$attachmentId] Detected an invalid incremental mac. Clearing and marking as a temporary failure, requiring the user to manually try again.")
+      SignalDatabase.attachments.clearIncrementalMacsForAttachmentAndAnyDuplicates(attachmentId, attachment.remoteKey, attachment.dataHash)
+      markFailed(attachmentId)
    }
  }

--- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2ViewModel.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/MediaPreviewV2ViewModel.kt
@@ -1,8 +1,12 @@
 package org.thoughtcrime.securesms.mediapreview

+import android.app.Notification
+import android.app.PendingIntent
 import android.content.Context
 import android.content.Intent
 import android.net.Uri
+import androidx.core.app.NotificationCompat
+import androidx.core.app.NotificationManagerCompat
 import androidx.lifecycle.ViewModel
 import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers
 import io.reactivex.rxjava3.core.Completable
@@ -11,15 +15,30 @@ import io.reactivex.rxjava3.core.Single
 import io.reactivex.rxjava3.disposables.CompositeDisposable
 import io.reactivex.rxjava3.kotlin.plusAssign
 import io.reactivex.rxjava3.schedulers.Schedulers
+import org.signal.core.util.PendingIntentFlags
+import org.signal.core.util.concurrent.SignalExecutors
+import org.signal.core.util.logging.Log
+import org.thoughtcrime.securesms.R
 import org.thoughtcrime.securesms.attachments.AttachmentId
 import org.thoughtcrime.securesms.attachments.DatabaseAttachment
 import org.thoughtcrime.securesms.database.MediaTable
+import org.thoughtcrime.securesms.database.SignalDatabase
+import org.thoughtcrime.securesms.dependencies.AppDependencies
+import org.thoughtcrime.securesms.logsubmit.SubmitDebugLogActivity
 import org.thoughtcrime.securesms.mediasend.Media
+import org.thoughtcrime.securesms.mms.PartUriParser
+import org.thoughtcrime.securesms.notifications.NotificationChannels
+import org.thoughtcrime.securesms.notifications.NotificationIds
+import org.thoughtcrime.securesms.util.RemoteConfig
 import org.thoughtcrime.securesms.util.rx.RxStore
 import java.util.Optional

 class MediaPreviewV2ViewModel : ViewModel() {

+  companion object {
+    private val TAG = Log.tag(MediaPreviewV2ViewModel::class)
+  }
+
  private val store = RxStore(MediaPreviewV2State())
  private val disposables = CompositeDisposable()
  private val repository: MediaPreviewRepository = MediaPreviewRepository()
@@ -111,6 +130,24 @@ class MediaPreviewV2ViewModel : ViewModel() {
    return repository.getMessagePositionIntent(context, messageId)
  }

+  fun onIncrementalMacError(uri: Uri) {
+    maybePostInvalidMacErrorNotification(AppDependencies.application)
+
+    val attachmentId = try {
+      val parser = PartUriParser(uri)
+      parser.partId
+    } catch (e: Exception) {
+      Log.w(TAG, "Got an incremental mac error, but could not parse the attachment data from the URI!", e)
+      return
+    }
+
+    SignalExecutors.BOUNDED.execute {
+      Log.w(TAG, "Got an incremental mac error for attachment $attachmentId, clearing the incremental mac data.")
+      val attachment = SignalDatabase.attachments.getAttachment(attachmentId) ?: return@execute
+      SignalDatabase.attachments.clearIncrementalMacsForAttachmentAndAnyDuplicates(attachmentId, attachment.remoteKey, attachment.dataHash)
+    }
+  }
+
  override fun onCleared() {
    disposables.dispose()
    store.dispose()
@@ -121,6 +158,21 @@ class MediaPreviewV2ViewModel : ViewModel() {
      oldState.copy(loadState = MediaPreviewV2State.LoadState.DATA_LOADED)
    }
  }
+
+  private fun maybePostInvalidMacErrorNotification(context: Context) {
+    if (!RemoteConfig.internalUser) {
+      return
+    }
+
+    val notification: Notification = NotificationCompat.Builder(context, NotificationChannels.getInstance().FAILURES)
+      .setSmallIcon(R.drawable.ic_notification)
+      .setContentTitle("[Internal-only] Bad incrementalMac!")
+      .setContentText("Tap to send a debug log")
+      .setContentIntent(PendingIntent.getActivity(context, 0, Intent(context, SubmitDebugLogActivity::class.java), PendingIntentFlags.mutable()))
+      .build()
+
+    NotificationManagerCompat.from(context).notify(NotificationIds.INTERNAL_ERROR, notification)
+  }
 }

 fun MediaTable.MediaRecord.toMedia(): Media? {
--- a/app/src/main/java/org/thoughtcrime/securesms/mediapreview/VideoMediaPreviewFragment.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/mediapreview/VideoMediaPreviewFragment.java
@@ -17,8 +17,10 @@ import androidx.media3.ui.LegacyPlayerControlView;

 import org.signal.core.util.concurrent.LifecycleDisposable;
 import org.signal.core.util.logging.Log;
+import org.signal.libsignal.protocol.incrementalmac.InvalidMacException;
 import org.thoughtcrime.securesms.R;
 import org.thoughtcrime.securesms.components.voice.VoiceNoteMediaControllerOwner;
+import org.thoughtcrime.securesms.mms.PartUriParser;
 import org.thoughtcrime.securesms.mms.VideoSlide;
 import org.thoughtcrime.securesms.util.MediaUtil;
 import org.thoughtcrime.securesms.video.VideoPlayer;
@@ -97,7 +99,10 @@ public final class VideoMediaPreviewFragment extends MediaPreviewFragment {
      }

      @Override
-      public void onError() {
+      public void onError(Exception e) {
+        if (e instanceof InvalidMacException) {
+          viewModel.onIncrementalMacError(uri);
+        }
        events.unableToPlayMedia();
      }
    });
--- a/app/src/main/java/org/thoughtcrime/securesms/mediasend/VideoEditorFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/mediasend/VideoEditorFragment.kt
@@ -97,7 +97,7 @@ class VideoEditorFragment : Fragment(), PositionDragListener, MediaSendPageFragm

        override fun onStopped() = Unit

-        override fun onError() {
+        override fun onError(e: Exception) {
          controller.onPlayerError()
        }
      })
@@ -126,7 +126,7 @@ class VideoEditorFragment : Fragment(), PositionDragListener, MediaSendPageFragm
          hud.showPlayButton()
        }

-        override fun onError() {
+        override fun onError(e: Exception) {
          controller.onPlayerError()
        }
      })
--- a/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/post/StoryPostFragment.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/stories/viewer/post/StoryPostFragment.kt
@@ -95,7 +95,7 @@ class StoryPostFragment : Fragment(R.layout.stories_post_fragment) {
      }

      override fun onStopped() {}
-      override fun onError() {
+      override fun onError(e: Exception) {
        requireCallback().onContentNotAvailable()
      }
    })
--- a/app/src/main/java/org/thoughtcrime/securesms/video/VideoPlayer.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/video/VideoPlayer.java
@@ -39,19 +39,24 @@ import androidx.media3.common.Player;
 import androidx.media3.common.Tracks;
 import androidx.media3.common.util.UnstableApi;
 import androidx.media3.exoplayer.ExoPlayer;
+import androidx.media3.exoplayer.analytics.AnalyticsListener;
 import androidx.media3.exoplayer.source.ClippingMediaSource;
 import androidx.media3.exoplayer.source.DefaultMediaSourceFactory;
+import androidx.media3.exoplayer.source.LoadEventInfo;
+import androidx.media3.exoplayer.source.MediaLoadData;
 import androidx.media3.exoplayer.source.MediaSource;
 import androidx.media3.ui.AspectRatioFrameLayout;
 import androidx.media3.ui.LegacyPlayerControlView;
 import androidx.media3.ui.PlayerView;

 import org.signal.core.util.logging.Log;
+import org.signal.libsignal.protocol.incrementalmac.InvalidMacException;
 import org.thoughtcrime.securesms.R;
 import org.thoughtcrime.securesms.dependencies.AppDependencies;
 import org.thoughtcrime.securesms.mediapreview.MediaPreviewPlayerControlView;
 import org.thoughtcrime.securesms.mms.VideoSlide;

+import java.io.IOException;
 import java.util.Objects;

@OptIn(markerClass = UnstableApi.class)
@@ -74,6 +79,7 @@ public class VideoPlayer extends FrameLayout {
  private long                                clippedStartUs;
  private ExoPlayerListener                   exoPlayerListener;
  private Player.Listener                     playerListener;
+  private AnalyticsListener                   analyticsListener;
  private boolean                             muted;
  private AudioFocusRequest                   audioFocusRequest;
  private boolean                             requestAudioFocus = true;
@@ -119,6 +125,15 @@ public class VideoPlayer extends FrameLayout {
    }

    this.exoPlayerListener = new ExoPlayerListener();
+    this.analyticsListener = new AnalyticsListener() {
+      @Override
+      public void onLoadError(EventTime eventTime, LoadEventInfo loadEventInfo, MediaLoadData mediaLoadData, IOException error, boolean wasCanceled) {
+        if (error instanceof InvalidMacException) {
+          Log.w(TAG, "Bad incremental mac!", error);
+          playerCallback.onError(error);
+        }
+      }
+    };
    this.playerListener    = new Player.Listener() {

      @Override
@@ -159,7 +174,6 @@ public class VideoPlayer extends FrameLayout {
            );
          }
        }
-
      }

      @Override
@@ -201,7 +215,7 @@ public class VideoPlayer extends FrameLayout {
      public void onPlayerError(@NonNull PlaybackException error) {
        Log.w(TAG, "A player error occurred", error);
        if (playerCallback != null) {
-          playerCallback.onError();
+          playerCallback.onError(error);
        }
      }
    };
@@ -226,6 +240,7 @@ public class VideoPlayer extends FrameLayout {
      exoPlayer = AppDependencies.getExoPlayerPool().require(poolTag);
      exoPlayer.addListener(exoPlayerListener);
      exoPlayer.addListener(playerListener);
+      exoPlayer.addAnalyticsListener(analyticsListener);
      exoView.setPlayer(exoPlayer);
      exoControls.setPlayer(exoPlayer);
      if (muted) {
@@ -513,6 +528,6 @@ public class VideoPlayer extends FrameLayout {

    void onStopped();

-    void onError();
+    void onError(Exception e);
  }
 }
--- a/app/src/main/java/org/thoughtcrime/securesms/video/exo/PartDataSource.java
+++ b/app/src/main/java/org/thoughtcrime/securesms/video/exo/PartDataSource.java
@@ -33,6 +33,7 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -75,6 +76,7 @@ class PartDataSource implements DataSource {
      final byte[] decodedKey = Base64.decode(attachmentKey);
      
      if (attachment.transferState == AttachmentTable.TRANSFER_RESTORE_IN_PROGRESS && attachment.archiveTransferState == AttachmentTable.ArchiveTransferState.FINISHED) {
+        Log.d(TAG, "Playing partial video content for archive attachment.");
        final File archiveFile = attachmentDatabase.getOrCreateTransferFile(attachment.attachmentId);
        try {
          String mediaName = DatabaseAttachmentArchiveUtil.requireMediaNameAsString(attachment);
@@ -83,10 +85,6 @@ class PartDataSource implements DataSource {
          MediaRootBackupKey.MediaKeyMaterial mediaKeyMaterial     = SignalStore.backup().getMediaRootBackupKey().deriveMediaSecretsFromMediaId(mediaId);
          long                                originalCipherLength = AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(attachment.size));

-          if (attachment.remoteDigest == null) {
-            throw new InvalidMessageException("Missing digest!");
-          }
-
          if (attachment.dataHash == null || attachment.dataHash.isEmpty()) {
            throw new InvalidMessageException("Missing plaintextHash!");
          }
@@ -96,13 +94,14 @@ class PartDataSource implements DataSource {
          throw new IOException("Error decrypting attachment stream!", e);
        }
      } else {
+        Log.d(TAG, "Playing partial video content for normal attachment.");
        final File transferFile = attachmentDatabase.getOrCreateTransferFile(attachment.attachmentId);
        try {
          long                                       streamLength   = AttachmentCipherStreamUtil.getCiphertextLength(PaddingInputStream.getPaddedSize(attachment.size));
          AttachmentCipherInputStream.StreamSupplier streamSupplier = () -> new TailerInputStream(() -> new FileInputStream(transferFile), streamLength);

          if (attachment.remoteDigest == null && attachment.dataHash == null) {
-            throw new InvalidMessageException("Missing digest!");
+            throw new InvalidMessageException("Missing digest and plaintextHash!");
          }

          IntegrityCheck integrityCheck = IntegrityCheck.forEncryptedDigestAndPlaintextHash(attachment.remoteDigest, attachment.dataHash);
@@ -119,6 +118,7 @@ class PartDataSource implements DataSource {

      Log.d(TAG, "Successfully loaded partial attachment file.");
    } else if (!inProgress || hasData) {
+      Log.d(TAG, "Playing a fully downloaded attachment.");
      this.inputStream = attachmentDatabase.getAttachmentStream(partUri.getPartId(), dataSpec.position);

      Log.d(TAG, "Successfully loaded completed attachment file.");