From fdc3e07be6d838254c1bddfd06ec5b3f89950da6 Mon Sep 17 00:00:00 2001 From: Nineko Date: Mon, 25 Dec 2023 16:28:56 +0100 Subject: [PATCH] feat: replaced flaky argon2 with better argon2 crate (#37) * feat: replaced flaky argon2 with better argon2 crate * fix: applied cargo fmt nightly * docs: added comment specifying what the settings for Argon2 mean * fix: made hashing error a bit more descriptive * fix: fixed incorrect value for Kib --- Cargo.lock | 57 ++++++++++++++------------------ Cargo.toml | 2 +- src/api/client_server/session.rs | 15 ++++++--- src/database/mod.rs | 18 +++++++--- src/lib.rs | 4 +-- src/service/globals/mod.rs | 14 ++++++-- src/service/mod.rs | 6 ++-- src/service/uiaa/mod.rs | 11 ++++-- src/utils/mod.rs | 19 +++++------ 9 files changed, 84 insertions(+), 62 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f332321c..2b3cd206 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -56,16 +56,16 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bddcadddf5e9015d310179a59bb28c4d4b9920ad0f11e8e14dbadf654890c9a6" [[package]] -name = "arrayref" -version = "0.3.7" +name = "argon2" +version = "0.5.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6b4930d2cb77ce62f89ee5d5289b4ac049559b1c45539271f5ed4fdc7db34545" - -[[package]] -name = "arrayvec" -version = "0.7.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" +checksum = "17ba4cac0a46bc1d2912652a751c47f2a9f3a7fe89bcae2275d418f5270402f9" +dependencies = [ + "base64ct", + "blake2", + "cpufeatures", + "password-hash", +] [[package]] name = "as_variant" @@ -257,14 +257,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b4682ae6287fcf752ecaabbfcc7b6f9b72aa33933dc23a554d853aea8eea8635" [[package]] -name = "blake2b_simd" -version = "1.0.2" +name = "blake2" +version = "0.10.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23285ad32269793932e830392f2fe2f83e26488fd3ec778883a93c8323735780" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" dependencies = [ - "arrayref", - "arrayvec", - "constant_time_eq", + "digest", ] [[package]] @@ -395,6 +393,7 @@ checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" name = "conduwuit" version = "0.7.0-alpha+conduwuit-0.1.1" dependencies = [ + "argon2", "async-trait", "axum", "axum-server", @@ -430,7 +429,6 @@ dependencies = [ "rocksdb", "ruma", "rusqlite", - "rust-argon2", "sd-notify", "serde", "serde_html_form", @@ -465,12 +463,6 @@ version = "0.2.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6051f239ecec86fde3410901ab7860d458d160371533842974fc61f96d15879b" -[[package]] -name = "constant_time_eq" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f7144d30dcf0fafbce74250a3963025d8d52177934239851c917d29f1df280c2" - [[package]] name = "core-foundation" version = "0.9.3" @@ -1755,6 +1747,17 @@ dependencies = [ "windows-targets", ] +[[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" @@ -2335,16 +2338,6 @@ dependencies = [ "smallvec", ] -[[package]] -name = "rust-argon2" -version = "2.0.0" -source = "git+https://github.com/sru-systems/rust-argon2?rev=e6cb5bf99643e565f4f0d103960d655dac9f3097#e6cb5bf99643e565f4f0d103960d655dac9f3097" -dependencies = [ - "base64", - "blake2b_simd", - "constant_time_eq", -] - [[package]] name = "rustc-demangle" version = "0.1.23" diff --git a/Cargo.toml b/Cargo.toml index e9dc01cd..2923def7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,7 @@ serde = { version = "1.0.193", features = ["rc"] } # Used for secure identifiers rand = "0.8.5" # Used to hash passwords -rust-argon2 = { git = "https://github.com/sru-systems/rust-argon2", rev = "e6cb5bf99643e565f4f0d103960d655dac9f3097" } +argon2 = "0.5" reqwest = { version = "0.11.22", default-features = false, features = ["rustls-tls-native-roots", "socks"] } # Used for conduit::Error type thiserror = "1.0.51" diff --git a/src/api/client_server/session.rs b/src/api/client_server/session.rs index 679d704f..cdc20174 100644 --- a/src/api/client_server/session.rs +++ b/src/api/client_server/session.rs @@ -1,5 +1,6 @@ use super::{DEVICE_ID_LENGTH, TOKEN_LENGTH}; use crate::{services, utils, Error, Result, Ruma}; +use argon2::{PasswordHash, PasswordVerifier}; use ruma::{ api::client::{ error::ErrorKind, @@ -9,7 +10,7 @@ use ruma::{ UserId, }; use serde::Deserialize; -use tracing::{info, warn}; +use tracing::{error, info, warn}; #[derive(Debug, Deserialize)] struct Claims { @@ -74,9 +75,15 @@ pub async fn login_route(body: Ruma) -> Result> = RwLock::new(None); +pub static SERVICES: RwLock>> = RwLock::new(None); -pub fn services() -> &'static Services { +pub fn services() -> &'static Services<'static> { SERVICES .read() .unwrap() diff --git a/src/service/globals/mod.rs b/src/service/globals/mod.rs index 4df22b1b..228424f5 100644 --- a/src/service/globals/mod.rs +++ b/src/service/globals/mod.rs @@ -1,4 +1,5 @@ mod data; +use argon2::Argon2; pub use data::Data; use ruma::{ serde::Base64, OwnedDeviceId, OwnedEventId, OwnedRoomId, OwnedServerName, @@ -51,7 +52,7 @@ type SyncHandle = ( Receiver>>, // rx ); -pub struct Service { +pub struct Service<'a> { pub db: &'static dyn Data, pub actual_destination_cache: Arc>, // actual_destination, host @@ -77,6 +78,7 @@ pub struct Service { pub rotate: RotationHandler, pub shutdown: AtomicBool, + pub argon: Argon2<'a>, } /// Handles "rotation" of long-polling requests. "Rotation" in this context is similar to "rotation" of log files and the like. @@ -140,7 +142,7 @@ impl Resolve for Resolver { } } -impl Service { +impl Service<'_> { pub fn load(db: &'static dyn Data, config: Config) -> Result { let keypair = db.load_keypair(); @@ -188,7 +190,12 @@ impl Service { RoomVersionId::V5, RoomVersionId::V11, ]; - + // 19456 Kib blocks, iterations = 2, parallelism = 1 for more info https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id + let argon = Argon2::new( + argon2::Algorithm::Argon2id, + argon2::Version::default(), + argon2::Params::new(19456, 2, 1, None).expect("valid parameters"), + ); let mut s = Self { db, config, @@ -219,6 +226,7 @@ impl Service { sync_receivers: RwLock::new(HashMap::new()), rotate: RotationHandler::new(), shutdown: AtomicBool::new(false), + argon, }; fs::create_dir_all(s.get_media_folder())?; diff --git a/src/service/mod.rs b/src/service/mod.rs index 74f120f9..883c567c 100644 --- a/src/service/mod.rs +++ b/src/service/mod.rs @@ -21,7 +21,7 @@ pub mod transaction_ids; pub mod uiaa; pub mod users; -pub struct Services { +pub struct Services<'a> { pub appservice: appservice::Service, pub pusher: pusher::Service, pub rooms: rooms::Service, @@ -30,13 +30,13 @@ pub struct Services { pub users: users::Service, pub account_data: account_data::Service, pub admin: Arc, - pub globals: globals::Service, + pub globals: globals::Service<'a>, pub key_backups: key_backups::Service, pub media: media::Service, pub sending: Arc, } -impl Services { +impl Services<'_> { pub fn build< D: appservice::Data + pusher::Data diff --git a/src/service/uiaa/mod.rs b/src/service/uiaa/mod.rs index ed39af99..2c51833a 100644 --- a/src/service/uiaa/mod.rs +++ b/src/service/uiaa/mod.rs @@ -1,5 +1,6 @@ mod data; +use argon2::{PasswordHash, PasswordVerifier}; pub use data::Data; use ruma::{ @@ -81,8 +82,14 @@ impl Service { // Check if password is correct if let Some(hash) = services().users.password_hash(&user_id)? { - let hash_matches = - argon2::verify_encoded(&hash, password.as_bytes()).unwrap_or(false); + let hash_matches = services() + .globals + .argon + .verify_password( + password.as_bytes(), + &PasswordHash::new(&hash).expect("valid hash in database"), + ) + .is_ok(); if !hash_matches { uiaainfo.auth_error = Some(ruma::api::client::error::StandardErrorBody { diff --git a/src/utils/mod.rs b/src/utils/mod.rs index 1bd6fde2..79e83f97 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -1,7 +1,7 @@ pub mod error; -use crate::{Error, Result}; -use argon2::{Config, Variant}; +use crate::{services, Error, Result}; +use argon2::{password_hash::SaltString, PasswordHasher}; use rand::prelude::*; use ring::digest; use ruma::{ @@ -72,14 +72,13 @@ pub fn random_string(length: usize) -> String { } /// Calculate a new hash for the given password -pub fn calculate_password_hash(password: &str) -> Result { - let hashing_config = Config { - variant: Variant::Argon2id, - ..Config::owasp2() // m=19456 (19 MiB), t=2, p=1 from https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#argon2id - }; - - let salt = random_string(32); - argon2::hash_encoded(password.as_bytes(), salt.as_bytes(), &hashing_config) +pub fn calculate_password_hash(password: &str) -> Result { + let salt = SaltString::generate(thread_rng()); + services() + .globals + .argon + .hash_password(password.as_bytes(), &salt) + .map(|it| it.to_string()) } #[tracing::instrument(skip(keys))]