-
-
Notifications
You must be signed in to change notification settings - Fork 127
fix(pair-code): keep a phone-number link alive past the QR rotation #1163
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.
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
Merged
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d9545c9
fix(pair-code): keep a phone-number link alive past the QR rotation
jlucaso1 108437b
fix(pair-code): close the races review found in the new lifetime guards
jlucaso1 4cbe76d
fix(pair-code): scope the new guards to what actually depends on them
jlucaso1 e27d01a
fix(pair): drive the QR re-render off the runtime clock, not tokio
jlucaso1 0bc7763
fix(pair-code): make the claim and the rotation hold under interleaving
jlucaso1 dda93f7
fix(pair): clear the pair-code slot on the lifecycle-less teardown too
jlucaso1 eb67af3
fix(pair-code): drop the deferred registration refresh instead of que…
jlucaso1 52f6126
fix(pair-code): return the claim before the error, and keep the ref o…
jlucaso1 4024b59
fix(events): append the new kind, and stop the QR only once pairing v…
jlucaso1 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| //! `<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 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. | ||
| 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; | ||
| } | ||
|
|
||
| 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" | ||
| ); | ||
| // Dropped, not queued. Replaying it later would have to be atomic with | ||
| // pair-success completing, with the flow being retired on any of four | ||
| // paths, and with the connection going away — four synchronisation | ||
| // points to recover a QR in a window where the phone-number flow is the | ||
| // one being used anyway. If that flow fails, the QR advertises a retired | ||
| // secret until the next reconnect, which is where the server asks again. | ||
| return; | ||
|
jlucaso1 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| rotate_companion_registration(client).await; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.