From 4422b63dfdee7ddca5ac4b9ffb79a2bfc5dfbfd3 Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Thu, 1 Jul 2021 10:12:02 +0200
Subject: [PATCH] better CSRF handling in forms

---
 matrix-authentication-service/src/csrf.rs     | 42 ++++++++++++-------
 matrix-authentication-service/src/handlers.rs |  2 +-
 .../src/handlers/views.rs                     |  6 +--
 matrix-authentication-service/src/main.rs     |  3 +-
 matrix-authentication-service/src/state.rs    |  2 +-
 5 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/matrix-authentication-service/src/csrf.rs b/matrix-authentication-service/src/csrf.rs
index 2c07dd6ba..87d2b600c 100644
--- a/matrix-authentication-service/src/csrf.rs
+++ b/matrix-authentication-service/src/csrf.rs
@@ -3,6 +3,7 @@ use std::convert::TryInto;
 use async_trait::async_trait;
 use csrf::CsrfProtection;
 use data_encoding::BASE64;
+use serde::Deserialize;
 use tide::{http::Cookie, Middleware};
 use time::Duration;
 
@@ -43,23 +44,34 @@ impl Middleware<State> for HasCsrf {
     }
 }
 
-pub fn verify_csrf(request: &tide::Request<State>, token: &str) -> tide::Result<()> {
-    // Verify CSRF from body
-    let state = request.state();
-    let protection = state.csrf_protection();
+/// A CSRF-protected form
+#[derive(Deserialize)]
+pub struct CsrfForm<T> {
+    csrf: String,
 
-    let cookie = request
-        .cookie("csrf")
-        .ok_or_else(|| anyhow::anyhow!("missing csrf cookie"))?; // TODO: proper error
-    let cookie = BASE64.decode(cookie.value().as_bytes())?;
-    let cookie = protection.parse_cookie(&cookie)?;
+    #[serde(flatten)]
+    inner: T,
+}
 
-    let token = BASE64.decode(token.as_bytes())?;
-    let token = protection.parse_token(&token)?;
+impl<T> CsrfForm<T> {
+    pub fn verify_csrf(self, request: &tide::Request<State>) -> tide::Result<T> {
+        // Verify CSRF from body
+        let state = request.state();
+        let protection = state.csrf_protection();
 
-    if protection.verify_token_pair(&token, &cookie) {
-        Ok(())
-    } else {
-        Err(tide::Error::from_str(400, "failed CSRF validation"))
+        let cookie = request
+            .cookie("csrf")
+            .ok_or_else(|| anyhow::anyhow!("missing csrf cookie"))?; // TODO: proper error
+        let cookie = BASE64.decode(cookie.value().as_bytes())?;
+        let cookie = protection.parse_cookie(&cookie)?;
+
+        let token = BASE64.decode(self.csrf.as_bytes())?;
+        let token = protection.parse_token(&token)?;
+
+        if protection.verify_token_pair(&token, &cookie) {
+            Ok(self.inner)
+        } else {
+            Err(tide::Error::from_str(400, "failed CSRF validation"))
+        }
     }
 }
diff --git a/matrix-authentication-service/src/handlers.rs b/matrix-authentication-service/src/handlers.rs
index 5d81e5a82..6d14d7a9b 100644
--- a/matrix-authentication-service/src/handlers.rs
+++ b/matrix-authentication-service/src/handlers.rs
@@ -1,7 +1,7 @@
 use async_trait::async_trait;
 use serde::Deserialize;
 use thiserror::Error;
-use tide::{sessions::SessionMiddleware, Middleware, Redirect, Server};
+use tide::{Middleware, Redirect, Server};
 use url::Url;
 
 use crate::{
diff --git a/matrix-authentication-service/src/handlers/views.rs b/matrix-authentication-service/src/handlers/views.rs
index 8dabecda6..864733b36 100644
--- a/matrix-authentication-service/src/handlers/views.rs
+++ b/matrix-authentication-service/src/handlers/views.rs
@@ -1,12 +1,12 @@
 use serde::Deserialize;
 use tide::{Redirect, Request, Response};
 
+use crate::csrf::CsrfForm;
 use crate::state::State;
 use crate::templates::common_context;
 
 #[derive(Deserialize)]
 struct LoginForm {
-    csrf: String,
     username: String,
     password: String,
 }
@@ -36,8 +36,8 @@ pub async fn login(req: Request<State>) -> tide::Result {
 }
 
 pub async fn login_post(mut req: Request<State>) -> tide::Result {
-    let form: LoginForm = req.body_form().await?;
-    crate::csrf::verify_csrf(&req, &form.csrf)?;
+    let form: CsrfForm<LoginForm> = req.body_form().await?;
+    let form = form.verify_csrf(&req)?;
     let state = req.state();
 
     let user = state
diff --git a/matrix-authentication-service/src/main.rs b/matrix-authentication-service/src/main.rs
index d34f81712..12e6cc872 100644
--- a/matrix-authentication-service/src/main.rs
+++ b/matrix-authentication-service/src/main.rs
@@ -14,8 +14,7 @@ use self::state::State;
 async fn main() -> tide::Result<()> {
     // Setup logging & tracing
     let fmt_layer = tracing_subscriber::fmt::layer();
-    let filter_layer =
-        EnvFilter::try_from_default_env().or_else(|_| EnvFilter::try_new("debug"))?;
+    let filter_layer = EnvFilter::try_from_default_env().or_else(|_| EnvFilter::try_new("info"))?;
 
     let subscriber = Registry::default().with(filter_layer).with(fmt_layer);
     subscriber.try_init()?;
diff --git a/matrix-authentication-service/src/state.rs b/matrix-authentication-service/src/state.rs
index ebe893fef..12976b542 100644
--- a/matrix-authentication-service/src/state.rs
+++ b/matrix-authentication-service/src/state.rs
@@ -1,7 +1,7 @@
 use std::sync::Arc;
 
 use async_trait::async_trait;
-use csrf::{AesGcmCsrfProtection, CsrfProtection};
+use csrf::AesGcmCsrfProtection;
 use tera::Tera;
 use tide::{
     sessions::{MemoryStore, SessionMiddleware, SessionStore},