From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id E4DDC1FF13E for ; Wed, 01 Jul 2026 12:32:10 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 12726214B0; Wed, 01 Jul 2026 12:31:54 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v4 6/8] pbs-client: use proxmox-https openssl callback Date: Wed, 1 Jul 2026 12:30:50 +0200 Message-ID: <20260701103120.1593265-7-d.csapak@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260701103120.1593265-1-d.csapak@proxmox.com> References: <20260701103120.1593265-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: AUE2JSJ4LVF2TX2VSR4OW76L6E3OBZPK X-Message-ID-Hash: AUE2JSJ4LVF2TX2VSR4OW76L6E3OBZPK X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: instead of implementing it here. This changes the behavior when giving a fingerprint explicitly when the certificate chain is trusted by openssl. Previously this would be accepted due to openssls checks, regardless if the given fingerprint would match or not. With this patch, a given fingerprint has higher priority than openssls validation. Signed-off-by: Dominik Csapak --- Cargo.toml | 2 +- pbs-client/src/http_client.rs | 165 ++++++++++++++++------------------ 2 files changed, 76 insertions(+), 91 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a625370cf..9530e1ccb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,7 +64,7 @@ proxmox-config-digest = "1" proxmox-daemon = "1" proxmox-fuse = "3" proxmox-docgen = "1" -proxmox-http = { version = "1.0.2", features = [ "client", "http-helpers", "api-types", "websocket" ] } # see below +proxmox-http = { version = "1.0.2", features = [ "client", "http-helpers", "api-types", "websocket", "tls" ] } # see below proxmox-human-byte = "1" proxmox-io = "1.0.1" # tools and client use "tokio" feature proxmox-lang = "1.1" diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs index a6fcafcfd..27f5a1782 100644 --- a/pbs-client/src/http_client.rs +++ b/pbs-client/src/http_client.rs @@ -1,4 +1,5 @@ use std::io::{IsTerminal, Write}; +use std::str::FromStr; use std::sync::{Arc, Mutex, RwLock}; use std::time::Duration; @@ -15,10 +16,7 @@ use hyper::http::{Request, Response}; use hyper_util::client::legacy::connect::dns::GaiResolver; use hyper_util::client::legacy::{Client, connect::HttpConnector}; use hyper_util::rt::{TokioExecutor, TokioIo}; -use openssl::{ - ssl::{SslConnector, SslMethod}, - x509::X509StoreContextRef, -}; +use openssl::ssl::{SslConnector, SslMethod}; use percent_encoding::percent_encode; use serde_json::{Value, json}; use xdg::BaseDirectories; @@ -28,10 +26,10 @@ use proxmox_sys::fs::{CreateOptions, file_get_json, replace_file}; use proxmox_sys::linux::tty; use proxmox_async::broadcast_future::BroadcastFuture; -use proxmox_http::Body; -use proxmox_http::ProxyConfig; use proxmox_http::client::HttpsConnector; use proxmox_http::uri::{build_authority, json_object_to_query}; +use proxmox_http::{Body, Fingerprint}; +use proxmox_http::{ProxyConfig, SslVerifyError, openssl_verify_callback}; use proxmox_log::{error, info, warn}; use proxmox_rate_limiter::RateLimiter; @@ -417,6 +415,14 @@ impl HttpClient { expected_fingerprint = load_fingerprint(options.prefix.as_ref().unwrap(), server); } + let expected_fingerprint = match expected_fingerprint { + Some(fp) => Some( + Fingerprint::from_str(&fp) + .map_err(|err| format_err!("could not parse fingerprint: {err}"))?, + ), + None => None, + }; + let mut ssl_connector_builder = SslConnector::builder(SslMethod::tls()).unwrap(); if options.verify_cert { @@ -425,30 +431,47 @@ impl HttpClient { let interactive = options.interactive; let fingerprint_cache = options.fingerprint_cache; let prefix = options.prefix.clone(); - let trust_openssl_valid = Arc::new(Mutex::new(true)); ssl_connector_builder.set_verify_callback( openssl::ssl::SslVerifyMode::PEER, - move |valid, ctx| match Self::verify_callback( + move |valid, ctx| match openssl_verify_callback( valid, ctx, - expected_fingerprint.as_ref(), - interactive, - Arc::clone(&trust_openssl_valid), + expected_fingerprint.clone(), ) { - Ok(None) => true, - Ok(Some(fingerprint)) => { - if fingerprint_cache { - if let Some(ref prefix) = prefix { - if let Err(err) = store_fingerprint(prefix, &server, &fingerprint) { - error!("{}", err); - } - } + Ok(()) => { + if let Some(fp) = &expected_fingerprint { + *verified_fingerprint.lock().unwrap() = Some(fp.to_string()); } - *verified_fingerprint.lock().unwrap() = Some(fingerprint); true - } + }, Err(err) => { - error!("certificate validation failed - {}", err); + match err { + SslVerifyError::NoCertificate => error!( + "certificate validation failed - context lacks current certificate" + ), + SslVerifyError::InvalidFingerprint(error_stack) => { + error!("certificate validation failed - failed to calculate FP - {error_stack}") + }, + SslVerifyError::UntrustedCertificate { fingerprint } => { + if interactive && std::io::stdin().is_terminal() { + match Self::interactive_fp_check(prefix.as_deref(), &server, verified_fingerprint.clone(), fingerprint_cache, fingerprint) { + Ok(()) => return true, + Err(err) => error!("certificate validation failed - {err}"), + } + } + } + SslVerifyError::FingerprintMismatch { fingerprint, expected } => { + warn!("WARNING: certificate fingerprint does not match expected fingerprint!"); + warn!("expected: {expected}"); + + if interactive && std::io::stdin().is_terminal() { + match Self::interactive_fp_check(prefix.as_deref(), &server, verified_fingerprint.clone(), fingerprint_cache, fingerprint) { + Ok(()) => return true, + Err(err) => error!("certificate validation failed - {err}"), + } + } + }, + } false } }, @@ -661,79 +684,41 @@ impl HttpClient { bail!("no password input mechanism available"); } - fn verify_callback( - openssl_valid: bool, - ctx: &mut X509StoreContextRef, - expected_fingerprint: Option<&String>, - interactive: bool, - trust_openssl: Arc>, - ) -> Result, Error> { - let mut trust_openssl_valid = trust_openssl.lock().unwrap(); - - // we can only rely on openssl's prevalidation if we haven't forced it earlier - if openssl_valid && *trust_openssl_valid { - return Ok(None); - } - - let cert = match ctx.current_cert() { - Some(cert) => cert, - None => bail!("context lacks current certificate."), - }; - - // force trust in case of a chain, but set flag to no longer trust prevalidation by openssl - if ctx.error_depth() > 0 { - *trust_openssl_valid = false; - return Ok(None); - } - - // leaf certificate - if we end up here, we have to verify the fingerprint! - let fp = match cert.digest(openssl::hash::MessageDigest::sha256()) { - Ok(fp) => fp, - Err(err) => bail!("failed to calculate certificate FP - {}", err), // should not happen - }; - let fp_string = hex::encode(fp); - let fp_string = fp_string - .as_bytes() - .chunks(2) - .map(|v| std::str::from_utf8(v).unwrap()) - .collect::>() - .join(":"); - - if let Some(expected_fingerprint) = expected_fingerprint { - let expected_fingerprint = expected_fingerprint.to_lowercase(); - if expected_fingerprint == fp_string { - return Ok(Some(fp_string)); - } else { - warn!("WARNING: certificate fingerprint does not match expected fingerprint!"); - warn!("expected: {}", expected_fingerprint); - } - } - - // If we're on a TTY, query the user - if interactive && std::io::stdin().is_terminal() { - info!("fingerprint: {}", fp_string); - loop { - eprint!("Are you sure you want to continue connecting? (y/n): "); - let _ = std::io::stdout().flush(); - use std::io::{BufRead, BufReader}; - let mut line = String::new(); - match BufReader::new(std::io::stdin()).read_line(&mut line) { - Ok(_) => { - let trimmed = line.trim(); - if trimmed == "y" || trimmed == "Y" { - return Ok(Some(fp_string)); - } else if trimmed == "n" || trimmed == "N" { - bail!("Certificate fingerprint was not confirmed."); - } else { - continue; + fn interactive_fp_check( + prefix: Option<&str>, + server: &str, + verified_fingerprint: Arc>>, + fingerprint_cache: bool, + fingerprint: Fingerprint, + ) -> Result<(), Error> { + info!("fingerprint: {fingerprint}"); + loop { + eprint!("Are you sure you want to continue connecting? (y/n): "); + let _ = std::io::stdout().flush(); + use std::io::{BufRead, BufReader}; + let mut line = String::new(); + match BufReader::new(std::io::stdin()).read_line(&mut line) { + Ok(_) => { + let trimmed = line.trim(); + if trimmed == "y" || trimmed == "Y" { + if fingerprint_cache && prefix.is_some() { + if let Err(err) = + store_fingerprint(prefix.unwrap(), server, &fingerprint.to_string()) + { + error!("{}", err); + } } + *verified_fingerprint.lock().unwrap() = Some(fingerprint.to_string()); + return Ok(()); + } else if trimmed == "n" || trimmed == "N" { + bail!("Certificate fingerprint was not confirmed."); + } else { + continue; } - Err(err) => bail!("Certificate fingerprint was not confirmed - {}.", err), } + Err(err) => bail!("Certificate fingerprint was not confirmed - {}.", err), } } - - bail!("Certificate fingerprint was not confirmed."); } pub async fn request(&self, mut req: Request) -> Result { -- 2.47.3