all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs
@ 2025-07-10 13:50 Shannon Sterz
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints Shannon Sterz
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Shannon Sterz @ 2025-07-10 13:50 UTC (permalink / raw)
  To: pbs-devel

# Summary

this series adds an authentication flow based on http only cookies for
proxmox backup server. at the moment this authentication flow is opt-in
in order to not break older clients that may still rely on the previous
authentication flow.

this series is split into three parts:

1. prepare proxmox-auth-api to be used with extjs and implements a new
   ticket endpoint for pbs. this endpoint requires clients to provide a
   boolean parameter `http-only` as `true` for it to switch to the new
   http-only based authentication flow.
2. adapt proxmox backup server's ui components to always use the http
   only based authentication flow. this should make cookies
   inaccessible to any javascript-based attack in the browser,
   providing an extra layer of security.
3. prepare pbs-client for potential servers that may no longer provide
   the previous authentication flow. the point of already adding this
   now, is to be prepare update-hesitant users for a future without the
   old authentication flow. if the old authentication flow is dropped in
   the future, more users will already have a client version that can
   adapt to the new flow.

# Why not opt the `pbs-client` into the new flow?

the client is deliberatelly not opted into the new authentication flow
for the following reasons:

- http only cookies are only an effective security mechanism within a
  browser context. the client simply does not benefit from this extra
  layer of protection. the attacks http only cookies protect against,
  simply don't exist here.
- opting the client in unconditionally isn't possible. older pbs servers
  would complain about the additional api parameters provided by the
  client. this means:

    + the client would either need to try the http-only flow and once
      that fails, fall back to the older authentication flow.
    + or query (and possibly cache) the server version and check if the
      version is new enough to support the new authentication flow.

both approaches are more error-prone and produce additional network
overhead than simply not opting the client into the new flow. causing
api errors may also produce a lot of false warnings when monitoring pbs.
so there are no benefits, but potential downsides. hence, the client
will not yet be opted into the new authentication flow.

proxmox:

Shannon Sterz (1):
  auth-api: include meta information required by extjs in api endpoints

 proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)


proxmox-backup:

Shannon Sterz (3):
  api: access: add opt-in http only ticket authentication flow
  ui: opt into the new http-only ticket authentication flow
  client: adapt pbs client to also handle http-only flows correctly

 pbs-client/src/http_client.rs | 70 ++++++++++++++++++++++++++++---
 src/api2/access/mod.rs        | 77 +++++++++++++++++++++++++++++++++--
 www/Application.js            | 12 +++++-
 www/LoginView.js              |  4 +-
 www/MainView.js               |  1 +
 www/Utils.js                  |  6 +++
 6 files changed, 159 insertions(+), 11 deletions(-)


Summary over all repositories:
  7 files changed, 169 insertions(+), 14 deletions(-)

--
Generated by git-murpp 0.8.1


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
@ 2025-07-10 13:50 ` Shannon Sterz
  2025-07-15 22:40   ` Thomas Lamprecht
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow Shannon Sterz
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Shannon Sterz @ 2025-07-10 13:50 UTC (permalink / raw)
  To: pbs-devel

otherwise extjs will assume the requests failed even though they were
successful.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/proxmox-auth-api/src/api/access.rs b/proxmox-auth-api/src/api/access.rs
index fdf52185..f5111d4a 100644
--- a/proxmox-auth-api/src/api/access.rs
+++ b/proxmox-auth-api/src/api/access.rs
@@ -6,7 +6,6 @@ use http::Response;
 use openssl::hash::MessageDigest;
 use serde_json::{json, Value};
 
-use proxmox_http::Body;
 use proxmox_rest_server::{extract_cookie, RestEnvironment};
 use proxmox_router::{
     http_err, ApiHandler, ApiMethod, ApiResponseFuture, Permission, RpcEnvironment,
@@ -93,7 +92,11 @@ fn logout_handler(
 
         Ok(Response::builder()
             .header(hyper::header::SET_COOKIE, host_cookie)
-            .body(Body::empty())?)
+            .body(
+                json!({"data": {}, "status": 200, "success": true })
+                    .to_string()
+                    .into(),
+            )?)
     })
 }
 
@@ -179,7 +182,11 @@ fn create_ticket_http_only(
             }
         }
 
-        Ok(response.body(json!({"data": ticket_response }).to_string().into())?)
+        Ok(response.body(
+            json!({"data": ticket_response, "status": 200, "success": true })
+                .to_string()
+                .into(),
+        )?)
     })
 }
 
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints Shannon Sterz
@ 2025-07-10 13:50 ` Shannon Sterz
  2025-07-23 12:57   ` Mira Limbeck
  2025-07-23 13:58   ` Maximiliano Sandoval
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 2/3] ui: opt into the new http-only " Shannon Sterz
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 13+ messages in thread
From: Shannon Sterz @ 2025-07-10 13:50 UTC (permalink / raw)
  To: pbs-devel

this new flow returns https only cookies providing an additional layer
of security for clients operating in a browser environment. opt-in
only to not break existing clients.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 src/api2/access/mod.rs | 77 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 4 deletions(-)

diff --git a/src/api2/access/mod.rs b/src/api2/access/mod.rs
index 832cdc66..b61b596e 100644
--- a/src/api2/access/mod.rs
+++ b/src/api2/access/mod.rs
@@ -2,14 +2,23 @@
 
 use anyhow::{bail, format_err, Error};
 
-use serde_json::Value;
+use hyper::header::CONTENT_TYPE;
+use hyper::http::request::Parts;
+use hyper::Response;
+use serde_json::{json, Value};
+
 use std::collections::HashMap;
 use std::collections::HashSet;
 
+use proxmox_auth_api::api::API_METHOD_CREATE_TICKET_HTTP_ONLY;
+use proxmox_auth_api::types::{CreateTicket, CreateTicketResponse};
 use proxmox_router::{
-    http_bail, http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap,
+    http_bail, http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture,
+    Permission, Router, RpcEnvironment, SubdirMap,
+};
+use proxmox_schema::{
+    api, AllOfSchema, ApiType, BooleanSchema, ObjectSchema, ParameterSchema, ReturnType,
 };
-use proxmox_schema::api;
 use proxmox_sortable_macro::sortable;
 
 use pbs_api_types::{
@@ -268,7 +277,9 @@ const SUBDIRS: SubdirMap = &sorted!([
     ),
     (
         "ticket",
-        &Router::new().post(&proxmox_auth_api::api::API_METHOD_CREATE_TICKET)
+        &Router::new()
+            .post(&API_METHOD_CREATE_TICKET_TOGGLE)
+            .delete(&proxmox_auth_api::api::API_METHOD_LOGOUT)
     ),
     ("openid", &openid::ROUTER),
     ("domains", &domain::ROUTER),
@@ -277,6 +288,64 @@ const SUBDIRS: SubdirMap = &sorted!([
     ("tfa", &tfa::ROUTER),
 ]);
 
+const API_METHOD_CREATE_TICKET_TOGGLE: ApiMethod = ApiMethod::new_full(
+    &proxmox_router::ApiHandler::AsyncHttpBodyParameters(&handle_ticket_toggle),
+    ParameterSchema::AllOf(&AllOfSchema::new(
+        "Either create a new HttpOnly ticket or a regular ticket.",
+        &[
+            &ObjectSchema::new(
+                "<INNER: Toggle between http only or legacy ticket endpoints.>",
+                &[(
+                    "http-only",
+                    true,
+                    &BooleanSchema::new(
+                        "Whether the http only authentication flow should be used.",
+                    )
+                    .default(false)
+                    .schema(),
+                )],
+            )
+            .schema(),
+            &CreateTicket::API_SCHEMA,
+        ],
+    )),
+)
+.returns(ReturnType::new(false, &CreateTicketResponse::API_SCHEMA))
+.protected(true)
+.access(None, &Permission::World);
+
+fn handle_ticket_toggle(
+    parts: Parts,
+    mut param: Value,
+    info: &'static ApiMethod,
+    mut rpcenv: Box<dyn RpcEnvironment>,
+) -> ApiResponseFuture {
+    // If the client specifies that they want to use http only cookies, prefer those.
+    if Some(true) == param["http-only"].take().as_bool() {
+        if let ApiHandler::AsyncHttpBodyParameters(handler) =
+            API_METHOD_CREATE_TICKET_HTTP_ONLY.handler
+        {
+            return handler(parts, param, info, rpcenv);
+        }
+    }
+
+    // Otherwise, default back to the previous ticket method.
+    Box::pin(async move {
+        let create_params: CreateTicket = serde_json::from_value(param)?;
+
+        let ticket_response =
+            proxmox_auth_api::api::create_ticket(create_params, rpcenv.as_mut()).await?;
+
+        let response = Response::builder().header(CONTENT_TYPE, "application/json");
+
+        Ok(response.body(
+            json!({"data": ticket_response, "status": 200, "success": true })
+                .to_string()
+                .into(),
+        )?)
+    })
+}
+
 pub const ROUTER: Router = Router::new()
     .get(&list_subdirs_api_method!(SUBDIRS))
     .subdirs(SUBDIRS);
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 2/3] ui: opt into the new http-only ticket authentication flow
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints Shannon Sterz
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow Shannon Sterz
@ 2025-07-10 13:50 ` Shannon Sterz
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 3/3] client: adapt pbs client to also handle http-only flows correctly Shannon Sterz
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2025-07-10 13:50 UTC (permalink / raw)
  To: pbs-devel

this should add additional protections for cookie stealing and xss
attacks.

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 www/Application.js | 12 +++++++++++-
 www/LoginView.js   |  4 +++-
 www/MainView.js    |  1 +
 www/Utils.js       |  6 ++++++
 4 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/www/Application.js b/www/Application.js
index 9e223522..fb0b2e14 100644
--- a/www/Application.js
+++ b/www/Application.js
@@ -18,7 +18,17 @@ Ext.define('PBS.Application', {
     logout: function () {
         var me = this;
         Proxmox.Utils.authClear();
-        me.changeView('loginview', true);
+        Proxmox.Utils.API2Request({
+            url: '/api2/extjs/access/ticket',
+            method: 'DELETE',
+            success: function () {
+                me.changeView('loginview', true);
+            },
+            failure: function ({ response }) {
+                // logout failed
+                console.error('could not log out', response);
+            }
+        });
     },
 
     changeView: function (view, skipCheck) {
diff --git a/www/LoginView.js b/www/LoginView.js
index 30e70d85..7cdf458b 100644
--- a/www/LoginView.js
+++ b/www/LoginView.js
@@ -83,6 +83,8 @@ Ext.define('PBS.LoginView', {
             }
             sp.set(saveunField.getStateId(), saveunField.getValue());
 
+            creds['http-only'] = true;
+
             try {
                 let resp = await Proxmox.Async.api2({
                     url: '/api2/extjs/access/ticket',
@@ -91,7 +93,7 @@ Ext.define('PBS.LoginView', {
                 });
 
                 let data = resp.result.data;
-                if (data.ticket.startsWith('PBS:!tfa!')) {
+                if (data.ticket?.startsWith('PBS:!tfa!')) {
                     data = await me.performTFAChallenge(data);
                 }
 
diff --git a/www/MainView.js b/www/MainView.js
index b5ae3605..70adfbce 100644
--- a/www/MainView.js
+++ b/www/MainView.js
@@ -172,6 +172,7 @@ Ext.define('PBS.MainView', {
                         params: {
                             username: Proxmox.UserName,
                             password: ticket,
+                            'http-only': true,
                         },
                         url: '/api2/json/access/ticket',
                         method: 'POST',
diff --git a/www/Utils.js b/www/Utils.js
index 30b4a6e7..7b165b67 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -8,6 +8,12 @@ Ext.define('PBS.Utils', {
     missingText: gettext('missing'),
 
     updateLoginData: function (data) {
+        if (data['ticket-info']) {
+            // we received a http only ticket response, the actually cookie is handled by the browser
+            // set the ticket field to use the information from `ticket-info`
+            data.ticket = data['ticket-info'];
+        }
+
         Proxmox.Utils.setAuthData(data);
     },
 
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [pbs-devel] [PATCH proxmox-backup 3/3] client: adapt pbs client to also handle http-only flows correctly
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
                   ` (2 preceding siblings ...)
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 2/3] ui: opt into the new http-only " Shannon Sterz
@ 2025-07-10 13:50 ` Shannon Sterz
  2025-07-23 12:56 ` [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Mira Limbeck
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2025-07-10 13:50 UTC (permalink / raw)
  To: pbs-devel

if we decide to make the http-only flow opt-out or remove the previous
authentication flow entirely, prepare the client to speak to servers
that only support http-only cookie tickets

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 pbs-client/src/http_client.rs | 70 ++++++++++++++++++++++++++++++++---
 1 file changed, 65 insertions(+), 5 deletions(-)

diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index 0b415cac..f6aedda1 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -7,6 +7,7 @@ use bytes::Bytes;
 use futures::*;
 use http_body_util::{BodyDataStream, BodyExt};
 use hyper::body::Incoming;
+use hyper::header::SET_COOKIE;
 use hyper::http::header::HeaderValue;
 use hyper::http::Uri;
 use hyper::http::{Request, Response};
@@ -111,11 +112,16 @@ mod resolver {
 /// certain error conditions. Keep it generous, to avoid false-positive under high load.
 const HTTP_TIMEOUT: Duration = Duration::from_secs(2 * 60);
 
+const PROXMOX_BACKUP_AUTH_COOKIE: &str = "PBSAuthCookie";
+const PROXMOX_BACKUP_PREFIXED_AUTH_COOKIE: &str = "__Host-PBSAuthCookie";
+
 #[derive(Clone)]
 pub struct AuthInfo {
     pub auth_id: Authid,
     pub ticket: String,
     pub token: String,
+    // Whether the server uses HttpOnly cookies for authentication
+    pub http_only: bool,
 }
 
 pub struct HttpClientOptions {
@@ -504,6 +510,7 @@ impl HttpClient {
             auth_id: auth_id.clone(),
             ticket: password.clone(),
             token: "".to_string(),
+            http_only: false,
         }));
 
         let server2 = server.to_string();
@@ -725,10 +732,18 @@ impl HttpClient {
                 HeaderValue::from_str(&enc_api_token).unwrap(),
             );
         } else {
+            let cookie_name = if auth.http_only {
+                // server has switched to http only flow, provide ticket in properly prefixed cookie
+                PROXMOX_BACKUP_PREFIXED_AUTH_COOKIE
+            } else {
+                PROXMOX_BACKUP_AUTH_COOKIE
+            };
+
             let enc_ticket = format!(
-                "PBSAuthCookie={}",
+                "{cookie_name}={}",
                 percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)
             );
+
             req.headers_mut()
                 .insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap());
             req.headers_mut().insert(
@@ -767,10 +782,18 @@ impl HttpClient {
 
         let auth = self.login().await?;
 
+        let cookie_name = if auth.http_only {
+            // server has switched to http only flow, provide ticket in properly prefixed cookie
+            PROXMOX_BACKUP_PREFIXED_AUTH_COOKIE
+        } else {
+            PROXMOX_BACKUP_AUTH_COOKIE
+        };
+
         let enc_ticket = format!(
-            "PBSAuthCookie={}",
+            "{cookie_name}={}",
             percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)
         );
+
         req.headers_mut()
             .insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap());
 
@@ -836,10 +859,18 @@ impl HttpClient {
                 HeaderValue::from_str(&enc_api_token).unwrap(),
             );
         } else {
+            let cookie_name = if auth.http_only {
+                // server has switched to http only flow, provide ticket in properly prefixed cookie
+                PROXMOX_BACKUP_PREFIXED_AUTH_COOKIE
+            } else {
+                PROXMOX_BACKUP_AUTH_COOKIE
+            };
+
             let enc_ticket = format!(
-                "PBSAuthCookie={}",
+                "{cookie_name}={}",
                 percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)
             );
+
             req.headers_mut()
                 .insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap());
             req.headers_mut().insert(
@@ -905,14 +936,43 @@ impl HttpClient {
             "/api2/json/access/ticket",
             Some(data),
         )?;
-        let cred = Self::api_request(client, req).await?;
+
+        let res = tokio::time::timeout(HTTP_TIMEOUT, client.request(req))
+            .await
+            .map_err(|_| format_err!("http request timed out"))??;
+
+        // check if the headers contain a newer http only cookie
+        let http_only_ticket = res
+            .headers()
+            .get_all(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] == PROXMOX_BACKUP_PREFIXED_AUTH_COOKIE =>
+                {
+                    Some(c[begin + 1..end].to_string())
+                }
+                _ => None,
+            })
+            .next();
+
+        // if the headers contained a new http only cookie, the server switched to providing these
+        // by default. this means that older cookies may no longer be supported, so switch to using
+        // the new cookie name.
+        let http_only = http_only_ticket.is_some();
+
+        let cred = Self::api_response(res).await?;
         let auth = AuthInfo {
             auth_id: cred["data"]["username"].as_str().unwrap().parse()?,
-            ticket: cred["data"]["ticket"].as_str().unwrap().to_owned(),
+            ticket: http_only_ticket
+                .or(cred["data"]["ticket"].as_str().map(|t| t.to_string()))
+                .unwrap(),
             token: cred["data"]["CSRFPreventionToken"]
                 .as_str()
                 .unwrap()
                 .to_owned(),
+            http_only,
         };
 
         Ok(auth)
-- 
2.39.5



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints Shannon Sterz
@ 2025-07-15 22:40   ` Thomas Lamprecht
       [not found]     ` <DBDBXGTI71WP.3V2J3DEMNK1DL@proxmox.com>
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2025-07-15 22:40 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Shannon Sterz

Am 10.07.25 um 15:50 schrieb Shannon Sterz:
> otherwise extjs will assume the requests failed even though they were
> successful.

Potentially dumb question, but is this then only returning that for the
extjs formatter or also for the json one?

> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/proxmox-auth-api/src/api/access.rs b/proxmox-auth-api/src/api/access.rs
> index fdf52185..f5111d4a 100644
> --- a/proxmox-auth-api/src/api/access.rs
> +++ b/proxmox-auth-api/src/api/access.rs
> @@ -6,7 +6,6 @@ use http::Response;
>  use openssl::hash::MessageDigest;
>  use serde_json::{json, Value};
>  
> -use proxmox_http::Body;
>  use proxmox_rest_server::{extract_cookie, RestEnvironment};
>  use proxmox_router::{
>      http_err, ApiHandler, ApiMethod, ApiResponseFuture, Permission, RpcEnvironment,
> @@ -93,7 +92,11 @@ fn logout_handler(
>  
>          Ok(Response::builder()
>              .header(hyper::header::SET_COOKIE, host_cookie)
> -            .body(Body::empty())?)
> +            .body(
> +                json!({"data": {}, "status": 200, "success": true })
> +                    .to_string()
> +                    .into(),
> +            )?)
>      })
>  }
>  
> @@ -179,7 +182,11 @@ fn create_ticket_http_only(
>              }
>          }
>  
> -        Ok(response.body(json!({"data": ticket_response }).to_string().into())?)
> +        Ok(response.body(
> +            json!({"data": ticket_response, "status": 200, "success": true })
> +                .to_string()
> +                .into(),
> +        )?)
>      })
>  }
>  



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints
       [not found]     ` <DBDBXGTI71WP.3V2J3DEMNK1DL@proxmox.com>
@ 2025-07-22 20:21       ` Thomas Lamprecht
  2025-07-23 15:18         ` Shannon Sterz
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Lamprecht @ 2025-07-22 20:21 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: Proxmox Backup Server development discussion

Am 16.07.25 um 10:17 schrieb Shannon Sterz:
> On Wed Jul 16, 2025 at 12:40 AM CEST, Thomas Lamprecht wrote:
>> Am 10.07.25 um 15:50 schrieb Shannon Sterz:
>>> otherwise extjs will assume the requests failed even though they were
>>> successful.
>>
>> Potentially dumb question, but is this then only returning that for the
>> extjs formatter or also for the json one?
> 
> sadly this impacts all formaters as this type of api handler returns a
> `Response` right away instead of a passing it through a formater like
> other handler types. the problem with the other handler types is, that
> they don't let you access the requests header values as easily.
> 
> the nicest way to resolve this would probably be to pass down the
> formater in `handle_api_request` [1] to endpoints that return an
> `ApiResponseFuture`. so that they can handle the formatting themselves
> (some api endpoints like those that download raw text files etc.
> probably won't need the formater).

It might be better to expose through the (RPC/REST) environment in the
mid term, could be useful for other stuff too. Or alternatively expose
a structured way to pass back a cookie if the methods signals support
in the schema.

But either way: we can go this route for now, while looking at this more
closely down the line, when it either causes problems (IMO unlikely) or
when we need something similar for another reason anyway and can clean
this up here then too.

Anyway, if you get someone to give this a spin and at least a T-b here,
ideally a review too, I'd be still open to take this in.


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
                   ` (3 preceding siblings ...)
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 3/3] client: adapt pbs client to also handle http-only flows correctly Shannon Sterz
@ 2025-07-23 12:56 ` Mira Limbeck
  2025-07-23 14:05 ` Maximiliano Sandoval
  2025-07-23 15:15 ` Shannon Sterz
  6 siblings, 0 replies; 13+ messages in thread
From: Mira Limbeck @ 2025-07-23 12:56 UTC (permalink / raw)
  To: pbs-devel

On 7/10/25 15:50, Shannon Sterz wrote:
> # Summary
> 
> this series adds an authentication flow based on http only cookies for
> proxmox backup server. at the moment this authentication flow is opt-in
> in order to not break older clients that may still rely on the previous
> authentication flow.
> 
> this series is split into three parts:
> 
> 1. prepare proxmox-auth-api to be used with extjs and implements a new
>    ticket endpoint for pbs. this endpoint requires clients to provide a
>    boolean parameter `http-only` as `true` for it to switch to the new
>    http-only based authentication flow.
> 2. adapt proxmox backup server's ui components to always use the http
>    only based authentication flow. this should make cookies
>    inaccessible to any javascript-based attack in the browser,
>    providing an extra layer of security.
> 3. prepare pbs-client for potential servers that may no longer provide
>    the previous authentication flow. the point of already adding this
>    now, is to be prepare update-hesitant users for a future without the
>    old authentication flow. if the old authentication flow is dropped in
>    the future, more users will already have a client version that can
>    adapt to the new flow.
> 
> # Why not opt the `pbs-client` into the new flow?
> 
> the client is deliberatelly not opted into the new authentication flow
> for the following reasons:
> 
> - http only cookies are only an effective security mechanism within a
>   browser context. the client simply does not benefit from this extra
>   layer of protection. the attacks http only cookies protect against,
>   simply don't exist here.
> - opting the client in unconditionally isn't possible. older pbs servers
>   would complain about the additional api parameters provided by the
>   client. this means:
> 
>     + the client would either need to try the http-only flow and once
>       that fails, fall back to the older authentication flow.
>     + or query (and possibly cache) the server version and check if the
>       version is new enough to support the new authentication flow.
> 
> both approaches are more error-prone and produce additional network
> overhead than simply not opting the client into the new flow. causing
> api errors may also produce a lot of false warnings when monitoring pbs.
> so there are no benefits, but potential downsides. hence, the client
> will not yet be opted into the new authentication flow.
> 
> proxmox:
> 
> Shannon Sterz (1):
>   auth-api: include meta information required by extjs in api endpoints
> 
>  proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> 
> proxmox-backup:
> 
> Shannon Sterz (3):
>   api: access: add opt-in http only ticket authentication flow
>   ui: opt into the new http-only ticket authentication flow
>   client: adapt pbs client to also handle http-only flows correctly
> 
>  pbs-client/src/http_client.rs | 70 ++++++++++++++++++++++++++++---
>  src/api2/access/mod.rs        | 77 +++++++++++++++++++++++++++++++++--
>  www/Application.js            | 12 +++++-
>  www/LoginView.js              |  4 +-
>  www/MainView.js               |  1 +
>  www/Utils.js                  |  6 +++
>  6 files changed, 159 insertions(+), 11 deletions(-)
> 
> 
> Summary over all repositories:
>   7 files changed, 169 insertions(+), 14 deletions(-)
> 
> --
> Generated by git-murpp 0.8.1
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 

Tested this series on a freshly installed pbs4, updated to the latest
version.
Tests:
- browser login -> check cookies for __Host-PBSAuthCookie
   -> had the `HttpOnly` flag set
- browser logout -> cookie got deleted
- old authentication flow (non-HttpOnly) with:
  - proxmox-backup-client backup via PVE 8 (pbs client 3) web interface
  - proxmox-backup-client list via PVE 8 (pbs client 3) web interface

So consider this:
Tested-by: Mira Limbeck <m.limbeck@proxmox.com>


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow Shannon Sterz
@ 2025-07-23 12:57   ` Mira Limbeck
  2025-07-23 13:58   ` Maximiliano Sandoval
  1 sibling, 0 replies; 13+ messages in thread
From: Mira Limbeck @ 2025-07-23 12:57 UTC (permalink / raw)
  To: pbs-devel

On 7/10/25 15:50, Shannon Sterz wrote:
> this new flow returns https only cookies providing an additional layer
> of security for clients operating in a browser environment. opt-in
> only to not break existing clients.
> 
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  src/api2/access/mod.rs | 77 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 73 insertions(+), 4 deletions(-)
> 
> diff --git a/src/api2/access/mod.rs b/src/api2/access/mod.rs
> index 832cdc66..b61b596e 100644
> --- a/src/api2/access/mod.rs
> +++ b/src/api2/access/mod.rs
> @@ -2,14 +2,23 @@
>  
>  use anyhow::{bail, format_err, Error};
>  
> -use serde_json::Value;
> +use hyper::header::CONTENT_TYPE;
> +use hyper::http::request::Parts;
> +use hyper::Response;
> +use serde_json::{json, Value};
> +
>  use std::collections::HashMap;
>  use std::collections::HashSet;
>  
> +use proxmox_auth_api::api::API_METHOD_CREATE_TICKET_HTTP_ONLY;
> +use proxmox_auth_api::types::{CreateTicket, CreateTicketResponse};
>  use proxmox_router::{
> -    http_bail, http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap,
> +    http_bail, http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture,
> +    Permission, Router, RpcEnvironment, SubdirMap,
> +};
> +use proxmox_schema::{
> +    api, AllOfSchema, ApiType, BooleanSchema, ObjectSchema, ParameterSchema, ReturnType,
>  };
> -use proxmox_schema::api;
>  use proxmox_sortable_macro::sortable;
>  
>  use pbs_api_types::{
> @@ -268,7 +277,9 @@ const SUBDIRS: SubdirMap = &sorted!([
>      ),
>      (
>          "ticket",
> -        &Router::new().post(&proxmox_auth_api::api::API_METHOD_CREATE_TICKET)
> +        &Router::new()
> +            .post(&API_METHOD_CREATE_TICKET_TOGGLE)
> +            .delete(&proxmox_auth_api::api::API_METHOD_LOGOUT)
>      ),
>      ("openid", &openid::ROUTER),
>      ("domains", &domain::ROUTER),
> @@ -277,6 +288,64 @@ const SUBDIRS: SubdirMap = &sorted!([
>      ("tfa", &tfa::ROUTER),
>  ]);
>  
> +const API_METHOD_CREATE_TICKET_TOGGLE: ApiMethod = ApiMethod::new_full(
> +    &proxmox_router::ApiHandler::AsyncHttpBodyParameters(&handle_ticket_toggle),
> +    ParameterSchema::AllOf(&AllOfSchema::new(
> +        "Either create a new HttpOnly ticket or a regular ticket.",
> +        &[
> +            &ObjectSchema::new(
> +                "<INNER: Toggle between http only or legacy ticket endpoints.>",
Should this also be named `HttpOnly` instead of `http only` for
consistency reasons?

> +                &[(
> +                    "http-only",
> +                    true,
> +                    &BooleanSchema::new(
> +                        "Whether the http only authentication flow should be used.",
Again `http only` here.

> +                    )
> +                    .default(false)
> +                    .schema(),
> +                )],
> +            )
> +            .schema(),
> +            &CreateTicket::API_SCHEMA,
> +        ],
> +    )),
> +)
> +.returns(ReturnType::new(false, &CreateTicketResponse::API_SCHEMA))
> +.protected(true)
> +.access(None, &Permission::World);
> +
> +fn handle_ticket_toggle(
> +    parts: Parts,
> +    mut param: Value,
> +    info: &'static ApiMethod,
> +    mut rpcenv: Box<dyn RpcEnvironment>,
> +) -> ApiResponseFuture {
> +    // If the client specifies that they want to use http only cookies, prefer those.
Again `http only` here.

> +    if Some(true) == param["http-only"].take().as_bool() {
> +        if let ApiHandler::AsyncHttpBodyParameters(handler) =
> +            API_METHOD_CREATE_TICKET_HTTP_ONLY.handler
> +        {
> +            return handler(parts, param, info, rpcenv);
> +        }
> +    }
> +
> +    // Otherwise, default back to the previous ticket method.
> +    Box::pin(async move {
> +        let create_params: CreateTicket = serde_json::from_value(param)?;
> +
> +        let ticket_response =
> +            proxmox_auth_api::api::create_ticket(create_params, rpcenv.as_mut()).await?;
> +
> +        let response = Response::builder().header(CONTENT_TYPE, "application/json");
> +
> +        Ok(response.body(
> +            json!({"data": ticket_response, "status": 200, "success": true })
> +                .to_string()
> +                .into(),
> +        )?)
> +    })
> +}
> +
>  pub const ROUTER: Router = Router::new()
>      .get(&list_subdirs_api_method!(SUBDIRS))
>      .subdirs(SUBDIRS);



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow
  2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow Shannon Sterz
  2025-07-23 12:57   ` Mira Limbeck
@ 2025-07-23 13:58   ` Maximiliano Sandoval
  1 sibling, 0 replies; 13+ messages in thread
From: Maximiliano Sandoval @ 2025-07-23 13:58 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

Shannon Sterz <s.sterz@proxmox.com> writes:

> this new flow returns https only cookies providing an additional layer
> of security for clients operating in a browser environment. opt-in
> only to not break existing clients.
>
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  src/api2/access/mod.rs | 77 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/src/api2/access/mod.rs b/src/api2/access/mod.rs
> index 832cdc66..b61b596e 100644
> --- a/src/api2/access/mod.rs
> +++ b/src/api2/access/mod.rs
> @@ -2,14 +2,23 @@
>  
>  use anyhow::{bail, format_err, Error};
>  
> -use serde_json::Value;
> +use hyper::header::CONTENT_TYPE;
> +use hyper::http::request::Parts;
> +use hyper::Response;
> +use serde_json::{json, Value};
> +
>  use std::collections::HashMap;
>  use std::collections::HashSet;
>  
> +use proxmox_auth_api::api::API_METHOD_CREATE_TICKET_HTTP_ONLY;
> +use proxmox_auth_api::types::{CreateTicket, CreateTicketResponse};
>  use proxmox_router::{
> -    http_bail, http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap,
> +    http_bail, http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture,
> +    Permission, Router, RpcEnvironment, SubdirMap,
> +};
> +use proxmox_schema::{
> +    api, AllOfSchema, ApiType, BooleanSchema, ObjectSchema, ParameterSchema, ReturnType,
>  };
> -use proxmox_schema::api;
>  use proxmox_sortable_macro::sortable;
>  
>  use pbs_api_types::{
> @@ -268,7 +277,9 @@ const SUBDIRS: SubdirMap = &sorted!([
>      ),
>      (
>          "ticket",
> -        &Router::new().post(&proxmox_auth_api::api::API_METHOD_CREATE_TICKET)
> +        &Router::new()
> +            .post(&API_METHOD_CREATE_TICKET_TOGGLE)
> +            .delete(&proxmox_auth_api::api::API_METHOD_LOGOUT)
>      ),
>      ("openid", &openid::ROUTER),
>      ("domains", &domain::ROUTER),
> @@ -277,6 +288,64 @@ const SUBDIRS: SubdirMap = &sorted!([
>      ("tfa", &tfa::ROUTER),
>  ]);
>  
> +const API_METHOD_CREATE_TICKET_TOGGLE: ApiMethod = ApiMethod::new_full(
> +    &proxmox_router::ApiHandler::AsyncHttpBodyParameters(&handle_ticket_toggle),
> +    ParameterSchema::AllOf(&AllOfSchema::new(
> +        "Either create a new HttpOnly ticket or a regular ticket.",
> +        &[
> +            &ObjectSchema::new(
> +                "<INNER: Toggle between http only or legacy ticket endpoints.>",
> +                &[(
> +                    "http-only",
> +                    true,
> +                    &BooleanSchema::new(
> +                        "Whether the http only authentication flow should be used.",
> +                    )
> +                    .default(false)
> +                    .schema(),
> +                )],
> +            )
> +            .schema(),
> +            &CreateTicket::API_SCHEMA,
> +        ],
> +    )),
> +)
> +.returns(ReturnType::new(false, &CreateTicketResponse::API_SCHEMA))
> +.protected(true)
> +.access(None, &Permission::World);
> +
> +fn handle_ticket_toggle(
> +    parts: Parts,
> +    mut param: Value,
> +    info: &'static ApiMethod,
> +    mut rpcenv: Box<dyn RpcEnvironment>,
> +) -> ApiResponseFuture {
> +    // If the client specifies that they want to use http only cookies, prefer those.
> +    if Some(true) == param["http-only"].take().as_bool() {
> +        if let ApiHandler::AsyncHttpBodyParameters(handler) =
> +            API_METHOD_CREATE_TICKET_HTTP_ONLY.handler
> +        {

This could benefit from a

            tracing::debug!("Client requests http-only cookies");

or similar.

> +            return handler(parts, param, info, rpcenv);
> +        }
> +    }
> +
> +    // Otherwise, default back to the previous ticket method.
> +    Box::pin(async move {
> +        let create_params: CreateTicket = serde_json::from_value(param)?;
> +
> +        let ticket_response =
> +            proxmox_auth_api::api::create_ticket(create_params, rpcenv.as_mut()).await?;
> +
> +        let response = Response::builder().header(CONTENT_TYPE, "application/json");
> +
> +        Ok(response.body(
> +            json!({"data": ticket_response, "status": 200, "success": true })
> +                .to_string()
> +                .into(),
> +        )?)
> +    })
> +}
> +
>  pub const ROUTER: Router = Router::new()
>      .get(&list_subdirs_api_method!(SUBDIRS))
>      .subdirs(SUBDIRS);

-- 
Maximiliano


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
                   ` (4 preceding siblings ...)
  2025-07-23 12:56 ` [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Mira Limbeck
@ 2025-07-23 14:05 ` Maximiliano Sandoval
  2025-07-23 15:15 ` Shannon Sterz
  6 siblings, 0 replies; 13+ messages in thread
From: Maximiliano Sandoval @ 2025-07-23 14:05 UTC (permalink / raw)
  To: Shannon Sterz; +Cc: pbs-devel

Shannon Sterz <s.sterz@proxmox.com> writes:

> # Summary
>
> this series adds an authentication flow based on http only cookies for
> proxmox backup server. at the moment this authentication flow is opt-in
> in order to not break older clients that may still rely on the previous
> authentication flow.
>
> this series is split into three parts:
>
> 1. prepare proxmox-auth-api to be used with extjs and implements a new
>    ticket endpoint for pbs. this endpoint requires clients to provide a
>    boolean parameter `http-only` as `true` for it to switch to the new
>    http-only based authentication flow.
> 2. adapt proxmox backup server's ui components to always use the http
>    only based authentication flow. this should make cookies
>    inaccessible to any javascript-based attack in the browser,
>    providing an extra layer of security.
> 3. prepare pbs-client for potential servers that may no longer provide
>    the previous authentication flow. the point of already adding this
>    now, is to be prepare update-hesitant users for a future without the
>    old authentication flow. if the old authentication flow is dropped in
>    the future, more users will already have a client version that can
>    adapt to the new flow.
>
> # Why not opt the `pbs-client` into the new flow?
>
> the client is deliberatelly not opted into the new authentication flow
> for the following reasons:
>
> - http only cookies are only an effective security mechanism within a
>   browser context. the client simply does not benefit from this extra
>   layer of protection. the attacks http only cookies protect against,
>   simply don't exist here.
> - opting the client in unconditionally isn't possible. older pbs servers
>   would complain about the additional api parameters provided by the
>   client. this means:
>
>     + the client would either need to try the http-only flow and once
>       that fails, fall back to the older authentication flow.
>     + or query (and possibly cache) the server version and check if the
>       version is new enough to support the new authentication flow.
>
> both approaches are more error-prone and produce additional network
> overhead than simply not opting the client into the new flow. causing
> api errors may also produce a lot of false warnings when monitoring pbs.
> so there are no benefits, but potential downsides. hence, the client
> will not yet be opted into the new authentication flow.
>
> proxmox:
>
> Shannon Sterz (1):
>   auth-api: include meta information required by extjs in api endpoints
>
>  proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
>
> proxmox-backup:
>
> Shannon Sterz (3):
>   api: access: add opt-in http only ticket authentication flow
>   ui: opt into the new http-only ticket authentication flow
>   client: adapt pbs client to also handle http-only flows correctly
>
>  pbs-client/src/http_client.rs | 70 ++++++++++++++++++++++++++++---
>  src/api2/access/mod.rs        | 77 +++++++++++++++++++++++++++++++++--
>  www/Application.js            | 12 +++++-
>  www/LoginView.js              |  4 +-
>  www/MainView.js               |  1 +
>  www/Utils.js                  |  6 +++
>  6 files changed, 159 insertions(+), 11 deletions(-)
>
>
> Summary over all repositories:
>   7 files changed, 169 insertions(+), 14 deletions(-)

Tested via modifying the client:

```diff
modified   pbs-client/src/http_client.rs
@@ -506,11 +506,20 @@ impl HttpClient {
             }
         };
 
+        let http_only = std::env::var("PROXMOX_BACKUP_CLIENT_HTTP_ONLY")
+            .ok()
+            .and_then(|s| s.parse::<i32>().ok())
+            .is_some_and(|res| res > 0);
+
+        if http_only {
+            info!("Using http-only authentication");
+        }
+
         let auth = Arc::new(RwLock::new(AuthInfo {
             auth_id: auth_id.clone(),
             ticket: password.clone(),
             token: "".to_string(),
-            http_only: false,
+            http_only,
         }));
 
         let server2 = server.to_string();
```

then `proxmox-backup-client login` works against a pbs running this
patch when then env variable is set.

When login via a web browser both cookies are set and cleared correctly
on logout.

The only thing I am missing is a bit of logging on the server side as
described in my reply, but this is not a blocker.

Consider this.

Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs
  2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
                   ` (5 preceding siblings ...)
  2025-07-23 14:05 ` Maximiliano Sandoval
@ 2025-07-23 15:15 ` Shannon Sterz
  6 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2025-07-23 15:15 UTC (permalink / raw)
  To: Shannon Sterz, pbs-devel

Superseeded-by: https://lore.proxmox.com/pbs-devel/20250723151356.264229-3-s.sterz@proxmox.com/T/#u

On Thu Jul 10, 2025 at 3:50 PM CEST, Shannon Sterz wrote:
> # Summary
>
> this series adds an authentication flow based on http only cookies for
> proxmox backup server. at the moment this authentication flow is opt-in
> in order to not break older clients that may still rely on the previous
> authentication flow.
>
> this series is split into three parts:
>
> 1. prepare proxmox-auth-api to be used with extjs and implements a new
>    ticket endpoint for pbs. this endpoint requires clients to provide a
>    boolean parameter `http-only` as `true` for it to switch to the new
>    http-only based authentication flow.
> 2. adapt proxmox backup server's ui components to always use the http
>    only based authentication flow. this should make cookies
>    inaccessible to any javascript-based attack in the browser,
>    providing an extra layer of security.
> 3. prepare pbs-client for potential servers that may no longer provide
>    the previous authentication flow. the point of already adding this
>    now, is to be prepare update-hesitant users for a future without the
>    old authentication flow. if the old authentication flow is dropped in
>    the future, more users will already have a client version that can
>    adapt to the new flow.
>
> # Why not opt the `pbs-client` into the new flow?
>
> the client is deliberatelly not opted into the new authentication flow
> for the following reasons:
>
> - http only cookies are only an effective security mechanism within a
>   browser context. the client simply does not benefit from this extra
>   layer of protection. the attacks http only cookies protect against,
>   simply don't exist here.
> - opting the client in unconditionally isn't possible. older pbs servers
>   would complain about the additional api parameters provided by the
>   client. this means:
>
>     + the client would either need to try the http-only flow and once
>       that fails, fall back to the older authentication flow.
>     + or query (and possibly cache) the server version and check if the
>       version is new enough to support the new authentication flow.
>
> both approaches are more error-prone and produce additional network
> overhead than simply not opting the client into the new flow. causing
> api errors may also produce a lot of false warnings when monitoring pbs.
> so there are no benefits, but potential downsides. hence, the client
> will not yet be opted into the new authentication flow.
>
> proxmox:
>
> Shannon Sterz (1):
>   auth-api: include meta information required by extjs in api endpoints
>
>  proxmox-auth-api/src/api/access.rs | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
>
> proxmox-backup:
>
> Shannon Sterz (3):
>   api: access: add opt-in http only ticket authentication flow
>   ui: opt into the new http-only ticket authentication flow
>   client: adapt pbs client to also handle http-only flows correctly
>
>  pbs-client/src/http_client.rs | 70 ++++++++++++++++++++++++++++---
>  src/api2/access/mod.rs        | 77 +++++++++++++++++++++++++++++++++--
>  www/Application.js            | 12 +++++-
>  www/LoginView.js              |  4 +-
>  www/MainView.js               |  1 +
>  www/Utils.js                  |  6 +++
>  6 files changed, 159 insertions(+), 11 deletions(-)
>
>
> Summary over all repositories:
>   7 files changed, 169 insertions(+), 14 deletions(-)
>
> --
> Generated by git-murpp 0.8.1



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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints
  2025-07-22 20:21       ` Thomas Lamprecht
@ 2025-07-23 15:18         ` Shannon Sterz
  0 siblings, 0 replies; 13+ messages in thread
From: Shannon Sterz @ 2025-07-23 15:18 UTC (permalink / raw)
  To: Thomas Lamprecht; +Cc: Proxmox Backup Server development discussion

On Tue Jul 22, 2025 at 10:21 PM CEST, Thomas Lamprecht wrote:
> Am 16.07.25 um 10:17 schrieb Shannon Sterz:
>> On Wed Jul 16, 2025 at 12:40 AM CEST, Thomas Lamprecht wrote:
>>> Am 10.07.25 um 15:50 schrieb Shannon Sterz:
>>>> otherwise extjs will assume the requests failed even though they were
>>>> successful.
>>>
>>> Potentially dumb question, but is this then only returning that for the
>>> extjs formatter or also for the json one?
>>
>> sadly this impacts all formaters as this type of api handler returns a
>> `Response` right away instead of a passing it through a formater like
>> other handler types. the problem with the other handler types is, that
>> they don't let you access the requests header values as easily.
>>
>> the nicest way to resolve this would probably be to pass down the
>> formater in `handle_api_request` [1] to endpoints that return an
>> `ApiResponseFuture`. so that they can handle the formatting themselves
>> (some api endpoints like those that download raw text files etc.
>> probably won't need the formater).
>
> It might be better to expose through the (RPC/REST) environment in the
> mid term, could be useful for other stuff too. Or alternatively expose
> a structured way to pass back a cookie if the methods signals support
> in the schema.
>
> But either way: we can go this route for now, while looking at this more
> closely down the line, when it either causes problems (IMO unlikely) or
> when we need something similar for another reason anyway and can clean
> this up here then too.
>
> Anyway, if you get someone to give this a spin and at least a T-b here,
> ideally a review too, I'd be still open to take this in.

alright, mira and maximiliano both tested the series and found mostly
cosmetic issues. i adopted them into a v2 and kept their T-b lines since
nothing really changed except for an additional `tracing::debug!`
statement. let me know if there is something else i can do.


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


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-07-23 15:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-10 13:50 [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Shannon Sterz
2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox 1/1] auth-api: include meta information required by extjs in api endpoints Shannon Sterz
2025-07-15 22:40   ` Thomas Lamprecht
     [not found]     ` <DBDBXGTI71WP.3V2J3DEMNK1DL@proxmox.com>
2025-07-22 20:21       ` Thomas Lamprecht
2025-07-23 15:18         ` Shannon Sterz
2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow Shannon Sterz
2025-07-23 12:57   ` Mira Limbeck
2025-07-23 13:58   ` Maximiliano Sandoval
2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 2/3] ui: opt into the new http-only " Shannon Sterz
2025-07-10 13:50 ` [pbs-devel] [PATCH proxmox-backup 3/3] client: adapt pbs client to also handle http-only flows correctly Shannon Sterz
2025-07-23 12:56 ` [pbs-devel] [PATCH proxmox{, -backup} 0/4] http only cookie based tickets for pbs Mira Limbeck
2025-07-23 14:05 ` Maximiliano Sandoval
2025-07-23 15:15 ` Shannon Sterz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal