From 0ee2e43b508d9e4d0dd8cf408b2565e2621f7fa5 Mon Sep 17 00:00:00 2001 From: Michael Wigard Date: Fri, 18 Oct 2024 19:09:04 +0200 Subject: [PATCH] fix: store number of cookie parts in a cookie this avoids problems with leftover cookie parts preventing decryption --- src/auth.rs | 42 +++++++++++++----------------------------- src/session.rs | 14 ++++---------- 2 files changed, 17 insertions(+), 39 deletions(-) diff --git a/src/auth.rs b/src/auth.rs index 321dbb1..3eec1a2 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -570,7 +570,6 @@ impl ConfiguredOidc { &new_nonce, self.plugin_config.cookie_name.as_str(), self.plugin_config.cookie_duration, - self.get_number_of_session_cookies() as u64, ); // Build cookie headers @@ -590,15 +589,7 @@ impl ConfiguredOidc { /// Clear the cookies and redirect to the base path or `end_session_endpoint`. fn logout(&self) -> Result { - let cookie_values = Session::make_cookie_values( - // This is a bit hacky, but we need to write something into the cookie to clear all cookies (otherwise the - // `make_cookie_values` function will not overwrite all cookies, as "" is an empty chunk) - "clear", - "", - &self.plugin_config.cookie_name, - 0, - self.get_number_of_session_cookies() as u64, - ); + let cookie_values = Session::make_cookie_values("", "", &self.plugin_config.cookie_name, 0); let mut headers = Session::make_set_cookie_headers(&cookie_values); @@ -734,7 +725,6 @@ impl ConfiguredOidc { &nonce, self.plugin_config.cookie_name.as_str(), self.plugin_config.cookie_duration, - self.get_number_of_session_cookies() as u64, ); // Build cookie headers @@ -841,24 +831,18 @@ impl ConfiguredOidc { .get_http_request_header("cookie") .ok_or(PluginError::SessionCookieNotFoundError)?; - // Split cookie by ; and filter for the cookie name. - let cookies = cookie - .split(';') - .filter(|x| x.contains(self.plugin_config.cookie_name.as_str())) - .filter(|x| !x.contains(format!("{}-nonce", self.plugin_config.cookie_name).as_str())); - - // Check if cookies have values - for cookie in cookies.clone() { - if cookie.split('=').collect::>().len() < 2 { - return Err(PluginError::SessionCookieNotFoundError); - } - } - - // Then split all cookies by = and get the second element before joining all values together. - let values = cookies - .map(|x| x.split('=').collect::>()[1]) - .collect::>() - // Join the cookie values together again. + let cookie_name = &self.plugin_config.cookie_name; + let num_parts: u8 = self + .get_cookie(&format!("{cookie_name}-parts")) + .unwrap_or_default() + .parse() + .map_err(|_| PluginError::SessionCookieNotFoundError)?; + + let values = (0..num_parts) + .into_iter() + .map(|i| self.get_cookie(&format!("{cookie_name}-{i}"))) + .collect::>>() + .ok_or(PluginError::SessionCookieNotFoundError)? .join(""); Ok(values) diff --git a/src/session.rs b/src/session.rs index f1562b4..dbc5773 100644 --- a/src/session.rs +++ b/src/session.rs @@ -89,7 +89,6 @@ impl Session { encoded_nonce: &str, cookie_name: &str, cookie_duration: u64, - number_current_cookies: u64, ) -> Vec { // Split every 4000 bytes let cookie_parts = encoded_cookie @@ -109,6 +108,10 @@ impl Session { cookie_values.push(cookie_value); } + let num_parts = cookie_values.len(); + let num_parts_cookie_value = format!("{cookie_name}-parts={num_parts}; Path=/; HttpOnly; Secure; Max-Age={cookie_duration}; "); + cookie_values.push(num_parts_cookie_value); + // Build nonce cookie value let nonce_cookie_value = format!( "{}-nonce={}; Path=/; HttpOnly; Secure; Max-Age={}; ", @@ -116,15 +119,6 @@ impl Session { ); cookie_values.push(nonce_cookie_value); - // Overwrite the old cookies because decryption will fail if older and expired cookies are - // still present. - for i in cookie_values.len()..number_current_cookies as usize { - cookie_values.push(format!( - "{}-{}=; Path=/; HttpOnly; Secure; Max-Age=0", - cookie_name, i - )); - } - cookie_values }