feat(client): report which <enc> failed to decrypt, and why

src/client.rs

@@ -85,6 +85,30 @@ impl Drop for DecryptedPayloadLease {
     }
 }
 
+/// Lease that keeps per-`<enc>` decrypt-failure events enabled for one consumer.
+///
+/// Dropping the final lease disables forwarding. The lease holds only a weak
+/// client reference, so it cannot keep the client alive.
+#[must_use = "dropping the lease immediately releases enc-decrypt-failure forwarding"]
+pub struct EncDecryptFailedLease {
+    client: std::sync::Weak<Client>,
+}
+
+impl Drop for EncDecryptFailedLease {
+    fn drop(&mut self) {
+        let Some(client) = self.client.upgrade() else {
+            return;
+        };
+        let previous = client
+            .enc_decrypt_failed_forwarding
+            .fetch_sub(1, Ordering::Relaxed);
+        debug_assert!(
+            previous > 0,
+            "enc-decrypt-failure forwarding lease underflow"
+        );
+    }
+}
+
 /// Lease that keeps raw decoded stanza events enabled for one consumer.
 ///
 /// Dropping the final lease disables forwarding. The lease holds only a weak
@@ -1566,6 +1590,12 @@ pub struct Client {
     /// forwarding.
     decrypted_payload_forwarding: AtomicUsize,
 
+    /// Number of consumers currently requesting `Event::EncDecryptFailed`
+    /// forwarding. Counted apart from `decrypted_payload_forwarding` so a
+    /// consumer that only watches failures does not turn on payload cloning,
+    /// and one that only watches successes pays nothing on the failure paths.
+    enc_decrypt_failed_forwarding: AtomicUsize,
+
     /// Gate and publisher for `Event::SentFrame`. Behind an `Arc` because the
     /// noise sender task reads it; see [`SentFrameTap`].
     pub(crate) sent_frame_tap: Arc<SentFrameTap>,

src/client/accessors.rs

@@ -88,6 +88,39 @@ impl Client {
         self.decrypted_payload_forwarding.load(Ordering::Relaxed) != 0
     }
 
+    /// Acquire per-`<enc>` decrypt-failure forwarding for one consumer.
+    ///
+    /// [`Event::EncDecryptFailed`] stays enabled until every acquired lease is
+    /// dropped. While none is held nothing is emitted and nothing is built: each
+    /// failure branch costs one relaxed atomic load.
+    ///
+    /// Separate from
+    /// [`acquire_decrypted_payload_forwarding`](Self::acquire_decrypted_payload_forwarding)
+    /// on purpose — a consumer that wants both halves of a stanza's decryption
+    /// holds both leases, and one that wants only failures does not make the
+    /// success path clone plaintext.
+    ///
+    /// [`Event::EncDecryptFailed`]: wacore::types::events::Event::EncDecryptFailed
+    pub fn acquire_enc_decrypt_failed_forwarding(self: &Arc<Self>) -> EncDecryptFailedLease {
+        let incremented = self
+            .enc_decrypt_failed_forwarding
+            .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |count| {
+                count.checked_add(1)
+            })
+            .is_ok();
+        assert!(
+            incremented,
+            "enc-decrypt-failure forwarding lease counter overflow"
+        );
+        EncDecryptFailedLease {
+            client: Arc::downgrade(self),
+        }
+    }
+
+    pub(crate) fn enc_decrypt_failed_forwarding_enabled(&self) -> bool {
+        self.enc_decrypt_failed_forwarding.load(Ordering::Relaxed) != 0
+    }
+
     /// Acquire sent-frame forwarding for one consumer.
     ///
     /// [`Event::SentFrame`] stays enabled until every acquired lease is dropped.

src/client/lifecycle.rs

@@ -510,6 +510,7 @@ impl Client {
             alloc_meter: std::sync::OnceLock::new(),
             raw_node_forwarding: AtomicUsize::new(0),
             decrypted_payload_forwarding: AtomicUsize::new(0),
+            enc_decrypt_failed_forwarding: AtomicUsize::new(0),
             sent_frame_tap,
             stanza_interceptors: std::sync::RwLock::new(Arc::new(Vec::new())),
             stanza_interceptor_count: AtomicUsize::new(0),

src/lib.rs

@@ -128,7 +128,7 @@ pub use client::{
 pub use client::{CallError, Voip};
 pub use client::{
     Client, ClientBuild, ClientBuilder, ClientBuilderError, Connection, DecryptedPayloadLease,
-    RawNodeLease, SentFrameLease,
+    EncDecryptFailedLease, RawNodeLease, SentFrameLease,
 };
 #[cfg(feature = "client-lifecycle")]
 #[cfg_attr(docsrs, doc(cfg(feature = "client-lifecycle")))]
@@ -249,7 +249,7 @@ pub mod prelude {
     pub use crate::bot::{Bot, BotBuilder, BotHandle, EventDelivery, MessageContext};
     pub use crate::client::{
         Client, ClientBuilder, ClientBuilderError, ClientError, Connection, DecryptedPayloadLease,
-        RawNodeLease, SentFrameLease,
+        EncDecryptFailedLease, RawNodeLease, SentFrameLease,
     };
     #[cfg(feature = "client-lifecycle")]
     #[cfg_attr(docsrs, doc(cfg(feature = "client-lifecycle")))]

src/message.rs

@@ -1,4 +1,5 @@
 use crate::client::Client;
+use crate::types::events::EncDecryptFailureReason;
 use crate::types::events::Event;
 use crate::types::message::MessageInfo;
 use log::{debug, warn};
@@ -154,8 +155,16 @@ enum MigrationDecryptResult {
     Decrypted,
     /// Server redelivered an already-processed message.
     Duplicate,
-    /// Migration didn't apply or still failed; caller sends a retry receipt.
-    NotDecrypted,
+    /// Migration didn't apply, or applied and the retry still failed; the
+    /// caller sends a retry receipt either way.
+    ///
+    /// `Some` carries the terminal cause when a migration actually ran and its
+    /// retry decrypt failed. Without it the caller would report the error that
+    /// sent it here — typically `NoSession` — for a message whose session was
+    /// in fact found and whose retry then failed a MAC or a store read.
+    /// `None` means nothing was migrated, so the caller's own error still is
+    /// the terminal one.
+    NotDecrypted(Option<EncDecryptFailureReason>),
 }
 
 #[derive(Clone, Copy, Debug, Default)]
@@ -262,6 +271,71 @@ fn decrypt_fail_log_level(mode: crate::types::events::DecryptFailMode) -> log::L
     }
 }
 
+/// Errors libsignal raises while turning bytes into a message of their declared
+/// type: too short to hold the signature, a version this build predates or does
+/// not know, or a body that is not the protobuf it claims. No key material is
+/// used to reach any of them.
+///
+/// Shared by the session and group arms so one libsignal error cannot be
+/// reported as a malformed envelope on one path and an unclassified failure on
+/// the other. `UnrecognizedMessageVersion` is deliberately absent: it is the
+/// *state* mismatch `group_decrypt` raises after parsing, not a parse failure —
+/// `UnrecognizedCiphertextVersion` is that one.
+fn is_malformed_envelope_error(e: &SignalProtocolError) -> bool {
+    matches!(
+        e,
+        SignalProtocolError::CiphertextMessageTooShort(_)
+            | SignalProtocolError::LegacyCiphertextVersion(_)
+            | SignalProtocolError::UnrecognizedCiphertextVersion(_)
+            | SignalProtocolError::InvalidProtobufEncoding
+    )
+}
+
+/// Cause reported for a libsignal error the decrypt arms do not name themselves.
+///
+/// Shared by the session catch-all and the group arm so the same error cannot be
+/// classified two ways. `BackendError` is local storage failing to answer — the
+/// store adapter wraps every backend error in it — and not the ciphertext
+/// failing: reporting it as a cryptographic error would blame the peer for our
+/// own disk, and corrupt any per-peer health signal built on this event.
+fn signal_error_reason(e: &SignalProtocolError) -> EncDecryptFailureReason {
+    if is_malformed_envelope_error(e) {
+        EncDecryptFailureReason::MalformedCiphertext
+    } else if matches!(e, SignalProtocolError::BackendError(_, _)) {
+        EncDecryptFailureReason::StorageFailure
+    } else if matches!(e, SignalProtocolError::KeyAgreementFailed(_)) {
+        // "the active crypto provider failed the key agreement" — our provider,
+        // not the sender's bytes, which were never judged.
+        EncDecryptFailureReason::LocalCryptoFailure
+    } else if matches!(e, SignalProtocolError::UnrecognizedMessageVersion(_)) {
+        // The group arm reaches this one through `group_decrypt_retry_reason`
+        // and calls it an invalid message. Naming it here too keeps a session
+        // `<enc>` and an `skmsg` from reporting the same rejection differently.
+        EncDecryptFailureReason::InvalidMessage
+    } else {
+        EncDecryptFailureReason::SignalError
+    }
+}
+
+/// Cause for a terminal error on the 1:1 session path, matching what the arms
+/// of `process_session_enc_batch` report for the same libsignal errors.
+///
+/// Used where an error reaches a reporting site that has no arm of its own —
+/// the PN→LID migration's retry decrypt — so a migrated session that then fails
+/// a MAC is not reported under the error that opened the migration.
+fn session_error_reason(e: &SignalProtocolError) -> EncDecryptFailureReason {
+    match e {
+        SignalProtocolError::SessionNotFound(_) => EncDecryptFailureReason::NoSession,
+        SignalProtocolError::BadMac(_) => EncDecryptFailureReason::BadMac,
+        SignalProtocolError::InvalidMessage(_, _) => EncDecryptFailureReason::InvalidMessage,
+        SignalProtocolError::InvalidPreKeyId | SignalProtocolError::InvalidSignedPreKeyId => {
+            EncDecryptFailureReason::UnknownPreKey
+        }
+        SignalProtocolError::UntrustedIdentity(_) => EncDecryptFailureReason::UntrustedIdentity,
+        other => signal_error_reason(other),
+    }
+}
+
 /// WA Web treats every `SignalDecryptionError` as `SignalRetryable`, so a
 /// sender-key desync must request a resend rather than NACK (which stops the
 /// server retransmitting). `None` = keep the NACK (genuinely non-Signal error).

src/message/msg_secret.rs

@@ -2,6 +2,29 @@
 
 use super::*;
 
+/// Map a bot-payload failure onto the cause reported for its `<enc>`.
+///
+/// The stage comes from [`BotMessageError::stage`] rather than from matching
+/// variants here, so a new variant upstream cannot quietly land in the wrong
+/// bucket. An envelope rejected on shape never reached the cipher, so calling
+/// it a MAC failure would make malformed wire data count as an authentication
+/// failure against the peer.
+///
+/// A `Secret` stage is `StorageFailure`, not `NoMessageSecret`: we found a row
+/// and it would not yield a key. `NoMessageSecret` is for the lookups that came
+/// back empty, which is the companion's state; a stored secret of the wrong
+/// length is ours.
+pub(super) fn msmsg_failure_reason(
+    error: &wacore::bot_message::BotMessageError,
+) -> EncDecryptFailureReason {
+    use wacore::bot_message::BotMessageFailure;
+    match error.stage() {
+        BotMessageFailure::Envelope => EncDecryptFailureReason::MalformedCiphertext,
+        BotMessageFailure::Secret => EncDecryptFailureReason::StorageFailure,
+        BotMessageFailure::Authentication => EncDecryptFailureReason::BadMac,
+    }
+}
+
 impl Client {
     /// Capture embedded `MessageContextInfo.message_secret` for add-on
     /// decrypts. Bot DMs keep the legacy LID key as a second entry.
@@ -485,6 +508,12 @@ impl Client {
                     "[msg:{}] failed to decode MessageSecretMessage: {e:?}",
                     info.id
                 );
+                self.report_enc_decrypt_failure(
+                    info,
+                    enc_index,
+                    enc_type,
+                    EncDecryptFailureReason::MalformedCiphertext,
+                );
                 self.spawn_nack(info, NackReason::ParsingError, None);
                 return;
             }
@@ -496,6 +525,12 @@ impl Client {
                 "[msg:{}] MessageSecretMessage missing enc_iv/enc_payload",
                 info.id
             );
+            self.report_enc_decrypt_failure(
+                info,
+                enc_index,
+                enc_type,
+                EncDecryptFailureReason::MalformedCiphertext,
+            );
             self.spawn_nack(info, NackReason::ParsingError, None);
             return;
         };
@@ -507,6 +542,12 @@ impl Client {
             Some(j) => j,
             None => {
                 log::warn!("[msg:{}] msmsg: no target_sender resolvable", info.id);
+                self.report_enc_decrypt_failure(
+                    info,
+                    enc_index,
+                    enc_type,
+                    EncDecryptFailureReason::NoMessageSecret,
+                );
                 self.spawn_nack(info, NackReason::MissingMessageSecret, None);
                 return;
             }
@@ -533,6 +574,12 @@ impl Client {
                     "[msg:{}] msmsg: <meta> missing target_id; cannot look up secret",
                     info.id
                 );
+                self.report_enc_decrypt_failure(
+                    info,
+                    enc_index,
+                    enc_type,
+                    EncDecryptFailureReason::NoMessageSecret,
+                );
                 self.spawn_nack(info, NackReason::MissingMessageSecret, None);
                 return;
             }
@@ -547,6 +594,12 @@ impl Client {
         // Store lookup: primary, then the LID/PN alternate. A backend error is
         // logged and treated as a miss (not a hard nack) so the resolver still
         // gets a chance — mirrors the secret-encrypted edit path.
+        //
+        // Treated as a miss for control flow, but not for reporting: "the store
+        // would not answer" is ours and "no secret here" is the companion's,
+        // and by the time the cause is named nothing else remembers which of
+        // the two emptied the lookup.
+        let mut lookup_failed = false;
         let buffered = self
             .msg_secret_buffer
             .lookup(&chat_for_lookup, &target_sender_str, target_id)
@@ -570,6 +623,7 @@ impl Client {
                     Ok(found) => found,
                     Err(e) => {
                         log::warn!("[msg:{}] msmsg: alternate lookup failed: {e:?}", info.id);
+                        lookup_failed = true;
                         None
                     }
                 },
@@ -578,6 +632,7 @@ impl Client {
                         "[msg:{}] backend error reading message_secret: {e:?}",
                         info.id
                     );
+                    lookup_failed = true;
                     None
                 }
             },
@@ -619,6 +674,16 @@ impl Client {
                             "[msg:{}] msmsg: no message_secret stored for target_id={target_id} (primary or alternate)",
                             info.id
                         );
+                        self.report_enc_decrypt_failure(
+                            info,
+                            enc_index,
+                            enc_type,
+                            if lookup_failed {
+                                EncDecryptFailureReason::StorageFailure
+                            } else {
+                                EncDecryptFailureReason::NoMessageSecret
+                            },
+                        );
                         self.spawn_nack(info, NackReason::MissingMessageSecret, None);
                         return;
                     }
@@ -681,6 +746,15 @@ impl Client {
                             "[msg:{}] msmsg AES-GCM open failed both attempts (primary={primary_err:?}, fallback={fallback_err:?})",
                             info.id
                         );
+                        // Both attempts see the same iv/payload, so a shape
+                        // rejection is identical on either and the primary
+                        // error decides.
+                        self.report_enc_decrypt_failure(
+                            info,
+                            enc_index,
+                            enc_type,
+                            msmsg_failure_reason(&primary_err),
+                        );
                         self.spawn_nack(info, NackReason::MissingMessageSecret, None);
                         return;
                     }
@@ -690,6 +764,12 @@ impl Client {
                         "[msg:{}] msmsg AES-GCM open failed and no fallback msg_id: {primary_err:?}",
                         info.id
                     );
+                    self.report_enc_decrypt_failure(
+                        info,
+                        enc_index,
+                        enc_type,
+                        msmsg_failure_reason(&primary_err),
+                    );
                     self.spawn_nack(info, NackReason::MissingMessageSecret, None);
                     return;
                 }
@@ -719,6 +799,12 @@ impl Client {
                     "[msg:{}] msmsg plaintext is not a Message proto: {e:?}",
                     info.id
                 );
+                self.report_enc_decrypt_failure(
+                    info,
+                    enc_index,
+                    enc_type,
+                    EncDecryptFailureReason::PlaintextUnusable,
+                );
                 self.spawn_nack(info, NackReason::ParsingError, None);
                 return;
             }