From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 8EC031FF185
	for <inbox@lore.proxmox.com>; Wed, 21 May 2025 10:45:59 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 0B03B12CC5;
	Wed, 21 May 2025 10:45:59 +0200 (CEST)
From: Dominik Csapak <d.csapak@proxmox.com>
To: pbs-devel@lists.proxmox.com
Date: Wed, 21 May 2025 10:45:20 +0200
Message-Id: <20250521084524.829496-2-d.csapak@proxmox.com>
X-Mailer: git-send-email 2.39.5
In-Reply-To: <20250521084524.829496-1-d.csapak@proxmox.com>
References: <20250521084524.829496-1-d.csapak@proxmox.com>
MIME-Version: 1.0
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.021 Adjusted score from AWL reputation of From: address
 BAYES_00                 -1.9 Bayes spam probability is 0 to 1%
 DMARC_MISSING             0.1 Missing DMARC policy
 KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to
 Validity was blocked. See
 https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more
 information.
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See
 http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more
 information. [lib.rs, tls.rs]
Subject: [pbs-devel] [PATCH proxmox 1/2] http: factor out openssl
 verification callback
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

with the 'tls' feature offers a callback method that can be used within
openssl's `set_verify_callback` with a given expected fingerprint.

The logic is inspired by our perl and proxmox-websocket-tunnel
verification logic:

Use openssl's verification if no fingerprint is pinned. If a fingerprint
is given, ignore openssl's verification and check if the leafs
certificate is a match.

This introduces a custom error type for this, since we need to handle
errors differently for different users, e.g. pbs-client wants to be able
to use a fingerprint cache and let the user accept it in interactive cli
sessions. For this we want the 'thiserror' crate, so move it to the
workspace Cargo.toml and depend from there. (also change this for
proxmox-openid)

One thing to note here is that  the APPLICATION_VERIFICATION error of
openssl is used to mark the case where an untrusted root or intermediate
certificate is trusted from the callback. When that happens, openssl
might return true for the following certificates (if nothing else is
wrong aside from a missing trust anchor), so the error is checked for
this special value to determine if the openssl validation can be
trusted.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 Cargo.toml                |  1 +
 proxmox-http/Cargo.toml   |  7 +++
 proxmox-http/src/lib.rs   |  5 +++
 proxmox-http/src/tls.rs   | 89 +++++++++++++++++++++++++++++++++++++++
 proxmox-openid/Cargo.toml |  2 +-
 5 files changed, 103 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-http/src/tls.rs

diff --git a/Cargo.toml b/Cargo.toml
index 95ade7d7..8aecc6ba 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -107,6 +107,7 @@ serde_json = "1.0"
 serde_plain = "1.0"
 syn = { version = "2", features = [ "full", "visit-mut" ] }
 tar = "0.4"
+thiserror = "1"
 tokio = "1.6"
 tokio-openssl = "0.6.1"
 tokio-stream = "0.1.0"
diff --git a/proxmox-http/Cargo.toml b/proxmox-http/Cargo.toml
index afa5981d..fb2eb26d 100644
--- a/proxmox-http/Cargo.toml
+++ b/proxmox-http/Cargo.toml
@@ -15,11 +15,13 @@ rust-version.workspace = true
 anyhow.workspace = true
 base64 = { workspace = true, optional = true }
 futures = { workspace = true, optional = true }
+hex = { workspace = true, optional = true }
 http = { workspace = true, optional = true }
 hyper = { workspace = true, optional = true }
 native-tls = { workspace = true, optional = true }
 openssl =  { version = "0.10", optional = true }
 serde_json = { workspace = true, optional = true }
+thiserror = { workspace = true, optional = true }
 tokio = { workspace = true, features = [], optional = true }
 tokio-openssl = { workspace = true, optional = true }
 tower-service.workspace = true
@@ -79,3 +81,8 @@ websocket = [
     "tokio?/io-util",
     "tokio?/sync",
 ]
+tls = [
+    "dep:hex",
+    "dep:openssl",
+    "dep:thiserror",
+]
diff --git a/proxmox-http/src/lib.rs b/proxmox-http/src/lib.rs
index 4770aaf4..a676acf9 100644
--- a/proxmox-http/src/lib.rs
+++ b/proxmox-http/src/lib.rs
@@ -35,3 +35,8 @@ pub use rate_limiter::{RateLimit, RateLimiter, RateLimiterVec, ShareableRateLimi
 mod rate_limited_stream;
 #[cfg(feature = "rate-limited-stream")]
 pub use rate_limited_stream::RateLimitedStream;
+
+#[cfg(feature = "tls")]
+mod tls;
+#[cfg(feature = "tls")]
+pub use tls::*;
diff --git a/proxmox-http/src/tls.rs b/proxmox-http/src/tls.rs
new file mode 100644
index 00000000..6da03cd9
--- /dev/null
+++ b/proxmox-http/src/tls.rs
@@ -0,0 +1,89 @@
+use openssl::x509::{X509StoreContextRef, X509VerifyResult};
+
+///
+/// Error type returned by failed [`openssl_verify_callback`].
+///
+#[derive(Debug, thiserror::Error)]
+pub enum SslVerifyError {
+    /// Occurs if no certificate is found in the current part of the chain. Should never happen!
+    #[error("SSL context lacks current certificate")]
+    NoCertificate,
+
+    /// Cannot calculate fingerprint from connection
+    #[error("failed to calculate fingerprint - {0}")]
+    InvalidFingerprint(openssl::error::ErrorStack),
+
+    /// Fingerprint match error
+    #[error("found fingerprint ({fingerprint}) does not match expected fingerprint ({expected})")]
+    FingerprintMismatch {
+        fingerprint: String,
+        expected: String,
+    },
+
+    /// Untrusted certificate with fingerprint information
+    #[error("certificate validation failed")]
+    UntrustedCertificate { fingerprint: String },
+}
+
+/// Intended as an openssl verification callback.
+///
+/// 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/
+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 cert = match ctx.current_cert() {
+        Some(cert) => cert,
+        None => {
+            return Err(SslVerifyError::NoCertificate);
+        }
+    };
+
+    if ctx.error_depth() > 0 {
+        // openssl was not valid, but we want to continue, so save that we don't trust openssl
+        ctx.set_error(X509VerifyResult::APPLICATION_VERIFICATION);
+        return Ok(());
+    }
+
+    let digest = cert
+        .digest(openssl::hash::MessageDigest::sha256())
+        .map_err(SslVerifyError::InvalidFingerprint)?;
+    let fingerprint = get_fingerprint_from_u8(&digest);
+
+    if let Some(expected_fp) = expected_fp {
+        if expected_fp.to_lowercase() == fingerprint.to_lowercase() {
+            ctx.set_error(X509VerifyResult::OK);
+            Ok(())
+        } else {
+            Err(SslVerifyError::FingerprintMismatch {
+                fingerprint,
+                expected: expected_fp.to_string(),
+            })
+        }
+    } else {
+        Err(SslVerifyError::UntrustedCertificate { fingerprint })
+    }
+}
+
+/// Returns the fingerprint from a byte slice ([`&[u8]`]) in the form `00:11:22:...`
+pub fn get_fingerprint_from_u8(fp: &[u8]) -> String {
+    let fp_string = hex::encode(fp);
+    let fp_string = fp_string
+        .as_bytes()
+        .chunks(2)
+        .map(|v| unsafe { std::str::from_utf8_unchecked(v) })
+        .collect::<Vec<_>>()
+        .join(":");
+
+    fp_string
+}
diff --git a/proxmox-openid/Cargo.toml b/proxmox-openid/Cargo.toml
index a5249214..4223d10c 100644
--- a/proxmox-openid/Cargo.toml
+++ b/proxmox-openid/Cargo.toml
@@ -18,7 +18,7 @@ http.workspace = true
 nix.workspace = true
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
-thiserror = "1"
+thiserror.workspace = true
 native-tls.workspace = true
 
 openidconnect = { version = "2.4", default-features = false, features = ["accept-rfc3339-timestamps"] }
-- 
2.39.5



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