Re: [PATCH proxmox-backup v4 6/8] pbs-client: use proxmox-https openssl callback

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

On Wed Jul 1, 2026 at 12:30 PM CEST, Dominik Csapak wrote:
> 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 <d.csapak@proxmox.com>
> ---
>  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

-->8 snip 8<--

> @@ -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"
> +                            ),

nit: i still think this could be misinterpreted and lead users to add
certificates to the trust store. i think this could be made clearer with
something like "certificate validation failed - could not get a
certificate necessary for verifiction". though, this error case isn't
likely to happen in real ussage anyway from what i can tell.

-->8 snip 8<--