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

[Dominik Csapak <d.csapak@proxmox.com>]

LGTM

Reviewed-by: Dominik Csapak <d.csapak@proxmox.com>
Tested-by: Dominik Csapak <d.csapak@proxmox.com>

On 7/25/25 13:24, Shannon Sterz wrote:
> 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);
>               }



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