From 5a5dfe36b967849c6814ebf7a56c1e3bc62e9a39 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Sat, 20 Jan 2024 21:57:55 +0000 Subject: [PATCH] Use our own sessionless XSRF tokens for login screen This is simpler and more performant than the Argon2 solution --- Cargo.lock | 145 +++++----------------------------- Cargo.toml | 2 +- src/web.rs | 26 +----- src/web/login.rs | 18 ++--- src/web/oauth_openid/token.rs | 2 +- src/web/sessionless_xsrf.rs | 42 ++++++++++ 6 files changed, 73 insertions(+), 162 deletions(-) create mode 100644 src/web/sessionless_xsrf.rs diff --git a/Cargo.lock b/Cargo.lock index 2ba5f21..a603609 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,41 +17,6 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" -[[package]] -name = "aead" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d122413f284cf2d62fb1b7db97e02edb8cda96d769b16e443a4f6195e35662b0" -dependencies = [ - "crypto-common", - "generic-array", -] - -[[package]] -name = "aes" -version = "0.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac1f845298e95f983ff1944b728ae08b8cebab80d684f0a832ed0fc74dfa27e2" -dependencies = [ - "cfg-if", - "cipher", - "cpufeatures", -] - -[[package]] -name = "aes-gcm" -version = "0.10.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "831010a0f742e1209b3bcea8fab6a8e149051ba6099432c8cb2cc117dec3ead1" -dependencies = [ - "aead", - "aes", - "cipher", - "ctr", - "ghash", - "subtle", -] - [[package]] name = "ahash" version = "0.3.8" @@ -289,24 +254,6 @@ dependencies = [ "syn 2.0.38", ] -[[package]] -name = "axum_csrf" -version = "0.7.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "362fcef4c28a7834c5c2e0b021b5a538a5d16a5be8c543edbf9e18e1d2206115" -dependencies = [ - "argon2", - "async-trait", - "axum-core", - "cookie", - "http", - "rand", - "thiserror", - "time", - "tower-layer", - "tower-service", -] - [[package]] name = "backtrace" version = "0.3.69" @@ -545,16 +492,6 @@ dependencies = [ "windows-targets 0.48.5", ] -[[package]] -name = "cipher" -version = "0.4.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "773f3b9af64447d2ce9850330c473515014aa235e6a783b02db81ff39e4a3dad" -dependencies = [ - "crypto-common", - "inout", -] - [[package]] name = "clang-sys" version = "1.6.1" @@ -679,13 +616,7 @@ version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7efb37c3e1ccb1ff97164ad95ac1606e8ccd35b3fa0a7d99a304c7f4a428cc24" dependencies = [ - "aes-gcm", - "base64", - "hmac", "percent-encoding", - "rand", - "sha2", - "subtle", "time", "version_check", ] @@ -806,19 +737,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" dependencies = [ "generic-array", - "rand_core", "typenum", ] -[[package]] -name = "ctr" -version = "0.9.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0369ee1ad671834580515889b80f2ea915f23b8be8d0daa4bbaf2ac5c7590835" -dependencies = [ - "cipher", -] - [[package]] name = "dashmap" version = "4.0.2" @@ -1277,16 +1198,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "ghash" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d930750de5717d2dd0b8c0d42c076c0e884c81a73e6cab859bbd2339c71e3e40" -dependencies = [ - "opaque-debug", - "polyval", -] - [[package]] name = "gimli" version = "0.28.0" @@ -1591,7 +1502,6 @@ dependencies = [ "argon2", "async-trait", "axum", - "axum_csrf", "base64", "blake2", "chrono", @@ -1614,6 +1524,7 @@ dependencies = [ "subtle", "time", "tokio", + "tower-cookies", "tower-http", "tracing", "tracing-subscriber", @@ -1692,15 +1603,6 @@ dependencies = [ "libc", ] -[[package]] -name = "inout" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a0c10553d664a4d0bcff9f4215d0aac67a639cc68ef660840afe309b807bc9f5" -dependencies = [ - "generic-array", -] - [[package]] name = "instant" version = "0.1.12" @@ -2170,12 +2072,6 @@ version = "1.18.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dd8b5dd2ae5ed71462c540258bedcb51965123ad7e7ccf4b9a8cafaa4a63576d" -[[package]] -name = "opaque-debug" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "624a8340c38c1b80fd549087862da4ba43e08858af025b236e509b6649fc13d5" - [[package]] name = "openssl" version = "0.10.59" @@ -2423,18 +2319,6 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "22686f4785f02a4fcc856d3b3bb19bf6c8160d103f7a99cc258bddd0251dc7f2" -[[package]] -name = "polyval" -version = "0.6.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d52cff9d1d4dee5fe6d03729099f4a310a41179e0a10dbf542039873f2e826fb" -dependencies = [ - "cfg-if", - "cpufeatures", - "opaque-debug", - "universal-hash", -] - [[package]] name = "portable-atomic" version = "1.4.3" @@ -3423,6 +3307,23 @@ dependencies = [ "tracing", ] +[[package]] +name = "tower-cookies" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40f38d941a2ffd8402b36e02ae407637a9caceb693aaf2edc910437db0f36984" +dependencies = [ + "async-trait", + "axum-core", + "cookie", + "futures-util", + "http", + "parking_lot", + "pin-project-lite", + "tower-layer", + "tower-service", +] + [[package]] name = "tower-http" version = "0.4.4" @@ -3626,16 +3527,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39ec24b3121d976906ece63c9daad25b85969647682eee313cb5779fdd69e14e" -[[package]] -name = "universal-hash" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fc1de2c688dc15305988b563c3854064043356019f97a4b46276fe734c4f07ea" -dependencies = [ - "crypto-common", - "subtle", -] - [[package]] name = "untrusted" version = "0.9.0" diff --git a/Cargo.toml b/Cargo.toml index 252fa24..b4c9b7a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,6 @@ edition = "2021" argon2 = "0.5.2" 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" @@ -34,6 +33,7 @@ sqlx = { version = "0.7.2", features = ["postgres", "runtime-tokio-rustls", "mac subtle = "2.5.0" time = "0.3.30" tokio = { version = "1.33.0", features = ["rt", "macros"] } +tower-cookies = "0.9.0" tower-http = { version = "0.4.4", features = ["trace"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } diff --git a/src/web.rs b/src/web.rs index fd323e0..de582ae 100644 --- a/src/web.rs +++ b/src/web.rs @@ -7,10 +7,9 @@ use axum::{ routing::{get, post}, Extension, Router, }; -use axum_csrf::{CsrfConfig, CsrfLayer}; use eyre::Context; use hornbeam::{initialise_template_manager, make_template_manager}; -use rand::{distributions::Alphanumeric, thread_rng, Rng}; +use tower_cookies::CookieManagerLayer; use tower_http::trace::TraceLayer; use tracing::{error, info}; @@ -30,6 +29,7 @@ use crate::{ pub mod login; pub mod oauth_openid; +pub mod sessionless_xsrf; make_template_manager! { static ref TEMPLATING = { @@ -62,28 +62,8 @@ pub async fn serve( "/.well-known/openid-configuration", get(oidc_discovery_configuration), ) - .layer(CsrfLayer::new( - CsrfConfig::new() - .with_lifetime(time::Duration::days(1)) - .with_cookie_name("CSRF") - .with_cookie_len(64) - .with_cookie_path("/") - .with_http_only(true) - .with_cookie_same_site(axum_csrf::SameSite::Strict) - .with_secure(true) - .with_key(Some(axum_csrf::Key::generate())) - // the salt is unimportant as the CSRF token in the cookie is encrypted by the above key anyway - // even without that it wouldn't really matter: the only point of the CSRF cookie is to prevent someone (not the recipient) from guessing it - // but it's here, so do it anyway? - .with_salt( - thread_rng() - .sample_iter(&Alphanumeric) - .take(32) - .map(char::from) - .collect::(), - ), - )) .layer(TraceLayer::new_for_http()) + .layer(CookieManagerLayer::new()) .layer(Extension(config)) .layer(Extension(secrets)) .layer(Extension(store)) diff --git a/src/web/login.rs b/src/web/login.rs index 8b68356..0cabf58 100644 --- a/src/web/login.rs +++ b/src/web/login.rs @@ -8,7 +8,6 @@ use axum::{ response::{Html, IntoResponse, Response}, 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}; @@ -16,6 +15,7 @@ use eyre::{bail, Context, ContextCompat}; use rand::{thread_rng, Rng}; use serde::Deserialize; use sqlx::types::Uuid; +use tower_cookies::Cookies; use tracing::error; use crate::{ @@ -24,7 +24,7 @@ use crate::{ store::IdCoopStore, }; -use super::WebResult; +use super::{sessionless_xsrf, WebResult}; /// Size of the login token itself pub const LOGIN_SESSION_TOKEN_BYTES: usize = 32; @@ -157,15 +157,13 @@ pub struct LoginQuery { pub async fn get_login( current_session: Option, Query(query): Query, - csrf: CsrfToken, + cookies: Cookies, ) -> Response { match current_session { Some(_session) => make_post_login_redirect(query.then), None => { - let csrf_token = csrf - .authenticity_token() - .expect("no reason a CSRF token should fail to be generated"); - (csrf, Html(format!("
UN PW (temporary form)
", csrf_token))).into_response() + let csrf_token = sessionless_xsrf::get_token(&cookies); + Html(format!("
UN PW (temporary form)
", csrf_token)).into_response() } } } @@ -194,14 +192,14 @@ fn render_login_retry_form() -> Response { pub async fn post_login( Query(query): Query, - csrf: CsrfToken, + cookies: Cookies, Extension(store): Extension>, Extension(config): Extension>, Form(form): Form, ) -> WebResult { - if csrf.verify(&form.csrf).is_err() { + if !sessionless_xsrf::check_token(&cookies, &form.csrf) { // Invalid CSRF token: try again - return Ok(get_login(None, Query(query), csrf).await); + return Ok(get_login(None, Query(query), cookies).await); } // retrieve user details diff --git a/src/web/oauth_openid/token.rs b/src/web/oauth_openid/token.rs index 68e69e5..52cdc7c 100644 --- a/src/web/oauth_openid/token.rs +++ b/src/web/oauth_openid/token.rs @@ -9,7 +9,7 @@ use axum::{ }; use base64::{prelude::BASE64_URL_SAFE_NO_PAD, Engine}; use blake2::Blake2s256; -use chrono::{DateTime, Duration, Utc}; +use chrono::{Duration, Utc}; use eyre::{bail, Context}; use josekit::{ jws::{alg::rsassa::RsassaJwsAlgorithm::Rs256, JwsHeader}, diff --git a/src/web/sessionless_xsrf.rs b/src/web/sessionless_xsrf.rs new file mode 100644 index 0000000..c317ccb --- /dev/null +++ b/src/web/sessionless_xsrf.rs @@ -0,0 +1,42 @@ +//! Implements a simple anti-XSRF measure for when we have no login session active (see LoginSession for one to use if we DO have a login session). +//1 +//! Tnis implements a simple 'Double Submit Cookie' pattern. +//! +//! Because we use a `__Host-` prefixed cookie, this is not vulnerable to any cookie fixation attacks in modern browsers. +//! +//! Even on older browsers, that type of attack is rare, so this 'naïve' scheme is fine for the purpose (rather than having to do anything complicated). + +use base64::{prelude::BASE64_URL_SAFE_NO_PAD, Engine}; +use rand::{thread_rng, Rng}; +use subtle::ConstantTimeEq; +use time::Duration; +use tower_cookies::{Cookie, Cookies}; + +pub const COOKIE_NAME: &str = "__Host-SessionlessXsrf"; + +/// Gets the Sessionless XSRF token to put into a form request +pub fn get_token(cookies: &Cookies) -> String { + if let Some(xsrf_cookie) = cookies.get(COOKIE_NAME) { + xsrf_cookie.value().to_owned() + } else { + let new_token = thread_rng().gen::<[u8; 16]>(); + let new_token_b64 = BASE64_URL_SAFE_NO_PAD.encode(new_token); + cookies.add( + Cookie::build(COOKIE_NAME, new_token_b64.clone()) + .path("/") + .http_only(true) + .secure(true) + .same_site(tower_cookies::cookie::SameSite::Strict) + .max_age(Duration::days(500)) + .finish(), + ); + new_token_b64 + } +} + +/// Checks a Sessionless XSRF token obtained from a form request +pub fn check_token(cookies: &Cookies, token: &str) -> bool { + let Some(xsrf_token) = cookies.get(COOKIE_NAME) else { return false }; + + bool::from(xsrf_token.value().as_bytes().ct_eq(token.as_bytes())) +}