Skip to content

Commit

Permalink
Auto merge of #13970 - weihanglo:libgit2@1.8.0, r=ehuss
Browse files Browse the repository at this point in the history
fix: adjust custom err from cert-check due to libgit2 1.8 change
  • Loading branch information
bors committed May 31, 2024
2 parents 4a86f6f + 0978162 commit ef56deb
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 19 deletions.
23 changes: 9 additions & 14 deletions src/cargo/sources/git/known_hosts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 _;
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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\
Expand Down Expand Up @@ -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\
Expand Down Expand Up @@ -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\
Expand All @@ -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\
Expand All @@ -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.
Expand Down
25 changes: 21 additions & 4 deletions src/cargo/sources/git/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand Down Expand Up @@ -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(())
})
}
Expand Down
2 changes: 1 addition & 1 deletion tests/testsuite/https.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit ef56deb

Please sign in to comment.