diff --git a/api/src/lib.rs b/api/src/lib.rs index 241b22b..bc082b5 100644 --- a/api/src/lib.rs +++ b/api/src/lib.rs @@ -386,6 +386,10 @@ pub trait Store { fn issue_code(&mut self, account: &str, kind: CodeKind) -> String; fn take_code(&mut self, account: &str, kind: CodeKind, code: &str) -> bool; + // Brute-force throttle for password authentication: how long the account is + // locked out (if at all), and a hook to record each attempt's outcome. + fn auth_lockout(&self, account: &str) -> Option; + fn note_auth(&mut self, account: &str, success: bool); fn verify_account(&mut self, account: &str) -> Result<(), RegError>; fn set_email(&mut self, account: &str, email: Option) -> Result<(), RegError>; fn group_nick(&mut self, nick: &str, account: &str) -> Result<(), RegError>; diff --git a/nickserv/src/identify.rs b/nickserv/src/identify.rs index bf9ae71..41f59ff 100644 --- a/nickserv/src/identify.rs +++ b/nickserv/src/identify.rs @@ -3,7 +3,7 @@ use fedserv_api::{Sender, ServiceCtx}; // IDENTIFY [account] : log in. The account defaults to the current // nick, so both the bare-password and account+password forms work. -pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: &dyn Store) { +pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: &mut dyn Store) { let (account_name, password) = match (args.get(1), args.get(2)) { (Some(account), Some(password)) => (*account, *password), (Some(password), None) => (from.nick, *password), @@ -17,14 +17,20 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: ctx.notice(me, from.uid, format!("\x02{account_name}\x02 isn't registered.")); return; } - match db.authenticate(account_name, password) { + // Refuse while throttled, so a password can't be brute-forced. + if let Some(secs) = db.auth_lockout(account_name) { + ctx.notice(me, from.uid, format!("Too many failed attempts. Please wait {secs}s and try again.")); + return; + } + // Take the result as owned so the account-store borrow ends before note_auth. + match db.authenticate(account_name, password).map(str::to_string) { Some(account) => { + db.note_auth(account_name, true); // Already identified to this account: don't re-fire the login. - if from.account == Some(account) { + if from.account == Some(account.as_str()) { ctx.notice(me, from.uid, format!("You're already identified as \x02{}\x02.", account)); return; } - let account = account.to_string(); ctx.login(from.uid, &account); ctx.notice(me, from.uid, format!("You're now identified as \x02{}\x02. Welcome back!", account)); // Apply the account's auto-join list (AJOIN). @@ -32,6 +38,9 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: ctx.force_join(from.uid, &entry.channel, &entry.key); } } - None => ctx.notice(me, from.uid, "Invalid password. Please try again."), + None => { + db.note_auth(account_name, false); + ctx.notice(me, from.uid, "Invalid password. Please try again."); + } } } diff --git a/nickserv/src/lib.rs b/nickserv/src/lib.rs index 1946a61..1afcadb 100644 --- a/nickserv/src/lib.rs +++ b/nickserv/src/lib.rs @@ -32,6 +32,8 @@ mod resetpass; mod confirm; #[path = "ajoin.rs"] mod ajoin; +#[path = "password.rs"] +mod password; pub struct NickServ { pub uid: String, diff --git a/nickserv/src/password.rs b/nickserv/src/password.rs new file mode 100644 index 0000000..2f8a710 --- /dev/null +++ b/nickserv/src/password.rs @@ -0,0 +1,50 @@ +// Password policy for REGISTER and SET PASSWORD. Anope enforces a minimum +// length and forbids the password matching the nick; we do the same, plus an +// upper bound so a huge password can't be used to burn CPU in the key +// derivation. Length is measured in characters, the cap in bytes (what the +// hasher actually processes). + +pub const MIN_PASSWORD_CHARS: usize = 8; +pub const MAX_PASSWORD_BYTES: usize = 300; + +// Validate a new password for the account/nick it will protect. On rejection, +// returns the message to show the user. +pub fn validate_password(password: &str, owner: &str) -> Result<(), &'static str> { + if password.chars().count() < MIN_PASSWORD_CHARS { + return Err("That password is too short — please use at least 8 characters."); + } + if password.len() > MAX_PASSWORD_BYTES { + return Err("That password is too long."); + } + if password.eq_ignore_ascii_case(owner) { + return Err("Your password can't be the same as your nick. Please choose another."); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn accepts_a_reasonable_password() { + assert!(validate_password("correct horse", "alice").is_ok()); + } + + #[test] + fn rejects_short_password() { + assert!(validate_password("short", "alice").is_err()); + assert!(validate_password("1234567", "alice").is_err()); // 7 chars + assert!(validate_password("12345678", "alice").is_ok()); // 8 chars + } + + #[test] + fn rejects_password_equal_to_nick() { + assert!(validate_password("Password", "password").is_err()); // case-insensitive + } + + #[test] + fn rejects_overlong_password() { + assert!(validate_password(&"x".repeat(MAX_PASSWORD_BYTES + 1), "alice").is_err()); + } +} diff --git a/nickserv/src/register.rs b/nickserv/src/register.rs index 6a1ecca..8aa863b 100644 --- a/nickserv/src/register.rs +++ b/nickserv/src/register.rs @@ -8,6 +8,10 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx) { ctx.notice(me, from.uid, "Syntax: REGISTER [email]"); return; }; + if let Err(reason) = crate::password::validate_password(password, from.nick) { + ctx.notice(me, from.uid, reason); + return; + } let email = args.get(2).map(|s| s.to_string()); ctx.defer_register(from.nick, *password, email, RegReply::NickServ { agent: me.to_string(), diff --git a/nickserv/src/set.rs b/nickserv/src/set.rs index 9894493..7d350b2 100644 --- a/nickserv/src/set.rs +++ b/nickserv/src/set.rs @@ -13,6 +13,10 @@ pub fn handle(me: &str, from: &Sender, args: &[&str], ctx: &mut ServiceCtx, db: ctx.notice(me, from.uid, "Syntax: SET PASSWORD "); return; }; + if let Err(reason) = crate::password::validate_password(password, account) { + ctx.notice(me, from.uid, reason); + return; + } // The link layer derives the new password off-thread, then commits it. ctx.defer_password(account, password, me, from.uid); } diff --git a/src/engine/db.rs b/src/engine/db.rs index f01abeb..b648e94 100644 --- a/src/engine/db.rs +++ b/src/engine/db.rs @@ -475,10 +475,33 @@ pub struct Db { email_brand: String, email_accent: String, email_logo: String, - // Node-local, non-persisted email codes: account -> (purpose, code, expiry). - codes: HashMap, + // Node-local, non-persisted email codes, keyed by account. + codes: HashMap, + // Node-local, non-persisted brute-force throttle for password authentication. + auth_fails: HashMap, } +// A pending emailed code and how many wrong guesses remain before it is burned. +struct PendingCode { + kind: CodeKind, + code: String, + deadline: Instant, + tries_left: u8, +} + +// Failed-authentication state for one account (in memory only, reset on restart). +struct AuthThrottle { + fails: u32, + locked_until: Option, +} + +// Wrong-code guesses tolerated before a code is invalidated (defence in depth on +// top of the code's own entropy). +const CODE_TRIES: u8 = 5; +// Free password attempts before the exponential backoff kicks in, and its cap. +const AUTH_FREE_TRIES: u32 = 3; +const AUTH_MAX_BACKOFF_SECS: u64 = 300; + fn key(name: &str) -> String { name.to_ascii_lowercase() } @@ -497,7 +520,7 @@ impl Db { apply(&mut accounts, &mut channels, &mut grouped, event); } tracing::info!(accounts = accounts.len(), channels = channels.len(), "account store loaded"); - Self { accounts, channels, grouped, log, scram_iterations: scram::DEFAULT_ITERATIONS, email_enabled: false, email_brand: "Network Services".to_string(), email_accent: "#4f46e5".to_string(), email_logo: String::new(), codes: HashMap::new() } + Self { accounts, channels, grouped, log, scram_iterations: scram::DEFAULT_ITERATIONS, email_enabled: false, email_brand: "Network Services".to_string(), email_accent: "#4f46e5".to_string(), email_logo: String::new(), codes: HashMap::new(), auth_fails: HashMap::new() } } /// Fold an entry authored by another node into the store — the services-side @@ -755,20 +778,57 @@ impl Db { /// Issue a fresh emailed code for `account` and purpose, valid for 15 minutes. pub fn issue_code(&mut self, account: &str, kind: CodeKind) -> String { let code = gen_code(); - self.codes.insert(key(account), (kind, code.clone(), Instant::now() + Duration::from_secs(900))); + self.codes.insert( + key(account), + PendingCode { kind, code: code.clone(), deadline: Instant::now() + Duration::from_secs(900), tries_left: CODE_TRIES }, + ); code } /// Consume a code for `account`: true if the purpose and code match and it - /// hasn't expired. + /// hasn't expired. A wrong guess burns a try and the code is dropped once + /// they run out, so it can't be ground down online. pub fn take_code(&mut self, account: &str, kind: CodeKind, code: &str) -> bool { let k = key(account); - match self.codes.get(&k) { - Some((ki, c, deadline)) if *ki == kind && c == code && *deadline > Instant::now() => { - self.codes.remove(&k); - true - } - _ => false, + let Some(pc) = self.codes.get_mut(&k) else { return false }; + if pc.deadline <= Instant::now() { + self.codes.remove(&k); + return false; + } + if pc.kind == kind && pc.code == code { + self.codes.remove(&k); + return true; + } + pc.tries_left = pc.tries_left.saturating_sub(1); + if pc.tries_left == 0 { + self.codes.remove(&k); + } + false + } + + /// Seconds the caller must wait before another password attempt on `account`, + /// or None if it is not currently throttled. + pub fn auth_lockout(&self, account: &str) -> Option { + let until = self.auth_fails.get(&key(account))?.locked_until?; + let now = Instant::now(); + (until > now).then(|| (until - now).as_secs() + 1) + } + + /// Record the outcome of a password attempt against `account`. Success clears + /// the counter; each failure past a few free tries grows an exponential + /// backoff, throttling guessing without a hard lockout a griefer could abuse. + pub fn note_auth(&mut self, account: &str, success: bool) { + let k = key(account); + if success { + self.auth_fails.remove(&k); + return; + } + let t = self.auth_fails.entry(k).or_insert(AuthThrottle { fails: 0, locked_until: None }); + t.fails += 1; + if t.fails > AUTH_FREE_TRIES { + let shift = (t.fails - AUTH_FREE_TRIES - 1).min(9); + let secs = (1u64 << shift).min(AUTH_MAX_BACKOFF_SECS); + t.locked_until = Some(Instant::now() + Duration::from_secs(secs)); } } @@ -1211,10 +1271,15 @@ fn glob_match(pattern: &str, text: &str) -> bool { } // A random 6-digit code for email verification / password reset. +// An unguessable emailed code: 8 characters from a 32-symbol unambiguous +// alphabet (~40 bits). 256 is an exact multiple of 32, so the byte->symbol map +// is bias-free. Long enough that it can't be brute-forced inside its 15-minute +// window even without the per-code attempt limit. fn gen_code() -> String { - let mut b = [0u8; 4]; + const ALPHABET: &[u8; 32] = b"23456789ABCDEFGHJKLMNPQRSTUVWXYZ"; // no 0/O/1/I + let mut b = [0u8; 8]; OsRng.fill_bytes(&mut b); - format!("{:06}", u32::from_le_bytes(b) % 1_000_000) + b.iter().map(|x| ALPHABET[(*x % 32) as usize] as char).collect() } fn hash_password(password: &str) -> Option { @@ -1286,6 +1351,12 @@ impl Store for Db { fn take_code(&mut self, account: &str, kind: CodeKind, code: &str) -> bool { Db::take_code(self, account, kind, code) } + fn auth_lockout(&self, account: &str) -> Option { + Db::auth_lockout(self, account) + } + fn note_auth(&mut self, account: &str, success: bool) { + Db::note_auth(self, account, success) + } fn verify_account(&mut self, account: &str) -> Result<(), RegError> { Db::verify_account(self, account) } @@ -1478,6 +1549,32 @@ mod tests { assert_eq!(db.ajoin_list("alice")[0].channel, "#dev"); } + // A wrong code is tolerated a few times, then the code is burned so it can't + // be ground down online even though it is short. + #[test] + fn code_burns_after_too_many_wrong_guesses() { + let mut db = Db::open(&tmp("code"), "N1"); + db.scram_iterations = 4096; + db.register("alice", "password", None).unwrap(); + let code = db.issue_code("alice", CodeKind::Reset); + for _ in 0..CODE_TRIES { + assert!(!db.take_code("alice", CodeKind::Reset, "WRONG000"), "wrong guess fails"); + } + assert!(!db.take_code("alice", CodeKind::Reset, &code), "the real code is now burned too"); + } + + // Password auth backs off after a few failures and the throttle clears on success. + #[test] + fn auth_throttle_locks_then_clears() { + let mut db = Db::open(&tmp("throttle"), "N1"); + for _ in 0..=AUTH_FREE_TRIES { + db.note_auth("mallory", false); + } + assert!(db.auth_lockout("mallory").is_some(), "locked out after the free tries"); + db.note_auth("mallory", true); + assert!(db.auth_lockout("mallory").is_none(), "a success clears the throttle"); + } + // Locally-authored events get an incrementing per-origin seq and a ticking // Lamport clock. #[test] diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 6684931..828e23e 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -1411,7 +1411,7 @@ mod tests { e.handle(NetEvent::UserConnect { uid: "000AAAAAB".into(), nick: "newbie".into(), host: "h".into() }); // REGISTER with an email defers; complete it as the link layer would. - let reg = e.handle(NetEvent::Privmsg { from: "000AAAAAB".into(), to: "42SAAAAAA".into(), text: "REGISTER pw newbie@example.org".into() }); + let reg = e.handle(NetEvent::Privmsg { from: "000AAAAAB".into(), to: "42SAAAAAA".into(), text: "REGISTER correcthorse newbie@example.org".into() }); let (account, password, email, reply) = reg.iter().find_map(|a| match a { NetAction::DeferRegister { account, password, email, reply } => Some((account.clone(), password.clone(), email.clone(), reply.clone())), _ => None,