From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id B67331FF13C for ; Thu, 11 Jun 2026 14:03:42 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 545EF365B; Thu, 11 Jun 2026 14:03:38 +0200 (CEST) From: Shannon Sterz To: pdm-devel@lists.proxmox.com Subject: [PATCH datacenter-manager 06/17] client: don't short-circuit on valid certificate when tls fp exists Date: Thu, 11 Jun 2026 14:03:16 +0200 Message-ID: <20260611120327.257523-7-s.sterz@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260611120327.257523-1-s.sterz@proxmox.com> References: <20260611120327.257523-1-s.sterz@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1781179364167 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.109 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 SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ZSXITBITQPH6MCPC7X3G3NH7XH3TE424 X-Message-ID-Hash: ZSXITBITQPH6MCPC7X3G3NH7XH3TE424 X-MailFrom: s.sterz@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: if a user intentionally sets a fingerprint, the assumption is that this acts similar to tls certificate pinning [1]. if we short-circuit return on certificates trusted by the system's trust store (meaning, if the `valid` parameter that is passed to the callback is true), this assumption is broken. any certificate that is trusted by the certificate store, whether there is one pinned for the corresponding host or not, is trusted. avoid this by not short-circuiting, but instead using that parameter only if we don't have a fingerprint stored for a given host. [1]: https://cheatsheetseries.owasp.org/cheatsheets/Pinning_Cheat_Sheet.html#what-is-pinning Signed-off-by: Shannon Sterz --- cli/client/src/env/fingerprint_cache.rs | 5 +++-- cli/client/src/env/mod.rs | 8 ++++++-- cli/client/src/main.rs | 6 +----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/cli/client/src/env/fingerprint_cache.rs b/cli/client/src/env/fingerprint_cache.rs index b9a9f972..af48c353 100644 --- a/cli/client/src/env/fingerprint_cache.rs +++ b/cli/client/src/env/fingerprint_cache.rs @@ -125,6 +125,7 @@ impl FingerprintCache { &self, hostname: &str, chain: &mut X509StoreContextRef, + valid: bool, ) -> Result { let cert = chain .current_cert() @@ -142,8 +143,8 @@ impl FingerprintCache { let fp = Fingerprint::try_from(&*fp) .map_err(|_| format_err!("unexpected fingerprint length"))?; - if !self.interactive { - return Ok(VerifyResult::unmodified(false)); + if !self.interactive || valid { + return Ok(VerifyResult::unmodified(valid)); } println!("Certificate SHA256 fingerprint: {fp}"); diff --git a/cli/client/src/env/mod.rs b/cli/client/src/env/mod.rs index 44a9388f..7c35e2a2 100644 --- a/cli/client/src/env/mod.rs +++ b/cli/client/src/env/mod.rs @@ -147,9 +147,13 @@ impl Env { Ok(()) } - pub fn verify_cert(&self, chain: &mut x509::X509StoreContextRef) -> Result { + pub fn verify_cert( + &self, + chain: &mut x509::X509StoreContextRef, + valid: bool, + ) -> Result { let result = match self.connect_args.host.as_deref() { - Some(server) => self.fingerprint_cache.verify(server, chain)?, + Some(server) => self.fingerprint_cache.verify(server, chain, valid)?, None => return Ok(false), }; diff --git a/cli/client/src/main.rs b/cli/client/src/main.rs index 4598a2bd..95b5b41f 100644 --- a/cli/client/src/main.rs +++ b/cli/client/src/main.rs @@ -47,11 +47,7 @@ pub fn client() -> Result, Error> { let address = env().url()?.parse()?; let options = TlsOptions::Callback(Box::new(|valid, store| { - if valid { - return true; - } - - match env().verify_cert(store) { + match env().verify_cert(store, valid) { Ok(b) => b, Err(err) => { eprintln!("failed to validate TLS certificate: {err}"); -- 2.47.3