Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v4 00/21] use HttpOnly cookies in new projects

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

Superseeded-by: https://lore.proxmox.com/pdm-devel/20250304144247.231089-1-s.sterz@proxmox.com/

On Tue Mar 4, 2025 at 1:04 PM CET, Shannon Sterz wrote:
> this patch series aims to improve the security of our authentication
> cookies for new projects such as anything based on the new yew-based
> toolkit. this is accomplished by several means:
>
> - cookies are now HttpOnly, which means client side JavaScript in a
>   browser has no access to the cookies anymore. this makes it harder to
>   steal cookies via malicious javascript code injected in the front-end.
>   (such as by downgrading a connection to http)
> - cookies are prefixed with `__Host-` by default (can be overriden in
>   the auth context), which means other subdomain's that did not set the
>   cookie have no more access to the cookie and cannot change it. this
>   means an attacker on another subdomain cannot overwrite the cookie
>   and, thus, trick a victim to perform actions with other credentials
>   than expected.
> - cookies are now `Secure` and `SameSite=Lax` by default. which means
>   cookies are only to be send in an https context and not on cross-site
>   requests (other than when a user initiates navigation).
>
> the first four patches in this series just add minor helpers and such to
> prepare for implementing a ticket endpoint in the `proxmox-auth-api`
> crate that can set tickets via a Set-Cookie header. such as adding a
> helper to express a unix epoch as http timestamp, setting cookies in an
> endpoint while still handling parameters in the request body and letting
> the auth context specify how to prefix the authentication cookie.
>
> the next four patches do the heavy lifting on the server side, mainly
> checking for the newly prefixed authentication cookie, implementing an
> endpoint that sets the cookie appropriatelly, and moving the existing
> ticket endpoint to use the same api types and handler as the new one.
> this is done in a way where the api itself stays the same for endusers.
> the last of these four commits also adds an endpoint to remove a ticket
> again, as browser-based clients can no longer do this by themselves.
>
> the next couple of patches adapt the `proxmox-login` and
> `proxmox-client` crates to deal with tickets stored in HttpOnly cookies.
> they also allow specifying a cookie name when creating a client, so that
> the cookie can be set in the appropriate header when needed. finally
> proxmox-yew-comp is adapted to also handle HttpOnly cookies correctly.
> since the client has no more access to the "real" ticket anymore, we
> return an unsigned "informational" ticket that has all the information
> needed by the client to refresh cookies (presuming that the correct
> HttpOnly cookie is appropriatelly handled by the context).
>
> for non-browser context, `proxmox-client` now checks for `Set-Cookie`
> headers as well in order to pick up on potential tickets there. this
> requires that the client is provided with an appropriate cookie name.
>
> the last commit adds the new endpoints to the datacenter-manager to
> already support them there correctly.
>
> ---
> changes since v3 thanks @ Wolfgang Bumiller & Maximiliano Sandoval
> - fixed a bug introduced in the new http only ticket endpoint introduced
>   by previous re-factoring (it would always panic due to a wrong
>   `unwrap`)
> - uncomment some `use` statements in the doc example for the new
>   `AsyncHttpBodyParameters` type endpoint
>
> changes since v2 thanks @ Wolfgang Bumiller & Maximiliano Sandoval
>
> - stop swalloing ticket parsing errors in the auth-api and proxmox-login
> - add a helper to create `Authentication`s instead of have the same code
>   three times
> - incorporate multiple minor nits and style improvements
>
> changes since v1 thanks @ Wolfgang Bumiller
>
> - moved common logic in the ticket endpoints to a separate handler and
>   use common types to improve parameter parsing and compatibility
> - only check `Set-Cookie` headers when a cookie name is provided and
>   only check cookies with a correct name in proxmox-client
> - pass through the cookie name if specify to proxmox-login in
>   proxmox-client
> - don't set informational tickets in the `set_auth_headers()` functions
>   in `proxmox-login`
> - smaller changes (nits, typos return types, dependency clean up where
>   possible etc.)
>
>
> *** MURPP HERE ***
>
> proxmox:
>
> Shannon Sterz (17):
>   time: add new `epoch_to_http_date` helper
>   rest-server: borrow parts parameter in `get_request_parameter`
>   router/rest-server: add new `AsyncHttpBodyParameters` api handler type
>   auth-api: extend `AuthContext` with prefixed cookie name
>   auth-api: check for new prefixed cookies as well
>   auth-api: introduce new CreateTicket and CreateTickeReponse api types
>   auth-api: add endpoint for issuing tickets as HttpOnly tickets
>   auth-api: make regular ticket endpoint use the new types and handler
>   auth-api: add logout method
>   login: add optional field for ticket_info and make password optional
>   login: make password optional when creating Login requests
>   login: add helpers to pass cookie values when parsing login responses
>   login: add `TicketResult::HttpOnly` member
>   login: add helper to check whether a ticket is just informational
>   login: add functions to specify full cookie names
>   client: add compatibility with HttpOnly cookies
>   client: specify cookie names for authentication headers where possible
>
>  proxmox-auth-api/Cargo.toml        |   4 +
>  proxmox-auth-api/src/api/access.rs | 240 +++++++++++++++++++++--------
>  proxmox-auth-api/src/api/mod.rs    |  53 +++++--
>  proxmox-auth-api/src/ticket.rs     |   5 +
>  proxmox-auth-api/src/types.rs      |  56 ++++++-
>  proxmox-client/src/client.rs       | 119 +++++++++++---
>  proxmox-login/src/api.rs           |   9 +-
>  proxmox-login/src/lib.rs           | 128 ++++++++++++---
>  proxmox-login/src/ticket.rs        |  53 ++++++-
>  proxmox-rest-server/src/rest.rs    |  21 ++-
>  proxmox-router/src/cli/command.rs  |  12 ++
>  proxmox-router/src/format.rs       |   6 +
>  proxmox-router/src/router.rs       |  45 ++++++
>  proxmox-time/src/posix.rs          |   9 ++
>  14 files changed, 622 insertions(+), 138 deletions(-)
>
>
> proxmox-yew-comp:
>
> Shannon Sterz (3):
>   HttpClient: add helpers to refresh HttpOnly cookies and remove them
>   LoginPanel/http helpers: add support for handling HttpOnly cookies
>   http helpers: ask server to remove `__Host-` prefixed cookie on logout
>
>  src/http_client_wasm.rs | 19 ++++++++++++++++++
>  src/http_helpers.rs     | 44 ++++++++++++++++++++++++++++++++++-------
>  src/login_panel.rs      |  5 ++++-
>  3 files changed, 60 insertions(+), 8 deletions(-)
>
>
> proxmox-datacenter-manager:
>
> Shannon Sterz (1):
>   api: switch ticket endpoint over to new http only endpoint
>
>  server/src/api/access/mod.rs | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
>
> Summary over all repositories:
>   18 files changed, 685 insertions(+), 147 deletions(-)
>
> --
> Generated by git-murpp 0.7.3



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