Skip to content

fix: WA Web-compliant DM multi-device fanout with phash validation #524

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jlucaso1 merged 8 commits into from
Apr 14, 2026
Merged

fix: WA Web-compliant DM multi-device fanout with phash validation #524

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d44d1a9
fix: encrypt DMs for all recipient devices, not just bare JID
mcaldas Apr 13, 2026
59709fc
fix: WA Web-compliant DM device fanout, phash validation, hosted filt…
jlucaso1 Apr 13, 2026
29477fb
fix: sender-device exclusion, scoped device invalidation, phash from …
jlucaso1 Apr 13, 2026
efdb89e
fix: skip device cache invalidation for status@broadcast
jlucaso1 Apr 13, 2026
20c2309
fix: try PN→LID session migration on InvalidPreKeyId
jlucaso1 Apr 13, 2026
c3a01d1
fix: don't send phash on DM stanzas (WA Web compliance)
jlucaso1 Apr 13, 2026
0b7fe97
fix: self-DM dedup, own device cache invalidation, phash boolean swap
jlucaso1 Apr 14, 2026
55997fb
fix: try PN→LID migration in post-identity-change InvalidPreKeyId path
jlucaso1 Apr 14, 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
82 changes: 49 additions & 33 deletions src/message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -833,27 +833,34 @@ impl Client {
any_duplicate = true;
} else if matches!(retry_err, SignalProtocolError::InvalidPreKeyId)
{
// InvalidPreKeyId after identity change means the sender is using
// an old prekey that we no longer have. This typically happens when:
// 1. The sender reinstalled WhatsApp and cached our old prekey bundle
// 2. The prekey they're using has been consumed or rotated out
//
// Solution: Send a retry receipt with a fresh prekey so the sender
// can establish a new session and resend the message.
log::warn!(
"[msg:{}] Decryption failed for {} due to InvalidPreKeyId after identity change. \
The sender is using an old prekey we no longer have. \
Sending retry receipt with fresh keys.",
info.id,
address
);

// Send retry receipt so the sender fetches our new prekey bundle
dispatched_undecryptable = self.handle_decrypt_failure(
info,
RetryReason::InvalidKeyId,
decrypt_fail_mode,
);
// Session may exist under PN address after identity change
if self
.try_pn_to_lid_migration_decrypt(
sender_encryption_jid,
&signal_address,
&parsed_message,
&mut adapter,
&mut rng,
&enc_type,
padding_version,
info,
)
.await
{
any_success = true;
} else {
log::warn!(
"[msg:{}] InvalidPreKeyId after identity change for {}. \
Sending retry receipt with fresh keys.",
info.id,
address
);
dispatched_undecryptable = self.handle_decrypt_failure(
info,
RetryReason::InvalidKeyId,
decrypt_fail_mode,
);
}
} else {
log::error!(
"[msg:{}] Decryption failed even after clearing untrusted identity for {}: {:?}",
Expand Down Expand Up @@ -951,18 +958,27 @@ impl Client {
self.handle_decrypt_failure(info, reason, decrypt_fail_mode);
continue;
} else if matches!(e, SignalProtocolError::InvalidPreKeyId) {
// InvalidPreKeyId means the sender is using a PreKey ID that we don't have.
// This typically happens when:
// 1. We were offline for a long time
// 2. The sender established a session with us using a prekey from the server
// 3. We never received the initial session-establishing message
// 4. Now we're receiving messages with counters 3, 4, 5... referencing that prekey
//
// The sender thinks they have a valid session, but we never had it.
// We need to send a retry receipt with fresh prekeys so the sender can:
// 1. Delete their old session
// 2. Fetch our new prekeys from the retry receipt
// 3. Create a NEW session and resend with counter 0
// InvalidPreKeyId on a PreKeyMessage can also mean the
// session exists under a PN address (legacy migration).
// Migrating lets Signal use the existing ratchet state
// instead of looking up the consumed one-time prekey.
if self
.try_pn_to_lid_migration_decrypt(
sender_encryption_jid,
&signal_address,
&parsed_message,
&mut adapter,
&mut rng,
&enc_type,
padding_version,
info,
)
.await
{
any_success = true;
continue;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

log::warn!(
"[msg:{}] Decryption failed for {} message from {} due to InvalidPreKeyId. \
Sender is using a prekey we don't have (likely session established while offline). \
Expand Down
83 changes: 64 additions & 19 deletions src/send.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ impl Client {
}

if let Some((rx, phash)) = ack {
self.spawn_phash_validation(rx, phash, to.clone(), false, request_id.clone());
self.spawn_phash_validation(rx, phash, to.clone(), true, request_id.clone());
}

self.update_sender_key_devices(&to_str, &prepared.skdm_devices)
Expand Down Expand Up @@ -666,6 +666,16 @@ impl Client {
log::warn!(
"Phash mismatch for {jid}: ours={our_phash}, server={server}. Invalidating caches."
);
// DM phash covers both recipient + own devices
// (WA Web: syncDeviceListJob([recipient, me]))
if !jid.is_group() && !jid.is_status_broadcast() {
client.invalidate_device_cache(&jid.user).await;
if let Some(own_pn) =
&client.persistence_manager.get_device_snapshot().await.pn
{
client.invalidate_device_cache(&own_pn.user).await;
}
}
Comment on lines +671 to +678

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Invalidate own device cache on DM phash mismatch

prepare_dm_stanza now hashes both recipient and own companion targets into DM phash, but this mismatch handler only invalidates the recipient user’s device cache (jid.user). If the mismatch is caused by our own stale companion list (e.g., a newly linked device), subsequent sends keep using the stale own-device registry and continue missing self-fanout/resync for that device. The DM phash self-healing path should also invalidate the local account’s device cache, not just the destination user.

Useful? React with 👍 / 👎.

client
.sender_key_device_cache
.invalidate(&jid.to_string())
Expand Down Expand Up @@ -856,6 +866,7 @@ impl Client {
let mut used_cached_tc_token_key: Option<String> = None;
let tc_issue_target = to.clone();

let mut dm_phash: Option<String> = None;
let stanza_to_send: wacore_binary::Node = if peer && !to.is_group() {
// Peer messages are only valid for individual users, not groups
// Resolve encryption JID and acquire lock ONLY for encryption
Expand Down Expand Up @@ -1056,20 +1067,55 @@ impl Client {
}
}

// DM fanout: bare recipient (device 0) + own companion devices.
// WA Web (MsgCreateFanoutStanza.js): for CHAT fanout with a single
// primary device, encrypts directly for that device only. Own devices
// get per-device enc for multi-device self-sync. The server routes
// the bare enc to the correct recipient device.
// DM fanout: all known recipient devices + own companions.
// WAWebSendUserMsgJob reads local device table only on the send
// path; WAWebDBDeviceListFanout excludes hosted devices.
let recipient_bare = self.resolve_encryption_jid(&to).await.to_non_ad();

// Populate device registry for retry handling
let _ = self.get_user_devices(std::slice::from_ref(&to)).await;
let own_devices = self.get_user_devices(std::slice::from_ref(own_jid)).await?;
// Local registry first; network warm only on miss to avoid
// unnecessary LID-migration side effects from get_user_devices
let mut recipient_cached = self.get_devices_from_registry(&recipient_bare).await;
if recipient_cached.is_none() {
let _ = self.get_user_devices(std::slice::from_ref(&to)).await;
recipient_cached = self.get_devices_from_registry(&recipient_bare).await;
}

let mut own_cached = self.get_devices_from_registry(own_jid).await;
if own_cached.is_none() {
let _ = self.get_user_devices(std::slice::from_ref(own_jid)).await;
own_cached = self.get_devices_from_registry(own_jid).await;
}

// Build device list, filter hosted in-place, reuse Vecs
let mut all_dm_jids = match recipient_cached {
Some(mut devices) => {
devices.retain(|j| !j.is_hosted());
devices
Comment on lines +1091 to +1093

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add recipient fallback when cached device list filters to empty

When recipient_cached exists but becomes empty after hosted-device filtering, this branch returns an empty recipient fanout instead of adding recipient_bare. In that case the DM stanza can be built with only own-device payloads (or no recipient payload at all), so the remote user cannot decrypt/receive the message until cache state changes. This is a send-path regression versus the previous unconditional bare-recipient fallback and is reproducible with empty/stale registry records or hosted-only cached entries.

Useful? React with 👍 / 👎.

}
// No record at all — bare JID, server handles fanout
None => vec![recipient_bare],
};

if let Some(mut own_devices) = own_cached {
own_devices.retain(|j| !j.is_hosted());
all_dm_jids.append(&mut own_devices);
Comment on lines +1099 to +1101

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Deduplicate merged DM fanout devices

This merge path can duplicate targets when the recipient is your own account ("message yourself"): recipient_cached and own_cached both contain the same devices, and append adds them twice. The duplicated all_dm_jids is then used for DM encryption, so the stanza can contain repeated <to> payloads and a phash over duplicates, which can trigger unnecessary phash-mismatch invalidations and resend churn for self-DMs. Deduplicate the merged list (after PN/LID normalization) before session setup/encryption.

Useful? React with 👍 / 👎.

}

let mut all_dm_jids = Vec::with_capacity(1 + own_devices.len());
all_dm_jids.push(recipient_bare);
all_dm_jids.extend(own_devices);
// Exclude exact sender device (WA Web: isMeDevice in getFanOutList)
// so ensure_e2e_sessions never creates a self-session
let own_lid = device_snapshot.lid.as_ref();
all_dm_jids.retain(|j| {
let is_sender = (j.is_same_user_as(own_jid) && j.device == own_jid.device)
|| own_lid.is_some_and(|lid| j.is_same_user_as(lid) && j.device == lid.device);
!is_sender
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// Dedup for self-DMs: recipient and own device lists overlap
// when sending to own account (WA Web uses Map keyed by toString)
{
let mut seen = std::collections::HashSet::with_capacity(all_dm_jids.len());
all_dm_jids.retain(|j| seen.insert(j.clone()));
}

self.ensure_e2e_sessions(&all_dm_jids).await?;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down Expand Up @@ -1098,7 +1144,7 @@ impl Client {

let mut stores = store_adapter.as_signal_stores();

wacore::send::prepare_dm_stanza(
let prepared = wacore::send::prepare_dm_stanza(
&mut stores,
self,
own_jid,
Expand All @@ -1111,13 +1157,12 @@ impl Client {
&extra_stanza_nodes,
all_dm_jids,
)
.await?
.await?;
dm_phash = prepared.phash;
prepared.node
};

let ack = if let Some(phash) = stanza_to_send
.attrs()
.optional_string("phash")
.map(|s| s.into_owned())
let ack = if let Some(phash) = dm_phash
&& let Some(msg_id) = stanza_to_send
Comment on lines +1165 to 1166

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Gate ACK phash validation on stanza phash, not DM-only state

This condition now checks dm_phash, which is only populated in the 1:1 DM branch, so normal group sends no longer register an ACK waiter even when their stanza carries a phash (set by prepare_group_stanza). As a result, group phash mismatches are never observed and the self-healing invalidation path in spawn_phash_validation no longer runs for group messages, leaving stale sender-key/device cache state in place after server-reported drift.

Useful? React with 👍 / 👎.

.attrs()
.optional_string("id")
Expand All @@ -1137,7 +1182,7 @@ impl Client {
}

if let Some((rx, phash, msg_id)) = ack {
self.spawn_phash_validation(rx, phash, tc_issue_target.clone(), true, msg_id);
self.spawn_phash_validation(rx, phash, tc_issue_target.clone(), false, msg_id);
}

if let Some(update) = skdm_update {
Expand Down
41 changes: 34 additions & 7 deletions wacore/src/send.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,16 @@ fn partition_dm_devices(
(recipient_devices, own_other_devices)
}

/// Result of `prepare_dm_stanza` — carries the stanza node and the
/// locally computed phash for server ACK validation.
pub struct PreparedDmStanza {
pub node: Node,
/// Locally computed phash from the sent device set. Not sent on the
/// wire (WA Web only sends phash for groups). Used by the caller to
/// compare against the server's ACK phash for device-list drift detection.
pub phash: Option<String>,
}

#[allow(clippy::too_many_arguments)]
pub async fn prepare_dm_stanza<
'a,
Expand All @@ -694,7 +704,7 @@ pub async fn prepare_dm_stanza<
edit: Option<crate::types::message::EditAttribute>,
extra_stanza_nodes: &[Node],
all_devices: Vec<Jid>,
) -> Result<Node> {
) -> Result<PreparedDmStanza> {
let reporting_result = generate_reporting_token(message, &request_id, &to_jid, &to_jid, None);

let message_for_encryption = if let Some(ref result) = reporting_result {
Expand All @@ -705,21 +715,29 @@ pub async fn prepare_dm_stanza<

let recipient_plaintext = MessageUtils::encode_and_pad(&message_for_encryption);

// Partition first so phash reflects the actual sent set (sender excluded)
let total_devices = all_devices.len();
let (recipient_devices, own_other_devices) =
partition_dm_devices(all_devices, own_jid, own_lid);

let phash = {
let mut sent = Vec::with_capacity(recipient_devices.len() + own_other_devices.len());
sent.extend_from_slice(&recipient_devices);
sent.extend_from_slice(&own_other_devices);
MessageUtils::participant_list_hash(&sent).ok()
};

let dsm = wa::Message {
device_sent_message: Some(Box::new(DeviceSentMessage {
destination_jid: Some(to_jid.to_string()),
message: Some(Box::new(message_for_encryption)),
phash: Some(String::new()),
phash: None, // WA Web only sets DSM phash for groups
})),
..Default::default()
};

let own_devices_plaintext = MessageUtils::encode_and_pad(&dsm);

let total_devices = all_devices.len();
let (recipient_devices, own_other_devices) =
partition_dm_devices(all_devices, own_jid, own_lid);

let mut participant_nodes = Vec::with_capacity(total_devices);
let mut includes_prekey_message = false;

Expand All @@ -730,6 +748,12 @@ pub async fn prepare_dm_stanza<

let mediatype = media_type_from_message(message);

// NOTE: WA Web has a bare-<enc> fast path for single primary device
// (WAWebSendMsgCreateFanoutStanza). Not implemented here because
// encrypt_for_devices always wraps in <to jid=...> nodes;
// a bare-enc mode would require refactoring the encryption layer.
// The <participants> form is accepted by the server regardless.

if !recipient_devices.is_empty() {
let result = encrypt_for_devices(
stores,
Expand Down Expand Up @@ -796,7 +820,10 @@ pub async fn prepare_dm_stanza<

let stanza = stanza_builder.children(message_content_nodes).build();

Ok(stanza)
Ok(PreparedDmStanza {
node: stanza,
phash,
})
}

pub async fn prepare_peer_stanza<S, I>(
Expand Down
Loading