From 5e8a529374f75d15061d6fddc5ddcdd3961a8dfd Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Sat, 31 Jul 2021 14:47:54 +0200 Subject: [PATCH] better CSRF form handling --- matrix-authentication-service/src/csrf.rs | 34 ------------------- .../src/filters/csrf.rs | 34 ++++++++++++++++++- .../src/handlers/views/login.rs | 14 ++------ .../src/handlers/views/logout.rs | 24 +++---------- .../src/handlers/views/reauth.rs | 19 ++++------- matrix-authentication-service/src/main.rs | 1 - 6 files changed, 47 insertions(+), 79 deletions(-) delete mode 100644 matrix-authentication-service/src/csrf.rs diff --git a/matrix-authentication-service/src/csrf.rs b/matrix-authentication-service/src/csrf.rs deleted file mode 100644 index 866e91f68..000000000 --- a/matrix-authentication-service/src/csrf.rs +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2021 The Matrix.org Foundation C.I.C. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use serde::Deserialize; - -use crate::filters::CsrfToken; - -/// A CSRF-protected form -#[derive(Deserialize)] -pub struct CsrfForm { - csrf: String, - - #[serde(flatten)] - inner: T, -} - -impl CsrfForm { - pub fn verify_csrf(self, token: &CsrfToken) -> anyhow::Result { - // Verify CSRF from request - token.verify_form_value(&self.csrf)?; - Ok(self.inner) - } -} diff --git a/matrix-authentication-service/src/filters/csrf.rs b/matrix-authentication-service/src/filters/csrf.rs index b7f6f9cff..f67ade5b6 100644 --- a/matrix-authentication-service/src/filters/csrf.rs +++ b/matrix-authentication-service/src/filters/csrf.rs @@ -18,7 +18,7 @@ use chrono::{DateTime, Duration, Utc}; use data_encoding::BASE64URL_NOPAD; use headers::SetCookie; -use serde::{Deserialize, Serialize}; +use serde::{de::DeserializeOwned, Deserialize, Serialize}; use serde_with::{serde_as, TimestampSeconds}; use warp::{filters::BoxedFilter, Filter, Rejection, Reply}; @@ -78,6 +78,23 @@ impl CsrfToken { } } +/// A CSRF-protected form +#[derive(Deserialize)] +struct CsrfForm { + csrf: String, + + #[serde(flatten)] + inner: T, +} + +impl CsrfForm { + fn verify_csrf(self, token: &CsrfToken) -> anyhow::Result { + // Verify CSRF from request + token.verify_form_value(&self.csrf)?; + Ok(self.inner) + } +} + pub fn csrf_token(cookies_config: &CookiesConfig) -> BoxedFilter<(CsrfToken,)> { super::cookies::encrypted("csrf", cookies_config) .and_then(move |token: CsrfToken| async move { @@ -118,3 +135,18 @@ where { save_encrypted("csrf", cookies_config) } + +pub fn protected_form(cookies_config: &CookiesConfig) -> BoxedFilter<(T,)> +where + T: DeserializeOwned + Send + 'static, +{ + csrf_token(cookies_config) + .and(warp::body::form()) + .and_then( + |csrf_token: CsrfToken, protected_form: CsrfForm| async move { + let form = protected_form.verify_csrf(&csrf_token).wrap_error()?; + Ok::<_, Rejection>(form) + }, + ) + .boxed() +} diff --git a/matrix-authentication-service/src/handlers/views/login.rs b/matrix-authentication-service/src/handlers/views/login.rs index 6ee814903..cb3108d75 100644 --- a/matrix-authentication-service/src/handlers/views/login.rs +++ b/matrix-authentication-service/src/handlers/views/login.rs @@ -20,10 +20,9 @@ use warp::{ use crate::{ config::{CookiesConfig, CsrfConfig}, - csrf::CsrfForm, errors::WrapError, filters::{ - csrf::{csrf_token, save_csrf_token, updated_csrf_token}, + csrf::{protected_form, save_csrf_token, updated_csrf_token}, session::{save_session, with_optional_session}, with_pool, with_templates, CsrfToken, }, @@ -52,9 +51,8 @@ pub(super) fn filter( .with(wrap_fn(save_csrf_token(cookies_config))); let post = warp::post() - .and(csrf_token(cookies_config)) .and(with_pool(pool)) - .and(warp::body::form()) + .and(protected_form(cookies_config)) .and_then(post) .untuple_one() .with(wrap_fn(save_session(cookies_config))); @@ -81,13 +79,7 @@ async fn get( )) } -async fn post( - csrf_token: CsrfToken, - db: PgPool, - form: CsrfForm, -) -> Result<(SessionInfo, impl Reply), Rejection> { - let form = form.verify_csrf(&csrf_token).wrap_error()?; - +async fn post(db: PgPool, form: LoginForm) -> Result<(SessionInfo, impl Reply), Rejection> { let session_info = login(&db, &form.username, &form.password) .await .wrap_error()?; diff --git a/matrix-authentication-service/src/handlers/views/logout.rs b/matrix-authentication-service/src/handlers/views/logout.rs index a8b7b7ed8..040caf3af 100644 --- a/matrix-authentication-service/src/handlers/views/logout.rs +++ b/matrix-authentication-service/src/handlers/views/logout.rs @@ -13,40 +13,26 @@ // limitations under the License. use sqlx::PgPool; -use warp::{filters::BoxedFilter, hyper::Uri, wrap_fn, Filter, Rejection, Reply}; +use warp::{filters::BoxedFilter, hyper::Uri, Filter, Rejection, Reply}; use crate::{ config::CookiesConfig, - csrf::CsrfForm, errors::WrapError, - filters::{ - csrf::{csrf_token, save_csrf_token}, - session::with_session, - with_pool, CsrfToken, - }, + filters::{csrf::protected_form, session::with_session, with_pool}, storage::SessionInfo, }; pub(super) fn filter(pool: &PgPool, cookies_config: &CookiesConfig) -> BoxedFilter<(impl Reply,)> { warp::post() .and(warp::path("logout")) - .and(csrf_token(cookies_config)) .and(with_session(pool, cookies_config)) .and(with_pool(pool)) - .and(warp::body::form()) + .and(protected_form(cookies_config)) .and_then(post) - .untuple_one() - .with(wrap_fn(save_csrf_token(cookies_config))) .boxed() } -async fn post( - token: CsrfToken, - session: SessionInfo, - pool: PgPool, - form: CsrfForm<()>, -) -> Result<(CsrfToken, impl Reply), Rejection> { - form.verify_csrf(&token).wrap_error()?; +async fn post(session: SessionInfo, pool: PgPool, _form: ()) -> Result { session.end(&pool).await.wrap_error()?; - Ok::<_, Rejection>((token, warp::redirect(Uri::from_static("/login")))) + Ok::<_, Rejection>(warp::redirect(Uri::from_static("/login"))) } diff --git a/matrix-authentication-service/src/handlers/views/reauth.rs b/matrix-authentication-service/src/handlers/views/reauth.rs index 3d3637e39..0f2921989 100644 --- a/matrix-authentication-service/src/handlers/views/reauth.rs +++ b/matrix-authentication-service/src/handlers/views/reauth.rs @@ -20,10 +20,9 @@ use warp::{ use crate::{ config::{CookiesConfig, CsrfConfig}, - csrf::CsrfForm, errors::WrapError, filters::{ - csrf::{csrf_token, save_csrf_token, updated_csrf_token}, + csrf::{protected_form, save_csrf_token, updated_csrf_token}, session::with_session, with_pool, with_templates, CsrfToken, }, @@ -51,13 +50,10 @@ pub(super) fn filter( .with(wrap_fn(save_csrf_token(cookies_config))); let post = warp::post() - .and(csrf_token(cookies_config)) .and(with_session(pool, cookies_config)) .and(with_pool(pool)) - .and(warp::body::form()) - .and_then(post) - .untuple_one() - .with(wrap_fn(save_csrf_token(cookies_config))); + .and(protected_form(cookies_config)) + .and_then(post); warp::path("reauth").and(get.or(post)).boxed() } @@ -81,14 +77,11 @@ async fn get( } async fn post( - csrf_token: CsrfToken, session: SessionInfo, pool: PgPool, - form: CsrfForm, -) -> Result<(CsrfToken, impl Reply), Rejection> { - let form = form.verify_csrf(&csrf_token).wrap_error()?; - + form: ReauthForm, +) -> Result { let _session = session.reauth(&pool, &form.password).await.wrap_error()?; - Ok((csrf_token, warp::redirect(Uri::from_static("/")))) + Ok(warp::redirect(Uri::from_static("/"))) } diff --git a/matrix-authentication-service/src/main.rs b/matrix-authentication-service/src/main.rs index 8d9eb6564..c742e9ed6 100644 --- a/matrix-authentication-service/src/main.rs +++ b/matrix-authentication-service/src/main.rs @@ -24,7 +24,6 @@ use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt, EnvFilte mod cli; mod config; -mod csrf; mod errors; mod filters; mod handlers;