[pbs-devel] [PATCH proxmox 2/3] auth-api: don't set `Expire` for HttpOnly cookies anymore

[Shannon Sterz <s.sterz@proxmox.com>]

previously users may have assumed that closing a browser will log them
out. this usually worked (see note below), as we defined the cookies
as "session cookies" by not setting `Expire` or `Max-Age`. clients
should remove such cookies when they are closed.

by setting `Expire` we broke this assumption as now browsers would
keep the cookie in place, even when closed, until they expired.

note: some browsers may never have behaved as expected here. a lot of
modern browsers have a "session restore" feature that would simply
restore such cookies when the session was restored. see also the
warning over in the mdn docs for `Set-Cookie` [1].

in any case, the tickets within the cookies were always valid for two
hours as we don't "revoke" tickets before they expire.

[1]:
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Set-Cookie#expiresdate

Reported-By: Dominik Csapak <d.csapak@proxmox.com>
Suggested-By: Dominik Csapak <d.csapak@proxmox.com>
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-auth-api/src/api/access.rs | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/proxmox-auth-api/src/api/access.rs b/proxmox-auth-api/src/api/access.rs
index f5111d4a..671a370b 100644
--- a/proxmox-auth-api/src/api/access.rs
+++ b/proxmox-auth-api/src/api/access.rs
@@ -158,25 +158,18 @@ fn create_ticket_http_only(
             // parse the ticket here, so we can use the correct timestamp of the `Expire` parameter
             // take the ticket here, so the option will be `None` in the response
             if let Some(ticket_str) = ticket_response.ticket.take() {
-                let ticket = Ticket::<ApiTicket>::parse(&ticket_str)?;
-
-                // see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#expiresdate
-                // see: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Date
-                // see: https://developer.mozilla.org/en-US/docs/Web/Security/Practical_implementation_guides/Cookies#expires
-                let expire =
-                    proxmox_time::epoch_to_http_date(ticket.time() + crate::TICKET_LIFETIME)?;
-
                 // this makes sure that ticket cookies:
                 // - Typically `__Host-`-prefixed: are only send to the specific domain that set
                 //   them and that scripts served via http cannot overwrite the cookie.
-                // - `Expires`: expire at the same time as the encoded timestamp in the ticket.
                 // - `Secure`: are only sent via https.
                 // - `SameSite=Lax`: are only sent on cross-site requests when the user is
                 //   navigating to the origin site from an external site.
                 // - `HttpOnly`: cookies are not readable to client-side javascript code.
-                let cookie = format!(
-                    "{host_cookie}={ticket_str}; Expires={expire}; Secure; SameSite=Lax; HttpOnly; Path=/;",
-                );
+                // - don't set `Expire` to keep cookie a session cookie. otherwise, we may break
+                //   security assumptions made by users previously. the expiration limit is still
+                //   enforced server side.
+                let cookie =
+                    format!("{host_cookie}={ticket_str}; Secure; SameSite=Lax; HttpOnly; Path=/;");
 
                 response = response.header(hyper::header::SET_COOKIE, cookie);
             }
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel