remove CSRF tokens (SameSite support is good enough)

author: Martin Fischer <martin@push-f.com> 2021-01-29 20:29:42 +0100
committer: Martin Fischer <martin@push-f.com> 2021-01-29 20:35:27 +0100
commit: c7d3bd087c49bdd0b33ed23ff583bf58ba705a1c (patch)
tree: c79624f5211438551583bec1638d2787d36277a2
parent: 10b5cd07f07cefd15450001375cd1c3e6927cc8a (diff)
5 files changed, 26 insertions, 146 deletions
diff --git a/Cargo.toml b/Cargo.toml
index 278de4e..fc4ba99 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -7,7 +7,7 @@ description = "A lightweight layer on top of hyper to facilitate building web ap
 repository = "https://git.push-f.com/sputnik"
 edition = "2018"
 categories = ["web-programming::http-server"]
-keywords = ["hyper", "web", "cookie", "csrf", "hmac"]
+keywords = ["hyper", "web", "cookie", "hmac"]
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
diff --git a/README.md b/README.md
index a3c0a71..9d61453 100644
--- a/README.md
+++ b/README.md
@@ -3,7 +3,7 @@
 A microframework based on [Hyper](https://hyper.rs/) providing traits to:
 
 * extend `http::request::Parts` with query parameter deserialization & cookie parsing
-* extend `hyper::Body` with form deserialization (and optional [CSRF](https://en.wikipedia.org/wiki/Cross-site_request_forgery) protection)
+* extend `hyper::Body` with form deserialization (and JSON deserialization with the `json` feature)
 * extend `http::response::Builder` with methods to set & delete cookies and set the Content-Type
 
 Furthermore Sputnik provides what's necessary to implement [signed & expiring
@@ -20,6 +20,12 @@ conversions for every error type, which you want to short-circuit with the `?`
 operator. This can be easily done with [thiserror](https://crates.io/crates/thiserror)
 because Sputnik restricts its error types to the `'static` lifetime.
 
+## Security Considerations
+
+Protect your application against [CSRF](https://en.wikipedia.org/wiki/Cross-site_request_forgery)
+by setting `SameSite` to `Lax` or `Strict` for your cookies and checking that the `Origin`
+header matches your domain name (especially if you have unauthenticated POST endpoints).
+
 ## Example
 
 ```rust
@@ -29,8 +35,8 @@ use hyper::{Method, Server, StatusCode, Body};
 use hyper::http::request::Parts;
 use hyper::http::response::Builder;
 use serde::Deserialize;
-use sputnik::{mime, request::{SputnikParts, SputnikBody, CsrfToken}, response::SputnikBuilder};
-use sputnik::request::CsrfProtectedFormError;
+use sputnik::{mime, request::{SputnikParts, SputnikBody}, response::SputnikBuilder};
+use sputnik::request::FormError;
 
 type Response = hyper::Response<Body>;
 
@@ -39,13 +45,13 @@ enum Error {
     #[error("page not found")]
     NotFound(String),
     #[error("{0}")]
-    CsrfError(#[from] CsrfProtectedFormError)
+    FormError(#[from] FormError)
 }
 
 fn render_error(err: Error) -> (StatusCode, String) {
     match err {
         Error::NotFound(msg) => (StatusCode::NOT_FOUND, msg),
-        Error::CsrfError(err) => (StatusCode::BAD_REQUEST, err.to_string()),
+        Error::FormError(err) => (StatusCode::BAD_REQUEST, err.to_string()),
     }
 }
 
@@ -57,22 +63,19 @@ async fn route(req: &mut Parts, body: Body) -> Result<Response, Error> {
     }
 }
 
-fn get_form(req: &mut Parts) -> Response {
+fn get_form(_req: &mut Parts) -> Response {
     Builder::new()
     .content_type(mime::TEXT_HTML)
     .body(
-        format!(
-            "<form method=post><input name=text>{}<button>Submit</button></form>",
-            CsrfToken::from_request(req).html_input()
-        ).into()
+        "<form method=post><input name=text> <button>Submit</button></form>".into()
     ).unwrap()
 }
 
 #[derive(Deserialize)]
 struct FormData {text: String}
 
-async fn post_form(req: &mut Parts, body: Body) -> Result<Response, Error> {
-    let msg: FormData = body.into_form_csrf(req).await?;
+async fn post_form(_req: &mut Parts, body: Body) -> Result<Response, Error> {
+    let msg: FormData = body.into_form().await?;
     Ok(Builder::new().body(
         format!("hello {}", msg.text).into()
     ).unwrap())
diff --git a/examples/csrf/Cargo.toml b/examples/form/Cargo.toml
index b6768ed..6f899df 100644
--- a/examples/csrf/Cargo.toml
+++ b/examples/form/Cargo.toml
@@ -1,5 +1,5 @@
 [package]
-name = "csrf"
+name = "sputnik-demo"
 version = "0.1.0"
 authors = ["Martin Fischer <martin@push-f.com>"]
 edition = "2018"
diff --git a/examples/csrf/src/main.rs b/examples/form/src/main.rs
index 53ea87f..e354f9f 100644
--- a/examples/csrf/src/main.rs
+++ b/examples/form/src/main.rs
@@ -4,8 +4,8 @@ use hyper::{Method, Server, StatusCode, Body};
 use hyper::http::request::Parts;
 use hyper::http::response::Builder;
 use serde::Deserialize;
-use sputnik::{mime, request::{SputnikParts, SputnikBody, CsrfToken}, response::SputnikBuilder};
-use sputnik::request::CsrfProtectedFormError;
+use sputnik::{mime, request::{SputnikParts, SputnikBody}, response::SputnikBuilder};
+use sputnik::request::FormError;
 
 type Response = hyper::Response<Body>;
 
@@ -14,13 +14,13 @@ enum Error {
     #[error("page not found")]
     NotFound(String),
     #[error("{0}")]
-    CsrfError(#[from] CsrfProtectedFormError)
+    FormError(#[from] FormError)
 }
 
 fn render_error(err: Error) -> (StatusCode, String) {
     match err {
         Error::NotFound(msg) => (StatusCode::NOT_FOUND, msg),
-        Error::CsrfError(err) => (StatusCode::BAD_REQUEST, err.to_string()),
+        Error::FormError(err) => (StatusCode::BAD_REQUEST, err.to_string()),
     }
 }
 
@@ -32,22 +32,19 @@ async fn route(req: &mut Parts, body: Body) -> Result<Response, Error> {
     }
 }
 
-fn get_form(req: &mut Parts) -> Response {
+fn get_form(_req: &mut Parts) -> Response {
     Builder::new()
     .content_type(mime::TEXT_HTML)
     .body(
-        format!(
-            "<form method=post><input name=text>{}<button>Submit</button></form>",
-            CsrfToken::from_request(req).html_input()
-        ).into()
+        "<form method=post><input name=text> <button>Submit</button></form>".into()
     ).unwrap()
 }
 
 #[derive(Deserialize)]
 struct FormData {text: String}
 
-async fn post_form(req: &mut Parts, body: Body) -> Result<Response, Error> {
-    let msg: FormData = body.into_form_csrf(req).await?;
+async fn post_form(_req: &mut Parts, body: Body) -> Result<Response, Error> {
+    let msg: FormData = body.into_form().await?;
     Ok(Builder::new().body(
         format!("hello {}", msg.text).into()
     ).unwrap())
diff --git a/src/request.rs b/src/request.rs
index d89248c..8059e07 100644
--- a/src/request.rs
+++ b/src/request.rs
@@ -2,17 +2,14 @@
 
 use cookie::Cookie;
 use mime::Mime;
-use serde::{Deserialize, de::DeserializeOwned};
+use serde::de::DeserializeOwned;
 use hyper::{HeaderMap, body::Bytes, header, http::request::Parts};
 use time::Duration;
 use std::{collections::HashMap, sync::Arc};
-use rand::{Rng, distributions::Alphanumeric};
 use async_trait::async_trait;
 
 use crate::response::{SputnikHeaders, delete_cookie};
 
-const CSRF_COOKIE_NAME : &str = "csrf";
-
 /// Adds convenience methods to [`http::request::Parts`](Parts).
 pub trait SputnikParts {
     /// Parses the query string of the request into a given struct.
@@ -79,16 +76,6 @@ impl SputnikParts for Parts {
     }
 }
 
-/// A cookie-based CSRF token to be used with [`SputnikBody::into_form_csrf`].
-#[derive(Clone)]
-pub struct CsrfToken(String);
-
-impl std::fmt::Display for CsrfToken {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0)
-    }
-}
-
 const FLASH_COOKIE_NAME: &str = "flash";
 
 /// Show the user a message after redirecting them.
@@ -150,75 +137,20 @@ impl Flash {
     }
 }
 
-impl CsrfToken {
-    /// Returns a CSRF token, either extracted from the `csrf` cookie or newly
-    /// generated if the cookie wasn't sent (in which case a set-cookie header is
-    /// appended to [`SputnikParts::response_headers`]).
-    ///
-    /// If there is no cookie, calling this method multiple times only generates
-    /// a new token on the first call, further calls return the previously
-    /// generated token.
-    pub fn from_request(req: &mut Parts) -> Self {
-        if let Some(token) = req.extensions.get::<CsrfToken>() {
-            return token.clone()
-        }
-        csrf_token_from_cookies(req)
-        .unwrap_or_else(|| {
-            let token: String = rand::thread_rng().sample_iter(
-                Alphanumeric
-                // must be HTML-safe because we embed it in CsrfToken::html_input
-            ).take(16).collect();
-            let mut c = Cookie::new(CSRF_COOKIE_NAME, token.clone());
-            c.set_secure(Some(true));
-            c.set_max_age(Some(Duration::hours(1)));
-
-            req.response_headers().set_cookie(c);
-            let token = CsrfToken(token);
-            req.extensions.insert(token.clone());
-            token
-        })
-    }
-
-    /// Returns a hidden HTML input to be embedded in forms that are received
-    /// with [`SputnikBody::into_form_csrf`].
-    pub fn html_input(&self) -> String {
-        format!("<input name=csrf type=hidden value=\"{}\">", self)
-    }
-}
-
 /// Adds deserialization methods to [`hyper::Body`].
 #[async_trait]
 pub trait SputnikBody {
     async fn into_bytes(self) -> Result<Bytes, BodyError>;
 
     /// Parses a `application/x-www-form-urlencoded` request body into a given struct.
-    ///
-    /// This does make you vulnerable to CSRF, so you normally want to use
-    /// [`SputnikBody::into_form_csrf()`] instead.
     async fn into_form<T: DeserializeOwned>(self) -> Result<T, FormError>;
 
-    /// Parses a `application/x-www-form-urlencoded` request body into a given struct.
-    /// Protects from CSRF by checking that the request body contains the same token retrieved from the cookies.
-    ///
-    /// The HTML form must embed a hidden input generated with [`CsrfToken::html_input`].
-    async fn into_form_csrf<T: DeserializeOwned>(self, req: &mut Parts) -> Result<T, CsrfProtectedFormError>;
-
     /// Attempts to deserialize the request body as JSON.
     #[cfg(feature = "json")]
     #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
     async fn into_json<T: DeserializeOwned>(self) -> Result<T, JsonError>;
 }
 
-fn csrf_token_from_cookies(req: &mut Parts) -> Option<CsrfToken> {
-    req.cookies()
-        .get(CSRF_COOKIE_NAME)
-        .map(|cookie| {
-            let token = CsrfToken(cookie.value().to_string());
-            req.extensions.insert(token.clone());
-            token
-        })
-}
-
 #[async_trait]
 impl SputnikBody for hyper::Body {
     async fn into_bytes(self) -> Result<Bytes, BodyError> {
@@ -230,19 +162,6 @@ impl SputnikBody for hyper::Body {
         Ok(serde_urlencoded::from_bytes::<T>(&full_body)?)
     }
 
-    async fn into_form_csrf<T: DeserializeOwned>(self, req: &mut Parts) -> Result<T, CsrfProtectedFormError> {
-        let full_body = self.into_bytes().await?;
-        let csrf_data = serde_urlencoded::from_bytes::<CsrfData>(&full_body).map_err(|_| CsrfProtectedFormError::NoCsrf)?;
-        match csrf_token_from_cookies(req) {
-            Some(token) => if token.to_string() == csrf_data.csrf {
-                Ok(serde_urlencoded::from_bytes::<T>(&full_body)?)
-            } else {
-                Err(CsrfProtectedFormError::Mismatch)
-            }
-            None => Err(CsrfProtectedFormError::NoCookie)
-        }
-    }
-
     #[cfg(feature = "json")]
     #[cfg_attr(docsrs, doc(cfg(feature = "json")))]
     async fn into_json<T: DeserializeOwned>(self) -> Result<T, JsonError> {
@@ -251,11 +170,6 @@ impl SputnikBody for hyper::Body {
     }
 }
 
-#[derive(Deserialize)]
-struct CsrfData {
-    csrf: String,
-}
-
 #[derive(thiserror::Error, Debug)]
 #[error("query deserialize error: {0}")]
 pub struct QueryError(pub serde_urlencoded::de::Error);
@@ -289,38 +203,4 @@ pub enum JsonError {
 
     #[error("json deserialize error: {0}")]
     Deserialize(#[from] serde_json::Error),
-}
-
-#[derive(thiserror::Error, Debug)]
-pub enum CsrfProtectedFormError {
-    #[error("{0}")]
-    Body(#[from] BodyError),
-
-    #[error("form deserialize error: {0}")]
-    Deserialize(#[from] serde_urlencoded::de::Error),
-
-    #[error("no csrf token in form data")]
-    NoCsrf,
-
-    #[error("no csrf cookie")]
-    NoCookie,
-
-    #[error("csrf parameter doesn't match csrf cookie")]
-    Mismatch,
-}
-
-#[cfg(test)]
-mod tests {
-    use hyper::Request;
-
-    use super::*;
-
-    #[test]
-    fn test_csrf_token() {
-        let mut parts = Request::new(hyper::Body::empty()).into_parts().0;
-        let tok1 = CsrfToken::from_request(&mut parts);
-        let tok2 = CsrfToken::from_request(&mut parts);
-        assert_eq!(tok1.to_string(), tok2.to_string());
-        assert_eq!(parts.response_headers().len(), 1);
-    }
 }
 \ No newline at end of file
author	Martin Fischer <martin@push-f.com>	2021-01-29 20:29:42 +0100
committer	Martin Fischer <martin@push-f.com>	2021-01-29 20:35:27 +0100
commit	c7d3bd087c49bdd0b33ed23ff583bf58ba705a1c (patch)
tree	c79624f5211438551583bec1638d2787d36277a2
parent	10b5cd07f07cefd15450001375cd1c3e6927cc8a (diff)