[pbs-devel] [PATCH proxmox 2/2] client: use proxmox-http's openssl verification callback

[Dominik Csapak <d.csapak@proxmox.com>]

This changes the validation logic by always checking the fingerprint of
the leaf certificate, ignoring the openssl verification if a fingerprint
is configured. This now aligns with our perl implementation and the one
for proxmox-websocket-tunnel.

Before, a valid certificate chain would have precedence over an explicit
fingerprint.

Additionally, fingerprints will be correctly checked only against the
leaf certificate instead of every part of the chain for which the
callback is called.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 proxmox-client/Cargo.toml    |  2 +-
 proxmox-client/src/client.rs | 48 ++++++------------------------------
 2 files changed, 9 insertions(+), 41 deletions(-)

diff --git a/proxmox-client/Cargo.toml b/proxmox-client/Cargo.toml
index c2682e77..477e9393 100644
--- a/proxmox-client/Cargo.toml
+++ b/proxmox-client/Cargo.toml
@@ -24,7 +24,7 @@ openssl = { workspace = true, optional = true }
 
 proxmox-login = { workspace = true, features = [ "http" ] }
 
-proxmox-http = { workspace = true, optional = true, features = [ "client" ] }
+proxmox-http = { workspace = true, optional = true, features = [ "client", "tls" ] }
 hyper = { workspace = true, optional = true }
 
 [dev-dependencies]
diff --git a/proxmox-client/src/client.rs b/proxmox-client/src/client.rs
index 6f1c9ef1..ce0c4841 100644
--- a/proxmox-client/src/client.rs
+++ b/proxmox-client/src/client.rs
@@ -9,9 +9,9 @@ use http::uri::PathAndQuery;
 use http::Method;
 use http::{StatusCode, Uri};
 use hyper::body::{Body, HttpBody};
-use openssl::hash::MessageDigest;
 use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode};
 use openssl::x509::{self, X509};
+use proxmox_http::get_fingerprint_from_u8;
 use proxmox_login::Ticket;
 use serde::Serialize;
 
@@ -109,10 +109,14 @@ impl Client {
             TlsOptions::Insecure => connector.set_verify(SslVerifyMode::NONE),
             TlsOptions::Fingerprint(expected_fingerprint) => {
                 connector.set_verify_callback(SslVerifyMode::PEER, move |valid, chain| {
-                    if valid {
-                        return true;
+                    let fp = get_fingerprint_from_u8(&expected_fingerprint);
+                    match proxmox_http::openssl_verify_callback(valid, chain, Some(&fp)) {
+                        Ok(()) => true,
+                        Err(err) => {
+                            log::error!("{err}");
+                            false
+                        }
                     }
-                    verify_fingerprint(chain, &expected_fingerprint)
                 });
             }
             TlsOptions::Callback(cb) => {
@@ -543,42 +547,6 @@ impl HttpApiClient for Client {
     }
 }
 
-fn verify_fingerprint(chain: &x509::X509StoreContextRef, expected_fingerprint: &[u8]) -> bool {
-    let Some(cert) = chain.current_cert() else {
-        log::error!("no certificate in chain?");
-        return false;
-    };
-
-    let fp = match cert.digest(MessageDigest::sha256()) {
-        Err(err) => {
-            log::error!("error calculating certificate fingerprint: {err}");
-            return false;
-        }
-        Ok(fp) => fp,
-    };
-
-    if expected_fingerprint != fp.as_ref() {
-        log::error!("bad fingerprint: {}", fp_string(&fp));
-        log::error!("expected fingerprint: {}", fp_string(expected_fingerprint));
-        return false;
-    }
-
-    true
-}
-
-fn fp_string(fp: &[u8]) -> String {
-    use std::fmt::Write as _;
-
-    let mut out = String::new();
-    for b in fp {
-        if !out.is_empty() {
-            out.push(':');
-        }
-        let _ = write!(out, "{b:02x}");
-    }
-    out
-}
-
 impl Error {
     pub(crate) fn internal<E>(context: &'static str, err: E) -> Self
     where
-- 
2.39.5



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel