Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
8 changes: 8 additions & 0 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,14 @@ pub struct Client {
pub(crate) connected_notifier: Arc<event_listener::Event>,
pub(crate) major_sync_task_sender: async_channel::Sender<MajorSyncTask>,
pub(crate) pairing_cancellation_tx: Arc<Mutex<Option<async_channel::Sender<()>>>>,
/// A `companion_reg_refresh` that arrived while a pair-code flow was
/// awaiting `pair-success`, and so could not rotate the ADV secret without
/// breaking that flow's HMAC. Applied when the flow ends without pairing.
pub(crate) pending_reg_refresh: portable_atomic::AtomicBool,
/// Asks the QR rotation task to re-render the ref it is already showing.
/// The payload embeds the adv secret, so a rotation has to reach the code
/// on screen and not just the next one.
pub(crate) pairing_qr_refresh_tx: Arc<Mutex<Option<async_channel::Sender<()>>>>,

/// State machine for pair code authentication flow.
/// Tracks the pending pair code request and ephemeral keys.
Expand Down
18 changes: 18 additions & 0 deletions src/client/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,9 @@ impl Client {
is_ready: Arc::new(AtomicBool::new(false)),
connected_notifier: Arc::new(event_listener::Event::new()),
major_sync_task_sender: tx,
pending_reg_refresh: portable_atomic::AtomicBool::new(false),
pairing_cancellation_tx: Arc::new(Mutex::new(None)),
pairing_qr_refresh_tx: Arc::new(Mutex::new(None)),
pair_code_state: Arc::new(Mutex::new(wacore::pair_code::PairCodeState::default())),
passkey_state: Arc::new(Mutex::new(crate::passkey::flow::PasskeyFlowState::default())),
passkey_opening: AtomicBool::new(false),
Expand Down Expand Up @@ -922,6 +924,20 @@ impl Client {
#[cfg(not(feature = "client-lifecycle"))]
pub(crate) async fn cleanup_connection_state(self: &Arc<Self>) {
self.cleanup_connection_state_inner().await;
self.clear_connection_scoped_pair_code().await;
}

/// A pair-code flow belongs to the connection that carried it: the pairing
/// ref and any in-flight `companion_hello` die with the socket, and the
/// server routes no `primary_hello` to a session it has dropped. Left
/// standing, the outstanding-code guard would reject the very request that
/// reconnecting exists to make.
///
/// Runs after the inner teardown, so the generation is already retired and
/// the transport already closed: a request that claims the slot from here
/// on is one the next connection will carry.
async fn clear_connection_scoped_pair_code(self: &Arc<Self>) {
*self.pair_code_state.lock().await = wacore::pair_code::PairCodeState::Idle;
Comment thread
jlucaso1 marked this conversation as resolved.
}
Comment thread
jlucaso1 marked this conversation as resolved.

#[cfg_attr(
Expand All @@ -932,6 +948,7 @@ impl Client {
pub(crate) async fn cleanup_connection_state(self: &Arc<Self>) {
if self.lifecycle.is_none() {
self.cleanup_connection_state_inner().await;
self.clear_connection_scoped_pair_code().await;
return;
}

Expand All @@ -951,6 +968,7 @@ impl Client {
Ok(Err(panic)) => std::panic::resume_unwind(panic),
Err(_) => error!("Detached connection cleanup stopped before completion"),
}
self.clear_connection_scoped_pair_code().await;
Comment thread
jlucaso1 marked this conversation as resolved.
}

async fn cleanup_connection_state_inner(&self) {
Expand Down
92 changes: 92 additions & 0 deletions src/handlers/notification/companion_reg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//! `<notification type="companion_reg_refresh">` — the server retiring an
//! unpaired companion's registration material.
//!
//! WA Web (`Handle/CompanionReqRefreshNotification.js`) accepts the stanza with
//! either a `companion_reg_refresh` or a `pair-device-rotate-qr` child, rejects
//! it outright when neither is present, and answers by regenerating the ADV
//! secret key. That key is what the QR payload advertises, so ignoring the
//! request leaves us handing out a QR built on a secret the server has retired.

use crate::client::Client;
use log::{debug, warn};
use std::sync::Arc;
use std::sync::atomic::Ordering;
use wacore_binary::NodeRef;

/// The two children WA Web's parser accepts on this notification.
const REFRESH_CHILDREN: [&str; 2] = ["companion_reg_refresh", "pair-device-rotate-qr"];

/// Rotate the ADV secret and re-render the QR that advertises it.
///
/// Also the landing point for a refresh that had to wait: see
/// [`apply_pending_reg_refresh`].
pub(crate) async fn rotate_companion_registration(client: &Arc<Client>) {
use rand::Rng as _;
let mut secret = [0u8; 32];
rand::make_rng::<rand::rngs::StdRng>().fill_bytes(&mut secret);
client
.persistence_manager
.process_command(wacore::store::commands::DeviceCommand::SetAdvSecretKey(
secret,
))
.await;
debug!(
target: "Client/PairRefresh",
"Rotated the adv secret the server asked to retire"
);
// The QR on screen embeds the old secret; re-render it rather than let it
// stay scannable until its ref expires.
client.refresh_pairing_qr().await;
}

/// Apply a refresh that was deferred to a pair-code flow which then ended
/// without pairing. Without this the server's request is simply lost, and the
/// QR keeps advertising material it asked to retire until the next reconnect.
pub(crate) async fn apply_pending_reg_refresh(client: &Arc<Client>) {
if client.pending_reg_refresh.swap(false, Ordering::Relaxed) {
debug!(target: "Client/PairRefresh", "Applying the registration refresh a pair-code flow had deferred");
rotate_companion_registration(client).await;
Comment thread
jlucaso1 marked this conversation as resolved.
Outdated
}
}

pub(super) async fn handle_companion_reg_refresh(client: &Arc<Client>, node: &NodeRef<'_>) {
if !REFRESH_CHILDREN
.iter()
.any(|tag| node.get_optional_child_by_tag(&[tag]).is_some())
{
warn!(
target: "Client/PairRefresh",
"companion_reg_refresh carries neither companion_reg_refresh nor pair-device-rotate-qr; ignoring"
);
return;
}

// The one place we knowingly diverge from WA Web, which rotates
// unconditionally. Once stage 2 has run, this is the secret the pending
// pair-success HMAC is computed over, and rotating it turns a link that was
// about to succeed into one that cannot. A code that is only displayed does
// not qualify: stage 2 derives its own secret when the phone answers, so
// deferring there would protect nothing while leaving the QR that shares
// this connection advertising material the server just retired.
{
// Held across the write, not just the check: stage 2 derives its secret
// and builds `companion_finish` under this same lock, so releasing it
// in between would let a `primary_hello` land in the gap and have its
// secret overwritten — the pair-success would then be verified against
// the wrong one.
let state = client.pair_code_state.lock().await;
if state.awaiting_pair_success() {
debug!(
target: "Client/PairRefresh",
"Server asked to refresh companion registration; keeping the adv secret a pending pair-success depends on"
);
// Deferred, not dropped: if that flow ends without pairing, the
// secret the server retired is still the one the QR advertises.
client.pending_reg_refresh.store(true, Ordering::Relaxed);
return;
}

rotate_companion_registration(client).await;
drop(state);
Comment thread
jlucaso1 marked this conversation as resolved.
Outdated
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
}
137 changes: 137 additions & 0 deletions src/handlers/notification/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ async fn handle_notification_impl(client: &Arc<Client>, node: Arc<OwnedNodeRef>)
"link_code_companion_reg" => {
crate::pair_code::handle_pair_code_notification(client, nr).await;
}
"companion_reg_refresh" => handle_companion_reg_refresh(client, nr).await,
"business" => handle_business_notification(client, nr).await,
"picture" => handle_picture_notification(client, nr),
"privacy_token" => handle_privacy_token_notification(client, nr).await,
Expand Down Expand Up @@ -83,11 +84,14 @@ async fn handle_notification_impl(client: &Arc<Client>, node: Arc<OwnedNodeRef>)
}
}

mod companion_reg;
mod device;
mod groups;
mod privacy_business;
mod profile;

pub(crate) use companion_reg::apply_pending_reg_refresh;
use companion_reg::*;
// `pub(crate)` re-export keeps `crate::handlers::notification::handle_local_identity_change`
// resolving for device_registry.rs.
pub(crate) use device::*;
Expand All @@ -113,6 +117,139 @@ mod tests {
crate::test_utils::node_to_owned_ref(&node)
}

// ── `companion_reg_refresh` (WA Web `Handle/CompanionReqRefreshNotification.js`)
//
// The server tells an unpaired companion to re-mint its registration. WA
// Web regenerates the ADV secret key and acks; we routed the stanza to the
// catch-all instead, so the QR we kept advertising carried a secret the
// server had already retired.

fn companion_reg_refresh_notif(child: &'static str) -> Node {
NodeBuilder::new("notification")
.attr("type", "companion_reg_refresh")
.attr("from", "s.whatsapp.net")
.attr("id", "reg-refresh-1")
.children([NodeBuilder::new(child).build()])
.build()
}

async fn adv_secret(client: &Arc<Client>) -> [u8; 32] {
client
.persistence_manager
.get_device_snapshot()
.adv_secret_key
}

#[tokio::test]
async fn companion_reg_refresh_rotates_the_adv_secret() {
let client = create_test_client().await;
let before = adv_secret(&client).await;

let notif = companion_reg_refresh_notif("companion_reg_refresh");
handle_notification_impl(&client, node_to_arc(notif)).await;

assert_ne!(
adv_secret(&client).await,
before,
"companion_reg_refresh must re-mint the ADV secret the QR advertises"
);
}

/// WA Web accepts either child tag on this notification.
#[tokio::test]
async fn pair_device_rotate_qr_is_the_same_request() {
let client = create_test_client().await;
let before = adv_secret(&client).await;

let notif = companion_reg_refresh_notif("pair-device-rotate-qr");
handle_notification_impl(&client, node_to_arc(notif)).await;

assert_ne!(adv_secret(&client).await, before);
}

/// WA Web's parser rejects the stanza when neither child is present; a
/// malformed notification must not silently rotate live key material.
#[tokio::test]
async fn companion_reg_refresh_without_a_known_child_is_ignored() {
let client = create_test_client().await;
let before = adv_secret(&client).await;

let notif = companion_reg_refresh_notif("something-else");
handle_notification_impl(&client, node_to_arc(notif)).await;

assert_eq!(adv_secret(&client).await, before);
}

/// Regression: the displayed-code window and the pending-link window are not
/// the same. A `primary_hello` accepted near the end of the 180s validity
/// leaves `companion_finish` waiting up to another minute for pair-success,
/// and the ADV secret that HMAC is computed over is already derived — but
/// the code itself has expired, so a validity-only guard would rotate right
/// through it.
#[tokio::test]
async fn companion_reg_refresh_waits_for_a_pending_pair_success() {
use wacore::libsignal::protocol::KeyPair;
use wacore::pair_code::{PairCodeState, PairCodeUtils};

let client = create_test_client().await;
let expired =
wacore::time::now_secs() - (PairCodeUtils::code_validity().as_secs() as i64 + 1);
*client.pair_code_state.lock().await = PairCodeState::WaitingForPhoneConfirmation {
pairing_ref: b"3@2:ref".to_vec(),
phone_jid: "15551234567".to_string(),
pair_code: "ABCD1234".to_string(),
ephemeral_keypair: Box::new(KeyPair::generate(
&mut rand::make_rng::<rand::rngs::StdRng>(),
)),
code_generation_ts: expired,
// Stage 2 ran: companion_finish is out and pair-success is pending.
primary_hello_attempt_count: 1,
};
let before = adv_secret(&client).await;

let notif = companion_reg_refresh_notif("companion_reg_refresh");
handle_notification_impl(&client, node_to_arc(notif)).await;

assert_eq!(
adv_secret(&client).await,
before,
"the pending pair-success HMAC is computed over this secret"
);
}

/// A code that is merely displayed does not depend on the current ADV
/// secret — stage 2 derives and persists a fresh one when the phone
/// answers. Deferring there would protect nothing while leaving the QR,
/// which shares the connection, advertising registration material the
/// server asked to retire for the code's whole validity window.
#[tokio::test]
async fn a_merely_displayed_code_does_not_defer_the_rotation() {
use wacore::libsignal::protocol::KeyPair;
use wacore::pair_code::PairCodeState;

let client = create_test_client().await;
*client.pair_code_state.lock().await = PairCodeState::WaitingForPhoneConfirmation {
pairing_ref: b"3@2:ref".to_vec(),
phone_jid: "15551234567".to_string(),
pair_code: "ABCD1234".to_string(),
ephemeral_keypair: Box::new(KeyPair::generate(
&mut rand::make_rng::<rand::rngs::StdRng>(),
)),
code_generation_ts: wacore::time::now_secs(),
primary_hello_attempt_count: 0,
};
let before = adv_secret(&client).await;

let notif = companion_reg_refresh_notif("companion_reg_refresh");
handle_notification_impl(&client, node_to_arc(notif)).await;

assert_ne!(
adv_secret(&client).await,
before,
"nothing depends on this secret yet, and the QR still needs it rotated"
);
}

#[test]
fn test_parse_device_add_notification() {
// Per WhatsApp Web: add operation has single device + key-index-list
Expand Down
Loading
Loading