From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 2828E1FF17C for ; Wed, 23 Jul 2025 17:14:16 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8C25516E88; Wed, 23 Jul 2025 17:15:35 +0200 (CEST) From: Shannon Sterz To: pbs-devel@lists.proxmox.com Date: Wed, 23 Jul 2025 17:14:02 +0200 Message-ID: <20250723151356.264229-11-s.sterz@proxmox.com> X-Mailer: git-send-email 2.47.2 In-Reply-To: <20250723151356.264229-3-s.sterz@proxmox.com> References: <20250723151356.264229-3-s.sterz@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753283691763 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.023 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup v2 3/3] client: adapt pbs client to also handle HttpOnly flows correctly X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" if we decide to make the HttpOnly flow opt-out or remove the previous authentication flow entirely, prepare the client to properly authenticate against such servers as well this does not opt the client into the new flow, as that has no real security benefits. however, doing so would require additional network traffic and/or state handling on the client to maintain backward compatability. this would be rather convoluted. hence, avoid doing so for now. Signed-off-by: Shannon Sterz Tested-by: Mira Limbeck Tested-by: Maximiliano Sandoval --- changes since v1: - consistently use "HttpOnly" instead of switching between "http only", "http-only" and "HttpOnly" in comments and user facing documentation, thanks @ Mira Limbeck 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..74b07624 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 HttpOnly 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 HttpOnly 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.47.2 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel