[PATCH datacenter-manager 06/17] client: don't short-circuit on valid certificate when tls fp exists

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

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 <s.sterz@proxmox.com>
---
 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<VerifyResult, Error> {
         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<bool, Error> {
+    pub fn verify_cert(
+        &self,
+        chain: &mut x509::X509StoreContextRef,
+        valid: bool,
+    ) -> Result<bool, Error> {
         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<PdmClient<Client>, 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