From 0978162ebe032e803dab4bc3e2094a51fc717f84 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Tue, 28 May 2024 00:28:17 +0000 Subject: [PATCH] fix: adjust custom err from cert-check due to libgit2 1.8 change libgit2 disallows overriding errors from certificate check since v1.8.0, so we store the error additionally and unwrap it later. See https://github.com/libgit2/libgit2/commit/9a9f220119d9647a352867b24b0556195cb26548 --- src/cargo/sources/git/known_hosts.rs | 23 +++++++++-------------- src/cargo/sources/git/utils.rs | 25 +++++++++++++++++++++---- tests/testsuite/https.rs | 2 +- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/src/cargo/sources/git/known_hosts.rs b/src/cargo/sources/git/known_hosts.rs index 8ce081194b0..25bcf478bd1 100644 --- a/src/cargo/sources/git/known_hosts.rs +++ b/src/cargo/sources/git/known_hosts.rs @@ -23,6 +23,7 @@ //! and revoked markers. See "FIXME" comments littered in this file. use crate::util::context::{Definition, GlobalContext, Value}; +use crate::CargoResult; use base64::engine::general_purpose::STANDARD; use base64::engine::general_purpose::STANDARD_NO_PAD; use base64::Engine as _; @@ -137,7 +138,7 @@ pub fn certificate_check( port: Option, config_known_hosts: Option<&Vec>>, diagnostic_home_config: &str, -) -> Result { +) -> CargoResult { let Some(host_key) = cert.as_hostkey() else { // Return passthrough for TLS X509 certificates to use whatever validation // was done in git2. @@ -150,13 +151,12 @@ pub fn certificate_check( _ => host.to_string(), }; // The error message must be constructed as a string to pass through the libgit2 C API. - let err_msg = match check_ssh_known_hosts(gctx, host_key, &host_maybe_port, config_known_hosts) - { + match check_ssh_known_hosts(gctx, host_key, &host_maybe_port, config_known_hosts) { Ok(()) => { return Ok(CertificateCheckStatus::CertificateOk); } Err(KnownHostError::CheckError(e)) => { - format!("error: failed to validate host key:\n{:#}", e) + anyhow::bail!("error: failed to validate host key:\n{:#}", e) } Err(KnownHostError::HostKeyNotFound { hostname, @@ -193,7 +193,7 @@ pub fn certificate_check( } msg }; - format!("error: unknown SSH host key\n\ + anyhow::bail!("error: unknown SSH host key\n\ The SSH host key for `{hostname}` is not known and cannot be validated.\n\ \n\ To resolve this issue, add the host key to {known_hosts_location}\n\ @@ -242,7 +242,7 @@ pub fn certificate_check( ) } }; - format!("error: SSH host key has changed for `{hostname}`\n\ + anyhow::bail!("error: SSH host key has changed for `{hostname}`\n\ *********************************\n\ * WARNING: HOST KEY HAS CHANGED *\n\ *********************************\n\ @@ -274,7 +274,7 @@ pub fn certificate_check( location, }) => { let key_type_short_name = key_type.short_name(); - format!( + anyhow::bail!( "error: Key has been revoked for `{hostname}`\n\ **************************************\n\ * WARNING: REVOKED HOST KEY DETECTED *\n\ @@ -288,7 +288,7 @@ pub fn certificate_check( ) } Err(KnownHostError::HostHasOnlyCertAuthority { hostname, location }) => { - format!("error: Found a `@cert-authority` marker for `{hostname}`\n\ + anyhow::bail!("error: Found a `@cert-authority` marker for `{hostname}`\n\ \n\ Cargo doesn't support certificate authorities for host key verification. It is\n\ recommended that the command line Git client is used instead. This can be achieved\n\ @@ -300,12 +300,7 @@ pub fn certificate_check( for more information.\n\ ") } - }; - Err(git2::Error::new( - git2::ErrorCode::GenericError, - git2::ErrorClass::Callback, - err_msg, - )) + } } /// Checks if the given host/host key pair is known. diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index 6117266559a..5ade00902ed 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -831,7 +831,10 @@ pub fn with_fetch_options( let config_known_hosts = ssh_config.and_then(|ssh| ssh.known_hosts.as_ref()); let diagnostic_home_config = gctx.diagnostic_home_config(); network::retry::with_retry(gctx, || { - with_authentication(gctx, url, git_config, |f| { + // Hack: libgit2 disallows overriding the error from check_cb since v1.8.0, + // so we store the error additionally and unwrap it later + let mut check_cb_result = Ok(()); + let auth_result = with_authentication(gctx, url, git_config, |f| { let port = Url::parse(url).ok().and_then(|url| url.port()); let mut last_update = Instant::now(); let mut rcb = git2::RemoteCallbacks::new(); @@ -840,14 +843,24 @@ pub fn with_fetch_options( let mut counter = MetricsCounter::<10>::new(0, last_update); rcb.credentials(f); rcb.certificate_check(|cert, host| { - super::known_hosts::certificate_check( + match super::known_hosts::certificate_check( gctx, cert, host, port, config_known_hosts, &diagnostic_home_config, - ) + ) { + Ok(status) => Ok(status), + Err(e) => { + check_cb_result = Err(e); + // This is not really used because it'll be overridden by libgit2 + // See https://github.com/libgit2/libgit2/commit/9a9f220119d9647a352867b24b0556195cb26548 + Err(git2::Error::from_str( + "invalid or unknown remote ssh hostkey", + )) + } + } }); rcb.transfer_progress(|stats| { let indexed_deltas = stats.indexed_deltas(); @@ -889,7 +902,11 @@ pub fn with_fetch_options( let mut opts = git2::FetchOptions::new(); opts.remote_callbacks(rcb); cb(opts) - })?; + }); + if auth_result.is_err() { + check_cb_result?; + } + auth_result?; Ok(()) }) } diff --git a/tests/testsuite/https.rs b/tests/testsuite/https.rs index e87ea6bec9a..8940722322c 100644 --- a/tests/testsuite/https.rs +++ b/tests/testsuite/https.rs @@ -33,7 +33,7 @@ fn self_signed_should_fail() { let err_msg = if cfg!(target_os = "macos") { "untrusted connection error; class=Ssl (16); code=Certificate (-17)" } else if cfg!(unix) { - "the SSL certificate is invalid; class=Ssl (16); code=Certificate (-17)" + "the SSL certificate is invalid; class=Ssl (16)[..]" } else if cfg!(windows) { "user cancelled certificate check; class=Http (34); code=Certificate (-17)" } else {