diff --git a/src/client/context_impl.rs b/src/client/context_impl.rs index c90fcbf12..0df4cb512 100644 --- a/src/client/context_impl.rs +++ b/src/client/context_impl.rs @@ -18,13 +18,14 @@ impl SendContextResolver for Client { &self, jids: &[Jid], ) -> Result, anyhow::Error> { - self.fetch_pre_keys(jids, None).await + // The fan-out has its own batch-level handling; it wants the bundles only. + self.fetch_pre_keys(jids, None).await.map(|o| o.bundles) } async fn fetch_prekeys_for_identity_check( &self, jids: &[Jid], - ) -> Result, anyhow::Error> { + ) -> Result { self.fetch_pre_keys(jids, Some(PreKeyFetchReason::Identity)) .await .map_err(|e| { diff --git a/src/client/sessions.rs b/src/client/sessions.rs index b20d4f641..cf14b75f6 100644 --- a/src/client/sessions.rs +++ b/src/client/sessions.rs @@ -323,9 +323,13 @@ impl Client { /// `wacore::request::ServerErrorCode` to cross the crate boundary. A downcast /// to either one alone silently answers `false` for the other, and the failure /// mode of that is the send failing exactly as it did before. +/// The `` the server attaches to a device it no longer knows. +const UNREGISTERED_DEVICE_CODE: u16 = 406; + fn is_device_unregistered(err: &anyhow::Error) -> bool { use crate::error::ErrorChainExt; - err.server_rejection().is_some_and(|r| r.code == 406) + err.server_rejection() + .is_some_and(|r| r.code == UNREGISTERED_DEVICE_CODE) } /// The distinct users named by `jids`, in first-seen order. @@ -457,6 +461,30 @@ impl Client { Err(e) => return Err(e), }; + // The server named these individually, which is the per-device signal a + // batch-wide failure cannot give: refresh exactly their device lists and + // leave the rest of the batch alone. The send continues, because the + // devices that did come back with a bundle are unaffected and skipping + // them would deliver to fewer devices for a reason that only concerns + // the named ones. + if !prekey_bundles.rejected.is_empty() { + let rejected: Vec = prekey_bundles + .rejected + .iter() + .filter(|device| device.code == UNREGISTERED_DEVICE_CODE) + .map(|device| device.jid.clone()) + .collect(); + if !rejected.is_empty() { + log::debug!( + "prekey fetch rejected {} of {} device(s) as unregistered; \ + refreshing their device lists", + rejected.len(), + jids.len() + ); + self.invalidate_device_caches_for(&rejected).await; + } + } + let mut adapter = self.signal_adapter().await; let mut rng = rand::make_rng::(); @@ -465,7 +493,10 @@ impl Client { let mut failed_count = 0; for jid in jids { - if let Some(bundle) = prekey_bundles.get(&jid.normalize_for_prekey_bundle()) { + if let Some(bundle) = prekey_bundles + .bundles + .get(&jid.normalize_for_prekey_bundle()) + { match self .install_prekey_bundle_cached(jid, bundle, &mut adapter, &mut rng) .await diff --git a/src/prekeys.rs b/src/prekeys.rs index c36e5f5b2..fda3156fe 100644 --- a/src/prekeys.rs +++ b/src/prekeys.rs @@ -13,7 +13,7 @@ use std::sync::atomic::Ordering; use wacore::iq::prekeys::{ DigestKeyBundleSpec, PreKeyCountSpec, PreKeyFetchReason, PreKeyFetchSpec, PreKeyUploadSpec, }; -use wacore::libsignal::protocol::{KeyPair, PreKeyBundle, PublicKey}; +use wacore::libsignal::protocol::{KeyPair, PublicKey}; use wacore::libsignal::store::record_helpers::encode_pre_key_record_to; use wacore::store::commands::DeviceCommand; use wacore_binary::Jid; @@ -147,7 +147,7 @@ impl Client { &self, jids: &[Jid], reason: Option, - ) -> Result, anyhow::Error> { + ) -> Result { let spec = match reason { Some(r) => PreKeyFetchSpec::with_reason(jids.to_vec(), r), None => PreKeyFetchSpec::new(jids.to_vec()), @@ -161,13 +161,13 @@ impl Client { // identity as the fallback in validateADVwithIdentityKey). let spec = spec.with_account_identities(self.collect_account_identities(jids).await); - let bundles = self.execute(spec).await?; + let outcome = self.execute(spec).await?; - for jid in bundles.keys() { + for jid in outcome.bundles.keys() { log::debug!("Successfully parsed pre-key bundle for {}", jid.observe()); } - Ok(bundles) + Ok(outcome) } /// Load, for each companion JID, its account (device 0) identity key from the diff --git a/wacore/benches/send_receive_benchmark.rs b/wacore/benches/send_receive_benchmark.rs index 37a617a17..7b61503d0 100644 --- a/wacore/benches/send_receive_benchmark.rs +++ b/wacore/benches/send_receive_benchmark.rs @@ -441,8 +441,8 @@ impl SendContextResolver for MockResolver { async fn fetch_prekeys_for_identity_check( &self, _: &[Jid], - ) -> Result, anyhow::Error> { - Ok(HashMap::new()) + ) -> Result { + Ok(wacore::prekeys::PreKeyFetchOutcome::default()) } async fn resolve_group_info( &self, diff --git a/wacore/src/client/context.rs b/wacore/src/client/context.rs index 93c0d517d..7a9fc250a 100644 --- a/wacore/src/client/context.rs +++ b/wacore/src/client/context.rs @@ -220,10 +220,13 @@ pub trait SendContextResolver: crate::sync_marker::MaybeSendSync { jids: &[Jid], ) -> Result, anyhow::Error>; + /// Returns the bundles alongside the devices the server rejected by name, + /// so a per-device rejection is not flattened into "no bundle" before the + /// fan-out can tell the two apart. async fn fetch_prekeys_for_identity_check( &self, jids: &[Jid], - ) -> Result, anyhow::Error>; + ) -> Result; async fn resolve_group_info(&self, jid: &Jid) -> Result, anyhow::Error>; diff --git a/wacore/src/iq/prekeys.rs b/wacore/src/iq/prekeys.rs index dd0c82f4e..f5b460464 100644 --- a/wacore/src/iq/prekeys.rs +++ b/wacore/src/iq/prekeys.rs @@ -144,7 +144,7 @@ impl PreKeyFetchSpec { } impl IqSpec for PreKeyFetchSpec { - type Response = std::collections::HashMap; + type Response = crate::prekeys::PreKeyFetchOutcome; fn build_iq(&self) -> InfoQuery<'static> { let content = PreKeyUtils::build_fetch_prekeys_request( diff --git a/wacore/src/prekeys.rs b/wacore/src/prekeys.rs index 8a236ba3c..bf9aaa2aa 100644 --- a/wacore/src/prekeys.rs +++ b/wacore/src/prekeys.rs @@ -5,6 +5,27 @@ use wacore_binary::Jid; use wacore_binary::builder::NodeBuilder; use wacore_binary::{Node, NodeRef}; +/// A device the server refused to hand a bundle for, named individually. +#[derive(Debug, Clone)] +pub struct RejectedDevice { + pub jid: Jid, + /// The `` the server attached; `406` means unregistered. + pub code: u16, +} + +/// What one prekey fetch produced: the bundles it did return, and the devices +/// it named as rejected. +/// +/// Kept apart because they call for opposite responses. A missing bundle is +/// ambiguous and the device is simply skipped this round; a rejected one is the +/// server telling us which cached device is gone, which is the only per-device +/// signal a batch-wide failure cannot give. +#[derive(Default)] +pub struct PreKeyFetchOutcome { + pub bundles: HashMap, + pub rejected: Vec, +} + pub struct PreKeyUtils; /// Compute SHA-1 digest of a key bundle for validation against server. @@ -175,21 +196,28 @@ impl PreKeyUtils { pub fn parse_prekeys_response( resp_node: &NodeRef<'_>, account_identities: &HashMap, - ) -> Result, anyhow::Error> { + ) -> Result { let list_node = resp_node .get_optional_child("list") .ok_or_else(|| anyhow::anyhow!(" not found in pre-key response"))?; let children = list_node.children().unwrap_or_default(); - let mut bundles = HashMap::with_capacity(children.len()); + let mut outcome = PreKeyFetchOutcome { + bundles: HashMap::with_capacity(children.len()), + rejected: Vec::new(), + }; for user_node_ref in children { if user_node_ref.tag != "user" { continue; } - let mut jid = user_node_ref - .attrs() - .jid("jid") - .normalize_for_prekey_bundle(); + // A `` whose jid is missing or unparseable names no device, and + // the default would name device 0 of an empty user -- an address a + // rejection must never be recorded against. + let Some(named) = user_node_ref.attrs().optional_jid("jid") else { + log::warn!("prekey response carried a with no usable jid; skipping"); + continue; + }; + let mut jid = named.normalize_for_prekey_bundle(); if jid.device == 0 && matches!( jid.server, @@ -201,6 +229,35 @@ impl PreKeyUtils { jid.user = CompactString::from(user_base); jid.device = device; } + // A rejected device answers with an `` in place of its key + // material, which is how the server names the one device a + // batch-wide failure cannot. Distinguished from a malformed bundle + // because only this one means "this device is gone": the caller + // refreshes that device list, and the rest of the batch is + // unaffected. Mirrors WA Web's `FetchKeyBundlesUserError` arm, + // which collects per-item errors alongside the bundles. + if let Some(error_node) = user_node_ref.get_optional_child("error") { + // Narrowed, not truncated: `65942 as u16` is 406, so a cast + // would let an out-of-range code arrive as the one value that + // makes us refresh a device list. A code we cannot read names + // nothing we can act on, so the entry is dropped rather than + // guessed at. + match error_node + .attrs() + .optional_u64("code") + .and_then(|raw| u16::try_from(raw).ok()) + { + Some(code) => { + log::debug!("prekey fetch rejected {} with code {code}", jid.observe()); + outcome.rejected.push(RejectedDevice { jid, code }); + } + None => log::warn!( + "prekey fetch rejected {} with an unreadable code; ignoring", + jid.observe() + ), + } + continue; + } let account_identity = account_identities.get(&jid); let bundle = match Self::node_to_pre_key_bundle_ref(&jid, user_node_ref, account_identity) { @@ -210,10 +267,10 @@ impl PreKeyUtils { continue; } }; - bundles.insert(jid, bundle); + outcome.bundles.insert(jid, bundle); } - Ok(bundles) + Ok(outcome) } fn node_to_pre_key_bundle_ref( @@ -508,7 +565,8 @@ mod tests { .build(); let bundles = PreKeyUtils::parse_prekeys_response(&response.as_node_ref(), &HashMap::new()) - .expect("parse bundles"); + .expect("parse bundles") + .bundles; assert!(bundles.contains_key(&base_jid)); assert!(!bundles.contains_key(&raw_jid)); @@ -518,6 +576,132 @@ mod tests { assert_eq!(parsed_jid.agent, 0); } + /// The server names a rejected device inside its own ``, which is the + /// per-device signal a batch-wide `406` cannot give. The rest of the batch + /// must come back intact, or one absent device would cost the send every + /// other device in it. + #[test] + fn a_rejected_user_is_named_without_costing_the_rest_of_the_batch() { + let good_jid = Jid::lid_device("100000012345678", 1); + let gone_jid = Jid::lid_device("100000087654321", 2); + + let good = + PreKeyBundleUserNode::from_bundle(good_jid.clone(), &create_mock_bundle(1), None) + .expect("build bundle node") + .into_node(); + + // What the server sends in place of key material for a device it no + // longer knows: an where the bundle would be. + let gone = NodeBuilder::new("user") + .attr("jid", NodeValue::Jid(gone_jid.clone())) + .children([NodeBuilder::new("error") + .attr("code", "406") + .attr("text", "not-acceptable") + .build()]) + .build(); + + let response = NodeBuilder::new("iq") + .children([NodeBuilder::new("list").children([good, gone]).build()]) + .build(); + + let outcome = PreKeyUtils::parse_prekeys_response(&response.as_node_ref(), &HashMap::new()) + .expect("a rejected user must not fail the whole parse"); + + assert!( + outcome.bundles.contains_key(&good_jid), + "the healthy device still gets its bundle" + ); + assert!( + !outcome.bundles.contains_key(&gone_jid), + "the rejected device has no bundle to give" + ); + assert_eq!(outcome.rejected.len(), 1); + assert_eq!(outcome.rejected[0].jid, gone_jid); + assert_eq!( + outcome.rejected[0].code, 406, + "the code is what separates an unregistered device from another refusal" + ); + } + + /// `65942 as u16` is exactly 406, so a truncating cast turns an + /// out-of-range code into the one value that makes the caller refresh a + /// device list. The rejection is dropped instead of being invented. + #[test] + fn an_out_of_range_error_code_is_not_narrowed_into_a_406() { + let gone_jid = Jid::lid_device("100000087654321", 2); + assert_eq!(65942_u64 as u16, 406, "the cast this guards against"); + + let gone = NodeBuilder::new("user") + .attr("jid", NodeValue::Jid(gone_jid.clone())) + .children([NodeBuilder::new("error").attr("code", "65942").build()]) + .build(); + let response = NodeBuilder::new("iq") + .children([NodeBuilder::new("list").children([gone]).build()]) + .build(); + + let outcome = PreKeyUtils::parse_prekeys_response(&response.as_node_ref(), &HashMap::new()) + .expect("parse"); + + assert!( + outcome.rejected.is_empty(), + "an unreadable code must not be recorded as a rejection" + ); + assert!(outcome.bundles.is_empty()); + } + + /// A `` with no usable jid names no device. Defaulting it would record + /// the rejection against device 0 of an empty user and refresh whatever + /// that resolves to. + #[test] + fn a_user_without_a_usable_jid_is_skipped_rather_than_defaulted() { + for attrs in [None, Some("not a jid")] { + let mut builder = NodeBuilder::new("user"); + if let Some(raw) = attrs { + builder = builder.attr("jid", raw); + } + let user = builder + .children([NodeBuilder::new("error").attr("code", "406").build()]) + .build(); + let response = NodeBuilder::new("iq") + .children([NodeBuilder::new("list").children([user]).build()]) + .build(); + + let outcome = + PreKeyUtils::parse_prekeys_response(&response.as_node_ref(), &HashMap::new()) + .expect("parse"); + + assert!( + outcome.rejected.is_empty(), + "a rejection with no device to name must not be recorded ({attrs:?})" + ); + } + } + + /// The code travels intact for values that do fit, since it is what decides + /// whether the caller acts at all. + #[test] + fn a_rejection_keeps_the_code_the_server_sent() { + for code in [400_u16, 406, 503] { + let jid = Jid::lid_device("100000011112222", 3); + let user = NodeBuilder::new("user") + .attr("jid", NodeValue::Jid(jid.clone())) + .children([NodeBuilder::new("error") + .attr("code", code.to_string()) + .build()]) + .build(); + let response = NodeBuilder::new("iq") + .children([NodeBuilder::new("list").children([user]).build()]) + .build(); + + let outcome = + PreKeyUtils::parse_prekeys_response(&response.as_node_ref(), &HashMap::new()) + .expect("parse"); + + assert_eq!(outcome.rejected.len(), 1); + assert_eq!(outcome.rejected[0].code, code); + } + } + fn parse_one(jid: Jid, device_identity: Option>) -> HashMap { let bundle = create_mock_bundle(jid.device as u32); let user_node = PreKeyBundleUserNode::from_bundle(jid, &bundle, device_identity) @@ -528,6 +712,7 @@ mod tests { .build(); PreKeyUtils::parse_prekeys_response(&response.as_node_ref(), &HashMap::new()) .expect("parse bundles") + .bundles } #[test] diff --git a/wacore/src/send/encrypt.rs b/wacore/src/send/encrypt.rs index b1fe8be92..7e54718cb 100644 --- a/wacore/src/send/encrypt.rs +++ b/wacore/src/send/encrypt.rs @@ -77,8 +77,12 @@ pub struct SignalStores<'a> { /// Check if an anyhow error is a 406 "not-acceptable" server error (device unregistered). /// Uses typed downcast to `ServerErrorCode` — the shared error type that the /// `SendContextResolver` impl wraps server errors in. +/// The `` the server attaches to a device it no longer knows. +pub(crate) const UNREGISTERED_DEVICE_CODE: u16 = 406; + pub(crate) fn is_device_unregistered_error(err: &anyhow::Error) -> bool { - crate::request::ServerErrorCode::from_anyhow(err).is_some_and(|e| e.code == 406) + crate::request::ServerErrorCode::from_anyhow(err) + .is_some_and(|e| e.code == UNREGISTERED_DEVICE_CODE) } pub struct EncryptResult { @@ -87,6 +91,9 @@ pub struct EncryptResult { pub encrypted_devices: Vec, /// True if any device returned 406 (unregistered) during prekey fetch. pub had_unregistered_device: bool, + /// The devices the server rejected by name, when it named them. Empty for a + /// batch-wide failure, which names nobody. + pub rejected_devices: Vec, } pub(crate) struct EncryptAttempt { @@ -111,6 +118,8 @@ pub struct EncryptForDevicesRaw { pub includes_prekey_message: bool, /// True if any device returned 406 (unregistered) during prekey fetch. pub had_unregistered_device: bool, + /// See [`EncryptResult::rejected_devices`]. + pub rejected_devices: Vec, } struct RawEncryptAttempt { @@ -440,6 +449,16 @@ pub struct SessionPlan { /// See [`record_encryption_override`]. encryption_overrides: Vec>, pub had_unregistered_device: bool, + /// Devices the server rejected *by name*. Empty when the whole batch + /// failed, since a batch-wide answer names nobody. + /// + /// Kept apart from the flag because they call for different recoveries: a + /// named set says exactly which device lists are stale, while a batch-wide + /// failure leaves the caller to infer it from what went unencrypted -- and + /// inferring it when the server did name the devices would sweep in every + /// device that merely lacked a bundle or failed session setup, refreshing + /// unrelated users for no reason. + pub rejected_devices: Vec, first_error: Option, } @@ -453,6 +472,7 @@ impl SessionPlan { device_count, encryption_overrides: Vec::new(), had_unregistered_device: false, + rejected_devices: Vec::new(), first_error: None, } } @@ -504,6 +524,7 @@ pub async fn ensure_sessions_for_devices( // Indices into `devices` for those needing prekey fetch. let mut indices_needing_prekeys: Vec = Vec::new(); let mut had_406 = false; + let mut rejected_devices: Vec = Vec::new(); let mut first_error = None; let mut reusable_addr = crate::types::jid::make_reusable_protocol_address(); @@ -565,15 +586,36 @@ pub async fn ensure_sessions_for_devices( .iter() .map(|&i| devices[i].clone()) .collect(); - // 406 on this batch is all-or-nothing — per-device retries just wasted + // A batch-wide 406 is all-or-nothing — per-device retries just wasted // N·RTT with the same failure. Mark `had_406` so the caller invalidates // the users and the next send re-fetches. Matches WA Web's // `GroupSkmsgJob`: log, continue without those devices. + // + // A per-device rejection is the better-informed case: the server names + // the device in its own ``, so it sets the same flag without + // condemning the rest of the batch. let prekey_bundles = match resolver .fetch_prekeys_for_identity_check(&jids_for_fetch) .await { - Ok(bundles) => bundles, + Ok(outcome) => { + rejected_devices.extend( + outcome + .rejected + .iter() + .filter(|device| device.code == UNREGISTERED_DEVICE_CODE) + .map(|device| device.jid.clone()), + ); + if !rejected_devices.is_empty() { + log::debug!( + "prekey fetch rejected {} of {} device(s) by name", + rejected_devices.len(), + jids_for_fetch.len() + ); + had_406 = true; + } + outcome.bundles + } Err(e) if is_device_unregistered_error(&e) => { // No server prekeys for these devices this round; the next send // re-fetches. Debug, not warn — a batch 406 would otherwise flood the log. @@ -699,6 +741,7 @@ pub async fn ensure_sessions_for_devices( device_count: devices.len(), encryption_overrides, had_unregistered_device: had_406, + rejected_devices, first_error, }) } @@ -773,6 +816,7 @@ pub(crate) async fn encrypt_for_devices_with_sessions_detailed( includes_prekey_message: raw.includes_prekey_message, encrypted_devices, had_unregistered_device: raw.had_unregistered_device, + rejected_devices: raw.rejected_devices.clone(), }, first_error, }) @@ -820,6 +864,7 @@ async fn encrypt_for_devices_with_sessions_raw_detailed( device_count: _, encryption_overrides, had_unregistered_device, + rejected_devices, mut first_error, } = plan; @@ -932,6 +977,7 @@ async fn encrypt_for_devices_with_sessions_raw_detailed( devices: encrypted, includes_prekey_message, had_unregistered_device, + rejected_devices, }, first_error, }) diff --git a/wacore/src/send/group.rs b/wacore/src/send/group.rs index 1ca667742..999ea5676 100644 --- a/wacore/src/send/group.rs +++ b/wacore/src/send/group.rs @@ -317,6 +317,8 @@ pub async fn prepare_group_stanza( } let mut had_unregistered_devices = false; + // Empty when the failure was batch-wide; see `stale_users_for`. + let mut skdm_rejected_devices: Vec = Vec::new(); let sender_key_name = make_sender_key_name(to_jid, &own_sending_jid.to_protocol_address()); @@ -433,6 +435,7 @@ pub async fn prepare_group_stanza( includes_prekey_message: result_includes_prekey, encrypted_devices, had_unregistered_device, + rejected_devices, } = result; if distribution_policy == SenderKeyDistributionPolicy::Required && (encrypted_devices.len() != distribution_list.len() @@ -445,15 +448,13 @@ pub async fn prepare_group_stanza( distribution_list.len() ) }); - let stale_device_users = if had_unregistered_device { - collect_stale_device_users( - Some(distribution_list), - &encrypted_devices, - group_info, - ) - } else { - Vec::new() - }; + let stale_device_users = stale_users_for( + had_unregistered_device, + &rejected_devices, + Some(distribution_list), + &encrypted_devices, + group_info, + ); return Err(RequiredSenderKeyDistributionError::new( error, stale_device_users, @@ -464,6 +465,7 @@ pub async fn prepare_group_stanza( includes_prekey_message |= result_includes_prekey; if had_unregistered_device { had_unregistered_devices = true; + skdm_rejected_devices.extend(rejected_devices); } skdm_encrypted_devices = encrypted_devices; @@ -580,15 +582,13 @@ pub async fn prepare_group_stanza( let stanza = stanza_builder.children(message_children).build(); - let stale_users = if had_unregistered_devices { - collect_stale_device_users( - distribution_list.as_deref(), - &skdm_encrypted_devices, - group_info, - ) - } else { - Vec::new() - }; + let stale_users = stale_users_for( + had_unregistered_devices, + &skdm_rejected_devices, + distribution_list.as_deref(), + &skdm_encrypted_devices, + group_info, + ); Ok(PreparedGroupStanza { node: stanza, @@ -626,6 +626,32 @@ pub(crate) fn build_group_phash_set(devices: &[Jid], own_sending_jid: &Jid) -> V /// emitted when the group knows the mapping — `invalidate_device_cache` needs /// both to clean up zombie records that were stored under whichever alias /// `update_device_list` canonicalised to at the time of the write. +/// Which users to re-resolve after a fan-out that hit an unregistered device. +/// +/// When the server named the devices, those are the answer, and only those: a +/// target can go unencrypted because its bundle was absent, malformed, or its +/// session setup failed, and refreshing those users would delete device +/// registries over failures that say nothing about the list being stale. +/// +/// A batch-wide failure names nobody, so there the unencrypted remainder is the +/// only available signal and every target in it is suspect -- which is sound, +/// because a batch-wide failure means none of them got a bundle either. +pub(crate) fn stale_users_for( + had_unregistered_device: bool, + rejected_devices: &[Jid], + distribution_list: Option<&[Jid]>, + encrypted_devices: &[Jid], + group_info: &GroupInfo, +) -> Vec { + if !had_unregistered_device { + return Vec::new(); + } + if rejected_devices.is_empty() { + return collect_stale_device_users(distribution_list, encrypted_devices, group_info); + } + collect_stale_device_users(Some(rejected_devices), &[], group_info) +} + pub(crate) fn collect_stale_device_users( distribution_list: Option<&[Jid]>, skdm_encrypted_devices: &[Jid], diff --git a/wacore/src/send/tests.rs b/wacore/src/send/tests.rs index d513b8eb3..a233d061c 100644 --- a/wacore/src/send/tests.rs +++ b/wacore/src/send/tests.rs @@ -423,6 +423,8 @@ struct MockSendContextResolver { identity_changes: std::sync::Mutex>, chain_lock_probe: Option, prekey_error_code: Option, + /// Devices the server names as rejected inside an otherwise fine response. + rejected_devices: Vec, } impl MockSendContextResolver { @@ -434,6 +436,7 @@ impl MockSendContextResolver { identity_changes: std::sync::Mutex::new(Vec::new()), chain_lock_probe: None, prekey_error_code: None, + rejected_devices: Vec::new(), } } @@ -466,6 +469,14 @@ impl MockSendContextResolver { self } + /// The server answers with bundles for the rest of the batch and an + /// `` for `jid`, which is how it names one absent device. + fn with_rejected_device(mut self, jid: Jid, code: u16) -> Self { + self.rejected_devices + .push(crate::prekeys::RejectedDevice { jid, code }); + self + } + fn with_prekey_error(mut self, code: u16) -> Self { self.prekey_error_code = Some(code); self @@ -493,7 +504,7 @@ impl SendContextResolver for MockSendContextResolver { async fn fetch_prekeys_for_identity_check( &self, jids: &[Jid], - ) -> Result> { + ) -> Result { if let Some(code) = self.prekey_error_code { return Err(anyhow::Error::new(crate::request::ServerErrorCode { code, @@ -527,7 +538,10 @@ impl SendContextResolver for MockSendContextResolver { } // If None, we intentionally omit it from the result (simulating server not returning it) } - Ok(result) + Ok(crate::prekeys::PreKeyFetchOutcome { + bundles: result, + rejected: self.rejected_devices.clone(), + }) } async fn resolve_group_info(&self, _jid: &Jid) -> Result> { @@ -2971,6 +2985,101 @@ mod collect_stale_device_users { info } + /// The case that separates a named rejection from an inferred one: one + /// device is rejected by name while another simply produced no bundle (an + /// absent or malformed one, or a session setup that failed). Only the named + /// device's user may be refreshed -- deleting the other user's device + /// registry would force a re-resolution over a failure that says nothing + /// about the list being stale. + #[test] + fn only_the_named_device_is_refreshed_when_the_server_named_it() { + use super::super::stale_users_for; + + let info = group_info_lid(&[]); + let delivered = lid_device("100000000000001", 1); + let named = lid_device("100000000000002", 2); + let merely_missing = lid_device("100000000000003", 3); + let dist = vec![delivered.clone(), named.clone(), merely_missing.clone()]; + + let out = stale_users_for(true, &[named], Some(&dist), &[delivered], &info); + let set: HashSet = out.into_iter().collect(); + + assert!(set.contains("100000000000002"), "the named device's user"); + assert!( + !set.contains("100000000000003"), + "a device that merely produced no bundle is not evidence of a stale list" + ); + assert_eq!(set.len(), 1); + } + + /// A batch-wide failure names nobody, so the unencrypted remainder is the + /// only signal left -- and every target in it is suspect, because none of + /// them got a bundle either. + #[test] + fn a_batch_wide_failure_falls_back_to_the_unencrypted_remainder() { + use super::super::stale_users_for; + + let info = group_info_lid(&[]); + let delivered = lid_device("100000000000001", 1); + let missing = lid_device("100000000000002", 2); + let dist = vec![delivered.clone(), missing]; + + let out = stale_users_for(true, &[], Some(&dist), &[delivered], &info); + let set: HashSet = out.into_iter().collect(); + + assert!(set.contains("100000000000002")); + assert_eq!(set.len(), 1); + } + + /// No unregistered device at all means nothing to refresh, whatever else + /// went unencrypted. + #[test] + fn nothing_is_refreshed_without_an_unregistered_device() { + use super::super::stale_users_for; + + let info = group_info_lid(&[]); + let dist = vec![lid_device("100000000000001", 1)]; + + assert!(stale_users_for(false, &[], Some(&dist), &[], &info).is_empty()); + } + + /// Closes the loop the named rejection opens: the rejected device gets no + /// bundle, so it is never in the encrypted set, so it surfaces here as a + /// user to re-resolve. This is the recovery — not the sender-key marking, + /// which deliberately covers the whole target set (WA Web + /// `markHasSenderKey(x, skDistribList)`). + #[test] + fn a_device_that_was_never_encrypted_for_is_reported_stale() { + let info = group_info_lid(&[]); + let delivered = lid_device("100000000000001", 1); + let rejected = lid_device("100000000000002", 9); + let dist = vec![delivered.clone(), rejected.clone()]; + + let out = collect_stale_device_users(Some(&dist), &[delivered], &info); + let set: HashSet = out.into_iter().collect(); + + assert!( + set.contains("100000000000002"), + "the device with no bundle must come back as stale" + ); + assert!( + !set.contains("100000000000001"), + "a device that did receive the SKDM is not stale" + ); + } + + /// The counterpart: when every target was encrypted for, nothing is stale, + /// so an ordinary group send does not invalidate any device list. + #[test] + fn a_fully_delivered_distribution_reports_nothing_stale() { + let info = group_info_lid(&[]); + let a = lid_device("100000000000001", 1); + let b = lid_device("100000000000002", 2); + let dist = vec![a.clone(), b.clone()]; + + assert!(collect_stale_device_users(Some(&dist), &[a, b], &info).is_empty()); + } + #[test] fn emits_lid_and_pn_alias_when_mapping_known() { let info = group_info_lid(&[("100000000000001", "15550000001")]); @@ -4509,6 +4618,116 @@ mod local_identity_change_on_send { ); } + /// The server names one device inside an otherwise fine response, and + /// that naming has to survive the resolver boundary: the fan-out sets + /// the same stale-device flag a batch-wide 406 would, so the group path + /// still refreshes the list after the send. Flattening the rejection + /// into "no bundle" loses it, and the stale device is kept forever. + #[tokio::test] + async fn a_named_rejection_reaches_the_fan_out_like_a_batch_failure() { + let warm: Jid = "5511900000061:0@s.whatsapp.net".parse().unwrap(); + let gone: Jid = "5511900000061:9@s.whatsapp.net".parse().unwrap(); + + let (mut session_store, mut identity_store) = + stores_with_sessions(std::slice::from_ref(&warm)).await; + let mut prekey_store = UnusedPreKeyStore; + let signed_prekey_store = UnusedSignedPreKeyStore; + let mut sender_key_store = MemSenderKeyStore::default(); + let mut stores = raw_fanout_stores( + &mut sender_key_store, + &mut session_store, + &mut identity_store, + &mut prekey_store, + &signed_prekey_store, + ); + + // Not `with_prekey_error`: the batch succeeds, and the server names + // the one device it will not hand a bundle for. + let resolver = MockSendContextResolver::new().with_rejected_device(gone.clone(), 406); + + let plan = ensure_sessions_for_devices( + &TokioTestRuntime, + &mut stores, + &resolver, + &[warm.clone(), gone.clone()], + ) + .await + .expect("a named rejection must not fail the fan-out"); + + assert!( + plan.had_unregistered_device, + "the named device must raise the same flag a batch 406 raises" + ); + } + + /// Only a `406` means "this device is gone". Another refusal code says + /// something else, and refreshing a device list over it costs a usync + /// for nothing. + #[tokio::test] + async fn a_rejection_that_is_not_a_406_leaves_the_device_list_alone() { + let warm: Jid = "5511900000071:0@s.whatsapp.net".parse().unwrap(); + let odd: Jid = "5511900000071:9@s.whatsapp.net".parse().unwrap(); + + let (mut session_store, mut identity_store) = + stores_with_sessions(std::slice::from_ref(&warm)).await; + let mut prekey_store = UnusedPreKeyStore; + let signed_prekey_store = UnusedSignedPreKeyStore; + let mut sender_key_store = MemSenderKeyStore::default(); + let mut stores = raw_fanout_stores( + &mut sender_key_store, + &mut session_store, + &mut identity_store, + &mut prekey_store, + &signed_prekey_store, + ); + let resolver = MockSendContextResolver::new().with_rejected_device(odd.clone(), 503); + + let plan = ensure_sessions_for_devices( + &TokioTestRuntime, + &mut stores, + &resolver, + &[warm.clone(), odd.clone()], + ) + .await + .expect("a non-406 rejection is still not a fan-out failure"); + + assert!( + !plan.had_unregistered_device, + "a 503 is not the server saying the device is unregistered" + ); + } + + /// A response with nothing rejected must not raise the flag either, or + /// every ordinary send would invalidate device lists. + #[tokio::test] + async fn a_clean_fetch_reports_no_unregistered_device() { + let warm: Jid = "5511900000081:0@s.whatsapp.net".parse().unwrap(); + + let (mut session_store, mut identity_store) = + stores_with_sessions(std::slice::from_ref(&warm)).await; + let mut prekey_store = UnusedPreKeyStore; + let signed_prekey_store = UnusedSignedPreKeyStore; + let mut sender_key_store = MemSenderKeyStore::default(); + let mut stores = raw_fanout_stores( + &mut sender_key_store, + &mut session_store, + &mut identity_store, + &mut prekey_store, + &signed_prekey_store, + ); + + let plan = ensure_sessions_for_devices( + &TokioTestRuntime, + &mut stores, + &MockSendContextResolver::new(), + std::slice::from_ref(&warm), + ) + .await + .expect("plan"); + + assert!(!plan.had_unregistered_device); + } + /// End to end through `prepare_dm_stanza`: recipient devices and own /// companion devices are two separate fan-outs but one participant /// list, recipients first.