Skip to content
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
49920f5
feat(client): report which <enc> failed to decrypt, and why
claude Aug 9, 2026
91f4e2a
style: rustfmt the shortened test helper signature
claude Aug 9, 2026
9c626bb
fix(client): name the terminal cause on three misclassified failure b…
claude Aug 9, 2026
2c2339f
fix(client): keep the redelivery path and local storage errors out of…
claude Aug 9, 2026
65fb7f6
docs(events): stop StorageFailure promising a redelivery it does not …
claude Aug 9, 2026
f93f919
fix(store): mark a corrupt stored record as ours, not as a bad cipher…
claude Aug 9, 2026
cbaf386
fix(client): route three more terminal errors through the shared clas…
claude Aug 9, 2026
b1014d8
fix(client): report what the migration retry failed on, not what open…
claude Aug 9, 2026
22db49d
fix(client): classify the identity retry with the session mapping
claude Aug 9, 2026
2a4fe67
fix(store): mark a corrupt stored identity as ours too
claude Aug 9, 2026
0adc457
fix(client): keep two more local failures off the peer's ledger
claude Aug 9, 2026
b179607
fix(client): stop losing three failure attributions
claude Aug 9, 2026
9e6bf2b
fix(signal): mark a corrupt stored session as ours, not the peer's
claude Aug 9, 2026
66d6296
docs(events): correct two reasons my own changes made wrong
claude Aug 9, 2026
7e7e065
docs(events): the duplicate rule stops at the duplicate
claude Aug 9, 2026
91417db
docs(events): UntrustedIdentity has a second producer
claude Aug 9, 2026
d20f3b9
fix(client): report the encs a teardown abandons
claude Aug 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>,
Expand Down
33 changes: 33 additions & 0 deletions src/client/accessors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions src/client/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")))]
Expand Down Expand Up @@ -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")))]
Expand Down
92 changes: 90 additions & 2 deletions src/message.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -262,6 +271,85 @@ 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) {
Comment thread
jlucaso1 marked this conversation as resolved.
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 e.is_stored_session_corruption() {
// A stored `SessionRecord` that decoded and then would not yield usable
// state. The predicate lives in libsignal because the distinction is
// drawn on `InvalidSessionStructure`'s message, and only the crate that
// writes those messages can keep the two in step.
EncDecryptFailureReason::StorageFailure
} else if matches!(e, SignalProtocolError::InvalidSenderKeySession) {
// A sender-key record that loaded but does not hold usable state: no
// chain key, a signing key that will not parse, or a chain whose derived
// key/IV the cipher rejects. Every site `group_decrypt` can reach it
// from is reading our stored record, which is why libsignal's own log
// there says the state is corrupt. The peer's copy is judged by
// `SignatureValidationFailed` and `InvalidMessage` instead.
EncDecryptFailureReason::StorageFailure
Comment thread
jlucaso1 marked this conversation as resolved.
} 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
Comment thread
jlucaso1 marked this conversation as resolved.
Comment thread
jlucaso1 marked this conversation as resolved.
Comment thread
jlucaso1 marked this conversation as resolved.
}
}

/// 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).
Expand Down
Loading
Loading