From: "Shannon Sterz" <s.sterz@proxmox.com>
To: "Dominik Csapak" <d.csapak@proxmox.com>,
<pve-devel@lists.proxmox.com>, <pbs-devel@lists.proxmox.com>
Subject: Re: [PATCH proxmox-backup v3 4/6] pbs-client: use proxmox-https openssl callback
Date: Thu, 25 Jun 2026 13:19:51 +0200 [thread overview]
Message-ID: <DJI38S7T88V8.37WFEYC372602@proxmox.com> (raw)
In-Reply-To: <20260617085949.1528300-5-d.csapak@proxmox.com>
On Wed Jun 17, 2026 at 10:59 AM 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>
-->8 snip 8<--
> @@ -425,30 +422,42 @@ 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.as_deref(),
> ) {
> - 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(()) => true,
> + Err(err) => {
> + match err {
> + SslVerifyError::NoCertificate => error!(
> + "certificate validation failed - context lacks current certificate"
tiny nit, i think a user is likely to misunderstand what "context" here
means and might try to add a given certificate to the system's trust
store. as they might assume that's the context referenced here. imo
something like "certificate validation failed - could not get a
certificate needed for validation from the server" might be clearer?
-->8 snip 8<--
next prev parent reply other threads:[~2026-06-25 11:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-17 8:59 [PATCH proxmox{,-backup,-websocket-tunnel} v3 0/6] unify openssl callback logic Dominik Csapak
2026-06-17 8:59 ` [PATCH proxmox v3 1/6] http: factor out openssl verification callback Dominik Csapak
2026-06-25 11:19 ` Shannon Sterz
2026-06-17 8:59 ` [PATCH proxmox v3 2/6] http: tls: use legacy behavior when PROXMOX_NEW_TLS_CHECK is not set Dominik Csapak
2026-06-25 11:19 ` Shannon Sterz
2026-06-17 8:59 ` [PATCH proxmox v3 3/6] client: use proxmox-http's openssl verification callback Dominik Csapak
2026-06-25 11:19 ` Shannon Sterz
2026-06-17 8:59 ` [PATCH proxmox-backup v3 4/6] pbs-client: use proxmox-https openssl callback Dominik Csapak
2026-06-25 11:19 ` Shannon Sterz [this message]
2026-06-17 8:59 ` [PATCH proxmox-backup v3 5/6] pbs-client: honor already verified fingerprint Dominik Csapak
2026-06-17 8:59 ` [PATCH proxmox-websocket-tunnel v3 6/6] use proxmox-http's openssl callback Dominik Csapak
2026-06-25 11:19 ` Shannon Sterz
2026-06-25 11:19 ` [PATCH proxmox{,-backup,-websocket-tunnel} v3 0/6] unify openssl callback logic Shannon Sterz
2026-06-25 11:22 ` [PATCH proxmox 1/3] http: tls: move PROXMOX_NEW_TLS_CHECK env var name into constant Shannon Sterz
2026-06-25 11:22 ` [PATCH proxmox 2/3] http: tls: implement `PartialEq` for `SslVerifyError` Shannon Sterz
2026-06-25 11:22 ` [PATCH proxmox 3/3] http: tls: add integration tests for openssl verify callbacks Shannon Sterz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DJI38S7T88V8.37WFEYC372602@proxmox.com \
--to=s.sterz@proxmox.com \
--cc=d.csapak@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox