Merge pull request #438 from sebadob/feat-global-cookie-encryption

Feat: global cookie encryption
sebadob · May 17, 2024 · 4fdb3f2 · 4fdb3f2
2 parents 4454bec + fd1fc40
commit 4fdb3f2
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 28 deletions.
diff --git a/rauthy-handlers/src/middleware/principal.rs b/rauthy-handlers/src/middleware/principal.rs
@@ -122,7 +122,7 @@ async fn get_session_from_cookie(
         None => {
             return Ok(None);
         }
-        Some(cookie) => cookie.value().to_string(),
+        Some(session_id) => session_id,
     };
 
     match Session::find(data, session_id).await {

diff --git a/rauthy-handlers/src/users.rs b/rauthy-handlers/src/users.rs
@@ -750,7 +750,7 @@ pub async fn post_webauthn_auth_start(
                     ));
                 }
                 Some(c) => {
-                    if c.value().len() != 48 {
+                    if c.len() != 48 {
                         return Err(ErrorResponse::new(
                             ErrorResponseType::BadRequest,
                             "Malformed magic link binding cookie".to_string(),

diff --git a/rauthy-models/src/api_cookie.rs b/rauthy-models/src/api_cookie.rs
@@ -1,18 +1,21 @@
 use actix_web::cookie::{Cookie, SameSite};
 use actix_web::dev::ServiceRequest;
 use actix_web::{cookie, HttpRequest};
+use cryptr::EncValue;
 use rauthy_common::constants::{CookieMode, COOKIE_MODE, COOKIE_SET_PATH};
+use rauthy_common::utils::{base64_decode, base64_encode};
+use semver::Op;
 use std::borrow::Cow;
 use std::fmt::Display;
 use tracing::warn;
 
 pub struct ApiCookie;
 
 impl ApiCookie {
-    pub fn build<'c, N, V>(name: N, value: V, max_age: i64) -> Cookie<'c>
+    pub fn build<'c, 'b, N, V>(name: N, value: V, max_age: i64) -> Cookie<'c>
     where
         N: Into<Cow<'c, str>> + Display,
-        V: Into<Cow<'c, str>> + Display,
+        V: Into<Cow<'b, str>> + Display,
     {
         let path = if *COOKIE_SET_PATH { "/auth" } else { "/" };
         let (name, secure, path) = match *COOKIE_MODE {
@@ -29,7 +32,12 @@ impl ApiCookie {
             cookie::time::Duration::seconds(max_age)
         };
 
-        Cookie::build(name, value)
+        // we always encrypt any cookie value
+        let enc =
+            EncValue::encrypt(value.into().as_bytes()).expect("ENC_VALUES not set up correctly");
+        let value_b64 = base64_encode(enc.into_bytes().as_ref());
+
+        Cookie::build(name, value_b64)
             .secure(secure)
             .http_only(true)
             .same_site(SameSite::Lax)
@@ -38,7 +46,7 @@ impl ApiCookie {
             .finish()
     }
 
-    pub fn from_req<'c, N>(req: &HttpRequest, cookie_name: N) -> Option<Cookie<'c>>
+    pub fn from_req<'c, N>(req: &HttpRequest, cookie_name: N) -> Option<String>
     where
         N: Into<Cow<'c, str>> + Display,
     {
@@ -47,10 +55,11 @@ impl ApiCookie {
             CookieMode::Secure => format!("__Secure-{}", cookie_name),
             CookieMode::DangerInsecure => cookie_name.to_string(),
         };
-        req.cookie(&name)
+        // req.cookie(&name)
+        Self::cookie_into_value(req.cookie(&name))
     }
 
-    pub fn from_svc_req<'c, N>(req: &ServiceRequest, cookie_name: N) -> Option<Cookie<'c>>
+    pub fn from_svc_req<'c, N>(req: &ServiceRequest, cookie_name: N) -> Option<String>
     where
         N: Into<Cow<'c, str>> + Display,
     {
@@ -59,6 +68,38 @@ impl ApiCookie {
             CookieMode::Secure => format!("__Secure-{}", cookie_name),
             CookieMode::DangerInsecure => cookie_name.to_string(),
         };
-        req.cookie(&name)
+        Self::cookie_into_value(req.cookie(&name))
+    }
+
+    pub fn cookie_into_value(cookie: Option<Cookie>) -> Option<String> {
+        match cookie {
+            None => None,
+            Some(cookie) => {
+                let val = cookie.value();
+                let bytes = base64_decode(val).ok()?;
+                let enc = EncValue::try_from(bytes).ok()?;
+                let dec = enc.decrypt().ok()?;
+                Some(String::from_utf8_lossy(dec.as_ref()).to_string())
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use cryptr::EncKeys;
+
+    #[test]
+    fn test_api_cookie() {
+        dotenvy::from_filename("rauthy.test.cfg").ok();
+        let _ = EncKeys::from_env().unwrap().init();
+
+        let val = "my_cookie_val_1337";
+
+        let cookie = ApiCookie::build("myCookie", val, 10);
+        let val_from = ApiCookie::cookie_into_value(Some(cookie)).unwrap();
+
+        assert_eq!(val, val_from);
     }
 }
diff --git a/rauthy-models/src/entity/auth_providers.rs b/rauthy-models/src/entity/auth_providers.rs
@@ -117,22 +117,20 @@ pub struct AuthProviderLinkCookie {
     pub user_email: String,
 }
 
-impl TryFrom<Cookie<'_>> for AuthProviderLinkCookie {
+impl TryFrom<&str> for AuthProviderLinkCookie {
     type Error = ErrorResponse;
 
-    fn try_from(value: Cookie) -> Result<AuthProviderLinkCookie, Self::Error> {
-        let bytes_enc = base64_url_decode(value.value())?;
-        let dec = EncValue::try_from(bytes_enc)?.decrypt()?;
-        let slf = bincode::deserialize::<AuthProviderLinkCookie>(dec.as_ref())?;
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        let bytes = base64_decode(value)?;
+        let slf = bincode::deserialize::<AuthProviderLinkCookie>(bytes.as_slice())?;
         Ok(slf)
     }
 }
 
 impl AuthProviderLinkCookie {
     pub fn build_cookie(&self) -> Result<Cookie<'_>, ErrorResponse> {
         let bytes = bincode::serialize(self)?;
-        let secret_enc = EncValue::encrypt(bytes.as_ref())?.into_bytes().to_vec();
-        let value = base64_url_encode(&secret_enc);
+        let value = base64_encode(&bytes);
         Ok(ApiCookie::build(PROVIDER_LINK_COOKIE, value, 300))
     }
 
@@ -677,11 +675,9 @@ impl AuthProviderCallback {
             .expect("write to always succeed");
         }
 
-        let id_enc = EncValue::encrypt(slf.callback_id.as_bytes())?;
-        let id_b64 = base64_encode(id_enc.into_bytes().as_ref());
         let cookie = ApiCookie::build(
             COOKIE_UPSTREAM_CALLBACK,
-            id_b64,
+            &slf.callback_id,
             UPSTREAM_AUTH_CALLBACK_TIMEOUT_SECS as i64,
         );
 
@@ -702,16 +698,12 @@ impl AuthProviderCallback {
         mut session: Session,
     ) -> Result<(AuthStep, Cookie<'a>), ErrorResponse> {
         // the callback id for the cache should be inside the encrypted cookie
-        let cookie = ApiCookie::from_req(req, COOKIE_UPSTREAM_CALLBACK).ok_or_else(|| {
+        let callback_id = ApiCookie::from_req(req, COOKIE_UPSTREAM_CALLBACK).ok_or_else(|| {
             ErrorResponse::new(
                 ErrorResponseType::Forbidden,
                 "Missing encrypted callback cookie".to_string(),
             )
         })?;
-        let bytes = base64_decode(cookie.value())?;
-        let plain = EncValue::try_from(bytes)?.decrypt()?;
-        let callback_id = String::from_utf8_lossy(plain.as_ref()).to_string();
-        debug!("callback_id from encrypted cookie: {}", callback_id);
 
         // validate state
         if callback_id != payload.state {
@@ -799,7 +791,7 @@ impl AuthProviderCallback {
         // extract a possibly existing provider link cookie for
         // linking an existing account to a provider
         let link_cookie = ApiCookie::from_req(req, PROVIDER_LINK_COOKIE)
-            .and_then(|c| AuthProviderLinkCookie::try_from(c).ok());
+            .and_then(|value| AuthProviderLinkCookie::try_from(value.as_str()).ok());
 
         // deserialize payload and validate the information
         let (user, provider_mfa_login) = match res.json::<AuthProviderTokenSet>().await {
@@ -1475,6 +1467,28 @@ struct OidcCodeRequestParams<'a> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use cryptr::EncKeys;
+
+    #[test]
+    fn test_auth_provider_link_cookie() {
+        dotenvy::from_filename("rauthy.test.cfg").ok();
+        let _ = EncKeys::from_env().unwrap().init();
+
+        let value = AuthProviderLinkCookie {
+            provider_id: "my_id_1337".to_string(),
+            user_id: "batman123".to_string(),
+            user_email: "batman@gotham.io".to_string(),
+        };
+
+        let cookie = value.build_cookie().unwrap();
+
+        let cookie_val = ApiCookie::cookie_into_value(Some(cookie)).unwrap();
+        let res = AuthProviderLinkCookie::try_from(cookie_val.as_str()).unwrap();
+
+        assert_eq!(value.provider_id, res.provider_id);
+        assert_eq!(value.user_id, res.user_id);
+        assert_eq!(value.user_email, res.user_email);
+    }
 
     // ... just to understand the query syntax
     #[test]

diff --git a/rauthy-models/src/entity/magic_links.rs b/rauthy-models/src/entity/magic_links.rs
@@ -206,7 +206,7 @@ impl MagicLink {
             let cookie_opt = ApiCookie::from_req(req, PWD_RESET_COOKIE);
             if let Some(cookie) = cookie_opt {
                 // the extracted cookie from the request starts with 'rauthy-pwd-reset='
-                if !cookie.value().ends_with(self.cookie.as_ref().unwrap()) {
+                if !cookie.ends_with(self.cookie.as_ref().unwrap()) {
                     if *PASSWORD_RESET_COOKIE_BINDING {
                         return Err(err);
                     } else {

diff --git a/rauthy-models/src/entity/webauthn.rs b/rauthy-models/src/entity/webauthn.rs
@@ -381,15 +381,15 @@ impl WebauthnCookie {
         Ok(ApiCookie::build(COOKIE_MFA, b64, max_age))
     }
 
-    pub fn parse_validate(cookie: &Option<Cookie>) -> Result<Self, ErrorResponse> {
+    pub fn parse_validate(cookie: &Option<String>) -> Result<Self, ErrorResponse> {
         if cookie.is_none() {
             return Err(ErrorResponse::new(
                 ErrorResponseType::BadRequest,
                 "Webauthn Cookie is missing".to_string(),
             ));
         }
         let cookie = cookie.as_ref().unwrap();
-        let bytes = base64_decode(cookie.value())?;
+        let bytes = base64_decode(cookie)?;
         let dec = EncValue::try_from(bytes)?.decrypt()?;
         let slf = bincode::deserialize::<Self>(&dec)?;