db: a gossiped re-home of an account is only a takeover when the credential also changed, so an authority re-provision or migration re-home no longer spuriously logs the user out
All checks were successful
CI / check (push) Successful in 3m44s
All checks were successful
CI / check (push) Successful in 3m44s
This commit is contained in:
parent
c64c3820a7
commit
db3e818089
2 changed files with 56 additions and 8 deletions
|
|
@ -1098,11 +1098,18 @@ impl Db {
|
||||||
/// the account name if this ingest changed its owner (a registration conflict
|
/// the account name if this ingest changed its owner (a registration conflict
|
||||||
/// resolved against the local claim), so the caller can log out stale sessions.
|
/// resolved against the local claim), so the caller can log out stale sessions.
|
||||||
pub fn ingest(&mut self, entry: LogEntry) -> std::io::Result<Option<IngestEffect>> {
|
pub fn ingest(&mut self, entry: LogEntry) -> std::io::Result<Option<IngestEffect>> {
|
||||||
// Snapshot the incoming account's local owner before applying, to detect a
|
// Snapshot the incoming account's local owner AND credential before applying,
|
||||||
// takeover (home changed) or a remote drop (it disappears).
|
// to tell a takeover (home moved to a *different* account) from a benign
|
||||||
let watched: Option<(String, Option<String>)> = match &entry.event {
|
// re-home (the same account re-asserted), and a remote drop (it disappears).
|
||||||
Event::AccountRegistered(a) => Some((a.name.clone(), self.account(&a.name).map(|c| c.home.clone()))),
|
let watched: Option<(String, Option<String>, Option<String>)> = match &entry.event {
|
||||||
Event::AccountDropped { account } => Some((account.clone(), self.account(account).map(|c| c.home.clone()))),
|
Event::AccountRegistered(a) => {
|
||||||
|
let c = self.account(&a.name);
|
||||||
|
Some((a.name.clone(), c.map(|c| c.home.clone()), c.and_then(|c| c.scram256.clone())))
|
||||||
|
}
|
||||||
|
Event::AccountDropped { account } => {
|
||||||
|
let c = self.account(account);
|
||||||
|
Some((account.clone(), c.map(|c| c.home.clone()), c.and_then(|c| c.scram256.clone())))
|
||||||
|
}
|
||||||
_ => None,
|
_ => None,
|
||||||
};
|
};
|
||||||
let applied = self.log.ingest(entry)?;
|
let applied = self.log.ingest(entry)?;
|
||||||
|
|
@ -1126,9 +1133,22 @@ impl Db {
|
||||||
if let Some(event) = applied {
|
if let Some(event) = applied {
|
||||||
apply(&mut self.accounts, &mut self.channels, &mut self.grouped, &mut self.bots, &mut self.host_cfg, &mut self.net, event);
|
apply(&mut self.accounts, &mut self.channels, &mut self.grouped, &mut self.bots, &mut self.host_cfg, &mut self.net, event);
|
||||||
}
|
}
|
||||||
if let Some((name, prev_home)) = watched {
|
if let Some((name, prev_home, prev_verifier)) = watched {
|
||||||
match (prev_home, self.account(&name).map(|c| c.home.clone())) {
|
let cur = self.account(&name);
|
||||||
(Some(prev), Some(cur)) if cur != prev => return Ok(Some(IngestEffect::TakenOver(name))),
|
match (prev_home, cur.map(|c| c.home.clone())) {
|
||||||
|
(Some(prev), Some(cur_home)) if cur_home != prev => {
|
||||||
|
// The account's home moved. Treat it as a hostile takeover — which
|
||||||
|
// logs the local user out with a "collided with another network"
|
||||||
|
// notice — ONLY if the credential changed too. A re-home that keeps
|
||||||
|
// the same verifier is the SAME account being re-asserted (an
|
||||||
|
// authority re-provision, an import, or an origin change during
|
||||||
|
// migration): the local login is still valid, so it must not be
|
||||||
|
// disrupted. This is what spuriously logged users out and (before
|
||||||
|
// the channel-keep fix) dropped their channels during the cutover.
|
||||||
|
if cur.and_then(|c| c.scram256.clone()) != prev_verifier {
|
||||||
|
return Ok(Some(IngestEffect::TakenOver(name)));
|
||||||
|
}
|
||||||
|
}
|
||||||
(Some(_), None) => return Ok(Some(IngestEffect::Dropped(name))),
|
(Some(_), None) => return Ok(Some(IngestEffect::Dropped(name))),
|
||||||
_ => {}
|
_ => {}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -54,6 +54,34 @@
|
||||||
assert_eq!(converge(&a, &a), "A");
|
assert_eq!(converge(&a, &a), "A");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A gossiped registration that wins the name (earlier ts) moves an account's
|
||||||
|
// home. That is a hostile takeover — logging the local user out — ONLY if the
|
||||||
|
// credential differs; re-asserting the SAME account (same verifier) must not.
|
||||||
|
#[test]
|
||||||
|
fn rehome_keeping_the_verifier_is_not_a_takeover() {
|
||||||
|
let acct = |home: &str, ts: u64, verifier: &str| Account {
|
||||||
|
name: "reverse".into(), email: None, ts, home: home.into(),
|
||||||
|
scram256: Some(verifier.into()), scram512: None, certfps: vec![], verified: true,
|
||||||
|
ajoin: vec![], suspension: None, memos: vec![], memo_ignore: vec![], memo_notify: true,
|
||||||
|
memo_limit: None, greet: String::new(), no_autoop: false, no_protect: false,
|
||||||
|
hide_status: false, vhost: None, vhost_request: None, last_seen: ts, noexpire: false,
|
||||||
|
expiry_warned: false, oper_note: None,
|
||||||
|
};
|
||||||
|
let reg = |origin: &str, a: Account| LogEntry::for_test(origin, 1, 1, Event::AccountRegistered(Box::new(a)));
|
||||||
|
|
||||||
|
// Same account, re-homed to B with the SAME verifier — not a takeover.
|
||||||
|
let mut db = Db::open(tmp("rehome-same"), "A");
|
||||||
|
db.ingest(reg("A", acct("A", 100, "V"))).unwrap();
|
||||||
|
assert!(db.ingest(reg("B", acct("B", 1, "V"))).unwrap().is_none(), "same-verifier re-home is not a takeover");
|
||||||
|
assert_eq!(db.account("reverse").unwrap().home, "B", "the home still moved (conflict resolved)");
|
||||||
|
|
||||||
|
// A genuinely different account (different verifier) winning the name is one.
|
||||||
|
let mut db2 = Db::open(tmp("rehome-diff"), "A");
|
||||||
|
db2.ingest(reg("A", acct("A", 100, "V"))).unwrap();
|
||||||
|
assert!(matches!(db2.ingest(reg("B", acct("B", 1, "OTHER"))).unwrap(), Some(IngestEffect::TakenOver(n)) if n == "reverse"),
|
||||||
|
"a different credential winning the name is a real takeover");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn channel_state_is_node_local_but_persists() {
|
fn channel_state_is_node_local_but_persists() {
|
||||||
let path = tmp("scope");
|
let path = tmp("scope");
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue