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,-websocket-tunnel} v3 0/6] unify openssl callback logic
Date: Thu, 25 Jun 2026 13:19:30 +0200 [thread overview]
Message-ID: <DJI38IK5B0IS.3I6Q9FCYQ0EY9@proxmox.com> (raw)
In-Reply-To: <20260617085949.1528300-1-d.csapak@proxmox.com>
On Wed Jun 17, 2026 at 10:59 AM CEST, Dominik Csapak wrote:
> There are currently 3+ slightly different implementations of the openssl
> verify callback in place. They differ in how an explicit fingerprint
> would be checked:
first off, thanks for tackling this. unifying this behavior makes a lot
of sense imo.
-->8 snip 8<--
> The last patch of the proxmox-http crate is to preserve backwards compatibility
> with the current pbs client behavior, but can be switched to the new 'correct'
> one via environment variable (which we might want to enable automatically for
> the websocket-tunnel?)
after discussing this with fabian we came to conclusion that this
behavior should probably be opt-out, not opt-in. we can opt-in existing
installs on update, but avoid having new installs and new users use the
old behavior by accident.
personally, websocket-tunnel should definitively be kept at its current
behavior. leting it regress to "wrong" behavior sounds wrong to me.
-->8 snip 8<--
> I tried to implement some tests, but due to the openssl interface this
> seems to be not really possible, except if we'd start a server + client
> in the tests (which seems overkill). But if anyone has an idea how we
> could test this code (and i mean not only it's interface, but the
> openssl connection behavior), I'd be glad.
as discussed off list, adding tests for this does make a lot of sense.
even if it means manually spawning a server that we can test against. we
already do something similar for other crates such as `proxmox-ldap` and
as you pointed out, we managed to get this "wrong" several times over,
so the effort is imo justified.
attached to this review, i send three patches that implement such tests.
these three are intended to be applied on top of your patches for the
"proxmox" repo. feel free to either incorporate them into your series or
adjust them/provide feedback on them :)
> This series partially overlaps/interferes with shannons recent series:
> https://lore.proxmox.com/pdm-devel/20260611120327.257523-1-s.sterz@proxmox.com/
>
> Depending on whether this or shannons series is applied first, I'd either
> send a follow-up for PDM or a new version rebased on shannons.
btw. same here, i'd also be up for merging parts of these series if it
makes reviewing/applying them easier.
-->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
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 ` Shannon Sterz [this message]
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=DJI38IK5B0IS.3I6Q9FCYQ0EY9@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