instead of implementing it here. This changes the behavior when giving a
fingerprint explicitly when the certificate chain is trusted by openssl.
Previously this would be accepted due to openssls checks, regardless if
the given fingerprint would match or not.
With this patch, a given fingerprint has higher priority than openssls
validation.
Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
---
Cargo.toml | 2 +-
pbs-client/src/http_client.rs | 151 ++++++++++++++--------------------
2 files changed, 62 insertions(+), 91 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index ea4133283..c8e59640d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -63,7 +63,7 @@ proxmox-compression = "1"
proxmox-config-digest = "1"
proxmox-daemon = "1"
proxmox-fuse = "1"
-proxmox-http = { version = "1", features = [ "client", "http-helpers",
"websocket" ] } # see below
+proxmox-http = { version = "1", features = [ "client", "http-helpers",
"websocket", "tls" ] } # see below
proxmox-human-byte = "1"
proxmox-io = "1.0.1" # tools and client use "tokio" feature
proxmox-lang = "1.1"
diff --git a/pbs-client/src/http_client.rs b/pbs-client/src/http_client.rs
index 74b076244..69ef1d6bb 100644
--- a/pbs-client/src/http_client.rs
+++ b/pbs-client/src/http_client.rs
@@ -15,10 +15,7 @@ use hyper::http::{Request, Response};
use hyper_util::client::legacy::connect::dns::GaiResolver;
use hyper_util::client::legacy::{connect::HttpConnector, Client};
use hyper_util::rt::{TokioExecutor, TokioIo};
-use openssl::{
- ssl::{SslConnector, SslMethod},
- x509::X509StoreContextRef,
-};
+use openssl::ssl::{SslConnector, SslMethod};
use percent_encoding::percent_encode;
use serde_json::{json, Value};
use xdg::BaseDirectories;
@@ -31,7 +28,7 @@ use proxmox_async::broadcast_future::BroadcastFuture;
use proxmox_http::client::HttpsConnector;
use proxmox_http::uri::{build_authority, json_object_to_query};
use proxmox_http::Body;
-use proxmox_http::{ProxyConfig, RateLimiter};
+use proxmox_http::{openssl_verify_callback, ProxyConfig, RateLimiter,
SslVerifyError};
use proxmox_log::{error, info, warn};
use pbs_api_types::percent_encoding::DEFAULT_ENCODE_SET;
@@ -413,30 +410,42 @@ impl HttpClient {
let interactive = options.interactive;
let fingerprint_cache = options.fingerprint_cache;
let prefix = options.prefix.clone();
- let trust_openssl_valid = Arc::new(Mutex::new(true));
ssl_connector_builder.set_verify_callback(
openssl::ssl::SslVerifyMode::PEER,
- move |valid, ctx| match Self::verify_callback(
+ move |valid, ctx| match openssl_verify_callback(
valid,
ctx,
- expected_fingerprint.as_ref(),
- interactive,
- Arc::clone(&trust_openssl_valid),
+ expected_fingerprint.as_deref(),
) {
- Ok(None) => true,
- Ok(Some(fingerprint)) => {
- if fingerprint_cache && prefix.is_some() {
- if let Err(err) =
- store_fingerprint(prefix.as_ref().unwrap(),
&server, &fingerprint)
- {
- error!("{}", err);
+ Ok(()) => true,
+ Err(err) => {
+ match err {
+ SslVerifyError::NoCertificate => error!(
+ "certificate validation failed - context lacks
current certificate"
+ ),
+ SslVerifyError::InvalidFingerprint(error_stack) =>
{
+ error!("certificate validation failed - failed
to calculate FP - {error_stack}")
+ },
+ SslVerifyError::UntrustedCertificate { fingerprint
} => {
+ if interactive &&
std::io::stdin().is_terminal() {
+ match
Self::interactive_fp_check(prefix.as_deref(), &server,
verified_fingerprint.clone(), fingerprint_cache, fingerprint) {
+ Ok(()) => return true,
+ Err(err) => error!("certificate
validation failed - {err}"),
+ }
+ }
}
+ SslVerifyError::FingerprintMismatch { fingerprint,
expected } => {
+ warn!("WARNING: certificate fingerprint does
not match expected fingerprint!");
+ warn!("expected: {expected}");
+
+ if interactive &&
std::io::stdin().is_terminal() {
+ match
Self::interactive_fp_check(prefix.as_deref(), &server,
verified_fingerprint.clone(), fingerprint_cache, fingerprint) {
+ Ok(()) => return true,
+ Err(err) => error!("certificate
validation failed - {err}"),
+ }
+ }
+ },
}
- *verified_fingerprint.lock().unwrap() =
Some(fingerprint);
- true
- }
- Err(err) => {
- error!("certificate validation failed - {}", err);
false
}
},
@@ -642,79 +651,41 @@ impl HttpClient {
bail!("no password input mechanism available");
}
- fn verify_callback(
- openssl_valid: bool,
- ctx: &mut X509StoreContextRef,
- expected_fingerprint: Option<&String>,
- interactive: bool,
- trust_openssl: Arc<Mutex<bool>>,
- ) -> Result<Option<String>, Error> {
- let mut trust_openssl_valid = trust_openssl.lock().unwrap();
-
- // we can only rely on openssl's prevalidation if we haven't forced it
earlier
- if openssl_valid && *trust_openssl_valid {
- return Ok(None);
- }
-
- let cert = match ctx.current_cert() {
- Some(cert) => cert,
- None => bail!("context lacks current certificate."),
- };
-
- // force trust in case of a chain, but set flag to no longer trust
prevalidation by openssl
- if ctx.error_depth() > 0 {
- *trust_openssl_valid = false;
- return Ok(None);
- }
-
- // leaf certificate - if we end up here, we have to verify the
fingerprint!
- let fp = match cert.digest(openssl::hash::MessageDigest::sha256()) {
- Ok(fp) => fp,
- Err(err) => bail!("failed to calculate certificate FP - {}", err),
// should not happen
- };
- let fp_string = hex::encode(fp);
- let fp_string = fp_string
- .as_bytes()
- .chunks(2)
- .map(|v| std::str::from_utf8(v).unwrap())
- .collect::<Vec<&str>>()
- .join(":");
-
- if let Some(expected_fingerprint) = expected_fingerprint {
- let expected_fingerprint = expected_fingerprint.to_lowercase();
- if expected_fingerprint == fp_string {
- return Ok(Some(fp_string));
- } else {
- warn!("WARNING: certificate fingerprint does not match
expected fingerprint!");
- warn!("expected: {}", expected_fingerprint);
- }
- }
-
- // If we're on a TTY, query the user
- if interactive && std::io::stdin().is_terminal() {
- info!("fingerprint: {}", fp_string);
- loop {
- eprint!("Are you sure you want to continue connecting? (y/n):
");
- let _ = std::io::stdout().flush();
- use std::io::{BufRead, BufReader};
- let mut line = String::new();
- match BufReader::new(std::io::stdin()).read_line(&mut line) {
- Ok(_) => {
- let trimmed = line.trim();
- if trimmed == "y" || trimmed == "Y" {
- return Ok(Some(fp_string));
- } else if trimmed == "n" || trimmed == "N" {
- bail!("Certificate fingerprint was not
confirmed.");
- } else {
- continue;
+ fn interactive_fp_check(
+ prefix: Option<&str>,
+ server: &str,
+ verified_fingerprint: Arc<Mutex<Option<String>>>,
+ fingerprint_cache: bool,
+ fingerprint: String,
+ ) -> Result<(), Error> {
+ info!("fingerprint: {fingerprint}");
+ loop {
+ eprint!("Are you sure you want to continue connecting? (y/n): ");
+ let _ = std::io::stdout().flush();
+ use std::io::{BufRead, BufReader};
+ let mut line = String::new();
+ match BufReader::new(std::io::stdin()).read_line(&mut line) {
+ Ok(_) => {
+ let trimmed = line.trim();
+ if trimmed == "y" || trimmed == "Y" {
+ if fingerprint_cache && prefix.is_some() {
+ if let Err(err) =
+ store_fingerprint(prefix.unwrap(), server,
&fingerprint)
+ {
+ error!("{}", err);
+ }
}
+ *verified_fingerprint.lock().unwrap() =
Some(fingerprint);
+ return Ok(());
+ } else if trimmed == "n" || trimmed == "N" {
+ bail!("Certificate fingerprint was not confirmed.");
+ } else {
+ continue;
}
- Err(err) => bail!("Certificate fingerprint was not
confirmed - {}.", err),
}
+ Err(err) => bail!("Certificate fingerprint was not confirmed -
{}.", err),
}
}
-
- bail!("Certificate fingerprint was not confirmed.");
}
pub async fn request(&self, mut req: Request<Body>) -> Result<Value,
Error> {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
