Skip to content

Commit

Permalink
fix: AEAP: Remove old peerstate verified_key instead of removing the …
Browse files Browse the repository at this point in the history
…whole peerstate (#5535)

When doing an AEAP transition, we mustn't just delete the old peerstate as this would break
encryption to it. This is critical for non-verified groups -- if we can't encrypt to the old
address, we can't securely remove it from the group (to add the new one instead).
  • Loading branch information
iequidoo committed May 30, 2024
1 parent 27bf4c3 commit 70ad323
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/peerstate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,10 +538,18 @@ impl Peerstate {
if let Some(old_addr) = old_addr {
// We are doing an AEAP transition to the new address and the SQL INSERT below will
// save the existing peerstate as belonging to this new address. We now need to
// delete the peerstate that belongs to the current address in case if the contact
// later wants to move back to the current address. Otherwise the old entry will be
// just found and updated instead of doing AEAP.
t.execute("DELETE FROM acpeerstates WHERE addr=?", (old_addr,))?;
// "unverify" the peerstate that belongs to the current address in case if the
// contact later wants to move back to the current address. Otherwise the old entry
// will be just found and updated instead of doing AEAP. We can't just delete the
// existing peerstate as this would break encryption to it. This is critical for
// non-verified groups -- if we can't encrypt to the old address, we can't securely
// remove it from the group (to add the new one instead).
t.execute(
"UPDATE acpeerstates \
SET verified_key=NULL, verified_key_fingerprint='', verifier='' \
WHERE addr=?",
(old_addr,),
)?;
}
t.execute(
"INSERT INTO acpeerstates (
Expand Down
6 changes: 6 additions & 0 deletions src/tests/aeap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,12 @@ async fn check_no_transition_done(groups: &[ChatId], old_alice_addr: &str, bob:
last_info_msg.is_none(),
"{last_info_msg:?} shouldn't be there (or it's an unrelated info msg)"
);

let sent = bob.send_text(*group, "hi").await;
let msg = Message::load_from_db(bob, sent.sender_msg_id)
.await
.unwrap();
assert_eq!(msg.get_showpadlock(), true);
}
}

Expand Down

0 comments on commit 70ad323

Please sign in to comment.