From d305f5bf776d902997dfed3656c5dbe430940736 Mon Sep 17 00:00:00 2001 From: Nikita Vilunov Date: Tue, 23 Apr 2024 16:31:00 +0000 Subject: [PATCH] argon2-based password hashing (#55) Reviewed-on: https://git.vilunov.me/lavina/lavina/pulls/55 --- Cargo.lock | 34 ++++++++++++++++++ crates/lavina-core/Cargo.toml | 2 ++ .../migrations/4_new_challenges.sql | 4 +++ crates/lavina-core/src/auth.rs | 35 ++++++++++++++----- crates/lavina-core/src/player.rs | 28 +++++++++------ crates/lavina-core/src/repo/auth.rs | 19 ++++++++++ crates/lavina-core/src/repo/mod.rs | 5 ++- crates/projection-irc/tests/lib.rs | 25 ++++++------- crates/projection-xmpp/tests/lib.rs | 7 ++-- 9 files changed, 123 insertions(+), 36 deletions(-) create mode 100644 crates/lavina-core/migrations/4_new_challenges.sql create mode 100644 crates/lavina-core/src/repo/auth.rs diff --git a/Cargo.lock b/Cargo.lock index a909a9a..9658ff6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -114,6 +114,18 @@ version = "1.0.82" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f538837af36e6f6a9be0faa67f9a314f8119e4e4b5867c6ab40ed60360142519" +[[package]] +name = "argon2" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072" +dependencies = [ + "base64ct", + "blake2", + "cpufeatures", + "password-hash", +] + [[package]] name = "assert_matches" version = "1.5.0" @@ -192,6 +204,15 @@ dependencies = [ "serde", ] +[[package]] +name = "blake2" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" +dependencies = [ + "digest", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -882,8 +903,10 @@ name = "lavina-core" version = "0.0.2-dev" dependencies = [ "anyhow", + "argon2", "chrono", "prometheus", + "rand_core", "serde", "sqlx", "tokio", @@ -1126,6 +1149,17 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core", + "subtle", +] + [[package]] name = "paste" version = "1.0.14" diff --git a/crates/lavina-core/Cargo.toml b/crates/lavina-core/Cargo.toml index c49f83d..ab26daf 100644 --- a/crates/lavina-core/Cargo.toml +++ b/crates/lavina-core/Cargo.toml @@ -11,3 +11,5 @@ tokio.workspace = true tracing.workspace = true prometheus.workspace = true chrono.workspace = true +argon2 = { version = "0.5.3" } +rand_core = { version = "0.6.4", features = ["getrandom"] } diff --git a/crates/lavina-core/migrations/4_new_challenges.sql b/crates/lavina-core/migrations/4_new_challenges.sql new file mode 100644 index 0000000..9017511 --- /dev/null +++ b/crates/lavina-core/migrations/4_new_challenges.sql @@ -0,0 +1,4 @@ +create table challenges_argon2_password( + user_id integer primary key not null, + hash string not null +); diff --git a/crates/lavina-core/src/auth.rs b/crates/lavina-core/src/auth.rs index ba465db..ccf962d 100644 --- a/crates/lavina-core/src/auth.rs +++ b/crates/lavina-core/src/auth.rs @@ -1,4 +1,7 @@ -use anyhow::Result; +use anyhow::{anyhow, Result}; +use argon2::password_hash::{PasswordHash, PasswordHasher, PasswordVerifier, SaltString}; +use argon2::Argon2; +use rand_core::OsRng; use crate::prelude::log; use crate::repo::Storage; @@ -26,21 +29,35 @@ impl<'a> Authenticator<'a> { let Some(stored_user) = self.storage.retrieve_user_by_name(login).await? else { return Ok(Verdict::UserNotFound); }; - let Some(expected_password) = stored_user.password else { - log::debug!("Password not defined for user '{}'", login); - return Ok(Verdict::InvalidPassword); - }; - if expected_password == provided_password { - return Ok(Verdict::Authenticated); + if let Some(argon2_hash) = stored_user.argon2_hash { + let argon2 = Argon2::default(); + let password_hash = + PasswordHash::new(&argon2_hash).map_err(|e| anyhow!("Failed to parse password hash: {e:?}"))?; + let password_verifier = argon2.verify_password(provided_password.as_bytes(), &password_hash); + if password_verifier.is_ok() { + return Ok(Verdict::Authenticated); + } + } + if let Some(expected_password) = stored_user.password { + if expected_password == provided_password { + return Ok(Verdict::Authenticated); + } } Ok(Verdict::InvalidPassword) } pub async fn set_password(&self, login: &str, provided_password: &str) -> Result { - let Some(_) = self.storage.retrieve_user_by_name(login).await? else { + let Some(u) = self.storage.retrieve_user_by_name(login).await? else { return Ok(UpdatePasswordResult::UserNotFound); }; - self.storage.set_password(login, provided_password).await?; + + let salt = SaltString::generate(&mut OsRng); + let argon2 = Argon2::default(); + let password_hash = argon2 + .hash_password(provided_password.as_bytes(), &salt) + .map_err(|e| anyhow!("Failed to hash password: {e:?}"))?; + + self.storage.set_argon2_challenge(u.id, password_hash.to_string().as_str()).await?; log::info!("Password changed for player {login}"); Ok(UpdatePasswordResult::PasswordUpdated) } diff --git a/crates/lavina-core/src/player.rs b/crates/lavina-core/src/player.rs index 3a58812..4d6f6cb 100644 --- a/crates/lavina-core/src/player.rs +++ b/crates/lavina-core/src/player.rs @@ -264,20 +264,26 @@ impl PlayerRegistry { } pub async fn get_or_launch_player(&mut self, id: &PlayerId) -> PlayerHandle { - let mut inner = self.0.write().await; + let inner = self.0.read().await; if let Some((handle, _)) = inner.players.get(id) { handle.clone() } else { - let (handle, fiber) = Player::launch( - id.clone(), - inner.room_registry.clone(), - inner.dialogs.clone(), - inner.storage.clone(), - ) - .await; - inner.players.insert(id.clone(), (handle.clone(), fiber)); - inner.metric_active_players.inc(); - handle + drop(inner); + let mut inner = self.0.write().await; + if let Some((handle, _)) = inner.players.get(id) { + handle.clone() + } else { + let (handle, fiber) = Player::launch( + id.clone(), + inner.room_registry.clone(), + inner.dialogs.clone(), + inner.storage.clone(), + ) + .await; + inner.players.insert(id.clone(), (handle.clone(), fiber)); + inner.metric_active_players.inc(); + handle + } } } diff --git a/crates/lavina-core/src/repo/auth.rs b/crates/lavina-core/src/repo/auth.rs new file mode 100644 index 0000000..f7c0d69 --- /dev/null +++ b/crates/lavina-core/src/repo/auth.rs @@ -0,0 +1,19 @@ +use anyhow::Result; + +use crate::repo::Storage; + +impl Storage { + pub async fn set_argon2_challenge(&self, user_id: u32, hash: &str) -> Result<()> { + let mut executor = self.conn.lock().await; + sqlx::query( + "insert into challenges_argon2_password(user_id, hash) + values (?, ?) + on conflict(user_id) do update set hash = excluded.hash;", + ) + .bind(user_id) + .bind(hash) + .execute(&mut *executor) + .await?; + Ok(()) + } +} diff --git a/crates/lavina-core/src/repo/mod.rs b/crates/lavina-core/src/repo/mod.rs index dfa93c6..9c7aff6 100644 --- a/crates/lavina-core/src/repo/mod.rs +++ b/crates/lavina-core/src/repo/mod.rs @@ -12,6 +12,7 @@ use tokio::sync::Mutex; use crate::prelude::*; +mod auth; mod dialog; mod room; mod user; @@ -42,8 +43,9 @@ impl Storage { pub async fn retrieve_user_by_name(&self, name: &str) -> Result> { let mut executor = self.conn.lock().await; let res = sqlx::query_as( - "select u.id, u.name, c.password + "select u.id, u.name, c.password, a.hash as argon2_hash from users u left join challenges_plain_password c on u.id = c.user_id + left join challenges_argon2_password a on u.id = a.user_id where u.name = ?;", ) .bind(name) @@ -175,6 +177,7 @@ pub struct StoredUser { pub id: u32, pub name: String, pub password: Option, + pub argon2_hash: Option>, } #[derive(FromRow)] diff --git a/crates/projection-irc/tests/lib.rs b/crates/projection-irc/tests/lib.rs index 6a90c46..2de4f9e 100644 --- a/crates/projection-irc/tests/lib.rs +++ b/crates/projection-irc/tests/lib.rs @@ -8,6 +8,7 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt, BufReader}; use tokio::net::tcp::{ReadHalf, WriteHalf}; use tokio::net::TcpStream; +use lavina_core::auth::Authenticator; use lavina_core::player::{JoinResult, PlayerId, SendMessageResult}; use lavina_core::repo::{Storage, StorageConfig}; use lavina_core::room::RoomId; @@ -27,7 +28,7 @@ impl<'a> TestScope<'a> { let (reader, writer) = stream.split(); let reader = BufReader::new(reader); let buffer = vec![]; - let timeout = Duration::from_millis(100); + let timeout = Duration::from_millis(1000); TestScope { reader, writer, @@ -159,7 +160,7 @@ async fn scenario_basic() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -188,7 +189,7 @@ async fn scenario_join_and_reboot() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -258,7 +259,7 @@ async fn scenario_force_join_msg() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream1 = TcpStream::connect(server.server.addr).await?; let mut s1 = TestScope::new(&mut stream1); @@ -324,9 +325,9 @@ async fn scenario_two_users() -> Result<()> { // test scenario server.storage.create_user("tester1").await?; - server.storage.set_password("tester1", "password").await?; + Authenticator::new(&server.storage).set_password("tester1", "password").await?; server.storage.create_user("tester2").await?; - server.storage.set_password("tester2", "password").await?; + Authenticator::new(&server.storage).set_password("tester2", "password").await?; let mut stream1 = TcpStream::connect(server.server.addr).await?; let mut s1 = TestScope::new(&mut stream1); @@ -388,7 +389,7 @@ async fn scenario_cap_full_negotiation() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -428,7 +429,7 @@ async fn scenario_cap_full_negotiation_nick_last() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -467,7 +468,7 @@ async fn scenario_cap_short_negotiation() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -505,7 +506,7 @@ async fn scenario_cap_sasl_fail() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -549,7 +550,7 @@ async fn terminate_socket_scenario() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -574,7 +575,7 @@ async fn server_time_capability() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); diff --git a/crates/projection-xmpp/tests/lib.rs b/crates/projection-xmpp/tests/lib.rs index be687a4..bece5d9 100644 --- a/crates/projection-xmpp/tests/lib.rs +++ b/crates/projection-xmpp/tests/lib.rs @@ -16,6 +16,7 @@ use tokio_rustls::rustls::client::ServerCertVerifier; use tokio_rustls::rustls::{ClientConfig, ServerName}; use tokio_rustls::TlsConnector; +use lavina_core::auth::Authenticator; use lavina_core::repo::{Storage, StorageConfig}; use lavina_core::LavinaCore; use projection_xmpp::{launch, RunningServer, ServerConfig}; @@ -158,7 +159,7 @@ async fn scenario_basic() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -210,7 +211,7 @@ async fn scenario_basic_without_headers() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream); @@ -260,7 +261,7 @@ async fn terminate_socket() -> Result<()> { // test scenario server.storage.create_user("tester").await?; - server.storage.set_password("tester", "password").await?; + Authenticator::new(&server.storage).set_password("tester", "password").await?; let mut stream = TcpStream::connect(server.server.addr).await?; let mut s = TestScope::new(&mut stream);