Skip to content

Commit

Permalink
fix: store number of cookie parts in a cookie
Browse files Browse the repository at this point in the history
this avoids problems with leftover cookie parts preventing decryption
  • Loading branch information
miwig authored and antonengelhardt committed Nov 4, 2024
1 parent 0565fd5 commit 0ee2e43
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 39 deletions.
42 changes: 13 additions & 29 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -590,15 +589,7 @@ impl ConfiguredOidc {

/// Clear the cookies and redirect to the base path or `end_session_endpoint`.
fn logout(&self) -> Result<Action, PluginError> {
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);

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::<Vec<&str>>().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::<Vec<&str>>()[1])
.collect::<Vec<&str>>()
// 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::<Option<Vec<String>>>()
.ok_or(PluginError::SessionCookieNotFoundError)?
.join("");

Ok(values)
Expand Down
14 changes: 4 additions & 10 deletions src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ impl Session {
encoded_nonce: &str,
cookie_name: &str,
cookie_duration: u64,
number_current_cookies: u64,
) -> Vec<String> {
// Split every 4000 bytes
let cookie_parts = encoded_cookie
Expand All @@ -109,22 +108,17 @@ 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={}; ",
cookie_name, &encoded_nonce, cookie_duration
);
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
}

Expand Down

0 comments on commit 0ee2e43

Please sign in to comment.