From a39f71c181b853d5d89809228918e4d43d39a5c5 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Thu, 24 Aug 2023 17:38:33 +0200 Subject: [PATCH] Handle cookies better by setting the right flags & expiration --- Cargo.lock | 1 - crates/axum-utils/Cargo.toml | 2 +- crates/axum-utils/src/cookies.rs | 131 +++++++++++++++--- crates/axum-utils/src/csrf.rs | 42 +++--- crates/axum-utils/src/lib.rs | 1 - crates/axum-utils/src/session.rs | 31 ++--- crates/cli/src/commands/server.rs | 10 +- crates/config/src/sections/secrets.rs | 2 +- crates/handlers/src/app_state.rs | 9 +- .../handlers/src/compat/login_sso_complete.rs | 7 +- crates/handlers/src/graphql/mod.rs | 8 +- crates/handlers/src/lib.rs | 6 +- .../src/oauth2/authorization/complete.rs | 7 +- .../handlers/src/oauth2/authorization/mod.rs | 7 +- crates/handlers/src/oauth2/consent.rs | 7 +- crates/handlers/src/test_utils.rs | 12 +- .../handlers/src/upstream_oauth2/authorize.rs | 6 +- .../handlers/src/upstream_oauth2/callback.rs | 5 +- crates/handlers/src/upstream_oauth2/cookie.rs | 34 ++--- crates/handlers/src/upstream_oauth2/link.rs | 7 +- .../handlers/src/views/account/emails/add.rs | 7 +- .../src/views/account/emails/verify.rs | 7 +- crates/handlers/src/views/account/password.rs | 9 +- crates/handlers/src/views/app.rs | 6 +- crates/handlers/src/views/index.rs | 6 +- crates/handlers/src/views/login.rs | 7 +- crates/handlers/src/views/logout.rs | 5 +- crates/handlers/src/views/reauth.rs | 7 +- crates/handlers/src/views/register.rs | 7 +- crates/keystore/Cargo.toml | 1 - crates/keystore/src/encrypter.rs | 12 +- 31 files changed, 242 insertions(+), 167 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88a7d044..c658fbc5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2909,7 +2909,6 @@ dependencies = [ "base64ct", "chacha20poly1305", "const-oid", - "cookie", "der", "ecdsa", "elliptic-curve", diff --git a/crates/axum-utils/Cargo.toml b/crates/axum-utils/Cargo.toml index dbe8830b..7c565a75 100644 --- a/crates/axum-utils/Cargo.toml +++ b/crates/axum-utils/Cargo.toml @@ -8,7 +8,7 @@ license = "Apache-2.0" [dependencies] async-trait = "0.1.73" axum = { version = "0.6.20", features = ["headers"] } -axum-extra = { version = "0.7.7", features = ["cookie-private"] } +axum-extra = { version = "0.7.7", features = ["cookie-private", "cookie-key-expansion"] } chrono.workspace = true data-encoding = "2.4.0" futures-util = "0.3.28" diff --git a/crates/axum-utils/src/cookies.rs b/crates/axum-utils/src/cookies.rs index c9132f41..afbea2b3 100644 --- a/crates/axum-utils/src/cookies.rs +++ b/crates/axum-utils/src/cookies.rs @@ -14,8 +14,18 @@ //! Private (encrypted) cookie jar, based on axum-extra's cookie jar +use std::convert::Infallible; + +use async_trait::async_trait; +use axum::{ + extract::{FromRef, FromRequestParts}, + response::{IntoResponseParts, ResponseParts}, +}; +use axum_extra::extract::cookie::{Cookie, Key, PrivateCookieJar, SameSite}; +use http::request::Parts; use serde::{de::DeserializeOwned, Serialize}; use thiserror::Error; +use url::Url; #[derive(Debug, Error)] #[error("could not decode cookie")] @@ -23,32 +33,113 @@ pub enum CookieDecodeError { Deserialize(#[from] serde_json::Error), } -pub trait CookieExt { - fn decode(&self) -> Result - where - T: DeserializeOwned; +/// Manages cookie options and encryption key +/// +/// This is meant to be accessible through axum's state via the [`FromRef`] +/// trait +#[derive(Clone)] +pub struct CookieManager { + options: CookieOption, + key: Key, +} + +impl CookieManager { + #[must_use] + pub const fn new(base_url: Url, key: Key) -> Self { + let options = CookieOption::new(base_url); + Self { options, key } + } #[must_use] - fn encode(self, t: &T) -> Self - where - T: Serialize; + pub fn derive_from(base_url: Url, key: &[u8]) -> Self { + let key = Key::derive_from(key); + Self::new(base_url, key) + } } -impl<'a> CookieExt for axum_extra::extract::cookie::Cookie<'a> { - fn decode(&self) -> Result - where - T: DeserializeOwned, - { - let decoded = serde_json::from_str(self.value())?; - Ok(decoded) +#[async_trait] +impl FromRequestParts for CookieJar +where + CookieManager: FromRef, + S: Send + Sync, +{ + type Rejection = Infallible; + + async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { + let cookie_manager = CookieManager::from_ref(state); + let inner = PrivateCookieJar::from_headers(&parts.headers, cookie_manager.key.clone()); + let options = cookie_manager.options.clone(); + + Ok(CookieJar { inner, options }) + } +} + +#[derive(Debug, Clone)] +struct CookieOption { + base_url: Url, +} + +impl CookieOption { + const fn new(base_url: Url) -> Self { + Self { base_url } } - fn encode(mut self, t: &T) -> Self - where - T: Serialize, - { - let encoded = serde_json::to_string(t).unwrap(); - self.set_value(encoded); + fn secure(&self) -> bool { + self.base_url.scheme() == "https" + } + + fn path(&self) -> &str { + self.base_url.path() + } + + fn apply<'a>(&self, mut cookie: Cookie<'a>) -> Cookie<'a> { + cookie.set_http_only(true); + cookie.set_secure(self.secure()); + cookie.set_path(self.path().to_owned()); + cookie.set_same_site(SameSite::Lax); + cookie + } +} + +/// A cookie jar which encrypts cookies & sets secure options +pub struct CookieJar { + inner: PrivateCookieJar, + options: CookieOption, +} + +impl CookieJar { + #[must_use] + pub fn save(mut self, key: &str, payload: &T, permanent: bool) -> Self { + let serialized = + serde_json::to_string(payload).expect("failed to serialize cookie payload"); + + let cookie = Cookie::new(key.to_owned(), serialized); + let mut cookie = self.options.apply(cookie); + + if permanent { + // XXX: this should use a clock + cookie.make_permanent(); + } + + self.inner = self.inner.add(cookie); + self } + + pub fn load(&self, key: &str) -> Result, CookieDecodeError> { + let Some(cookie) = self.inner.get(key) else { + return Ok(None); + }; + + let decoded = serde_json::from_str(cookie.value())?; + Ok(Some(decoded)) + } +} + +impl IntoResponseParts for CookieJar { + type Error = Infallible; + + fn into_response_parts(self, res: ResponseParts) -> Result { + self.inner.into_response_parts(res) + } } diff --git a/crates/axum-utils/src/csrf.rs b/crates/axum-utils/src/csrf.rs index 9886fedb..045d6db0 100644 --- a/crates/axum-utils/src/csrf.rs +++ b/crates/axum-utils/src/csrf.rs @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -use axum_extra::extract::cookie::{Cookie, PrivateCookieJar}; use chrono::{DateTime, Duration, Utc}; use data_encoding::{DecodeError, BASE64URL_NOPAD}; use mas_storage::Clock; @@ -21,7 +20,7 @@ use serde::{Deserialize, Serialize}; use serde_with::{serde_as, TimestampSeconds}; use thiserror::Error; -use crate::{cookies::CookieDecodeError, CookieExt}; +use crate::cookies::{CookieDecodeError, CookieJar}; /// Failed to validate CSRF token #[derive(Debug, Error)] @@ -118,36 +117,41 @@ pub trait CsrfExt { C: Clock; } -impl CsrfExt for PrivateCookieJar { +impl CsrfExt for CookieJar { fn csrf_token(self, clock: &C, rng: R) -> (CsrfToken, Self) where R: RngCore, C: Clock, { - let jar = self; - let mut cookie = jar.get("csrf").unwrap_or_else(|| Cookie::new("csrf", "")); - cookie.set_path("/"); - cookie.set_http_only(true); - let now = clock.now(); - let new_token = cookie - .decode() - .ok() - .and_then(|token: CsrfToken| token.verify_expiration(now).ok()) - .unwrap_or_else(|| CsrfToken::generate(now, rng, Duration::hours(1))) - .refresh(now, Duration::hours(1)); + let maybe_token = match self.load::("csrf") { + Ok(Some(token)) => { + let token = token.verify_expiration(now); - let cookie = cookie.encode(&new_token); - let jar = jar.add(cookie); - (new_token, jar) + // If the token is expired, just ignore it + token.ok() + } + Ok(None) => None, + Err(e) => { + tracing::warn!("Failed to decode CSRF cookie: {}", e); + None + } + }; + + let token = maybe_token.map_or_else( + || CsrfToken::generate(now, rng, Duration::hours(1)), + |token| token.refresh(now, Duration::hours(1)), + ); + + let jar = self.save("csrf", &token, false); + (token, jar) } fn verify_form(&self, clock: &C, form: ProtectedForm) -> Result where C: Clock, { - let cookie = self.get("csrf").ok_or(CsrfError::Missing)?; - let token: CsrfToken = cookie.decode()?; + let token: CsrfToken = self.load("csrf")?.ok_or(CsrfError::Missing)?; let token = token.verify_expiration(clock.now())?; token.verify_form_value(&form.csrf)?; Ok(form.inner) diff --git a/crates/axum-utils/src/lib.rs b/crates/axum-utils/src/lib.rs index 90fb6aba..799f66a4 100644 --- a/crates/axum-utils/src/lib.rs +++ b/crates/axum-utils/src/lib.rs @@ -34,7 +34,6 @@ pub mod user_authorization; pub use axum; pub use self::{ - cookies::CookieExt, fancy_error::FancyError, session::{SessionInfo, SessionInfoExt}, }; diff --git a/crates/axum-utils/src/session.rs b/crates/axum-utils/src/session.rs index 92f06329..a2c6d325 100644 --- a/crates/axum-utils/src/session.rs +++ b/crates/axum-utils/src/session.rs @@ -12,13 +12,12 @@ // See the License for the specific language governing permissions and // limitations under the License. -use axum_extra::extract::cookie::{Cookie, PrivateCookieJar}; use mas_data_model::BrowserSession; use mas_storage::{user::BrowserSessionRepository, RepositoryAccess}; use serde::{Deserialize, Serialize}; use ulid::Ulid; -use crate::CookieExt; +use crate::cookies::CookieJar; /// An encrypted cookie to save the session ID #[derive(Serialize, Deserialize, Debug, Default, Clone)] @@ -79,26 +78,22 @@ pub trait SessionInfoExt { } } -impl SessionInfoExt for PrivateCookieJar { +impl SessionInfoExt for CookieJar { fn session_info(self) -> (SessionInfo, Self) { - let jar = self; - let mut cookie = jar - .get("session") - .unwrap_or_else(|| Cookie::new("session", "")); - cookie.set_path("/"); - cookie.set_http_only(true); - let session_info = cookie.decode().unwrap_or_default(); + let info = match self.load("session") { + Ok(Some(s)) => s, + Ok(None) => SessionInfo::default(), + Err(e) => { + tracing::error!("failed to load session cookie: {}", e); + SessionInfo::default() + } + }; - let cookie = cookie.encode(&session_info); - let jar = jar.add(cookie); - (session_info, jar) + let jar = self.update_session_info(&info); + (info, jar) } fn update_session_info(self, info: &SessionInfo) -> Self { - let mut cookie = Cookie::new("session", ""); - cookie.set_path("/"); - cookie.set_http_only(true); - let cookie = cookie.encode(&info); - self.add(cookie) + self.save("session", info, true) } } diff --git a/crates/cli/src/commands/server.rs b/crates/cli/src/commands/server.rs index de91fa7e..45416ba2 100644 --- a/crates/cli/src/commands/server.rs +++ b/crates/cli/src/commands/server.rs @@ -18,7 +18,7 @@ use anyhow::Context; use clap::Parser; use itertools::Itertools; use mas_config::AppConfig; -use mas_handlers::{AppState, HttpClientFactory, MatrixHomeserver}; +use mas_handlers::{AppState, CookieManager, HttpClientFactory, MatrixHomeserver}; use mas_listener::{server::Server, shutdown::ShutdownStream}; use mas_matrix_synapse::SynapseConnection; use mas_router::UrlBuilder; @@ -52,6 +52,11 @@ impl Options { let span = info_span!("cli.run.init").entered(); let config: AppConfig = root.load_config()?; + // XXX: there should be a generic config verification step + if config.http.public_base.path() != "/" { + anyhow::bail!("The http.public_base path is not set to /, this is not supported"); + } + // Connect to the database info!("Connecting to the database"); let pool = database_from_config(&config.database).await?; @@ -73,6 +78,8 @@ impl Options { .context("could not import keys from config")?; let encrypter = config.secrets.encrypter(); + let cookie_manager = + CookieManager::derive_from(config.http.public_base.clone(), &config.secrets.encryption); // Load and compile the WASM policies (and fallback to the default embedded one) info!("Loading and compiling the policy module"); @@ -140,6 +147,7 @@ impl Options { pool, templates, key_store, + cookie_manager, encrypter, url_builder, homeserver, diff --git a/crates/config/src/sections/secrets.rs b/crates/config/src/sections/secrets.rs index 05eedee3..9e80216b 100644 --- a/crates/config/src/sections/secrets.rs +++ b/crates/config/src/sections/secrets.rs @@ -73,7 +73,7 @@ pub struct SecretsConfig { example = "example_secret" )] #[serde_as(as = "serde_with::hex::Hex")] - encryption: [u8; 32], + pub encryption: [u8; 32], /// List of private keys to use for signing and encrypting payloads #[serde(default)] diff --git a/crates/handlers/src/app_state.rs b/crates/handlers/src/app_state.rs index c71467dd..d9a8e96b 100644 --- a/crates/handlers/src/app_state.rs +++ b/crates/handlers/src/app_state.rs @@ -20,7 +20,7 @@ use axum::{ response::IntoResponse, }; use hyper::StatusCode; -use mas_axum_utils::http_client_factory::HttpClientFactory; +use mas_axum_utils::{cookies::CookieManager, http_client_factory::HttpClientFactory}; use mas_keystore::{Encrypter, Keystore}; use mas_policy::PolicyFactory; use mas_router::UrlBuilder; @@ -42,6 +42,7 @@ pub struct AppState { pub pool: PgPool, pub templates: Templates, pub key_store: Keystore, + pub cookie_manager: CookieManager, pub encrypter: Encrypter, pub url_builder: UrlBuilder, pub homeserver: MatrixHomeserver, @@ -161,6 +162,12 @@ impl FromRef for PasswordManager { } } +impl FromRef for CookieManager { + fn from_ref(input: &AppState) -> Self { + input.cookie_manager.clone() + } +} + #[async_trait] impl FromRequestParts for BoxClock { type Rejection = Infallible; diff --git a/crates/handlers/src/compat/login_sso_complete.rs b/crates/handlers/src/compat/login_sso_complete.rs index 099ab9ca..8ed24a00 100644 --- a/crates/handlers/src/compat/login_sso_complete.rs +++ b/crates/handlers/src/compat/login_sso_complete.rs @@ -20,14 +20,13 @@ use axum::{ extract::{Form, Path, Query, State}, response::{Html, IntoResponse, Redirect, Response}, }; -use axum_extra::extract::PrivateCookieJar; use chrono::Duration; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, FancyError, SessionInfoExt, }; use mas_data_model::Device; -use mas_keystore::Encrypter; use mas_router::{CompatLoginSsoAction, PostAuthAction, Route}; use mas_storage::{ compat::{CompatSessionRepository, CompatSsoLoginRepository}, @@ -63,7 +62,7 @@ pub async fn get( clock: BoxClock, mut repo: BoxRepository, State(templates): State, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(id): Path, Query(params): Query, ) -> Result { @@ -129,7 +128,7 @@ pub async fn post( clock: BoxClock, mut repo: BoxRepository, State(templates): State, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(id): Path, Query(params): Query, Form(form): Form>, diff --git a/crates/handlers/src/graphql/mod.rs b/crates/handlers/src/graphql/mod.rs index 74b4bd6d..1790366c 100644 --- a/crates/handlers/src/graphql/mod.rs +++ b/crates/handlers/src/graphql/mod.rs @@ -25,13 +25,11 @@ use axum::{ response::{Html, IntoResponse, Response}, Json, TypedHeader, }; -use axum_extra::extract::PrivateCookieJar; use futures_util::TryStreamExt; use headers::{authorization::Bearer, Authorization, ContentType, HeaderValue}; use hyper::header::CACHE_CONTROL; -use mas_axum_utils::{FancyError, SessionInfo, SessionInfoExt}; +use mas_axum_utils::{cookies::CookieJar, FancyError, SessionInfo, SessionInfoExt}; use mas_graphql::{Requester, Schema}; -use mas_keystore::Encrypter; use mas_matrix::HomeserverConnection; use mas_storage::{ BoxClock, BoxRepository, BoxRng, Clock, Repository, RepositoryError, SystemClock, @@ -228,7 +226,7 @@ pub async fn post( State(schema): State, clock: BoxClock, repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, content_type: Option>, authorization: Option>>, body: BodyStream, @@ -268,7 +266,7 @@ pub async fn get( State(schema): State, clock: BoxClock, repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, authorization: Option>>, RawQuery(query): RawQuery, ) -> Result { diff --git a/crates/handlers/src/lib.rs b/crates/handlers/src/lib.rs index 0a1d3034..db409d6e 100644 --- a/crates/handlers/src/lib.rs +++ b/crates/handlers/src/lib.rs @@ -47,7 +47,7 @@ use hyper::{ }, StatusCode, Version, }; -use mas_axum_utils::FancyError; +use mas_axum_utils::{cookies::CookieJar, FancyError}; use mas_http::CorsLayerExt; use mas_keystore::{Encrypter, Keystore}; use mas_policy::PolicyFactory; @@ -87,7 +87,7 @@ macro_rules! impl_from_error_for_route { }; } -pub use mas_axum_utils::http_client_factory::HttpClientFactory; +pub use mas_axum_utils::{cookies::CookieManager, http_client_factory::HttpClientFactory}; pub use self::{app_state::AppState, compat::MatrixHomeserver, graphql::schema as graphql_schema}; @@ -110,6 +110,7 @@ where BoxRepository: FromRequestParts, BoxClock: FromRequestParts, Encrypter: FromRef, + CookieJar: FromRequestParts, { let mut router = Router::new().route( "/graphql", @@ -267,6 +268,7 @@ where UrlBuilder: FromRef, Arc: FromRef, BoxRepository: FromRequestParts, + CookieJar: FromRequestParts, Encrypter: FromRef, Templates: FromRef, Keystore: FromRef, diff --git a/crates/handlers/src/oauth2/authorization/complete.rs b/crates/handlers/src/oauth2/authorization/complete.rs index 6a67cd14..687c1c64 100644 --- a/crates/handlers/src/oauth2/authorization/complete.rs +++ b/crates/handlers/src/oauth2/authorization/complete.rs @@ -18,11 +18,10 @@ use axum::{ extract::{Path, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; -use mas_axum_utils::{csrf::CsrfExt, SessionInfoExt}; +use mas_axum_utils::{cookies::CookieJar, csrf::CsrfExt, SessionInfoExt}; use mas_data_model::{AuthorizationGrant, BrowserSession, Client, Device}; -use mas_keystore::{Encrypter, Keystore}; +use mas_keystore::Keystore; use mas_policy::{EvaluationResult, PolicyFactory}; use mas_router::{PostAuthAction, Route, UrlBuilder}; use mas_storage::{ @@ -96,7 +95,7 @@ pub(crate) async fn get( State(url_builder): State, State(key_store): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(grant_id): Path, ) -> Result { let (session_info, cookie_jar) = cookie_jar.session_info(); diff --git a/crates/handlers/src/oauth2/authorization/mod.rs b/crates/handlers/src/oauth2/authorization/mod.rs index 1fd4b52c..ecc3af3d 100644 --- a/crates/handlers/src/oauth2/authorization/mod.rs +++ b/crates/handlers/src/oauth2/authorization/mod.rs @@ -18,11 +18,10 @@ use axum::{ extract::{Form, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; -use mas_axum_utils::{csrf::CsrfExt, SessionInfoExt}; +use mas_axum_utils::{cookies::CookieJar, csrf::CsrfExt, SessionInfoExt}; use mas_data_model::{AuthorizationCode, Pkce}; -use mas_keystore::{Encrypter, Keystore}; +use mas_keystore::Keystore; use mas_policy::PolicyFactory; use mas_router::{PostAuthAction, Route, UrlBuilder}; use mas_storage::{ @@ -146,7 +145,7 @@ pub(crate) async fn get( State(key_store): State, State(url_builder): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Form(params): Form, ) -> Result { // First, figure out what client it is diff --git a/crates/handlers/src/oauth2/consent.rs b/crates/handlers/src/oauth2/consent.rs index 5f8c9d23..916c3681 100644 --- a/crates/handlers/src/oauth2/consent.rs +++ b/crates/handlers/src/oauth2/consent.rs @@ -18,14 +18,13 @@ use axum::{ extract::{Form, Path, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, SessionInfoExt, }; use mas_data_model::{AuthorizationGrantStage, Device}; -use mas_keystore::Encrypter; use mas_policy::PolicyFactory; use mas_router::{PostAuthAction, Route}; use mas_storage::{ @@ -84,7 +83,7 @@ pub(crate) async fn get( State(policy_factory): State>, State(templates): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(grant_id): Path, ) -> Result { let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -149,7 +148,7 @@ pub(crate) async fn post( clock: BoxClock, State(policy_factory): State>, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(grant_id): Path, Form(form): Form>, ) -> Result { diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index 18510155..3575d81f 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -24,7 +24,7 @@ use axum::{ }; use headers::{Authorization, ContentType, HeaderMapExt, HeaderName}; use hyper::{header::CONTENT_TYPE, Request, Response, StatusCode}; -use mas_axum_utils::http_client_factory::HttpClientFactory; +use mas_axum_utils::{cookies::CookieManager, http_client_factory::HttpClientFactory}; use mas_keystore::{Encrypter, JsonWebKey, JsonWebKeySet, Keystore, PrivateKey}; use mas_matrix::{HomeserverConnection, MockHomeserverConnection}; use mas_policy::PolicyFactory; @@ -59,6 +59,7 @@ pub(crate) struct TestState { pub pool: PgPool, pub templates: Templates, pub key_store: Keystore, + pub cookie_manager: CookieManager, pub encrypter: Encrypter, pub url_builder: UrlBuilder, pub homeserver: MatrixHomeserver, @@ -95,6 +96,8 @@ impl TestState { let key_store = Keystore::new(jwks); let encrypter = Encrypter::new(&[0x42; 32]); + let cookie_manager = + CookieManager::derive_from("https://example.com".parse()?, &[0x42; 32]); let password_manager = PasswordManager::new([(1, Hasher::argon2id(None))])?; @@ -135,6 +138,7 @@ impl TestState { pool, templates, key_store, + cookie_manager, encrypter, url_builder, homeserver, @@ -317,6 +321,12 @@ impl FromRef for PasswordManager { } } +impl FromRef for CookieManager { + fn from_ref(input: &TestState) -> Self { + input.cookie_manager.clone() + } +} + #[async_trait] impl FromRequestParts for BoxClock { type Rejection = Infallible; diff --git a/crates/handlers/src/upstream_oauth2/authorize.rs b/crates/handlers/src/upstream_oauth2/authorize.rs index 392b47a0..83588a5d 100644 --- a/crates/handlers/src/upstream_oauth2/authorize.rs +++ b/crates/handlers/src/upstream_oauth2/authorize.rs @@ -16,10 +16,8 @@ use axum::{ extract::{Path, Query, State}, response::{IntoResponse, Redirect}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; -use mas_axum_utils::http_client_factory::HttpClientFactory; -use mas_keystore::Encrypter; +use mas_axum_utils::{cookies::CookieJar, http_client_factory::HttpClientFactory}; use mas_oidc_client::requests::authorization_code::AuthorizationRequestData; use mas_router::UrlBuilder; use mas_storage::{ @@ -68,7 +66,7 @@ pub(crate) async fn get( State(http_client_factory): State, mut repo: BoxRepository, State(url_builder): State, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(provider_id): Path, Query(query): Query, ) -> Result { diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index 0d28c3fc..4e1131fd 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -16,9 +16,8 @@ use axum::{ extract::{Path, Query, State}, response::IntoResponse, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; -use mas_axum_utils::http_client_factory::HttpClientFactory; +use mas_axum_utils::{cookies::CookieJar, http_client_factory::HttpClientFactory}; use mas_jose::claims::ClaimError; use mas_keystore::{Encrypter, Keystore}; use mas_oidc_client::requests::{ @@ -133,7 +132,7 @@ pub(crate) async fn get( State(url_builder): State, State(encrypter): State, State(keystore): State, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(provider_id): Path, Query(params): Query, ) -> Result { diff --git a/crates/handlers/src/upstream_oauth2/cookie.rs b/crates/handlers/src/upstream_oauth2/cookie.rs index 99e666f3..aa4a797c 100644 --- a/crates/handlers/src/upstream_oauth2/cookie.rs +++ b/crates/handlers/src/upstream_oauth2/cookie.rs @@ -14,14 +14,12 @@ // TODO: move that to a standalone cookie manager -use axum_extra::extract::{cookie::Cookie, PrivateCookieJar}; use chrono::{DateTime, Duration, NaiveDateTime, Utc}; -use mas_axum_utils::CookieExt; +use mas_axum_utils::cookies::CookieJar; use mas_router::PostAuthAction; use mas_storage::Clock; use serde::{Deserialize, Serialize}; use thiserror::Error; -use time::OffsetDateTime; use ulid::Ulid; /// Name of the cookie @@ -62,30 +60,24 @@ pub struct UpstreamSessionNotFound; impl UpstreamSessions { /// Load the upstreams sessions cookie - pub fn load(cookie_jar: &PrivateCookieJar) -> Self { - cookie_jar - .get(COOKIE_NAME) - .and_then(|c| c.decode().ok()) - .unwrap_or_default() + pub fn load(cookie_jar: &CookieJar) -> Self { + match cookie_jar.load(COOKIE_NAME) { + Ok(Some(sessions)) => sessions, + Ok(None) => Self::default(), + Err(e) => { + tracing::warn!("Invalid upstream sessions cookie: {}", e); + Self::default() + } + } } /// Save the upstreams sessions to the cookie jar - pub fn save(self, cookie_jar: PrivateCookieJar, clock: &C) -> PrivateCookieJar + pub fn save(self, cookie_jar: CookieJar, clock: &C) -> CookieJar where C: Clock, { - let now = clock.now(); - let this = self.expire(now); - let mut cookie = Cookie::named(COOKIE_NAME).encode(&this); - cookie.set_path("/"); - cookie.set_http_only(true); - - let expiration = now + Duration::seconds(SESSION_MAX_TIME_SECS); - let expiration = OffsetDateTime::from_unix_timestamp(expiration.timestamp()) - .expect("invalid unix timestamp"); - cookie.set_expires(expiration); - - cookie_jar.add(cookie) + let this = self.expire(clock.now()); + cookie_jar.save(COOKIE_NAME, &this, false) } fn expire(mut self, now: DateTime) -> Self { diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index 7d0c0da4..b2db2712 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -17,15 +17,14 @@ use axum::{ response::{Html, IntoResponse}, Form, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, SessionInfoExt, }; use mas_data_model::{UpstreamOAuthProviderImportPreference, User}; use mas_jose::jwt::Jwt; -use mas_keystore::Encrypter; use mas_storage::{ job::{JobRepositoryExt, ProvisionUserJob}, upstream_oauth2::{UpstreamOAuthLinkRepository, UpstreamOAuthSessionRepository}, @@ -170,7 +169,7 @@ pub(crate) async fn get( clock: BoxClock, mut repo: BoxRepository, State(templates): State, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(link_id): Path, ) -> Result { let sessions_cookie = UpstreamSessionsCookie::load(&cookie_jar); @@ -350,7 +349,7 @@ pub(crate) async fn post( mut rng: BoxRng, clock: BoxClock, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Path(link_id): Path, Form(form): Form>, ) -> Result { diff --git a/crates/handlers/src/views/account/emails/add.rs b/crates/handlers/src/views/account/emails/add.rs index 8afd3fd9..49036010 100644 --- a/crates/handlers/src/views/account/emails/add.rs +++ b/crates/handlers/src/views/account/emails/add.rs @@ -16,12 +16,11 @@ use axum::{ extract::{Form, Query, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, FancyError, SessionInfoExt, }; -use mas_keystore::Encrypter; use mas_router::Route; use mas_storage::{ job::{JobRepositoryExt, VerifyEmailJob}, @@ -44,7 +43,7 @@ pub(crate) async fn get( clock: BoxClock, State(templates): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -70,7 +69,7 @@ pub(crate) async fn post( mut rng: BoxRng, clock: BoxClock, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Query(query): Query, Form(form): Form>, ) -> Result { diff --git a/crates/handlers/src/views/account/emails/verify.rs b/crates/handlers/src/views/account/emails/verify.rs index 3a344440..44f48903 100644 --- a/crates/handlers/src/views/account/emails/verify.rs +++ b/crates/handlers/src/views/account/emails/verify.rs @@ -17,12 +17,11 @@ use axum::{ extract::{Form, Path, Query, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, FancyError, SessionInfoExt, }; -use mas_keystore::Encrypter; use mas_router::Route; use mas_storage::{ job::{JobRepositoryExt, ProvisionUserJob}, @@ -53,7 +52,7 @@ pub(crate) async fn get( mut repo: BoxRepository, Query(query): Query, Path(id): Path, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -96,7 +95,7 @@ pub(crate) async fn get( pub(crate) async fn post( clock: BoxClock, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Query(query): Query, Path(id): Path, Form(form): Form>, diff --git a/crates/handlers/src/views/account/password.rs b/crates/handlers/src/views/account/password.rs index 2628bd4b..4b2544a1 100644 --- a/crates/handlers/src/views/account/password.rs +++ b/crates/handlers/src/views/account/password.rs @@ -18,13 +18,12 @@ use axum::{ http::StatusCode, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, FancyError, SessionInfoExt, }; use mas_data_model::BrowserSession; -use mas_keystore::Encrypter; use mas_router::Route; use mas_storage::{ user::{BrowserSessionRepository, UserPasswordRepository}, @@ -51,7 +50,7 @@ pub(crate) async fn get( State(templates): State, State(password_manager): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { // If the password manager is disabled, we can go back to the account page. if !password_manager.is_enabled() { @@ -75,7 +74,7 @@ async fn render( clock: &impl Clock, templates: Templates, session: BrowserSession, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(clock, rng); @@ -95,7 +94,7 @@ pub(crate) async fn post( State(password_manager): State, State(templates): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Form(form): Form>, ) -> Result { if !password_manager.is_enabled() { diff --git a/crates/handlers/src/views/app.rs b/crates/handlers/src/views/app.rs index 5f5a2971..45507215 100644 --- a/crates/handlers/src/views/app.rs +++ b/crates/handlers/src/views/app.rs @@ -16,9 +16,7 @@ use axum::{ extract::State, response::{Html, IntoResponse}, }; -use axum_extra::extract::PrivateCookieJar; -use mas_axum_utils::{FancyError, SessionInfoExt}; -use mas_keystore::Encrypter; +use mas_axum_utils::{cookies::CookieJar, FancyError, SessionInfoExt}; use mas_router::{PostAuthAction, Route}; use mas_storage::BoxRepository; use mas_templates::{AppContext, Templates}; @@ -27,7 +25,7 @@ use mas_templates::{AppContext, Templates}; pub async fn get( State(templates): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (session_info, cookie_jar) = cookie_jar.session_info(); let session = session_info.load_session(&mut repo).await?; diff --git a/crates/handlers/src/views/index.rs b/crates/handlers/src/views/index.rs index 4cdb3182..a61bbc42 100644 --- a/crates/handlers/src/views/index.rs +++ b/crates/handlers/src/views/index.rs @@ -16,9 +16,7 @@ use axum::{ extract::State, response::{Html, IntoResponse}, }; -use axum_extra::extract::PrivateCookieJar; -use mas_axum_utils::{csrf::CsrfExt, FancyError, SessionInfoExt}; -use mas_keystore::Encrypter; +use mas_axum_utils::{cookies::CookieJar, csrf::CsrfExt, FancyError, SessionInfoExt}; use mas_router::UrlBuilder; use mas_storage::{BoxClock, BoxRepository, BoxRng}; use mas_templates::{IndexContext, TemplateContext, Templates}; @@ -30,7 +28,7 @@ pub async fn get( State(templates): State, State(url_builder): State, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); diff --git a/crates/handlers/src/views/login.rs b/crates/handlers/src/views/login.rs index f9168003..57dc46da 100644 --- a/crates/handlers/src/views/login.rs +++ b/crates/handlers/src/views/login.rs @@ -16,14 +16,13 @@ use axum::{ extract::{Form, Query, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, CsrfToken, ProtectedForm}, FancyError, SessionInfoExt, }; use mas_data_model::BrowserSession; -use mas_keystore::Encrypter; use mas_router::{Route, UpstreamOAuth2Authorize}; use mas_storage::{ upstream_oauth2::UpstreamOAuthProviderRepository, @@ -58,7 +57,7 @@ pub(crate) async fn get( State(templates): State, mut repo: BoxRepository, Query(query): Query, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -109,7 +108,7 @@ pub(crate) async fn post( State(templates): State, mut repo: BoxRepository, Query(query): Query, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Form(form): Form>, ) -> Result { if !password_manager.is_enabled() { diff --git a/crates/handlers/src/views/logout.rs b/crates/handlers/src/views/logout.rs index 35dc731e..0bc1c6cd 100644 --- a/crates/handlers/src/views/logout.rs +++ b/crates/handlers/src/views/logout.rs @@ -13,12 +13,11 @@ // limitations under the License. use axum::{extract::Form, response::IntoResponse}; -use axum_extra::extract::PrivateCookieJar; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, FancyError, SessionInfoExt, }; -use mas_keystore::Encrypter; use mas_router::{PostAuthAction, Route}; use mas_storage::{user::BrowserSessionRepository, BoxClock, BoxRepository}; @@ -26,7 +25,7 @@ use mas_storage::{user::BrowserSessionRepository, BoxClock, BoxRepository}; pub(crate) async fn post( clock: BoxClock, mut repo: BoxRepository, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Form(form): Form>>, ) -> Result { let form = cookie_jar.verify_form(&clock, form)?; diff --git a/crates/handlers/src/views/reauth.rs b/crates/handlers/src/views/reauth.rs index de5d990c..62ceeb5e 100644 --- a/crates/handlers/src/views/reauth.rs +++ b/crates/handlers/src/views/reauth.rs @@ -17,13 +17,12 @@ use axum::{ extract::{Form, Query, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, ProtectedForm}, FancyError, SessionInfoExt, }; -use mas_keystore::Encrypter; use mas_router::Route; use mas_storage::{ user::{BrowserSessionRepository, UserPasswordRepository}, @@ -49,7 +48,7 @@ pub(crate) async fn get( State(templates): State, mut repo: BoxRepository, Query(query): Query, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { if !password_manager.is_enabled() { // XXX: do something better here @@ -89,7 +88,7 @@ pub(crate) async fn post( State(password_manager): State, mut repo: BoxRepository, Query(query): Query, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Form(form): Form>, ) -> Result { if !password_manager.is_enabled() { diff --git a/crates/handlers/src/views/register.rs b/crates/handlers/src/views/register.rs index def96e97..cdab522c 100644 --- a/crates/handlers/src/views/register.rs +++ b/crates/handlers/src/views/register.rs @@ -18,14 +18,13 @@ use axum::{ extract::{Form, Query, State}, response::{Html, IntoResponse, Response}, }; -use axum_extra::extract::PrivateCookieJar; use hyper::StatusCode; use lettre::Address; use mas_axum_utils::{ + cookies::CookieJar, csrf::{CsrfExt, CsrfToken, ProtectedForm}, FancyError, SessionInfoExt, }; -use mas_keystore::Encrypter; use mas_policy::PolicyFactory; use mas_router::Route; use mas_storage::{ @@ -63,7 +62,7 @@ pub(crate) async fn get( State(password_manager): State, mut repo: BoxRepository, Query(query): Query, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, ) -> Result { let (csrf_token, cookie_jar) = cookie_jar.csrf_token(&clock, &mut rng); let (session_info, cookie_jar) = cookie_jar.session_info(); @@ -104,7 +103,7 @@ pub(crate) async fn post( State(templates): State, mut repo: BoxRepository, Query(query): Query, - cookie_jar: PrivateCookieJar, + cookie_jar: CookieJar, Form(form): Form>, ) -> Result { if !password_manager.is_enabled() { diff --git a/crates/keystore/Cargo.toml b/crates/keystore/Cargo.toml index d9834a40..64c171e1 100644 --- a/crates/keystore/Cargo.toml +++ b/crates/keystore/Cargo.toml @@ -8,7 +8,6 @@ license = "Apache-2.0" [dependencies] aead = { version = "0.5.2", features = ["std"] } const-oid = { version = "0.9.5", features = ["std"] } -cookie = { version = "0.17.0", features = ["key-expansion", "private"] } der = { version = "0.7.8", features = ["std"] } ecdsa = { version = "0.16.8", features = ["std"] } elliptic-curve = { version = "0.13.5", features = ["std", "pem", "sec1"] } diff --git a/crates/keystore/src/encrypter.rs b/crates/keystore/src/encrypter.rs index c57763de..da728cca 100644 --- a/crates/keystore/src/encrypter.rs +++ b/crates/keystore/src/encrypter.rs @@ -17,23 +17,15 @@ use std::sync::Arc; use aead::Aead; use base64ct::{Base64, Encoding}; use chacha20poly1305::{ChaCha20Poly1305, KeyInit}; -use cookie::Key; use generic_array::GenericArray; use thiserror::Error; /// Helps encrypting and decrypting data #[derive(Clone)] pub struct Encrypter { - cookie_key: Arc, aead: Arc, } -impl From for Key { - fn from(e: Encrypter) -> Self { - e.cookie_key.as_ref().clone() - } -} - #[derive(Debug, Error)] #[error("Decryption error")] pub enum DecryptError { @@ -46,12 +38,10 @@ impl Encrypter { /// Creates an [`Encrypter`] out of an encryption key #[must_use] pub fn new(key: &[u8; 32]) -> Self { - let cookie_key = Key::derive_from(&key[..]); - let cookie_key = Arc::new(cookie_key); let key = GenericArray::from_slice(key); let aead = ChaCha20Poly1305::new(key); let aead = Arc::new(aead); - Self { cookie_key, aead } + Self { aead } } /// Encrypt a payload