From 60fa0d5427c8f15a05a33502426924c4ad6a8051 Mon Sep 17 00:00:00 2001 From: Maciej Piechotka Date: Wed, 24 Apr 2019 12:49:56 -0700 Subject: [PATCH] Store visit and login timestamp in the identity cookie (#502) This allows to verify time of login or last visit and therfore limiting the danger of leaked cookies. --- src/middleware/identity.rs | 436 ++++++++++++++++++++++++++++++++----- 1 file changed, 381 insertions(+), 55 deletions(-) diff --git a/src/middleware/identity.rs b/src/middleware/identity.rs index ba03366fa..7a7604d67 100644 --- a/src/middleware/identity.rs +++ b/src/middleware/identity.rs @@ -49,10 +49,12 @@ //! ``` use std::cell::RefCell; use std::rc::Rc; +use std::time::SystemTime; use actix_service::{Service, Transform}; use futures::future::{ok, Either, FutureResult}; use futures::{Future, IntoFuture, Poll}; +use serde::{Deserialize, Serialize}; use time::Duration; use crate::cookie::{Cookie, CookieJar, Key, SameSite}; @@ -284,84 +286,133 @@ where struct CookieIdentityInner { key: Key, + key_v2: Key, name: String, path: String, domain: Option, secure: bool, max_age: Option, same_site: Option, + visit_deadline: Option, + login_deadline: Option, +} + +#[derive(Deserialize, Serialize, Debug)] +struct CookieValue { + identity: String, + #[serde(skip_serializing_if = "Option::is_none")] + login_timestamp: Option, + #[serde(skip_serializing_if = "Option::is_none")] + visit_timestamp: Option, +} + +#[derive(Debug)] +struct CookieIdentityExtention { + login_timestamp: Option } impl CookieIdentityInner { fn new(key: &[u8]) -> CookieIdentityInner { + let key_v2: Vec = key.iter().chain([1, 0, 0, 0].iter()).map(|e| *e).collect(); CookieIdentityInner { key: Key::from_master(key), + key_v2: Key::from_master(&key_v2), name: "actix-identity".to_owned(), path: "/".to_owned(), domain: None, secure: true, max_age: None, same_site: None, + visit_deadline: None, + login_deadline: None, } } fn set_cookie( &self, resp: &mut ServiceResponse, - id: Option, + value: Option, ) -> Result<()> { - let some = id.is_some(); - { - let id = id.unwrap_or_else(String::new); - let mut cookie = Cookie::new(self.name.clone(), id); - cookie.set_path(self.path.clone()); - cookie.set_secure(self.secure); - cookie.set_http_only(true); + let add_cookie = value.is_some(); + let val = value.map(|val| if !self.legacy_supported() { + serde_json::to_string(&val) + } else { + Ok(val.identity) + }); + let mut cookie = Cookie::new(self.name.clone(), val.unwrap_or_else(|| Ok(String::new()))?); + cookie.set_path(self.path.clone()); + cookie.set_secure(self.secure); + cookie.set_http_only(true); - if let Some(ref domain) = self.domain { - cookie.set_domain(domain.clone()); - } - - if let Some(max_age) = self.max_age { - cookie.set_max_age(max_age); - } - - if let Some(same_site) = self.same_site { - cookie.set_same_site(same_site); - } - - let mut jar = CookieJar::new(); - if some { - jar.private(&self.key).add(cookie); - } else { - jar.add_original(cookie.clone()); - jar.private(&self.key).remove(cookie); - } - - for cookie in jar.delta() { - let val = HeaderValue::from_str(&cookie.to_string())?; - resp.headers_mut().append(header::SET_COOKIE, val); - } + if let Some(ref domain) = self.domain { + cookie.set_domain(domain.clone()); } + if let Some(max_age) = self.max_age { + cookie.set_max_age(max_age); + } + + if let Some(same_site) = self.same_site { + cookie.set_same_site(same_site); + } + + let mut jar = CookieJar::new(); + let key = if self.legacy_supported() {&self.key} else {&self.key_v2}; + if add_cookie { + jar.private(&key).add(cookie); + } else { + jar.add_original(cookie.clone()); + jar.private(&key).remove(cookie); + } + for cookie in jar.delta() { + let val = HeaderValue::from_str(&cookie.to_string())?; + resp.headers_mut().append(header::SET_COOKIE, val); + } Ok(()) } - fn load(&self, req: &ServiceRequest) -> Option { - if let Ok(cookies) = req.cookies() { - for cookie in cookies.iter() { - if cookie.name() == self.name { - let mut jar = CookieJar::new(); - jar.add_original(cookie.clone()); + fn load(&self, req: &ServiceRequest) -> Option { + let cookie = req.cookie(&self.name)?; + let mut jar = CookieJar::new(); + jar.add_original(cookie.clone()); + let res = if self.legacy_supported() { + jar.private(&self.key).get(&self.name).map(|n| CookieValue { + identity: n.value().to_string(), + login_timestamp: None, + visit_timestamp: None + }) + } else { + None + }; + res.or_else(|| jar.private(&self.key_v2).get(&self.name).and_then(|c| self.parse(c))) + } - let cookie_opt = jar.private(&self.key).get(&self.name); - if let Some(cookie) = cookie_opt { - return Some(cookie.value().into()); - } - } + fn parse(&self, cookie: Cookie) -> Option { + let value: CookieValue = serde_json::from_str(cookie.value()).ok()?; + let now = SystemTime::now(); + if let Some(visit_deadline) = self.visit_deadline { + if now.duration_since(value.visit_timestamp?).ok()? > visit_deadline.to_std().ok()? { + return None; } } - None + if let Some(login_deadline) = self.login_deadline { + if now.duration_since(value.login_timestamp?).ok()? > login_deadline.to_std().ok()? { + return None; + } + } + Some(value) + } + + fn legacy_supported(&self) -> bool { + self.visit_deadline.is_none() && self.login_deadline.is_none() + } + + fn always_update_cookie(&self) -> bool { + self.visit_deadline.is_some() + } + + fn requires_oob_data(&self) -> bool { + self.login_deadline.is_some() } } @@ -443,6 +494,18 @@ impl CookieIdentityPolicy { Rc::get_mut(&mut self.0).unwrap().same_site = Some(same_site); self } + + /// Accepts only users whose cookie has been seen before the given deadline + pub fn visit_deadline(mut self, value: Duration) -> CookieIdentityPolicy { + Rc::get_mut(&mut self.0).unwrap().visit_deadline = Some(value); + self + } + + /// Accepts only users which has been authenticated before the given deadline + pub fn login_deadline(mut self, value: Duration) -> CookieIdentityPolicy { + Rc::get_mut(&mut self.0).unwrap().login_deadline = Some(value); + self + } } impl IdentityPolicy for CookieIdentityPolicy { @@ -450,7 +513,12 @@ impl IdentityPolicy for CookieIdentityPolicy { type ResponseFuture = Result<(), Error>; fn from_request(&self, req: &mut ServiceRequest) -> Self::Future { - Ok(self.0.load(req)) + Ok(self.0.load(req).map(|CookieValue {identity, login_timestamp, ..}| { + if self.0.requires_oob_data() { + req.extensions_mut().insert(CookieIdentityExtention { login_timestamp }); + } + identity + })) } fn to_response( @@ -459,9 +527,28 @@ impl IdentityPolicy for CookieIdentityPolicy { changed: bool, res: &mut ServiceResponse, ) -> Self::ResponseFuture { - if changed { - let _ = self.0.set_cookie(res, id); - } + let _ = if changed { + let login_timestamp = SystemTime::now(); + self.0.set_cookie(res, id.map(|identity| CookieValue { + identity, + login_timestamp: self.0.login_deadline.map(|_| login_timestamp), + visit_timestamp: self.0.visit_deadline.map(|_| login_timestamp) + })) + } else if self.0.always_update_cookie() && id.is_some() { + let visit_timestamp = SystemTime::now(); + let mut login_timestamp = None; + if self.0.requires_oob_data() { + let CookieIdentityExtention { login_timestamp: lt } = res.request().extensions_mut().remove().unwrap(); + login_timestamp = lt; + } + self.0.set_cookie(res, Some(CookieValue { + identity: id.unwrap(), + login_timestamp, + visit_timestamp: self.0.visit_deadline.map(|_| visit_timestamp) + })) + } else { + Ok(()) + }; Ok(()) } } @@ -473,14 +560,20 @@ mod tests { use crate::test::{self, TestRequest}; use crate::{web, App, HttpResponse}; + use std::borrow::Borrow; + + const COOKIE_KEY_MASTER: [u8; 32] = [0; 32]; + const COOKIE_NAME: &'static str = "actix_auth"; + const COOKIE_LOGIN: &'static str = "test"; + #[test] fn test_identity() { let mut srv = test::init_service( App::new() .wrap(IdentityService::new( - CookieIdentityPolicy::new(&[0; 32]) + CookieIdentityPolicy::new(&COOKIE_KEY_MASTER) .domain("www.rust-lang.org") - .name("actix_auth") + .name(COOKIE_NAME) .path("/") .secure(true), )) @@ -492,7 +585,7 @@ mod tests { } })) .service(web::resource("/login").to(|id: Identity| { - id.remember("test".to_string()); + id.remember(COOKIE_LOGIN.to_string()); HttpResponse::Ok() })) .service(web::resource("/logout").to(|id: Identity| { @@ -537,9 +630,9 @@ mod tests { let mut srv = test::init_service( App::new() .wrap(IdentityService::new( - CookieIdentityPolicy::new(&[0; 32]) + CookieIdentityPolicy::new(&COOKIE_KEY_MASTER) .domain("www.rust-lang.org") - .name("actix_auth") + .name(COOKIE_NAME) .path("/") .max_age_time(duration) .secure(true), @@ -563,9 +656,9 @@ mod tests { let mut srv = test::init_service( App::new() .wrap(IdentityService::new( - CookieIdentityPolicy::new(&[0; 32]) + CookieIdentityPolicy::new(&COOKIE_KEY_MASTER) .domain("www.rust-lang.org") - .name("actix_auth") + .name(COOKIE_NAME) .path("/") .max_age(seconds) .secure(true), @@ -582,4 +675,237 @@ mod tests { let c = resp.response().cookies().next().unwrap().to_owned(); assert_eq!(Duration::seconds(seconds as i64), c.max_age().unwrap()); } + + fn create_identity_server CookieIdentityPolicy + Sync + Send + Clone + 'static>(f: F) -> impl actix_service::Service, Error = actix_http::Error> { + test::init_service( + App::new() + .wrap(IdentityService::new(f(CookieIdentityPolicy::new(&COOKIE_KEY_MASTER).secure(false).name(COOKIE_NAME)))) + .service(web::resource("/").to(|id: Identity| { + let identity = id.identity(); + if identity.is_none() { + id.remember(COOKIE_LOGIN.to_string()) + } + web::Json(identity) + })) + ) + } + + fn legacy_login_cookie(identity: &'static str) -> Cookie<'static> { + let mut jar = CookieJar::new(); + jar.private(&Key::from_master(&COOKIE_KEY_MASTER)).add(Cookie::new(COOKIE_NAME, identity)); + jar.get(COOKIE_NAME).unwrap().clone() + } + + fn login_cookie(identity: &'static str, login_timestamp: Option, visit_timestamp: Option) -> Cookie<'static> { + let mut jar = CookieJar::new(); + let key: Vec = COOKIE_KEY_MASTER.iter().chain([1, 0, 0, 0].iter()).map(|e| *e).collect(); + jar.private(&Key::from_master(&key)).add(Cookie::new(COOKIE_NAME, serde_json::to_string(&CookieValue { + identity: identity.to_string(), + login_timestamp, + visit_timestamp + }).unwrap())); + jar.get(COOKIE_NAME).unwrap().clone() + } + + fn assert_logged_in(response: &mut ServiceResponse, identity: Option<&str>) { + use bytes::BytesMut; + use futures::Stream; + let bytes = + test::block_on(response.take_body().fold(BytesMut::new(), |mut b, c| { + b.extend(c); + Ok::<_, Error>(b) + })) + .unwrap(); + let resp: Option = serde_json::from_slice(&bytes[..]).unwrap(); + assert_eq!(resp.as_ref().map(|s| s.borrow()), identity); + } + + fn assert_legacy_login_cookie(response: &mut ServiceResponse, identity: &str) { + let mut cookies = CookieJar::new(); + for cookie in response.headers().get_all(header::SET_COOKIE) { + cookies.add(Cookie::parse(cookie.to_str().unwrap().to_string()).unwrap()); + } + let cookie = cookies.private(&Key::from_master(&COOKIE_KEY_MASTER)).get(COOKIE_NAME).unwrap(); + assert_eq!(cookie.value(), identity); + } + + enum LoginTimestampCheck { + NoTimestamp, + NewTimestamp, + OldTimestamp(SystemTime) + } + + enum VisitTimeStampCheck { + NoTimestamp, + NewTimestamp + } + + fn assert_login_cookie(response: &mut ServiceResponse, identity: &str, login_timestamp: LoginTimestampCheck, visit_timestamp: VisitTimeStampCheck) { + let mut cookies = CookieJar::new(); + for cookie in response.headers().get_all(header::SET_COOKIE) { + cookies.add(Cookie::parse(cookie.to_str().unwrap().to_string()).unwrap()); + } + let key: Vec = COOKIE_KEY_MASTER.iter().chain([1, 0, 0, 0].iter()).map(|e| *e).collect(); + let cookie = cookies.private(&Key::from_master(&key)).get(COOKIE_NAME).unwrap(); + let cv: CookieValue = serde_json::from_str(cookie.value()).unwrap(); + assert_eq!(cv.identity, identity); + let now = SystemTime::now(); + let t30sec_ago = now - Duration::seconds(30).to_std().unwrap(); + match login_timestamp { + LoginTimestampCheck::NoTimestamp => assert_eq!(cv.login_timestamp, None), + LoginTimestampCheck::NewTimestamp => assert!(t30sec_ago <= cv.login_timestamp.unwrap() && cv.login_timestamp.unwrap() <= now), + LoginTimestampCheck::OldTimestamp(old_timestamp) => assert_eq!(cv.login_timestamp, Some(old_timestamp)) + } + match visit_timestamp { + VisitTimeStampCheck::NoTimestamp => assert_eq!(cv.visit_timestamp, None), + VisitTimeStampCheck::NewTimestamp => assert!(t30sec_ago <= cv.visit_timestamp.unwrap() && cv.visit_timestamp.unwrap() <= now) + } + } + + fn assert_no_login_cookie(response: &mut ServiceResponse) { + let mut cookies = CookieJar::new(); + for cookie in response.headers().get_all(header::SET_COOKIE) { + cookies.add(Cookie::parse(cookie.to_str().unwrap().to_string()).unwrap()); + } + assert!(cookies.get(COOKIE_NAME).is_none()); + } + + #[test] + fn test_identity_legacy_cookie_is_set() { + let mut srv = create_identity_server(|c| c); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_legacy_login_cookie(&mut resp, COOKIE_LOGIN); + } + + #[test] + fn test_identity_legacy_cookie_works() { + let mut srv = create_identity_server(|c| c); + let cookie = legacy_login_cookie(COOKIE_LOGIN); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, Some(COOKIE_LOGIN)); + assert_no_login_cookie(&mut resp); + } + + #[test] + fn test_identity_legacy_cookie_rejected_if_visit_timestamp_needed() { + let mut srv = create_identity_server(|c| c.visit_deadline(Duration::days(90))); + let cookie = legacy_login_cookie(COOKIE_LOGIN); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::NoTimestamp, VisitTimeStampCheck::NewTimestamp); + } + + #[test] + fn test_identity_legacy_cookie_rejected_if_login_timestamp_needed() { + let mut srv = create_identity_server(|c| c.login_deadline(Duration::days(90))); + let cookie = legacy_login_cookie(COOKIE_LOGIN); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::NewTimestamp, VisitTimeStampCheck::NoTimestamp); + } + + #[test] + fn test_identity_cookie_rejected_if_login_timestamp_needed() { + let mut srv = create_identity_server(|c| c.login_deadline(Duration::days(90))); + let cookie = login_cookie(COOKIE_LOGIN, None, Some(SystemTime::now())); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::NewTimestamp, VisitTimeStampCheck::NoTimestamp); + } + + #[test] + fn test_identity_cookie_rejected_if_visit_timestamp_needed() { + let mut srv = create_identity_server(|c| c.visit_deadline(Duration::days(90))); + let cookie = login_cookie(COOKIE_LOGIN, Some(SystemTime::now()), None); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::NoTimestamp, VisitTimeStampCheck::NewTimestamp); + } + + #[test] + fn test_identity_cookie_rejected_if_login_timestamp_too_old() { + let mut srv = create_identity_server(|c| c.login_deadline(Duration::days(90))); + let cookie = login_cookie(COOKIE_LOGIN, Some(SystemTime::now() - Duration::days(180).to_std().unwrap()), None); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::NewTimestamp, VisitTimeStampCheck::NoTimestamp); + } + + #[test] + fn test_identity_cookie_rejected_if_visit_timestamp_too_old() { + let mut srv = create_identity_server(|c| c.visit_deadline(Duration::days(90))); + let cookie = login_cookie(COOKIE_LOGIN, None, Some(SystemTime::now() - Duration::days(180).to_std().unwrap())); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, None); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::NoTimestamp, VisitTimeStampCheck::NewTimestamp); + } + + #[test] + fn test_identity_cookie_not_updated_on_login_deadline() { + let mut srv = create_identity_server(|c| c.login_deadline(Duration::days(90))); + let cookie = login_cookie(COOKIE_LOGIN, Some(SystemTime::now()), None); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, Some(COOKIE_LOGIN)); + assert_no_login_cookie(&mut resp); + } + + #[test] + fn test_identity_cookie_updated_on_visit_deadline() { + let mut srv = create_identity_server(|c| c.visit_deadline(Duration::days(90)).login_deadline(Duration::days(90))); + let timestamp = SystemTime::now() - Duration::days(1).to_std().unwrap(); + let cookie = login_cookie(COOKIE_LOGIN, Some(timestamp), Some(timestamp)); + let mut resp = test::call_service( + &mut srv, + TestRequest::with_uri("/") + .cookie(cookie.clone()) + .to_request() + ); + assert_logged_in(&mut resp, Some(COOKIE_LOGIN)); + assert_login_cookie(&mut resp, COOKIE_LOGIN, LoginTimestampCheck::OldTimestamp(timestamp), VisitTimeStampCheck::NewTimestamp); + } }