* [pdm-devel] [PATCH proxmox v3 01/21] time: add new `epoch_to_http_date` helper
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 02/21] rest-server: borrow parts parameter in `get_request_parameter` Shannon Sterz
` (21 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
this makes it easy to generate RFC9110 preferred HTTP Dates.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-time/src/posix.rs | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/proxmox-time/src/posix.rs b/proxmox-time/src/posix.rs
index bb60ba04..79fcb1b6 100644
--- a/proxmox-time/src/posix.rs
+++ b/proxmox-time/src/posix.rs
@@ -380,6 +380,15 @@ pub fn epoch_to_rfc2822(epoch: i64) -> Result<String, Error> {
Ok(rfc2822_date)
}
+/// Convert an epoch to an RFC9110 preferred HTTP Date format.
+///
+/// see: <https://httpwg.org/specs/rfc9110.html#http.date>
+pub fn epoch_to_http_date(epoch: i64) -> Result<String, Error> {
+ let gmtime = gmtime(epoch)?;
+ let locale = Locale::new(libc::LC_ALL, Locale::C)?;
+ strftime_l("%a, %d %b %Y %H:%M:%S GMT", &gmtime, &locale)
+}
+
#[test]
fn test_leap_seconds() {
let convert_reconvert = |epoch| {
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 02/21] rest-server: borrow parts parameter in `get_request_parameter`
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 01/21] time: add new `epoch_to_http_date` helper Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 03/21] router/rest-server: add new `AsyncHttpBodyParameters` api handler type Shannon Sterz
` (20 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
this function does not require ownership of the parts parameter, so we
can simply borrow it. this way we can pass the parameter on to a
handler that consumes them even when using `get_request_parameter`
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-rest-server/src/rest.rs | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/proxmox-rest-server/src/rest.rs b/proxmox-rest-server/src/rest.rs
index d23ed776..78339b75 100644
--- a/proxmox-rest-server/src/rest.rs
+++ b/proxmox-rest-server/src/rest.rs
@@ -383,7 +383,7 @@ fn parse_query_parameters<S: 'static + BuildHasher + Send>(
async fn get_request_parameters<S: 'static + BuildHasher + Send>(
param_schema: ParameterSchema,
- parts: Parts,
+ parts: &Parts,
req_body: Body,
uri_param: HashMap<String, String, S>,
) -> Result<Value, Error> {
@@ -433,7 +433,7 @@ async fn get_request_parameters<S: 'static + BuildHasher + Send>(
param_schema.verify_json(¶ms)?;
Ok(params)
} else {
- parse_query_parameters(param_schema, utf8_data, &parts, &uri_param)
+ parse_query_parameters(param_schema, utf8_data, parts, &uri_param)
}
}
@@ -548,7 +548,7 @@ pub(crate) async fn handle_api_request<Env: RpcEnvironment, S: 'static + BuildHa
}
ApiHandler::StreamSync(handler) => {
let params =
- get_request_parameters(info.parameters, parts, req_body, uri_param).await?;
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
match (handler)(params, info, &mut rpcenv) {
Ok(iter) if accept_json_seq => handle_sync_stream_as_json_seq(iter),
Ok(iter) => iter
@@ -559,7 +559,7 @@ pub(crate) async fn handle_api_request<Env: RpcEnvironment, S: 'static + BuildHa
}
ApiHandler::StreamAsync(handler) => {
let params =
- get_request_parameters(info.parameters, parts, req_body, uri_param).await?;
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
match (handler)(params, info, &mut rpcenv).await {
Ok(stream) if accept_json_seq => handle_stream_as_json_seq(stream),
Ok(stream) => stream
@@ -571,25 +571,25 @@ pub(crate) async fn handle_api_request<Env: RpcEnvironment, S: 'static + BuildHa
}
ApiHandler::SerializingSync(handler) => {
let params =
- get_request_parameters(info.parameters, parts, req_body, uri_param).await?;
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
(handler)(params, info, &mut rpcenv)
.and_then(|data| formatter.format_data_streaming(data, &rpcenv))
}
ApiHandler::SerializingAsync(handler) => {
let params =
- get_request_parameters(info.parameters, parts, req_body, uri_param).await?;
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
(handler)(params, info, &mut rpcenv)
.await
.and_then(|data| formatter.format_data_streaming(data, &rpcenv))
}
ApiHandler::Sync(handler) => {
let params =
- get_request_parameters(info.parameters, parts, req_body, uri_param).await?;
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
(handler)(params, info, &mut rpcenv).map(|data| formatter.format_data(data, &rpcenv))
}
ApiHandler::Async(handler) => {
let params =
- get_request_parameters(info.parameters, parts, req_body, uri_param).await?;
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
(handler)(params, info, &mut rpcenv)
.await
.map(|data| formatter.format_data(data, &rpcenv))
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 03/21] router/rest-server: add new `AsyncHttpBodyParameters` api handler type
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 01/21] time: add new `epoch_to_http_date` helper Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 02/21] rest-server: borrow parts parameter in `get_request_parameter` Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 04/21] auth-api: extend `AuthContext` with prefixed cookie name Shannon Sterz
` (19 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
this allows us to write api handlers that have access to a request's
headers and to create a low level response while being able to also
specify the parameter in the request's body. this is useful for
endpoints that should not use url parameters, but still need to
access/set specific headers.
previously, `AsyncHttp` did not allow for parameters that were marked
as non-optional to be passed in the body of a request.
as a side-effect, the body is parsed by the rest server to check for
parameters and consumed. so it cannot be passed on by the handler.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-rest-server/src/rest.rs | 5 ++++
proxmox-router/src/cli/command.rs | 12 +++++++++
proxmox-router/src/format.rs | 6 +++++
proxmox-router/src/router.rs | 45 +++++++++++++++++++++++++++++++
4 files changed, 68 insertions(+)
diff --git a/proxmox-rest-server/src/rest.rs b/proxmox-rest-server/src/rest.rs
index 78339b75..350ce957 100644
--- a/proxmox-rest-server/src/rest.rs
+++ b/proxmox-rest-server/src/rest.rs
@@ -546,6 +546,11 @@ pub(crate) async fn handle_api_request<Env: RpcEnvironment, S: 'static + BuildHa
let params = parse_query_parameters(info.parameters, "", &parts, &uri_param)?;
(handler)(parts, req_body, params, info, Box::new(rpcenv)).await
}
+ ApiHandler::AsyncHttpBodyParameters(handler) => {
+ let params =
+ get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
+ (handler)(parts, params, info, Box::new(rpcenv)).await
+ }
ApiHandler::StreamSync(handler) => {
let params =
get_request_parameters(info.parameters, &parts, req_body, uri_param).await?;
diff --git a/proxmox-router/src/cli/command.rs b/proxmox-router/src/cli/command.rs
index 01f64d19..2ca2356a 100644
--- a/proxmox-router/src/cli/command.rs
+++ b/proxmox-router/src/cli/command.rs
@@ -107,6 +107,12 @@ async fn handle_simple_command_future(
ApiHandler::AsyncHttp(_) => {
bail!("CliHandler does not support ApiHandler::AsyncHttp - internal error")
}
+ #[cfg(feature = "server")]
+ ApiHandler::AsyncHttpBodyParameters(_) => {
+ bail!(
+ "CliHandler does not support ApiHandler::AsyncHttpBodyParameters - internal error"
+ )
+ }
};
match result {
@@ -159,6 +165,12 @@ pub(crate) fn handle_simple_command<'cli>(
ApiHandler::AsyncHttp(_) => {
bail!("CliHandler does not support ApiHandler::AsyncHttp - internal error");
}
+ #[cfg(feature = "server")]
+ ApiHandler::AsyncHttpBodyParameters(_) => {
+ bail!(
+ "CliHandler does not support ApiHandler::AsyncHttpBodyParameters - internal error"
+ );
+ }
};
match result {
diff --git a/proxmox-router/src/format.rs b/proxmox-router/src/format.rs
index 67568af0..be40895a 100644
--- a/proxmox-router/src/format.rs
+++ b/proxmox-router/src/format.rs
@@ -32,6 +32,12 @@ fn dump_method_definition(method: &str, path: &str, def: Option<&ApiMethod>) ->
method = if method == "GET" { "DOWNLOAD" } else { method };
}
+ #[cfg(feature = "server")]
+ if let ApiHandler::AsyncHttpBodyParameters(_) = api_method.handler {
+ method = if method == "POST" { "UPLOAD" } else { method };
+ method = if method == "GET" { "DOWNLOAD" } else { method };
+ }
+
let res = format!(
"**{} {}**\n\n{}{}\n\n{}",
method, path, description, param_descr, return_descr
diff --git a/proxmox-router/src/router.rs b/proxmox-router/src/router.rs
index 33b598da..12ba863e 100644
--- a/proxmox-router/src/router.rs
+++ b/proxmox-router/src/router.rs
@@ -435,6 +435,44 @@ pub type ApiAsyncHttpHandlerFn = &'static (dyn Fn(
pub type ApiResponseFuture =
Pin<Box<dyn Future<Output = Result<Response<Body>, anyhow::Error>> + Send>>;
+/// Asynchronous HTTP API handlers with parameters specified in their bodies
+///
+/// They get low level access to request and response data, but it is also possible to specify
+/// their parameters in the request body.
+///
+/// ```
+/// # use serde_json::{json, Value};
+/// #
+/// use hyper::{Body, Response, http::request::Parts};
+///
+/// use proxmox_router::{ApiHandler, ApiMethod, ApiResponseFuture, RpcEnvironment};
+/// use proxmox_schema::ObjectSchema;
+///
+/// fn low_level_hello(
+/// parts: Parts,
+/// param: Value,
+/// info: &ApiMethod,
+/// rpcenv: Box<dyn RpcEnvironment>,
+/// ) -> ApiResponseFuture {
+/// Box::pin(async move {
+/// let response = http::Response::builder()
+/// .status(200)
+/// .body(Body::from("Hello world!"))?;
+/// Ok(response)
+/// })
+/// }
+///
+/// const API_METHOD_LOW_LEVEL_HELLO_BODY_PARAMETER: ApiMethod = ApiMethod::new(
+/// &ApiHandler::AsyncHttpBodyParameters(&low_level_hello),
+/// &ObjectSchema::new("Hello World Example (low level)", &[])
+/// );
+/// ```
+#[cfg(feature = "server")]
+pub type ApiAsyncHttpHandlerBodyParametersFn = &'static (dyn Fn(Parts, Value, &'static ApiMethod, Box<dyn RpcEnvironment>) -> ApiResponseFuture
+ + Send
+ + Sync
+ + 'static);
+
/// Enum for different types of API handler functions.
#[non_exhaustive]
pub enum ApiHandler {
@@ -446,6 +484,8 @@ pub enum ApiHandler {
StreamAsync(StreamApiAsyncHandlerFn),
#[cfg(feature = "server")]
AsyncHttp(ApiAsyncHttpHandlerFn),
+ #[cfg(feature = "server")]
+ AsyncHttpBodyParameters(ApiAsyncHttpHandlerBodyParametersFn),
}
#[cfg(feature = "test-harness")]
@@ -478,6 +518,11 @@ impl PartialEq for ApiHandler {
(ApiHandler::AsyncHttp(l), ApiHandler::AsyncHttp(r)) => {
core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r)
}
+ #[cfg(feature = "server")]
+ (
+ ApiHandler::AsyncHttpBodyParameters(l),
+ ApiHandler::AsyncHttpBodyParameters(r),
+ ) => core::mem::transmute::<_, usize>(l) == core::mem::transmute::<_, usize>(r),
_ => false,
}
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 04/21] auth-api: extend `AuthContext` with prefixed cookie name
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (2 preceding siblings ...)
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 03/21] router/rest-server: add new `AsyncHttpBodyParameters` api handler type Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 05/21] auth-api: check for new prefixed cookies as well Shannon Sterz
` (18 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
this adds the function `prefixed_auth_cookie_name` to the
`AuthContext` trait. said function can be used by users of this crate
to modify the expected prefix of the auth cookie. most products
should be able to use the default of `__Host-` though, so this also
adds a default implementation.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-auth-api/src/api/mod.rs | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/proxmox-auth-api/src/api/mod.rs b/proxmox-auth-api/src/api/mod.rs
index a6f9d425..b75c8602 100644
--- a/proxmox-auth-api/src/api/mod.rs
+++ b/proxmox-auth-api/src/api/mod.rs
@@ -1,7 +1,7 @@
use std::future::Future;
use std::net::IpAddr;
use std::pin::Pin;
-use std::sync::Mutex;
+use std::sync::{Mutex, OnceLock};
use anyhow::{format_err, Error};
use percent_encoding::percent_decode_str;
@@ -84,6 +84,16 @@ pub trait AuthContext: Send + Sync {
let _ = (userid, password, path, privs, port);
Ok(None)
}
+
+ /// The auth cookie with a prefix. Usually this will be `__Host-`. However, products that don't
+ /// want a prefix or need a different one such as `__Secure-` should override the default
+ /// implementation.
+ ///
+ /// See: <https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie#cookie_prefixes>
+ fn prefixed_auth_cookie_name(&self) -> &'static str {
+ static HOST_COOKIE: OnceLock<String> = OnceLock::new();
+ HOST_COOKIE.get_or_init(|| format!("__Host-{}", self.auth_cookie_name()))
+ }
}
/// When verifying TFA challenges we need to be able to update the TFA config without interference
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 05/21] auth-api: check for new prefixed cookies as well
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (3 preceding siblings ...)
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 04/21] auth-api: extend `AuthContext` with prefixed cookie name Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 06/21] auth-api: introduce new CreateTicket and CreateTickeReponse api types Shannon Sterz
` (17 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
this makes sure that newly generated cookies that are prefixed with,
for example, `__Host-`, for security purposes, are correctly picked
up on. otherwise, the new cookies would not be able to yield proper
authentication.
currently this still falls back to insecure non-prefixed cookies. we
should deprecate them in the long-term and remove this fallback.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-auth-api/src/api/mod.rs | 36 ++++++++++++++++++++++++---------
1 file changed, 27 insertions(+), 9 deletions(-)
diff --git a/proxmox-auth-api/src/api/mod.rs b/proxmox-auth-api/src/api/mod.rs
index b75c8602..32d18299 100644
--- a/proxmox-auth-api/src/api/mod.rs
+++ b/proxmox-auth-api/src/api/mod.rs
@@ -203,18 +203,36 @@ fn extract_auth_data(
auth_context: &dyn AuthContext,
headers: &http::HeaderMap,
) -> Option<AuthData> {
- if let Some(raw_cookie) = headers.get(http::header::COOKIE) {
- if let Ok(cookie) = raw_cookie.to_str() {
- if let Some(ticket) = extract_cookie(cookie, auth_context.auth_cookie_name()) {
- let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
- Some(Ok(v)) => Some(v.to_owned()),
- _ => None,
- };
- return Some(AuthData::User(UserAuthData { ticket, csrf_token }));
- }
+ let mut ticket = None;
+ let cookie_name = auth_context.auth_cookie_name();
+ let host_cookie = auth_context.prefixed_auth_cookie_name();
+ let cookies = headers
+ .get_all(http::header::COOKIE)
+ .iter()
+ .filter_map(|c| c.to_str().ok());
+
+ for cookie in cookies {
+ // check if we got a `__Host-` prefixed cookie first and break the loop if we do so we use
+ // that cookie.
+ if let Some(extracted_ticket) = extract_cookie(cookie, host_cookie) {
+ ticket = Some(extracted_ticket);
+ break;
+ // if no such prefixed cookie exists, try to fall back to a non-prefixed one
+ // TODO: remove this once we do not support less secure non-prefixed cookies anymore
+ } else if let Some(extracted_ticket) = extract_cookie(cookie, cookie_name) {
+ ticket = Some(extracted_ticket)
}
}
+ if let Some(ticket) = ticket {
+ let csrf_token = match headers.get("CSRFPreventionToken").map(|v| v.to_str()) {
+ Some(Ok(v)) => Some(v.to_owned()),
+ _ => None,
+ };
+
+ return Some(AuthData::User(UserAuthData { ticket, csrf_token }));
+ }
+
let token_prefix = auth_context.auth_token_prefix();
match headers.get(http::header::AUTHORIZATION).map(|v| v.to_str()) {
Some(Ok(v)) => {
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 06/21] auth-api: introduce new CreateTicket and CreateTickeReponse api types
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (4 preceding siblings ...)
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 05/21] auth-api: check for new prefixed cookies as well Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 07/21] auth-api: add endpoint for issuing tickets as HttpOnly tickets Shannon Sterz
` (16 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
these types are used for creating a ticket and responding to a new
ticket request.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-auth-api/src/types.rs | 56 ++++++++++++++++++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/proxmox-auth-api/src/types.rs b/proxmox-auth-api/src/types.rs
index 64c580a5..81c43ab6 100644
--- a/proxmox-auth-api/src/types.rs
+++ b/proxmox-auth-api/src/types.rs
@@ -417,7 +417,7 @@ impl<'a> TryFrom<&'a str> for &'a TokennameRef {
}
/// A complete user id consisting of a user name and a realm
-#[derive(Clone, Debug, PartialEq, Eq, Hash, Ord, PartialOrd, UpdaterType)]
+#[derive(Clone, Debug, Default, PartialEq, Eq, Hash, Ord, PartialOrd, UpdaterType)]
pub struct Userid {
data: String,
name_len: usize,
@@ -676,6 +676,60 @@ impl TryFrom<String> for Authid {
}
}
+#[api]
+/// The parameter object for creating new ticket.
+#[derive(Debug, Default, Deserialize, Serialize)]
+pub struct CreateTicket {
+ /// User name
+ pub username: Userid,
+
+ /// The secret password. This can also be a valid ticket. Only optional if the ticket is
+ /// provided in a cookie header and only if the endpoint supports this.
+ #[serde(default)]
+ pub password: Option<String>,
+
+ /// Verify ticket, and check if user have access 'privs' on 'path'.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub path: Option<String>,
+
+ /// Verify ticket, and check if user have access 'privs' on 'path'.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub privs: Option<String>,
+
+ /// Port for verifying terminal tickets.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub port: Option<u16>,
+
+ /// The signed TFA challenge string the user wants to respond to.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ #[serde(rename = "tfa-challenge")]
+ pub tfa_challenge: Option<String>,
+}
+
+#[api]
+/// The API response for a ticket call.
+#[derive(Debug, Default, Deserialize, Serialize)]
+pub struct CreateTicketResponse {
+ /// The CSRF prevention token.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ #[serde(rename = "CSRFPreventionToken")]
+ pub csrfprevention_token: Option<String>,
+
+ /// The ticket as is supposed to be used in the authentication header. Not provided here if the
+ /// endpoint uses HttpOnly cookies to supply the actual ticket.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub ticket: Option<String>,
+
+ /// Like a full ticket, except the signature is missing. Useful in HttpOnly-contexts
+ /// (browsers).
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ #[serde(rename = "ticket-info")]
+ pub ticket_info: Option<String>,
+
+ /// The userid.
+ pub username: Userid,
+}
+
#[test]
fn test_token_id() {
let userid: Userid = "test@pam".parse().expect("parsing Userid failed");
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 07/21] auth-api: add endpoint for issuing tickets as HttpOnly tickets
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (5 preceding siblings ...)
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 06/21] auth-api: introduce new CreateTicket and CreateTickeReponse api types Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 08/21] auth-api: make regular ticket endpoint use the new types and handler Shannon Sterz
` (15 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
this adds a new endpoint for requesting tickets. instead of returning
the ticket in the responses body, the ticket is set as a HttpOnly
cookie. this has a couple of advantages:
- the cookie cannot be stolen if an attacker downgrades the connection
to http and injects malicious javascript (`HttpOnly`)
- we don't need to rely on the client to make sure that the cookie is
only send in the appropriate context and only over https
connections (`Secure`, `SameSite`).
- the cookie cannot be overwritten by other subdomains, insecure
connections etc. (the default is to prefix them with `__Host-`)
this follows the best practice guide for secure cookies from MDN
[1]. we also set the cookies to expire when the ticket would so that
the browser removes the cookie once the ticket isn't valid anymore.
the endpoint still returns a ticket that only contains the
informational portions of the ticket but not a valid signature. this
is helpful to let clients know when to refresh the ticket by querying
this endpoint again. it still protects the cookie, though, as it
isn't a valid ticket by itself.
[1]: https://developer.mozilla.org/en-US/docs/Web/Security/Practical_implementation_guides/Cookies
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-auth-api/Cargo.toml | 4 +
proxmox-auth-api/src/api/access.rs | 168 ++++++++++++++++++++++++++++-
proxmox-auth-api/src/api/mod.rs | 5 +-
proxmox-auth-api/src/ticket.rs | 5 +
4 files changed, 177 insertions(+), 5 deletions(-)
diff --git a/proxmox-auth-api/Cargo.toml b/proxmox-auth-api/Cargo.toml
index 49398775..848c947c 100644
--- a/proxmox-auth-api/Cargo.toml
+++ b/proxmox-auth-api/Cargo.toml
@@ -22,6 +22,7 @@ base64 = { workspace = true, optional = true }
libc = { workspace = true, optional = true }
log = { workspace = true, optional = true }
http = { workspace = true, optional = true }
+hyper = { workspace = true, optional = true }
nix = { workspace = true, optional = true }
openssl = { workspace = true, optional = true }
pam-sys = { workspace = true, optional = true }
@@ -37,6 +38,7 @@ proxmox-router = { workspace = true, optional = true }
proxmox-schema = { workspace = true, optional = true, features = [ "api-macro", "api-types" ] }
proxmox-sys = { workspace = true, optional = true }
proxmox-tfa = { workspace = true, optional = true, features = [ "api" ] }
+proxmox-time = { workspace = true, optional = true }
[features]
default = []
@@ -48,11 +50,13 @@ api = [
"ticket",
"dep:http",
+ "dep:hyper",
"dep:serde_json",
"dep:proxmox-rest-server",
"dep:proxmox-router",
"dep:proxmox-tfa",
+ "dep:proxmox-time",
]
pam-authenticator = [ "api", "dep:libc", "dep:log", "dep:pam-sys" ]
password-authenticator = [
diff --git a/proxmox-auth-api/src/api/access.rs b/proxmox-auth-api/src/api/access.rs
index f7d52e95..273d7701 100644
--- a/proxmox-auth-api/src/api/access.rs
+++ b/proxmox-auth-api/src/api/access.rs
@@ -1,18 +1,25 @@
//! Provides the "/access/ticket" API call.
use anyhow::{bail, format_err, Error};
+use http::request::Parts;
+use http::Response;
+use hyper::Body;
use openssl::hash::MessageDigest;
use serde_json::{json, Value};
-use proxmox_rest_server::RestEnvironment;
-use proxmox_router::{http_err, Permission, RpcEnvironment};
-use proxmox_schema::{api, api_types::PASSWORD_SCHEMA};
+use proxmox_rest_server::{extract_cookie, RestEnvironment};
+use proxmox_router::{
+ http_err, ApiHandler, ApiMethod, ApiResponseFuture, Permission, RpcEnvironment,
+};
+use proxmox_schema::{
+ api, api_types::PASSWORD_SCHEMA, AllOfSchema, ApiType, ParameterSchema, ReturnType,
+};
use proxmox_tfa::api::TfaChallenge;
use super::ApiTicket;
use super::{auth_context, HMACKey};
use crate::ticket::Ticket;
-use crate::types::{Authid, Userid};
+use crate::types::{Authid, CreateTicket, CreateTicketResponse, Userid};
#[allow(clippy::large_enum_variant)]
enum AuthResult {
@@ -132,6 +139,159 @@ pub async fn create_ticket(
}
}
+
+pub const API_METHOD_CREATE_TICKET_HTTP_ONLY: ApiMethod = ApiMethod::new_full(
+ &ApiHandler::AsyncHttpBodyParameters(&create_ticket_http_only),
+ ParameterSchema::AllOf(&AllOfSchema::new(
+ "Get a new ticket as an HttpOnly cookie. Supports tickets via cookies.",
+ &[&CreateTicket::API_SCHEMA],
+ )),
+)
+.returns(ReturnType::new(false, &CreateTicketResponse::API_SCHEMA))
+.protected(true)
+.access(None, &Permission::World);
+
+fn create_ticket_http_only(
+ parts: Parts,
+ param: Value,
+ _info: &ApiMethod,
+ rpcenv: Box<dyn RpcEnvironment>,
+) -> ApiResponseFuture {
+ Box::pin(async move {
+ let auth_context = auth_context()?;
+ let host_cookie = auth_context.prefixed_auth_cookie_name();
+ let mut create_params: CreateTicket = serde_json::from_value(param)?;
+
+ // previously to refresh a ticket, the old ticket was provided as a password via this
+ // endpoint's parameters. however, once the ticket is set as an HttpOnly cookie, some
+ // clients won't have access to it anymore. so we need to check whether the ticket is set
+ // in a cookie here too.
+ //
+ // only check the newer `__Host-` prefixed cookies here as older tickets should be
+ // provided via the password parameter anyway.
+ create_params.password = parts
+ .headers
+ // there is a `cookie_from_header` function we could use, but it seems to fail when
+ // multiple cookie headers are set
+ .get_all(http::header::COOKIE)
+ .iter()
+ .filter_map(|c| c.to_str().ok())
+ // after this only `__Host-{Cookie Name}` cookies are in the iterator
+ .filter_map(|c| extract_cookie(c, host_cookie))
+ // so this should just give us the first one if it exists
+ .next()
+ // if not use the parameter
+ .or(create_params.password);
+
+ let env: &RestEnvironment = rpcenv
+ .as_any()
+ .downcast_ref::<RestEnvironment>()
+ .ok_or(format_err!("detected wrong RpcEnvironment type"))?;
+
+ let mut ticket_response = handle_ticket_creation(create_params, env).await?;
+
+ // if `ticket_info` is set, we want to return the ticket in a `SET_COOKIE` header and not
+ // the response body
+ if ticket_response.ticket_info.is_some() {
+ // parse the ticket here, so we can use the correct timestamp of the `Expire` parameter
+ if let Some(ref ticket) = ticket_response.ticket.take() {
+ let ticket = Ticket::<ApiTicket>::parse(ticket)?;
+
+ // 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}={}; Expires={}; Secure; SameSite=Lax; HttpOnly; Path=/;",
+ // the unwrap here is safe, as we already parse the ticket above, it must be set
+ ticket_response.ticket.unwrap(),
+ expire,
+ );
+
+ return Ok(Response::builder()
+ .header(hyper::header::SET_COOKIE, cookie)
+ // unset the ticket here explicitly, so that JavaScript clients can't access it
+ .body(Body::from(
+ json!({"data": CreateTicketResponse { ticket: None, .. ticket_response} })
+ .to_string(),
+ ))?);
+ }
+ }
+
+ Ok(Response::builder().body(Body::from(json!({"data": ticket_response }).to_string()))?)
+ })
+}
+
+async fn handle_ticket_creation(
+ create_params: CreateTicket,
+ env: &RestEnvironment,
+) -> Result<CreateTicketResponse, Error> {
+ let username = create_params.username;
+ let password = create_params
+ .password
+ .ok_or(format_err!("no password provided"))?;
+
+ match authenticate_user(
+ &username,
+ &password,
+ create_params.path,
+ create_params.privs,
+ create_params.port,
+ create_params.tfa_challenge,
+ env,
+ )
+ .await
+ {
+ Ok(AuthResult::Success) => Ok(CreateTicketResponse {
+ username,
+ ..Default::default()
+ }),
+ Ok(AuthResult::CreateTicket) => {
+ let auth_context = auth_context()?;
+ let api_ticket = ApiTicket::Full(username.clone());
+ let mut ticket = Ticket::new(auth_context.auth_prefix(), &api_ticket)?;
+ let csrfprevention_token =
+ assemble_csrf_prevention_token(auth_context.csrf_secret(), &username);
+
+ env.log_auth(username.as_str());
+
+ Ok(CreateTicketResponse {
+ username,
+ ticket: Some(ticket.sign(auth_context.keyring(), None)?),
+ ticket_info: Some(ticket.ticket_info()),
+ csrfprevention_token: Some(csrfprevention_token),
+ })
+ }
+ Ok(AuthResult::Partial(challenge)) => {
+ let auth_context = auth_context()?;
+ let api_ticket = ApiTicket::Partial(challenge);
+ let ticket = Ticket::new(auth_context.auth_prefix(), &api_ticket)?
+ .sign(auth_context.keyring(), Some(username.as_str()))?;
+
+ Ok(CreateTicketResponse {
+ username,
+ ticket: Some(ticket),
+ csrfprevention_token: Some("invalid".to_string()),
+ ..Default::default()
+ })
+ }
+ Err(err) => {
+ env.log_failed_auth(Some(username.to_string()), &err.to_string());
+ Err(http_err!(UNAUTHORIZED, "permission check failed."))
+ }
+ }
+}
+
async fn authenticate_user(
userid: &Userid,
password: &str,
diff --git a/proxmox-auth-api/src/api/mod.rs b/proxmox-auth-api/src/api/mod.rs
index 32d18299..3ee2d0e1 100644
--- a/proxmox-auth-api/src/api/mod.rs
+++ b/proxmox-auth-api/src/api/mod.rs
@@ -18,7 +18,10 @@ mod ticket;
use crate::ticket::Ticket;
use access::verify_csrf_prevention_token;
-pub use access::{assemble_csrf_prevention_token, create_ticket, API_METHOD_CREATE_TICKET};
+pub use access::{
+ assemble_csrf_prevention_token, create_ticket, API_METHOD_CREATE_TICKET,
+ API_METHOD_CREATE_TICKET_HTTP_ONLY,
+};
pub use ticket::{ApiTicket, PartialTicket};
/// Authentication realms are used to manage users: authenticate, change password or remove.
diff --git a/proxmox-auth-api/src/ticket.rs b/proxmox-auth-api/src/ticket.rs
index 498e9385..59293492 100644
--- a/proxmox-auth-api/src/ticket.rs
+++ b/proxmox-auth-api/src/ticket.rs
@@ -227,6 +227,11 @@ where
_type_marker: PhantomData,
})
}
+
+ pub fn ticket_info(&self) -> String {
+ // append a `::ticketinfo` signature to distinguish the ticket info from proper tickets
+ format!("{}::ticketinfo", self.ticket_data())
+ }
}
#[cfg(test)]
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 08/21] auth-api: make regular ticket endpoint use the new types and handler
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (6 preceding siblings ...)
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 07/21] auth-api: add endpoint for issuing tickets as HttpOnly tickets Shannon Sterz
@ 2025-02-27 14:06 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 09/21] auth-api: add logout method Shannon Sterz
` (14 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:06 UTC (permalink / raw)
To: pdm-devel
so we can re-use more code between the different ticket endpoints
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-auth-api/src/api/access.rs | 94 +++---------------------------
1 file changed, 9 insertions(+), 85 deletions(-)
diff --git a/proxmox-auth-api/src/api/access.rs b/proxmox-auth-api/src/api/access.rs
index 273d7701..fb72b443 100644
--- a/proxmox-auth-api/src/api/access.rs
+++ b/proxmox-auth-api/src/api/access.rs
@@ -11,9 +11,7 @@ use proxmox_rest_server::{extract_cookie, RestEnvironment};
use proxmox_router::{
http_err, ApiHandler, ApiMethod, ApiResponseFuture, Permission, RpcEnvironment,
};
-use proxmox_schema::{
- api, api_types::PASSWORD_SCHEMA, AllOfSchema, ApiType, ParameterSchema, ReturnType,
-};
+use proxmox_schema::{api, AllOfSchema, ApiType, ParameterSchema, ReturnType};
use proxmox_tfa::api::TfaChallenge;
use super::ApiTicket;
@@ -36,51 +34,14 @@ enum AuthResult {
#[api(
input: {
properties: {
- username: {
- type: Userid,
- },
- password: {
- schema: PASSWORD_SCHEMA,
- },
- path: {
- type: String,
- description: "Path for verifying terminal tickets.",
- optional: true,
- },
- privs: {
- type: String,
- description: "Privilege for verifying terminal tickets.",
- optional: true,
- },
- port: {
- type: Integer,
- description: "Port for verifying terminal tickets.",
- optional: true,
- },
- "tfa-challenge": {
- type: String,
- description: "The signed TFA challenge string the user wants to respond to.",
- optional: true,
- },
+ create_params: {
+ type: CreateTicket,
+ flatten: true,
+ }
},
},
returns: {
- properties: {
- username: {
- type: String,
- description: "User name.",
- },
- ticket: {
- type: String,
- description: "Auth ticket.",
- },
- CSRFPreventionToken: {
- type: String,
- description:
- "Cross Site Request Forgery Prevention Token. \
- For partial tickets this is the string \"invalid\".",
- },
- },
+ type: CreateTicketResponse,
},
protected: true,
access: {
@@ -91,52 +52,15 @@ enum AuthResult {
///
/// Returns: An authentication ticket with additional infos.
pub async fn create_ticket(
- username: Userid,
- password: String,
- path: Option<String>,
- privs: Option<String>,
- port: Option<u16>,
- tfa_challenge: Option<String>,
+ create_params: CreateTicket,
rpcenv: &mut dyn RpcEnvironment,
-) -> Result<Value, Error> {
+) -> Result<CreateTicketResponse, Error> {
let env: &RestEnvironment = rpcenv
.as_any()
.downcast_ref::<RestEnvironment>()
.ok_or_else(|| format_err!("detected wrong RpcEnvironment type"))?;
- match authenticate_user(&username, &password, path, privs, port, tfa_challenge, env).await {
- Ok(AuthResult::Success) => Ok(json!({ "username": username })),
- Ok(AuthResult::CreateTicket) => {
- let auth_context = auth_context()?;
- let api_ticket = ApiTicket::Full(username.clone());
- let ticket = Ticket::new(auth_context.auth_prefix(), &api_ticket)?
- .sign(auth_context.keyring(), None)?;
- let token = assemble_csrf_prevention_token(auth_context.csrf_secret(), &username);
-
- env.log_auth(username.as_str());
-
- Ok(json!({
- "username": username,
- "ticket": ticket,
- "CSRFPreventionToken": token,
- }))
- }
- Ok(AuthResult::Partial(challenge)) => {
- let auth_context = auth_context()?;
- let api_ticket = ApiTicket::Partial(challenge);
- let ticket = Ticket::new(auth_context.auth_prefix(), &api_ticket)?
- .sign(auth_context.keyring(), Some(username.as_str()))?;
- Ok(json!({
- "username": username,
- "ticket": ticket,
- "CSRFPreventionToken": "invalid",
- }))
- }
- Err(err) => {
- env.log_failed_auth(Some(username.to_string()), &err.to_string());
- Err(http_err!(UNAUTHORIZED, "permission check failed."))
- }
- }
+ handle_ticket_creation(create_params, env).await
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 09/21] auth-api: add logout method
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (7 preceding siblings ...)
2025-02-27 14:06 ` [pdm-devel] [PATCH proxmox v3 08/21] auth-api: make regular ticket endpoint use the new types and handler Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 10/21] login: add optional field for ticket_info and make password optional Shannon Sterz
` (13 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
adds a new endpoint that is useful when dealing with HttpOnly cookies
that cannot be removed by client-side javascript (and by extension
wasm) code. the logout handle simply removes the cookie that is used
for storing the current ticket. this works the same way as it does in
the front-end: by setting an expired cookie with the same name.
as cookies are now prefixed with `__Host-` by default, the cookie here
also needs to be `Secure` and have the same `Path` to not be rejected
by the browser before it can remove the old cookie.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-auth-api/src/api/access.rs | 29 ++++++++++++++++++++++++++++-
proxmox-auth-api/src/api/mod.rs | 2 +-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/proxmox-auth-api/src/api/access.rs b/proxmox-auth-api/src/api/access.rs
index fb72b443..5bc65026 100644
--- a/proxmox-auth-api/src/api/access.rs
+++ b/proxmox-auth-api/src/api/access.rs
@@ -11,7 +11,7 @@ use proxmox_rest_server::{extract_cookie, RestEnvironment};
use proxmox_router::{
http_err, ApiHandler, ApiMethod, ApiResponseFuture, Permission, RpcEnvironment,
};
-use proxmox_schema::{api, AllOfSchema, ApiType, ParameterSchema, ReturnType};
+use proxmox_schema::{api, AllOfSchema, ApiType, ObjectSchema, ParameterSchema, ReturnType};
use proxmox_tfa::api::TfaChallenge;
use super::ApiTicket;
@@ -63,6 +63,33 @@ pub async fn create_ticket(
handle_ticket_creation(create_params, env).await
}
+pub const API_METHOD_LOGOUT: ApiMethod = ApiMethod::new(
+ &ApiHandler::AsyncHttpBodyParameters(&logout_handler),
+ &ObjectSchema::new("", &[]),
+)
+.protected(true)
+.access(None, &Permission::World);
+
+fn logout_handler(
+ _parts: Parts,
+ _param: Value,
+ _info: &ApiMethod,
+ _rpcenv: Box<dyn RpcEnvironment>,
+) -> ApiResponseFuture {
+ Box::pin(async move {
+ // unset authentication cookie by setting an invalid one. needs the same `Path` and
+ // `Secure` parameter to not be rejected by some browsers. also use the same `HttpOnly` and
+ // `SameSite` parameters just in case.
+ let host_cookie = format!(
+ "{}=; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; SameSite=Lax; HttpOnly; Path=/;",
+ auth_context()?.prefixed_auth_cookie_name()
+ );
+
+ Ok(Response::builder()
+ .header(hyper::header::SET_COOKIE, host_cookie)
+ .body(Body::empty())?)
+ })
+}
pub const API_METHOD_CREATE_TICKET_HTTP_ONLY: ApiMethod = ApiMethod::new_full(
&ApiHandler::AsyncHttpBodyParameters(&create_ticket_http_only),
diff --git a/proxmox-auth-api/src/api/mod.rs b/proxmox-auth-api/src/api/mod.rs
index 3ee2d0e1..e176ea01 100644
--- a/proxmox-auth-api/src/api/mod.rs
+++ b/proxmox-auth-api/src/api/mod.rs
@@ -20,7 +20,7 @@ use access::verify_csrf_prevention_token;
pub use access::{
assemble_csrf_prevention_token, create_ticket, API_METHOD_CREATE_TICKET,
- API_METHOD_CREATE_TICKET_HTTP_ONLY,
+ API_METHOD_CREATE_TICKET_HTTP_ONLY, API_METHOD_LOGOUT,
};
pub use ticket::{ApiTicket, PartialTicket};
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 10/21] login: add optional field for ticket_info and make password optional
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (8 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 09/21] auth-api: add logout method Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 11/21] login: make password optional when creating Login requests Shannon Sterz
` (12 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
tickets created through the new HttpOnly ticket endpoint won't return
a ticket in the password field. so this field will be left empty.
hence make it optional.
the endpoint does return a ticket_info parameter, though, that
includes the information when a ticket needs to be refreshed. so add
a new optional field for that too.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/api.rs | 9 ++++++++-
proxmox-login/src/lib.rs | 4 ++--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/proxmox-login/src/api.rs b/proxmox-login/src/api.rs
index aa8575fe..b7107312 100644
--- a/proxmox-login/src/api.rs
+++ b/proxmox-login/src/api.rs
@@ -21,7 +21,8 @@ pub struct CreateTicket {
pub otp: Option<String>,
/// The secret password. This can also be a valid ticket.
- pub password: String,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ pub password: Option<String>,
/// Verify ticket, and check if user have access 'privs' on 'path'
#[serde(default, skip_serializing_if = "Option::is_none")]
@@ -61,6 +62,12 @@ pub struct CreateTicketResponse {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub ticket: Option<String>,
+ /// A purely informational ticket that can be used to gather information about when the actual
+ /// ticket needs to be refreshed.
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ #[serde(rename = "ticket-info")]
+ pub ticket_info: Option<String>,
+
/// The full userid with the `@realm` part.
pub username: String,
}
diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index 5842d62a..d7798b62 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -129,7 +129,7 @@ impl Login {
let request = api::CreateTicket {
new_format: self.pve_compat.then_some(true),
username: self.userid.clone(),
- password: self.password.clone(),
+ password: Some(self.password.clone()),
..Default::default()
};
@@ -279,7 +279,7 @@ impl SecondFactorChallenge {
let request = api::CreateTicket {
new_format: self.pve_compat.then_some(true),
username: self.userid.clone(),
- password: data.to_string(),
+ password: Some(data.to_string()),
tfa_challenge: Some(self.ticket.clone()),
..Default::default()
};
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 11/21] login: make password optional when creating Login requests
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (9 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 10/21] login: add optional field for ticket_info and make password optional Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 12/21] login: add helpers to pass cookie values when parsing login responses Shannon Sterz
` (11 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
in certain context (for example, the browser), no password needs to be
provided when using HttpOnly cookies as they are handle by said
context. so make renewing ticket with password optional and add a new
helper function that does not require a password.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/lib.rs | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index d7798b62..3b3558d0 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -50,7 +50,7 @@ pub struct Request {
pub struct Login {
api_url: String,
userid: String,
- password: String,
+ password: Option<String>,
pve_compat: bool,
}
@@ -96,7 +96,18 @@ impl Login {
api_url: normalize_url(api_url.into()),
pve_compat: ticket.product() == "PVE",
userid: ticket.userid().to_string(),
- password: ticket.into(),
+ password: Some(ticket.into()),
+ }
+ }
+
+ /// Prepare a request with the assumption that the context handles the ticket (usually in a
+ /// browser via an HttpOnly cookie).
+ pub fn renew_with_cookie(api_url: impl Into<String>, userid: impl Into<String>) -> Self {
+ Self {
+ api_url: normalize_url(api_url.into()),
+ pve_compat: false,
+ userid: userid.into(),
+ password: None,
}
}
@@ -109,7 +120,7 @@ impl Login {
Self {
api_url: normalize_url(api_url.into()),
userid: userid.into(),
- password: password.into(),
+ password: Some(password.into()),
pve_compat: false,
}
}
@@ -129,7 +140,7 @@ impl Login {
let request = api::CreateTicket {
new_format: self.pve_compat.then_some(true),
username: self.userid.clone(),
- password: Some(self.password.clone()),
+ password: self.password.clone(),
..Default::default()
};
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 12/21] login: add helpers to pass cookie values when parsing login responses
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (10 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 11/21] login: make password optional when creating Login requests Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 13/21] login: add `TicketResult::HttpOnly` member Shannon Sterz
` (10 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
depending on the context a client may or may not have access to
HttpOnly cookies. this change allows them to pass such values to
`proxmox-login` to take them into account when parsing login
responses.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/lib.rs | 89 +++++++++++++++++++++++++++++++++-------
1 file changed, 75 insertions(+), 14 deletions(-)
diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index 3b3558d0..52282052 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -162,10 +162,27 @@ impl Login {
&self,
body: &T,
) -> Result<TicketResult, ResponseError> {
- self.response_bytes(body.as_ref())
+ self.response_bytes(None, body.as_ref())
}
- fn response_bytes(&self, body: &[u8]) -> Result<TicketResult, ResponseError> {
+ /// Parse the result body of a [`CreateTicket`](api::CreateTicket) API request taking into
+ /// account potential tickets obtained via a `Set-Cookie` header.
+ ///
+ /// On success, this will either yield an [`Authentication`] or a [`SecondFactorChallenge`] if
+ /// Two-Factor-Authentication is required.
+ pub fn response_with_cookie_ticket<T: ?Sized + AsRef<[u8]>>(
+ &self,
+ cookie_ticket: Option<Ticket>,
+ body: &T,
+ ) -> Result<TicketResult, ResponseError> {
+ self.response_bytes(cookie_ticket, body.as_ref())
+ }
+
+ fn response_bytes(
+ &self,
+ cookie_ticket: Option<Ticket>,
+ body: &[u8],
+ ) -> Result<TicketResult, ResponseError> {
use ticket::TicketResponse;
let response: api::ApiResponse<api::CreateTicketResponse> = serde_json::from_slice(body)?;
@@ -175,6 +192,14 @@ impl Login {
return Err("ticket response contained unexpected userid".into());
}
+ // if a ticket was provided via a cookie, use it like a normal ticket
+ if let Some(ticket) = cookie_ticket {
+ check_ticket_userid(ticket.userid(), &self.userid)?;
+ return Ok(TicketResult::Full(
+ self.authentication_for(ticket, response)?,
+ ));
+ }
+
let ticket: TicketResponse = match response.ticket {
Some(ticket) => ticket.parse()?,
None => return Err("missing ticket".into()),
@@ -183,15 +208,7 @@ impl Login {
Ok(match ticket {
TicketResponse::Full(ticket) => {
check_ticket_userid(ticket.userid(), &self.userid)?;
- TicketResult::Full(Authentication {
- csrfprevention_token: response
- .csrfprevention_token
- .ok_or("missing CSRFPreventionToken in ticket response")?,
- clustername: response.clustername,
- api_url: self.api_url.clone(),
- userid: response.username,
- ticket,
- })
+ TicketResult::Full(self.authentication_for(ticket, response)?)
}
TicketResponse::Tfa(ticket, challenge) => {
@@ -205,6 +222,22 @@ impl Login {
}
})
}
+
+ fn authentication_for(
+ &self,
+ ticket: Ticket,
+ response: api::CreateTicketResponse,
+ ) -> Result<Authentication, ResponseError> {
+ Ok(Authentication {
+ csrfprevention_token: response
+ .csrfprevention_token
+ .ok_or("missing CSRFPreventionToken in ticket response")?,
+ clustername: response.clustername,
+ api_url: self.api_url.clone(),
+ userid: response.username,
+ ticket,
+ })
+ }
}
/// This is the result of a ticket call. It will either yield a final ticket, or a TFA challenge.
@@ -310,10 +343,24 @@ impl SecondFactorChallenge {
&self,
body: &T,
) -> Result<Authentication, ResponseError> {
- self.response_bytes(body.as_ref())
+ self.response_bytes(None, body.as_ref())
}
- fn response_bytes(&self, body: &[u8]) -> Result<Authentication, ResponseError> {
+ /// Deal with the API's response object to extract the ticket either from a cookie or the
+ /// response itself.
+ pub fn response_with_cookie_ticket<T: ?Sized + AsRef<[u8]>>(
+ &self,
+ cookie_ticket: Option<Ticket>,
+ body: &T,
+ ) -> Result<Authentication, ResponseError> {
+ self.response_bytes(cookie_ticket, body.as_ref())
+ }
+
+ fn response_bytes(
+ &self,
+ cookie_ticket: Option<Ticket>,
+ body: &[u8],
+ ) -> Result<Authentication, ResponseError> {
let response: api::ApiResponse<api::CreateTicketResponse> = serde_json::from_slice(body)?;
let response = response.data.ok_or("missing response data")?;
@@ -321,7 +368,21 @@ impl SecondFactorChallenge {
return Err("ticket response contained unexpected userid".into());
}
- let ticket: Ticket = response.ticket.ok_or("no ticket in response")?.parse()?;
+ // get the ticket from:
+ // 1. the cookie if possible -> new HttpOnly authentication outside of the browser
+ // 2. just the `ticket_info` -> new HttpOnly authentication inside a browser context or
+ // similar, assume the ticket is handle by that
+ // 3. the `ticket` field -> old authentication flow where we handle the ticket ourselves
+ let ticket: Ticket = cookie_ticket
+ .ok_or(ResponseError::from("no ticket in response"))
+ .or_else(|e| {
+ response
+ .ticket_info
+ .or(response.ticket)
+ .ok_or(e)
+ .and_then(|t| t.parse().map_err(|e: TicketError| e.into()))
+ })?;
+
check_ticket_userid(ticket.userid(), &self.userid)?;
Ok(Authentication {
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 13/21] login: add `TicketResult::HttpOnly` member
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (11 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 12/21] login: add helpers to pass cookie values when parsing login responses Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 14/21] login: add helper to check whether a ticket is just informational Shannon Sterz
` (9 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
this allows client to be aware that the ticket they manage is
informational only and that the real ticket should have been set via
a HttpOnly cookie.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/lib.rs | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/proxmox-login/src/lib.rs b/proxmox-login/src/lib.rs
index 52282052..e97ece7b 100644
--- a/proxmox-login/src/lib.rs
+++ b/proxmox-login/src/lib.rs
@@ -200,9 +200,22 @@ impl Login {
));
}
+ // `ticket_info` is set when the server sets the ticket via an HttpOnly cookie. this also
+ // means we do not have access to the cookie itself which happens for example in a browser.
+ // assume that the cookie is handled properly by the context (browser) and don't worry
+ // about handling it ourselves.
+ if let Some(ref ticket) = response.ticket_info {
+ let ticket = ticket.parse()?;
+ return Ok(TicketResult::HttpOnly(
+ self.authentication_for(ticket, response)?,
+ ));
+ }
+
+ // old authentication flow where we needed to handle the ticket ourselves even in the
+ // browser etc.
let ticket: TicketResponse = match response.ticket {
- Some(ticket) => ticket.parse()?,
- None => return Err("missing ticket".into()),
+ Some(ref ticket) => ticket.parse()?,
+ None => return Err("no ticket information in response".into()),
};
Ok(match ticket {
@@ -250,6 +263,9 @@ pub enum TicketResult {
/// The response returned a Two-Factor-Authentication challenge.
TfaRequired(SecondFactorChallenge),
+
+ /// The response returned a valid ticket as an HttpOnly cookie.
+ HttpOnly(Authentication),
}
/// A ticket call can returned a TFA challenge. The user should inspect the
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 14/21] login: add helper to check whether a ticket is just informational
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (12 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 13/21] login: add `TicketResult::HttpOnly` member Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 15/21] login: add functions to specify full cookie names Shannon Sterz
` (8 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
tickets that end in `::ticketinfo` are not properly signed and just
include information such as the timestamp when the ticket was created.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/ticket.rs | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/proxmox-login/src/ticket.rs b/proxmox-login/src/ticket.rs
index 24606da5..dc70f913 100644
--- a/proxmox-login/src/ticket.rs
+++ b/proxmox-login/src/ticket.rs
@@ -94,6 +94,12 @@ impl Ticket {
}
}
+ /// Returns true when this is not a signed ticket, but just the information contained in a
+ /// ticket without a valid signature
+ pub fn is_info_only(&self) -> bool {
+ self.data.ends_with("::ticketinfo")
+ }
+
/// Get the cookie in the form `<PRODUCT>AuthCookie=Ticket`.
pub fn cookie(&self) -> String {
format!("{}AuthCookie={}", self.product(), self.data)
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 15/21] login: add functions to specify full cookie names
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (13 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 14/21] login: add helper to check whether a ticket is just informational Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 16/21] client: add compatibility with HttpOnly cookies Shannon Sterz
` (7 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
previously the name in which the ticket was send was derived by the
product abbreviation in the ticket itself. the assumption was that
authentication cookies would always have a name like this:
`<PRODUCT_ABBREVIATION>AuthCookie`.
this commit adds helpers that allow specifying the cookie's name by
users of this crate.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-login/src/ticket.rs | 47 ++++++++++++++++++++++++++++++++-----
1 file changed, 41 insertions(+), 6 deletions(-)
diff --git a/proxmox-login/src/ticket.rs b/proxmox-login/src/ticket.rs
index dc70f913..4b28f26e 100644
--- a/proxmox-login/src/ticket.rs
+++ b/proxmox-login/src/ticket.rs
@@ -104,6 +104,10 @@ impl Ticket {
pub fn cookie(&self) -> String {
format!("{}AuthCookie={}", self.product(), self.data)
}
+
+ pub fn cookie_with_name(&self, name: &str) -> String {
+ format!("{name}={}", self.data)
+ }
}
/// Whether a ticket should be refreshed or is already invalid and needs to be completely renewed.
@@ -239,19 +243,50 @@ impl Authentication {
self.ticket.cookie()
}
+ /// Get the ticket cookie in the form `<name>Ticket`.
+ pub fn cookie_with_name(&self, name: &str) -> String {
+ self.ticket.cookie_with_name(name)
+ }
+
#[cfg(feature = "http")]
/// Add authentication headers to a request.
///
/// This is equivalent to doing:
/// ```ignore
- /// request
- /// .header(http::header::COOKIE, auth.cookie())
- /// .header(proxmox_login::CSRF_HEADER_NAME, &auth.csrfprevention_token)
+ /// let request = if self.ticket.is_info_only() {
+ /// request
+ /// } else {
+ /// request.header(http::header::COOKIE, self.cookie())
+ /// };
+ /// request.header(crate::CSRF_HEADER_NAME, &self.csrfprevention_token)
/// ```
pub fn set_auth_headers(&self, request: http::request::Builder) -> http::request::Builder {
- request
- .header(http::header::COOKIE, self.cookie())
- .header(crate::CSRF_HEADER_NAME, &self.csrfprevention_token)
+ let request = if self.ticket.is_info_only() {
+ // don't set the cookie header if we don't have access to a full ticket
+ request
+ } else {
+ request.header(http::header::COOKIE, self.cookie())
+ };
+
+ request.header(crate::CSRF_HEADER_NAME, &self.csrfprevention_token)
+ }
+
+ #[cfg(feature = "http")]
+ /// Add authentication headers to a request and specify the name of the cookie in which the
+ /// ticket is set.
+ pub fn set_auth_headers_with_cookie_name(
+ &self,
+ request: http::request::Builder,
+ name: &str,
+ ) -> http::request::Builder {
+ let request = if self.ticket.is_info_only() {
+ // don't set the cookie header if we don't have access to a full ticket
+ request
+ } else {
+ request.header(http::header::COOKIE, self.cookie_with_name(name))
+ };
+
+ request.header(crate::CSRF_HEADER_NAME, &self.csrfprevention_token)
}
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 16/21] client: add compatibility with HttpOnly cookies
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (14 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 15/21] login: add functions to specify full cookie names Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 17/21] client: specify cookie names for authentication headers where possible Shannon Sterz
` (6 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
this should make it possible to use the proxmox-client crate outside
of context where HttpOnly cookies are handled for us. if a cookie
name is provided to a client, it tries to find a corresponding
`Set-Cookie` header in the login response and passes tries to parse
it as a ticket. that ticket is then passed on to proxmox-login like
any regular ticket.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-client/src/client.rs | 69 +++++++++++++++++++++++++++---------
1 file changed, 52 insertions(+), 17 deletions(-)
diff --git a/proxmox-client/src/client.rs b/proxmox-client/src/client.rs
index 9b078a98..07a53873 100644
--- a/proxmox-client/src/client.rs
+++ b/proxmox-client/src/client.rs
@@ -12,6 +12,7 @@ use hyper::body::{Body, HttpBody};
use openssl::hash::MessageDigest;
use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode};
use openssl::x509::{self, X509};
+use proxmox_login::Ticket;
use serde::Serialize;
use proxmox_login::ticket::Validity;
@@ -67,6 +68,7 @@ pub struct Client {
auth: Mutex<Option<Arc<AuthenticationKind>>>,
client: Arc<proxmox_http::client::Client>,
pve_compat: bool,
+ cookie_name: Option<String>,
}
impl Client {
@@ -75,6 +77,13 @@ impl Client {
Client::with_client(api_url, Arc::new(proxmox_http::client::Client::new()))
}
+ pub fn new_with_cookie(api_url: Uri, cookie_name: &str) -> Self {
+ let mut client =
+ Client::with_client(api_url, Arc::new(proxmox_http::client::Client::new()));
+ client.set_cookie_name(cookie_name);
+ client
+ }
+
/// Instantiate a client for an API with a given HTTP client instance.
pub fn with_client(api_url: Uri, client: Arc<proxmox_http::client::Client>) -> Self {
Self {
@@ -82,6 +91,7 @@ impl Client {
auth: Mutex::new(None),
client,
pve_compat: false,
+ cookie_name: None,
}
}
@@ -174,6 +184,10 @@ impl Client {
self.pve_compat = compatibility;
}
+ pub fn set_cookie_name(&mut self, cookie_name: &str) {
+ self.cookie_name = Some(cookie_name.to_string());
+ }
+
/// Get the currently used API url.
pub fn api_url(&self) -> &Uri {
&self.api_url
@@ -309,7 +323,10 @@ impl Client {
Ok(())
}
- async fn do_login_request(&self, request: proxmox_login::Request) -> Result<Vec<u8>, Error> {
+ async fn do_login_request(
+ &self,
+ request: proxmox_login::Request,
+ ) -> Result<(Option<Ticket>, Vec<u8>), Error> {
let request = http::Request::builder()
.method(Method::POST)
.uri(request.url)
@@ -330,10 +347,26 @@ impl Client {
return Err(Error::api(api_response.status(), "authentication failed"));
}
- let (_, body) = api_response.into_parts();
+ let (parts, body) = api_response.into_parts();
let body = read_body(body).await?;
- Ok(body)
+ let ticket: Option<Ticket> = self.cookie_name.as_ref().and_then(|cookie_name| {
+ parts
+ .headers
+ .get_all(http::header::SET_COOKIE)
+ .iter()
+ .filter_map(|c| c.to_str().ok())
+ .filter_map(|c| match (c.find('='), c.find(';')) {
+ (Some(begin), Some(end)) if begin < end && &c[..begin] == cookie_name => {
+ Some(&c[begin + 1..end])
+ }
+ _ => None,
+ })
+ .filter_map(|t| t.parse().ok())
+ .next()
+ });
+
+ Ok((ticket, body))
}
/// Attempt to refresh the current ticket.
@@ -349,10 +382,10 @@ impl Client {
let login = Login::renew(self.api_url.to_string(), auth.ticket.to_string())
.map_err(Error::Ticket)?;
- let api_response = self.do_login_request(login.request()).await?;
+ let (ticket, api_response) = self.do_login_request(login.request()).await?;
- match login.response(&api_response)? {
- TicketResult::Full(auth) => {
+ match login.response_with_cookie_ticket(ticket, &api_response)? {
+ TicketResult::Full(auth) | TicketResult::HttpOnly(auth) => {
*self.auth.lock().unwrap() = Some(Arc::new(auth.into()));
Ok(())
}
@@ -373,15 +406,17 @@ impl Client {
pub async fn login(&self, login: Login) -> Result<Option<SecondFactorChallenge>, Error> {
let login = login.pve_compatibility(self.pve_compat);
- let api_response = self.do_login_request(login.request()).await?;
-
- Ok(match login.response(&api_response)? {
- TicketResult::TfaRequired(challenge) => Some(challenge),
- TicketResult::Full(auth) => {
- *self.auth.lock().unwrap() = Some(Arc::new(auth.into()));
- None
- }
- })
+ let (ticket, api_response) = self.do_login_request(login.request()).await?;
+
+ Ok(
+ match login.response_with_cookie_ticket(ticket, &api_response)? {
+ TicketResult::TfaRequired(challenge) => Some(challenge),
+ TicketResult::Full(auth) | TicketResult::HttpOnly(auth) => {
+ *self.auth.lock().unwrap() = Some(Arc::new(auth.into()));
+ None
+ }
+ },
+ )
}
/// Attempt to finish a 2nd factor login.
@@ -393,9 +428,9 @@ impl Client {
challenge: SecondFactorChallenge,
challenge_response: proxmox_login::Request,
) -> Result<(), Error> {
- let api_response = self.do_login_request(challenge_response).await?;
+ let (ticket, api_response) = self.do_login_request(challenge_response).await?;
- let auth = challenge.response(&api_response)?;
+ let auth = challenge.response_with_cookie_ticket(ticket, &api_response)?;
*self.auth.lock().unwrap() = Some(Arc::new(auth.into()));
Ok(())
}
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH proxmox v3 17/21] client: specify cookie names for authentication headers where possible
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (15 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 16/21] client: add compatibility with HttpOnly cookies Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH yew-comp v3 18/21] HttpClient: add helpers to refresh HttpOnly cookies and remove them Shannon Sterz
` (5 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
if the client knows the auth cookie's name, it now passes it on to the
relevant parts of `proxmox-login` so that the ticket is send the
correct cookie
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
proxmox-client/src/client.rs | 50 +++++++++++++++++++++++++++++++-----
1 file changed, 44 insertions(+), 6 deletions(-)
diff --git a/proxmox-client/src/client.rs b/proxmox-client/src/client.rs
index 07a53873..a1b4eee2 100644
--- a/proxmox-client/src/client.rs
+++ b/proxmox-client/src/client.rs
@@ -222,8 +222,12 @@ impl Client {
json_body: Option<String>,
// send an `Accept: application/json-seq` header.
streaming: bool,
+ cookie_name: &Option<String>,
) -> Result<(http::response::Parts, hyper::Body), Error> {
- let mut request = auth.set_auth_headers(Request::builder().method(method).uri(uri));
+ let mut request = auth.set_auth_headers_with_cookie_name(
+ Request::builder().method(method).uri(uri),
+ cookie_name,
+ );
if streaming {
request = request.header(http::header::ACCEPT, "application/json-seq");
}
@@ -270,9 +274,18 @@ impl Client {
method: Method,
uri: Uri,
json_body: Option<String>,
+ cookie_name: &Option<String>,
) -> Result<HttpApiResponse, Error> {
- let (response, body) =
- Self::send_authenticated_request(client, auth, method, uri, json_body, false).await?;
+ let (response, body) = Self::send_authenticated_request(
+ client,
+ auth,
+ method,
+ uri,
+ json_body,
+ false,
+ cookie_name,
+ )
+ .await?;
let body = read_body(body).await?;
let content_type = match response.headers.get(http::header::CONTENT_TYPE) {
@@ -475,7 +488,7 @@ impl HttpApiClient for Client {
let auth = self.login_auth()?;
let uri = self.build_uri(path_and_query)?;
let client = Arc::clone(&self.client);
- Self::authenticated_request(client, auth, method, uri, params).await
+ Self::authenticated_request(client, auth, method, uri, params, &self.cookie_name).await
})
}
@@ -500,8 +513,16 @@ impl HttpApiClient for Client {
let auth = self.login_auth()?;
let uri = self.build_uri(path_and_query)?;
let client = Arc::clone(&self.client);
- let (response, body) =
- Self::send_authenticated_request(client, auth, method, uri, params, true).await?;
+ let (response, body) = Self::send_authenticated_request(
+ client,
+ auth,
+ method,
+ uri,
+ params,
+ true,
+ &self.cookie_name,
+ )
+ .await?;
let content_type = match response.headers.get(http::header::CONTENT_TYPE) {
None => None,
@@ -575,6 +596,23 @@ impl AuthenticationKind {
}
}
+ pub fn set_auth_headers_with_cookie_name(
+ &self,
+ request: http::request::Builder,
+ cookie_name: &Option<String>,
+ ) -> http::request::Builder {
+ match self {
+ AuthenticationKind::Ticket(auth) => {
+ if let Some(name) = cookie_name {
+ auth.set_auth_headers_with_cookie_name(request, name)
+ } else {
+ auth.set_auth_headers(request)
+ }
+ }
+ AuthenticationKind::Token(auth) => auth.set_auth_headers(request),
+ }
+ }
+
pub fn userid(&self) -> &str {
match self {
AuthenticationKind::Ticket(auth) => &auth.userid,
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH yew-comp v3 18/21] HttpClient: add helpers to refresh HttpOnly cookies and remove them
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (16 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH proxmox v3 17/21] client: specify cookie names for authentication headers where possible Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH yew-comp v3 19/21] LoginPanel/http helpers: add support for handling HttpOnly cookies Shannon Sterz
` (4 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
the `refresh` function handles a refresh without needing a ticket, the
assumption is that the ticket is handled by an HttpOnly cookie that we
don't have access to.
the `logout` function requests a removal of the ticket by the server
as we cannot remove properly protected cookies from the client-side.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
src/http_client_wasm.rs | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/src/http_client_wasm.rs b/src/http_client_wasm.rs
index e838e79..22ccbdf 100644
--- a/src/http_client_wasm.rs
+++ b/src/http_client_wasm.rs
@@ -123,6 +123,25 @@ impl HttpClientWasm {
Ok(login.response(&resp)?)
}
+ pub async fn refresh(&self, username: impl Into<String>) -> Result<TicketResult, Error> {
+ let username = username.into();
+
+ let login = Login::renew_with_cookie("", username);
+ let request = login.request();
+ let request =
+ Self::post_request_builder(&request.url, request.content_type, &request.body)?;
+ let resp = self.fetch_request_text(request).await?;
+
+ Ok(login.response(&resp)?)
+ }
+
+
+ pub async fn logout(&self) -> Result<(), Error> {
+ self.request::<()>(http::Method::DELETE, "/api2/extjs/access/ticket", None)
+ .await?;
+ Ok(())
+ }
+
pub async fn login_tfa(
&self,
challenge: Rc<proxmox_login::SecondFactorChallenge>,
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH yew-comp v3 19/21] LoginPanel/http helpers: add support for handling HttpOnly cookies
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (17 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH yew-comp v3 18/21] HttpClient: add helpers to refresh HttpOnly cookies and remove them Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH yew-comp v3 20/21] http helpers: ask server to remove `__Host-` prefixed cookie on logout Shannon Sterz
` (3 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
this makes sure that we handle tickets that are stored in in HttpOnly
cookies correctly by:
- assuming that the properly sign ticket is in a cookie we can't
access when refreshing tickets and only an informational ticket is
available to us.
- handling `TicketResult::HttpOnly` correctly
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
src/http_helpers.rs | 30 ++++++++++++++++++++++++++----
src/login_panel.rs | 5 ++++-
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/http_helpers.rs b/src/http_helpers.rs
index 9f0403f..e2489cc 100644
--- a/src/http_helpers.rs
+++ b/src/http_helpers.rs
@@ -108,11 +108,25 @@ async fn ticket_refresh_loop() {
}
Validity::Refresh => {
let client = CLIENT.with(|c| Rc::clone(&*c.borrow()));
- if let Ok(TicketResult::Full(auth)) =
+
+ // if the ticket is not signed, there is no point in sending it, assume we
+ // are using a HttpOnly cookie that is properly handled by the
+ // browser/cookie anyway
+ let result = if data.ticket.is_info_only() {
+ client.refresh(&data.userid).await
+ } else {
client.login(&data.userid, &data.ticket.to_string()).await
- {
- log::info!("ticket_refresh_loop: Got ticket update.");
- client.set_auth(auth.clone());
+ };
+
+ match result {
+ // TODO: eventually deprecate support for `TicketResult::Full` and
+ // throw an error. this package should only ever be used in a browser
+ // context where authentication info should be set via HttpOnly cookies.
+ Ok(TicketResult::Full(auth)) | Ok(TicketResult::HttpOnly(auth)) => {
+ log::info!("ticket_refresh_loop: Got ticket update.");
+ client.set_auth(auth.clone());
+ }
+ _ => { /* do nothing */ }
}
}
Validity::Valid => { /* do nothing */ }
@@ -157,11 +171,19 @@ pub async fn http_login(
.await?;
match ticket_result {
+ // TODO: eventually deprecate support for `TicketResult::Full` and
+ // throw an error. this package should only ever be used in a browser
+ // context where authentication info should be set via HttpOnly cookies.
TicketResult::Full(auth) => {
client.set_auth(auth.clone());
update_global_client(client);
Ok(TicketResult::Full(auth))
}
+ TicketResult::HttpOnly(auth) => {
+ client.set_auth(auth.clone());
+ update_global_client(client);
+ Ok(TicketResult::HttpOnly(auth))
+ }
challenge => Ok(challenge),
}
}
diff --git a/src/login_panel.rs b/src/login_panel.rs
index d979bbb..aecbf80 100644
--- a/src/login_panel.rs
+++ b/src/login_panel.rs
@@ -74,7 +74,10 @@ impl ProxmoxLoginPanel {
let link = ctx.link().clone();
self.async_pool.spawn(async move {
match crate::http_login(username, password, realm).await {
- Ok(TicketResult::Full(info)) => {
+ // TODO: eventually deprecate support for `TicketResult::Full` and
+ // throw an error. this package should only ever be used in a browser
+ // context where authentication info should be set via HttpOnly cookies.
+ Ok(TicketResult::Full(info)) | Ok(TicketResult::HttpOnly(info)) => {
link.send_message(Msg::Login(info));
}
Ok(TicketResult::TfaRequired(challenge)) => {
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH yew-comp v3 20/21] http helpers: ask server to remove `__Host-` prefixed cookie on logout
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (18 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH yew-comp v3 19/21] LoginPanel/http helpers: add support for handling HttpOnly cookies Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:07 ` [pdm-devel] [PATCH datacenter-manager v3 21/21] api: switch ticket endpoint over to new http only endpoint Shannon Sterz
` (2 subsequent siblings)
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
when clearing the authentication data, we can no longer remove the
cookie that stores the ticket once it is properly protected. so ask
the server to do this for us.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
src/http_helpers.rs | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/http_helpers.rs b/src/http_helpers.rs
index e2489cc..b1425d5 100644
--- a/src/http_helpers.rs
+++ b/src/http_helpers.rs
@@ -148,11 +148,19 @@ pub fn http_get_auth() -> Option<Authentication> {
CLIENT.with(move |c| c.borrow().get_auth())
}
+thread_local! {
+ static LOGOUT_GUARD: RefCell<Option<AsyncAbortGuard>> = const { RefCell::new(None) };
+}
+
pub fn http_clear_auth() {
- CLIENT.with(move |c| {
- c.borrow_mut().clear_auth();
- crate::clear_auth_cookie(c.borrow().product().auth_cookie_name());
+ let abort_guard = AsyncAbortGuard::spawn(async move {
+ let client = CLIENT.with(|c| Rc::clone(&*c.borrow()));
+ let _ = client.logout().await;
+ client.clear_auth();
+ crate::clear_auth_cookie(client.product().auth_cookie_name());
});
+
+ LOGOUT_GUARD.with_borrow_mut(|v| *v = Some(abort_guard));
}
pub async fn http_login(
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* [pdm-devel] [PATCH datacenter-manager v3 21/21] api: switch ticket endpoint over to new http only endpoint
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (19 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH yew-comp v3 20/21] http helpers: ask server to remove `__Host-` prefixed cookie on logout Shannon Sterz
@ 2025-02-27 14:07 ` Shannon Sterz
2025-02-27 14:08 ` [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
2025-03-04 12:08 ` Shannon Sterz
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:07 UTC (permalink / raw)
To: pdm-devel
also adds a delete method for remove "__Host-"-prefixed HttpOnly
cookies.
Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
server/src/api/access/mod.rs | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/server/src/api/access/mod.rs b/server/src/api/access/mod.rs
index 738d382..befe2f1 100644
--- a/server/src/api/access/mod.rs
+++ b/server/src/api/access/mod.rs
@@ -29,7 +29,9 @@ const SUBDIRS: SubdirMap = &sorted!([
("tfa", &tfa::ROUTER),
(
"ticket",
- &Router::new().post(&proxmox_auth_api::api::API_METHOD_CREATE_TICKET),
+ &Router::new()
+ .post(&proxmox_auth_api::api::API_METHOD_CREATE_TICKET_HTTP_ONLY)
+ .delete(&proxmox_auth_api::api::API_METHOD_LOGOUT),
),
("users", &users::ROUTER),
]);
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (20 preceding siblings ...)
2025-02-27 14:07 ` [pdm-devel] [PATCH datacenter-manager v3 21/21] api: switch ticket endpoint over to new http only endpoint Shannon Sterz
@ 2025-02-27 14:08 ` Shannon Sterz
2025-03-04 12:08 ` Shannon Sterz
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-02-27 14:08 UTC (permalink / raw)
To: Shannon Sterz, pdm-devel
On Thu Feb 27, 2025 at 3:06 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 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.)
>
>
>
> 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 | 247 +++++++++++++++++++++--------
> 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, 629 insertions(+), 138 deletions(-)
>
sorry forgot to add Maximiliano's R-b tag for the v2:
Reviewed-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>
> 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, 692 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
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects
2025-02-27 14:06 [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
` (21 preceding siblings ...)
2025-02-27 14:08 ` [pdm-devel] [PATCH datacenter-manager/proxmox/yew-comp v3 00/21] use HttpOnly cookies in new projects Shannon Sterz
@ 2025-03-04 12:08 ` Shannon Sterz
22 siblings, 0 replies; 24+ messages in thread
From: Shannon Sterz @ 2025-03-04 12:08 UTC (permalink / raw)
To: Shannon Sterz, pdm-devel
Superseeded-by: https://lore.proxmox.com/pdm-devel/20250304120506.135617-1-s.sterz@proxmox.com/
On Thu Feb 27, 2025 at 3:06 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 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.)
>
>
>
> 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 | 247 +++++++++++++++++++++--------
> 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, 629 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, 692 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
^ permalink raw reply [flat|nested] 24+ messages in thread