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 libgit2/libgit2@9a9f220
rust-lang · May 28, 2024 · aa2ad97 · aa2ad97
1 parent 95eeafa
commit aa2ad97
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 29 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -44,8 +44,8 @@ curl = "0.4.46"
 curl-sys = "0.4.72"
 filetime = "0.2.23"
 flate2 = { version = "1.0.30", default-features = false, features = ["zlib"] }
-git2 = "0.18.3"
-git2-curl = "0.19.0"
+git2 = { git = "https://github.com/rust-lang/git2-rs.git", rev = "b318ea3d2cbf79eed2eaa4af8ef74d8522d7ce9a" }
+git2-curl = { git = "https://github.com/rust-lang/git2-rs.git", rev = "b318ea3d2cbf79eed2eaa4af8ef74d8522d7ce9a" }
 gix = { version = "0.63.0", default-features = false, features = ["blocking-http-transport-curl", "progress-tree", "revision", "parallel", "dirwalk"] }
 glob = "0.3.1"
 handlebars = { version = "5.1.2", features = ["dir_source"] }
@@ -61,7 +61,7 @@ itertools = "0.12.1"
 jobserver = "0.1.31"
 lazycell = "1.3.0"
 libc = "0.2.154"
-libgit2-sys = "0.16.2"
+libgit2-sys = { git = "https://github.com/rust-lang/git2-rs.git", rev = "b318ea3d2cbf79eed2eaa4af8ef74d8522d7ce9a" }
 libloading = "0.8.3"
 memchr = "2.7.2"
 miow = "0.6.0"

diff --git 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<u16>,
     config_known_hosts: Option<&Vec<Value<String>>>,
     diagnostic_home_config: &str,
-) -> Result<CertificateCheckStatus, git2::Error> {
+) -> CargoResult<CertificateCheckStatus> {
     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
@@ -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
@@ -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 {