Re: [PATCH proxmox v3 2/6] http: tls: use legacy behavior when PROXMOX_NEW_TLS_CHECK is not set

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

On Wed Jun 17, 2026 at 10:59 AM CEST, Dominik Csapak wrote:
> if that environment variable is not set to "1", give the openssl result
> priority, and potentially ignore a given fingerprint that is not
> matching. If that's the case, print a warning.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>

-->8 snip 8<--

>  /// Intended as an openssl verification callback.
>  ///
> -/// The following things are checked:
> +/// If the 'PROXMOX_NEW_TLS_CHECK' environment variable is set to "1",
> +/// the following things are checked:
>  ///
>  /// * If no fingerprint is given, return the openssl verification result
> -/// * If a fingerprint is given, do:
> -///     * Ignore all non-leaf certificates/
> +/// * If a fingerprint is given, ignore all non-leaf certificates
> +///
> +/// Otherwise, we trust the openssl result if the whole chain was trusted
>  pub fn openssl_verify_callback(
>      openssl_valid: bool,
>      ctx: &mut X509StoreContextRef,
>      expected_fp: Option<&str>,
>  ) -> Result<(), SslVerifyError> {
>      let trust_openssl = ctx.error() != X509VerifyResult::APPLICATION_VERIFICATION;
> -    if expected_fp.is_none() && openssl_valid && trust_openssl {
> -        return Ok(());
> +
> +    let new_check = matches!(std::env::var("PROXMOX_NEW_TLS_CHECK").as_deref(), Ok("1"));
> +
> +    if openssl_valid && trust_openssl {
> +        if new_check && expected_fp.is_none() {
> +            return Ok(());
> +        }
> +
> +        // legacy mode: skip all valid certs except the leaf, so we can warn if fingerprint does not match
> +        if !new_check && ctx.error_depth() > 0 {

this check is probably wrong from what i can tell. assume a certificate
chain with the certificate of the root ca being in the systems trust
store and no fingerprint was provided:

1. the root ca's certificate is provided. since it is in the systems
   trust store `openssl_valid` and `trust_openssl` are true.  we are in
   the old verification logic, so `new_check` is `false`.
   `ctx.error_depth()` is `> 0` as we aren't at the leaf yet.
2. the facts outlined hold true for all certificates in the chain until
   we reach the leaf certificate. `trust_openssl` is never set to false,
   as that would happen further down the callback (not that setting an
   error here would *not* make the behavior correct).
3. the leaf certificate will not enter this block, as
   `ctx.error_depth() > 0` no longer holds true.

> +            return Ok(());
> +        }
>      }
>
>      let cert = match ctx.current_cert() {
> @@ -50,7 +62,7 @@ pub fn openssl_verify_callback(
>      };
>
>      if ctx.error_depth() > 0 {

3. (cont.) same here

> -        // openssl was not valid, but we want to continue, so save that we don't trust openssl
> +        // if openssl is not valid, and we want to continue, save that we don't trust openssl
>          ctx.set_error(X509VerifyResult::APPLICATION_VERIFICATION);
>          return Ok(());
>      }
> @@ -65,6 +77,13 @@ pub fn openssl_verify_callback(
>              ctx.set_error(X509VerifyResult::OK);
>              Ok(())
>          } else {
> +            if !new_check && openssl_valid && trust_openssl {

4. since no fingerprint was provided, we won't get here

> +                log::warn!(
> +                    "Certificate chain valid, but fingerprint does not match, ignoring fingerprint! To prioritize the fingerprint, set `PROXMOX_NEW_TLS_CHECK=1` in your environment."
> +                );
> +                return Ok(());
> +            }
> +
>              Err(SslVerifyError::FingerprintMismatch {
>                  fingerprint,
>                  expected: expected_fp.to_string(),

5. instead the `else` branch of the outer-most `if` statement here will
be taken, reporting the certificate as
`SslVerifyError::UntrustedCertificate`.

however, that is wrong. the certificate should be trusted as openssl
validated the certificate and no fingerprint was provided.