diff --git a/Cargo.lock b/Cargo.lock index a19871e4..e5b79b03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2684,7 +2684,6 @@ name = "mas-cli" version = "0.1.0" dependencies = [ "anyhow", - "argon2", "atty", "axum", "camino", @@ -3101,21 +3100,16 @@ dependencies = [ name = "mas-storage" version = "0.1.0" dependencies = [ - "anyhow", - "argon2", "chrono", "mas-data-model", "mas-iana", "mas-jose", "oauth2-types", - "password-hash", "rand 0.8.5", - "rand_chacha 0.3.1", "serde", "serde_json", "sqlx", "thiserror", - "tokio", "tracing", "ulid", "url", diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index ef5c443c..e50a0802 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -7,7 +7,6 @@ license = "Apache-2.0" [dependencies] anyhow = "1.0.66" -argon2 = { version = "0.4.1", features = ["password-hash"] } atty = "0.2.14" axum = "0.6.1" camino = "1.1.1" diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 413812bf..33066416 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -18,18 +18,20 @@ use hyper::StatusCode; use mas_data_model::{CompatSession, CompatSsoLoginState, Device, TokenType}; use mas_storage::{ compat::{ - add_compat_access_token, add_compat_refresh_token, compat_login, - get_compat_sso_login_by_token, mark_compat_sso_login_as_exchanged, + add_compat_access_token, add_compat_refresh_token, get_compat_sso_login_by_token, + mark_compat_sso_login_as_exchanged, start_compat_session, }, + user::{add_user_password, lookup_user_by_username, lookup_user_password}, Clock, }; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, skip_serializing_none, DurationMilliSeconds}; use sqlx::{PgPool, Postgres, Transaction}; use thiserror::Error; +use zeroize::Zeroizing; use super::{MatrixError, MatrixHomeserver}; -use crate::impl_from_error_for_route; +use crate::{impl_from_error_for_route, passwords::PasswordManager}; #[derive(Debug, Serialize)] #[serde(tag = "type")] @@ -132,8 +134,14 @@ pub enum RouteError { #[error("unsupported login method")] Unsupported, - #[error("login failed")] - LoginFailed, + #[error("user not found")] + UserNotFound, + + #[error("user has no password")] + NoPassword, + + #[error("password verification failed")] + PasswordVerificationFailed(#[source] anyhow::Error), #[error("login took too long")] LoginTookTooLong, @@ -158,11 +166,13 @@ impl IntoResponse for RouteError { error: "Invalid login type", status: StatusCode::BAD_REQUEST, }, - Self::LoginFailed => MatrixError { - errcode: "M_UNAUTHORIZED", - error: "Invalid username/password", - status: StatusCode::FORBIDDEN, - }, + Self::UserNotFound | Self::NoPassword | Self::PasswordVerificationFailed(_) => { + MatrixError { + errcode: "M_UNAUTHORIZED", + error: "Invalid username/password", + status: StatusCode::FORBIDDEN, + } + } Self::LoginTookTooLong => MatrixError { errcode: "M_UNAUTHORIZED", error: "Login token expired", @@ -180,6 +190,7 @@ impl IntoResponse for RouteError { #[tracing::instrument(skip_all, err)] pub(crate) async fn post( + State(password_manager): State, State(pool): State, State(homeserver): State, Json(input): Json, @@ -190,7 +201,7 @@ pub(crate) async fn post( Credentials::Password { identifier: Identifier::User { user }, password, - } => user_password_login(&mut txn, user, password).await?, + } => user_password_login(&password_manager, &mut txn, user, password).await?, Credentials::Token { token } => token_login(&mut txn, &clock, &token).await?, @@ -295,16 +306,53 @@ async fn token_login( } async fn user_password_login( + password_manager: &PasswordManager, txn: &mut Transaction<'_, Postgres>, username: String, password: String, ) -> Result { let (clock, mut rng) = crate::clock_and_rng(); - let device = Device::generate(&mut rng); - let session = compat_login(txn, &mut rng, &clock, &username, &password, device) + // Find the user + let user = lookup_user_by_username(&mut *txn, &username) + .await? + .ok_or(RouteError::UserNotFound)?; + + // Lookup its password + let user_password = lookup_user_password(&mut *txn, &user) + .await? + .ok_or(RouteError::NoPassword)?; + + // Verify the password + let password = Zeroizing::new(password.into_bytes()); + + let new_password_hash = password_manager + .verify_and_upgrade( + &mut rng, + user_password.version, + password, + user_password.hashed_password.clone(), + ) .await - .map_err(|_| RouteError::LoginFailed)?; + .map_err(RouteError::PasswordVerificationFailed)?; + + if let Some((version, hashed_password)) = new_password_hash { + // Save the upgraded password if needed + add_user_password( + &mut *txn, + &mut rng, + &clock, + &user, + version, + hashed_password, + Some(user_password), + ) + .await?; + } + + // Now that the user credentials have been verified, start a new compat session + let device = Device::generate(&mut rng); + let session = start_compat_session(&mut *txn, &mut rng, &clock, user, device).await?; Ok(session) } diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index ed308c29..2d3d1812 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -208,6 +208,7 @@ where UrlBuilder: FromRef, PgPool: FromRef, MatrixHomeserver: FromRef, + PasswordManager: FromRef, { Router::new() .route( diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index 8a6f58aa..01560c12 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -6,20 +6,15 @@ edition = "2021" license = "Apache-2.0" [dependencies] -tokio = "1.23.0" sqlx = { version = "0.6.2", features = ["runtime-tokio-rustls", "postgres", "migrate", "chrono", "offline", "json", "uuid"] } chrono = { version = "0.4.23", features = ["serde"] } serde = { version = "1.0.149", features = ["derive"] } serde_json = "1.0.89" thiserror = "1.0.37" -anyhow = "1.0.66" tracing = "0.1.37" # Password hashing -argon2 = { version = "0.4.1", features = ["password-hash"] } -password-hash = { version = "0.4.2", features = ["std"] } rand = "0.8.5" -rand_chacha = "0.3.1" url = { version = "2.3.1", features = ["serde"] } uuid = "1.2.2" ulid = { version = "1.0.0", features = ["uuid", "serde"] } diff --git a/crates/storage/src/compat.rs b/crates/storage/src/compat.rs index d1464e89..6d4be8e0 100644 --- a/crates/storage/src/compat.rs +++ b/crates/storage/src/compat.rs @@ -12,8 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use anyhow::Context; -use argon2::{Argon2, PasswordHash}; use chrono::{DateTime, Duration, Utc}; use mas_data_model::{ CompatAccessToken, CompatRefreshToken, CompatSession, CompatSsoLogin, CompatSsoLoginState, @@ -21,7 +19,6 @@ use mas_data_model::{ }; use rand::Rng; use sqlx::{Acquire, PgExecutor, Postgres, QueryBuilder}; -use tokio::task; use tracing::{info_span, Instrument}; use ulid::Ulid; use url::Url; @@ -29,7 +26,6 @@ use uuid::Uuid; use crate::{ pagination::{process_page, QueryBuilderExt}, - user::lookup_user_by_username, Clock, DatabaseError, DatabaseInconsistencyError, LookupResultExt, }; @@ -284,91 +280,6 @@ pub async fn lookup_active_compat_refresh_token( Ok(Some((refresh_token, access_token, session))) } -#[tracing::instrument( - skip_all, - fields( - user.username = username, - user.id, - compat_session.id, - compat_session.device.id = device.as_str(), - ), - err(Debug), -)] -pub async fn compat_login( - conn: impl Acquire<'_, Database = Postgres> + Send, - mut rng: impl Rng + Send, - clock: &Clock, - username: &str, - password: &str, - device: Device, -) -> Result { - // TODO: that should be split and not verify the password hash here - let mut txn = conn.begin().await.context("could not start transaction")?; - - // First, lookup the user - let user = lookup_user_by_username(&mut txn, username) - .await? - .context("Could not lookup username")?; - tracing::Span::current().record("user.id", tracing::field::display(user.id)); - - // Now, fetch the hashed password from the user associated with that session - let hashed_password: String = sqlx::query_scalar!( - r#" - SELECT up.hashed_password - FROM user_passwords up - WHERE up.user_id = $1 - ORDER BY up.created_at DESC - LIMIT 1 - "#, - Uuid::from(user.id), - ) - .fetch_one(&mut txn) - .instrument(tracing::info_span!("Lookup hashed password")) - .await?; - - // TODO: pass verifiers list as parameter - // Verify the password in a blocking thread to avoid blocking the async executor - let password = password.to_owned(); - task::spawn_blocking(move || { - let context = Argon2::default(); - let hasher = PasswordHash::new(&hashed_password)?; - hasher.verify_password(&[&context], &password) - }) - .instrument(tracing::info_span!("Verify hashed password")) - .await??; - - let created_at = clock.now(); - let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng); - tracing::Span::current().record("compat_session.id", tracing::field::display(id)); - - sqlx::query!( - r#" - INSERT INTO compat_sessions - (compat_session_id, user_id, device_id, created_at) - VALUES ($1, $2, $3, $4) - "#, - Uuid::from(id), - Uuid::from(user.id), - device.as_str(), - created_at, - ) - .execute(&mut txn) - .instrument(tracing::info_span!("Insert compat session")) - .await - .context("could not insert compat session")?; - - let session = CompatSession { - id, - user, - device, - created_at, - finished_at: None, - }; - - txn.commit().await.context("could not commit transaction")?; - Ok(session) -} - #[tracing::instrument( skip_all, fields( @@ -895,6 +806,48 @@ pub async fn get_compat_sso_login_by_token( Ok(Some(res.try_into()?)) } +#[tracing::instrument( + skip_all, + fields( + %user.id, + compat_session.id, + compat_session.device.id = device.as_str(), + ), + err, +)] +pub async fn start_compat_session( + executor: impl PgExecutor<'_>, + mut rng: impl Rng + Send, + clock: &Clock, + user: User, + device: Device, +) -> Result { + let created_at = clock.now(); + let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng); + tracing::Span::current().record("compat_session.id", tracing::field::display(id)); + + sqlx::query!( + r#" + INSERT INTO compat_sessions (compat_session_id, user_id, device_id, created_at) + VALUES ($1, $2, $3, $4) + "#, + Uuid::from(id), + Uuid::from(user.id), + device.as_str(), + created_at, + ) + .execute(executor) + .await?; + + Ok(CompatSession { + id, + user, + device, + created_at, + finished_at: None, + }) +} + #[tracing::instrument( skip_all, fields( @@ -920,31 +873,7 @@ pub async fn fullfill_compat_sso_login( let mut txn = conn.begin().await?; - let created_at = clock.now(); - let id = Ulid::from_datetime_with_source(created_at.into(), &mut rng); - tracing::Span::current().record("compat_session.id", tracing::field::display(id)); - - sqlx::query!( - r#" - INSERT INTO compat_sessions (compat_session_id, user_id, device_id, created_at) - VALUES ($1, $2, $3, $4) - "#, - Uuid::from(id), - Uuid::from(user.id), - device.as_str(), - created_at, - ) - .execute(&mut txn) - .instrument(tracing::info_span!("Insert compat session")) - .await?; - - let session = CompatSession { - id, - user, - device, - created_at, - finished_at: None, - }; + let session = start_compat_session(&mut txn, &mut rng, clock, user, device).await?; let fulfilled_at = clock.now(); sqlx::query!(