From 1d737c261b5fc33742c8bf9d9e465a2911dfc986 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Sat, 13 Jan 2024 19:50:38 +0000 Subject: [PATCH] Add login sessions --- ...b5571eddce6f76d80db323622db9648d918c3.json | 16 ++ ...740664e29e4faf414e4546e24c909a0bdfa67.json | 34 +++ Cargo.lock | 1 + Cargo.toml | 1 + migrations/20231205205358_login_sessions.sql | 21 ++ src/bin/idcoop.rs | 5 +- src/passwords.rs | 19 +- src/store.rs | 51 ++++ src/web.rs | 26 +- src/web/login.rs | 224 ++++++++++++++++-- src/web/oauth_openid.rs | 2 +- src/web/oauth_openid/authorisation.rs | 2 +- 12 files changed, 366 insertions(+), 36 deletions(-) create mode 100644 .sqlx/query-2633347de5bc78129d60926bbeab5571eddce6f76d80db323622db9648d918c3.json create mode 100644 .sqlx/query-7a2835e1bed065bddf7cec4a5a6740664e29e4faf414e4546e24c909a0bdfa67.json create mode 100644 migrations/20231205205358_login_sessions.sql diff --git a/.sqlx/query-2633347de5bc78129d60926bbeab5571eddce6f76d80db323622db9648d918c3.json b/.sqlx/query-2633347de5bc78129d60926bbeab5571eddce6f76d80db323622db9648d918c3.json new file mode 100644 index 0000000..5934efe --- /dev/null +++ b/.sqlx/query-2633347de5bc78129d60926bbeab5571eddce6f76d80db323622db9648d918c3.json @@ -0,0 +1,16 @@ +{ + "db_name": "PostgreSQL", + "query": "INSERT INTO login_sessions (login_session_token_hash, user_id, started_at_utc, csrf_secret)\n VALUES ($1, $2, NOW(), $3)", + "describe": { + "columns": [], + "parameters": { + "Left": [ + "Bytea", + "Uuid", + "Bytea" + ] + }, + "nullable": [] + }, + "hash": "2633347de5bc78129d60926bbeab5571eddce6f76d80db323622db9648d918c3" +} diff --git a/.sqlx/query-7a2835e1bed065bddf7cec4a5a6740664e29e4faf414e4546e24c909a0bdfa67.json b/.sqlx/query-7a2835e1bed065bddf7cec4a5a6740664e29e4faf414e4546e24c909a0bdfa67.json new file mode 100644 index 0000000..934b03a --- /dev/null +++ b/.sqlx/query-7a2835e1bed065bddf7cec4a5a6740664e29e4faf414e4546e24c909a0bdfa67.json @@ -0,0 +1,34 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT user_name, user_id, csrf_secret\n FROM login_sessions INNER JOIN users USING (user_id)\n WHERE login_session_token_hash = $1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "user_name", + "type_info": "Text" + }, + { + "ordinal": 1, + "name": "user_id", + "type_info": "Uuid" + }, + { + "ordinal": 2, + "name": "csrf_secret", + "type_info": "Bytea" + } + ], + "parameters": { + "Left": [ + "Bytea" + ] + }, + "nullable": [ + false, + false, + false + ] + }, + "hash": "7a2835e1bed065bddf7cec4a5a6740664e29e4faf414e4546e24c909a0bdfa67" +} diff --git a/Cargo.lock b/Cargo.lock index 2e527c9..2ba5f21 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1593,6 +1593,7 @@ dependencies = [ "axum", "axum_csrf", "base64", + "blake2", "chrono", "clap", "comfy-table", diff --git a/Cargo.toml b/Cargo.toml index 82bea65..252fa24 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ async-trait = "0.1.74" axum = { version = "0.6.20", features = ["tracing", "macros", "headers"] } axum_csrf = { version = "0.7.2", features = ["layer"] } base64 = "0.21.5" +blake2 = "0.10.6" chrono = "0.4.31" clap = { version = "4.4.6", features = ["derive"] } comfy-table = "7.1.0" diff --git a/migrations/20231205205358_login_sessions.sql b/migrations/20231205205358_login_sessions.sql new file mode 100644 index 0000000..9a7f54c --- /dev/null +++ b/migrations/20231205205358_login_sessions.sql @@ -0,0 +1,21 @@ +-- Create a table to store the login sessions of users. + +CREATE TABLE login_sessions ( + login_session_id SERIAL NOT NULL PRIMARY KEY, + login_session_token_hash BYTEA NOT NULL UNIQUE, + user_id UUID NOT NULL REFERENCES users(user_id), + started_at_utc TIMESTAMP NOT NULL, + csrf_secret BYTEA NOT NULL +); + +COMMENT ON TABLE login_sessions IS 'A login session is equivalent to one browser session where the user has logged in. The session is identified by a cookie.'; + +COMMENT ON COLUMN login_sessions.login_session_id IS 'Synthetic numeric ID for the session, used for cross-referencing only. Should not be public.'; + +COMMENT ON COLUMN login_sessions.login_session_token_hash IS 'BLAKE2s-256 hash of the login session token (session cookie value).'; + +COMMENT ON COLUMN login_sessions.user_id IS 'ID of the user that this login session is for.'; + +COMMENT ON COLUMN login_sessions.started_at_utc IS 'Timestamp in UTC when the session was started.'; + +COMMENT ON COLUMN login_sessions.csrf_secret IS 'Key for a Blake2sMac256 which is used to prevent Cross-Site Request Forgery.'; diff --git a/src/bin/idcoop.rs b/src/bin/idcoop.rs index 19a8768..c6f03ac 100644 --- a/src/bin/idcoop.rs +++ b/src/bin/idcoop.rs @@ -201,10 +201,7 @@ async fn handle_user_command(command: UserCommand, config: &Configuration) -> ey })}).await? else { bail!("No user by that name."); }; - print!( - "Change password for {} ({}):\n=> ", - user.user_name, user.user_id - ); + println!("Change password for {} ({}):", user.user_name, user.user_id); let mut buf_line = String::new(); stdin() .read_line(&mut buf_line) diff --git a/src/passwords.rs b/src/passwords.rs index f464b47..98f1258 100644 --- a/src/passwords.rs +++ b/src/passwords.rs @@ -1,4 +1,7 @@ -use argon2::{password_hash::SaltString, Algorithm, Argon2, Params, PasswordHasher, Version}; +use argon2::{ + password_hash::SaltString, Algorithm, Argon2, Params, PasswordHash, PasswordHasher, + PasswordVerifier, Version, +}; use eyre::eyre; use rand::rngs::OsRng; @@ -8,7 +11,6 @@ pub fn create_password_hash( password: &str, config: &PasswordHashingConfig, ) -> eyre::Result { - eprintln!("hash {password:?}"); let salt = SaltString::generate(&mut OsRng); let argon2 = Argon2::new( Algorithm::Argon2id, @@ -22,6 +24,15 @@ pub fn create_password_hash( .map(|h| h.to_string()) } -pub fn check_hash(password: String, hash: &str) -> eyre::Result { - todo!() +pub fn check_hash(password: &str, hash: &str) -> eyre::Result { + let argon2 = Argon2::new( + Algorithm::Argon2id, + Version::default(), + // the params here are irrelevant: the PasswordHash contains the needed params. + Params::default(), + ); + let hash = + PasswordHash::new(hash).map_err(|err| eyre!("failed to decode password hash: {err:?}"))?; + + Ok(argon2.verify_password(password.as_bytes(), &hash).is_ok()) } diff --git a/src/store.rs b/src/store.rs index 2e5a67b..8bc0bf5 100644 --- a/src/store.rs +++ b/src/store.rs @@ -1,9 +1,15 @@ use chrono::NaiveDateTime; +use eyre::eyre; use eyre::Context; use futures::future::BoxFuture; use sqlx::{types::Uuid, Connection, PgPool, Postgres, Transaction}; use tracing::error; +use crate::web::login::{ + LoginSession, LOGIN_SESSION_CSRF_SECRET_BYTES, LOGIN_SESSION_TOKEN_BYTES, + LOGIN_SESSION_TOKEN_HASH_BYTES, +}; + /// Postgres-backed storage for IdCoop pub struct IdCoopStore { db_pool: PgPool, @@ -154,4 +160,49 @@ impl<'a, 'txn> IdCoopStoreTxn<'a, 'txn> { .await .context("failed to list users") } + + pub async fn create_login_session( + &mut self, + login_session_token_hash: &[u8; LOGIN_SESSION_TOKEN_HASH_BYTES], + user_id: Uuid, + csrf_secret: &[u8; LOGIN_SESSION_CSRF_SECRET_BYTES], + ) -> eyre::Result<()> { + sqlx::query!( + "INSERT INTO login_sessions (login_session_token_hash, user_id, started_at_utc, csrf_secret) + VALUES ($1, $2, NOW(), $3)", + login_session_token_hash, user_id, csrf_secret + ) + .execute(&mut **self.txn) + .await + .context("failed to create login session")?; + Ok(()) + } + + pub async fn lookup_login_session( + &mut self, + login_session_token_hash: &[u8; LOGIN_SESSION_TOKEN_HASH_BYTES], + ) -> eyre::Result> { + let row_opt = sqlx::query!( + " + SELECT user_name, user_id, csrf_secret + FROM login_sessions INNER JOIN users USING (user_id) + WHERE login_session_token_hash = $1 + ", + login_session_token_hash + ) + .fetch_optional(&mut **self.txn) + .await + .context("failed to lookup login session")?; + + let Some(row) = row_opt else { return Ok(None); }; + + Ok(Some(LoginSession { + user_name: row.user_name, + user_id: row.user_id, + csrf_secret: row + .csrf_secret + .try_into() + .map_err(|_| eyre!("cannot retrieve login session: has invalid CSRF token"))?, + })) + } } diff --git a/src/web.rs b/src/web.rs index eaa495d..fd323e0 100644 --- a/src/web.rs +++ b/src/web.rs @@ -3,7 +3,7 @@ use std::{net::SocketAddr, sync::Arc}; use axum::{ extract::ConnectInfo, http::{StatusCode, Uri}, - response::{IntoResponse, Response}, + response::{Html, IntoResponse, Response}, routing::{get, post}, Extension, Router, }; @@ -12,7 +12,7 @@ use eyre::Context; use hornbeam::{initialise_template_manager, make_template_manager}; use rand::{distributions::Alphanumeric, thread_rng, Rng}; use tower_http::trace::TraceLayer; -use tracing::info; +use tracing::{error, info}; use crate::{ config::{Configuration, SecretConfig}, @@ -121,3 +121,25 @@ fn make_login_redirect(then_uri: Uri) -> Response { ) .into_response() } + +/// Wrapper to make `eyre::Error: IntoResponse`. +pub struct InternalEyreError(eyre::Error); + +impl From for InternalEyreError { + fn from(value: eyre::Error) -> Self { + InternalEyreError(value) + } +} + +impl IntoResponse for InternalEyreError { + fn into_response(self) -> Response { + error!("internal error processing request: {:?}", self.0); + ( + StatusCode::INTERNAL_SERVER_ERROR, + Html("

500 Internal Server Error

A fault in our system means that your request could not be handled. Please try again soon or contact the administrator if the issue persists.

") + ) + .into_response() + } +} + +pub type WebResult = Result; diff --git a/src/web/login.rs b/src/web/login.rs index d940a0d..b220b3e 100644 --- a/src/web/login.rs +++ b/src/web/login.rs @@ -1,16 +1,98 @@ +use std::sync::Arc; + use async_trait::async_trait; use axum::{ extract::{FromRequestParts, Query}, headers::Cookie, http::{request::Parts, uri::PathAndQuery, HeaderValue, StatusCode}, response::{Html, IntoResponse, Response}, - Form, TypedHeader, + Extension, Form, TypedHeader, }; use axum_csrf::CsrfToken; +use base64::{prelude::BASE64_URL_SAFE_NO_PAD, Engine}; +use blake2::{digest::Mac, Blake2s256, Blake2sMac256, Digest}; +use chrono::{DateTime, Duration, TimeZone, Utc}; +use eyre::{bail, Context, ContextCompat}; +use rand::{thread_rng, Rng}; use serde::Deserialize; +use sqlx::types::Uuid; +use tracing::error; + +use crate::{ + config::{Configuration, PasswordHashingConfig}, + passwords::{check_hash, create_password_hash}, + store::IdCoopStore, +}; + +use super::WebResult; + +/// Size of the login token itself +pub const LOGIN_SESSION_TOKEN_BYTES: usize = 32; + +/// Size of a Blake2s hash of the login token (which is what we store in the database) +pub const LOGIN_SESSION_TOKEN_HASH_BYTES: usize = 32; + +/// Size of the CSRF secret in bytes; this is a Blake2sMac256 salt size. +/// TODO the Blake2sMac256 also has a 'personal' item which might give us more bytes to play with and +/// perhaps we should be using that too. +pub const LOGIN_SESSION_CSRF_SECRET_BYTES: usize = 8; pub struct LoginSession { - pub username: String, + pub user_name: String, + pub user_id: Uuid, + pub csrf_secret: [u8; LOGIN_SESSION_CSRF_SECRET_BYTES], +} + +/// CSRF token expiry time is 1 week. +pub const CSRF_TOKEN_EXPIRY_TIME: Duration = Duration::milliseconds(1000 * 86400 * 7); + +impl LoginSession { + /// Generates a CSRF token which is bound to this session and expires 1 week in the future. + pub fn generate_csrf_token(&self, now: DateTime) -> eyre::Result { + let now_timestamp = now.timestamp(); + let now_8bytes = now_timestamp.to_be_bytes(); + + let mac_tag_bytes = Blake2sMac256::new_with_salt_and_personal(&self.csrf_secret, &[], &[])? + .chain_update(&now_8bytes) + .finalize() + .into_bytes(); + let mac_b64 = BASE64_URL_SAFE_NO_PAD.encode(&mac_tag_bytes); + + Ok(format!("{now_timestamp}.{mac_b64}")) + } + + /// Validates a CSRF token to check it is bound to this session and hasn't expired (is less than a week old). + pub fn validate_csrf_token(&self, token: &str, now: DateTime) -> eyre::Result<()> { + let (timestamp_str, mac_b64) = token + .split_once('.') + .context("CSRF token in wrong format")?; + let timestamp: i64 = timestamp_str + .parse() + .context("timestamp in CSRF token is wrong")?; + let mac_tag_bytes: Vec = BASE64_URL_SAFE_NO_PAD + .decode(mac_b64) + .context("failed to b64decode the MAC in CSRF token")?; + + let timestamp_8bytes = timestamp.to_be_bytes(); + + Blake2sMac256::new_with_salt_and_personal(&self.csrf_secret, &[], &[])? + .chain_update(×tamp_8bytes) + .verify_slice(&mac_tag_bytes) + .context("bad MAC in CSRF token")?; + + // At this point, the MAC is correct. All that's left is to check that the timestamp isn't too old. + + if now.signed_duration_since( + Utc.timestamp_opt(timestamp, 0) + .earliest() + .context("CSRF timestamp not valid")?, + ) > CSRF_TOKEN_EXPIRY_TIME + { + bail!("CSRF token expired."); + } + + Ok(()) + } } #[async_trait] @@ -21,25 +103,48 @@ where type Rejection = (StatusCode, &'static str); async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { - let cookie_val = - if let Ok(cookies) = TypedHeader::::from_request_parts(parts, state).await { - cookies.get("dummy").map(str::to_owned) - } else { - None - }; + let Ok(cookies) = TypedHeader::::from_request_parts(parts, state).await else { + return Err((StatusCode::UNAUTHORIZED, "No login session.")); + }; + let Some(cookie_val) = cookies.get("__Host-LoginSession").map(str::to_owned) else { + return Err((StatusCode::UNAUTHORIZED, "No login session.")); + }; + let Ok(login_session_token) = BASE64_URL_SAFE_NO_PAD.decode(&cookie_val) else { + return Err(( + StatusCode::UNAUTHORIZED, + "Invalid login session token." + )); + }; + if login_session_token.len() != LOGIN_SESSION_TOKEN_BYTES { + return Err((StatusCode::UNAUTHORIZED, "Invalid login session token.")); + } + let login_session_token_hash: [u8; LOGIN_SESSION_TOKEN_HASH_BYTES] = + Blake2s256::digest(&login_session_token).into(); - if cookie_val.is_some() { - Ok(LoginSession { - username: "dummy".to_owned(), + let db_store = Extension::>::from_request_parts(parts, state) + .await + .expect("no db store; this is a programming error"); + + match db_store + .txn(|mut txn| { + Box::pin(async move { txn.lookup_login_session(&login_session_token_hash).await }) }) + .await + { + Ok(Some(session)) => Ok(session), + Ok(None) => { + return Err((StatusCode::UNAUTHORIZED, "Invalid login session.")); + } + Err(err) => { + error!("failed to check login session: {err:?}"); + return Err(( + StatusCode::INTERNAL_SERVER_ERROR, + "A fault occurred when checking your login status. If the issue persists, please contact an administrator.", + )); + } + } // TODO do we want a middleware to renew the cookie? - } else { - Err(( - StatusCode::UNAUTHORIZED, - "Currently do not support login sessions", - )) - } } } @@ -59,39 +164,110 @@ pub async fn get_login( let csrf_token = csrf .authenticity_token() .expect("no reason a CSRF token should fail to be generated"); - (csrf, Html(format!("
", csrf_token))).into_response() + (csrf, Html(format!("
UN PW (temporary form)
", csrf_token))).into_response() } } } #[derive(Deserialize)] pub struct PostLoginForm { + username: String, + password: String, csrf: String, } +fn dummy_password_hash( + password: String, + password_hash_config: &PasswordHashingConfig, +) -> WebResult { + // TODO(security): concurrency-limited password hashing + let _hash = create_password_hash(&password, password_hash_config) + .context("unable to hash password!")?; + + Ok(render_login_retry_form()) +} + +fn render_login_retry_form() -> Response { + (StatusCode::UNAUTHORIZED, "Wrong username or password!").into_response() // TODO(ui): this should re-render the login form for another go +} + pub async fn post_login( Query(query): Query, csrf: CsrfToken, + Extension(store): Extension>, + Extension(config): Extension>, Form(form): Form, -) -> Response { +) -> WebResult { if csrf.verify(&form.csrf).is_err() { // Invalid CSRF token: try again - return get_login(None, Query(query), csrf).await; + return Ok(get_login(None, Query(query), csrf).await); } + + // retrieve user details + // N.B. it may be that there is no user returned in this step, but we need to + // do a password hash check anyway to avoid user enumeration problems. + let user_opt = store + .txn(|mut txn| Box::pin(async move { txn.lookup_user_by_name(form.username).await })) + .await + .context("failed to look up user for login")?; + + // verify credentials + let user = match user_opt { + Some(user) => match &user.password_hash { + Some(password_hash) => match check_hash(&form.password, password_hash) { + Ok(is_correct) => { + if is_correct { + user + } else { + return Ok(render_login_retry_form()); + } + } + Err(err) => { + error!("failed to check password hash: {err:?}"); + return dummy_password_hash(form.password, &config.password_hashing); + } + }, + None => { + return dummy_password_hash(form.password, &config.password_hashing); + } + }, + None => { + return dummy_password_hash(form.password, &config.password_hashing); + } + }; + + // Generate a login session token and store the hash in our database + let login_session_token = thread_rng().gen::<[u8; LOGIN_SESSION_TOKEN_BYTES]>(); + let login_session_token_b64 = BASE64_URL_SAFE_NO_PAD.encode(login_session_token); + let login_session_token_hash: [u8; LOGIN_SESSION_TOKEN_HASH_BYTES] = + Blake2s256::digest(&login_session_token).into(); + let csrf_secret = thread_rng().gen::<[u8; LOGIN_SESSION_CSRF_SECRET_BYTES]>(); + + // store session in the database + store + .txn(|mut txn| { + Box::pin(async move { + txn.create_login_session(&login_session_token_hash, user.user_id, &csrf_secret) + .await + }) + }) + .await + .context("failed to store session in database")?; + let expiry_date = chrono::Utc::now() + chrono::Duration::days(500); let expiry_date_rfc1123 = expiry_date.format("%a, %d %b %Y %H:%M:%S GMT"); - ( + Ok(( [( "Set-Cookie", HeaderValue::from_str(&format!( - "dummy=1; Path=/; HttpOnly; SameSite=Strict; Secure; Expires={}", - expiry_date_rfc1123 + "__Host-LoginSession={}; Path=/; HttpOnly; SameSite=Strict; Secure; Expires={}", + login_session_token_b64, expiry_date_rfc1123 )) .expect("no reason we should fail to make a cookie"), )], make_post_login_redirect(query.then), ) - .into_response() + .into_response()) } fn make_post_login_redirect(then: Option) -> Response { diff --git a/src/web/oauth_openid.rs b/src/web/oauth_openid.rs index 0994690..31c6104 100644 --- a/src/web/oauth_openid.rs +++ b/src/web/oauth_openid.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use axum::{http::StatusCode, response::IntoResponse, Extension, Json}; -use josekit::jwk::{Jwk, JwkSet}; +use josekit::jwk::Jwk; use serde::Serialize; use crate::config::{Configuration, SecretConfig}; diff --git a/src/web/oauth_openid/authorisation.rs b/src/web/oauth_openid/authorisation.rs index 65af42c..41ffdbf 100644 --- a/src/web/oauth_openid/authorisation.rs +++ b/src/web/oauth_openid/authorisation.rs @@ -214,7 +214,7 @@ async fn show_consent_page( csrf, Html(format!( "hi {}, consent to {}?
", - login_session.username, client_config.name, csrf_token + login_session.user_name, client_config.name, csrf_token )) ) .into_response()