ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

git: only try to use ssh-agent once per connection

Browse source 
As reported in #1970, SSH authentication would sometimes run into a
loop where it repeatedly tries to use ssh-agent for authentication
without making progess. The problem can be reproduced by simply
removing `$SSH_AUTH_KEY` from your environment (and not having a Git
credentials helper configured, I think).

This seems to be a bug introduced by b104f8e154c21. That commit meant
to make it so we attempt to use ssh-agent and fall back to using
(password-less) keys after that. The problem is that
`git2::Cred::ssh_key_from_agent()` just returns an object that will be
used later for looking up the credentials from ssh-agent, so the call
will not fail because ssh-agent is not reachable.

This commit attempts to fix the problem by having the credentials
callback attempt to use ssh-agent only once.
This commit is contained in:
Martin von Zweigbergk 2023-08-07 21:30:23 -07:00 committed by Martin von Zweigbergk
parent a9e5e97025
commit c752b43db1
2 changed files with 13 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -50,6 +50,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed bugs
* SSH authentication could hang when ssh-agent couldn't be reached
[#1970](https://github.com/martinvonz/jj/issues/1970)
## [0.8.0] - 2023-07-09
### Breaking changes

32
lib/src/git.rs
View file

@ -878,6 +878,7 @@ impl<'a> RemoteCallbacks<'a> {
}
// TODO: We should expose the callbacks to the caller instead -- the library
// crate shouldn't read environment variables.
let mut tried_ssh_agent = false;
callbacks.credentials(move |url, username_from_url, allowed_types| {
let span = tracing::debug_span!("RemoteCallbacks.credentials");
let _ = span.enter();
@ -890,28 +891,15 @@ impl<'a> RemoteCallbacks<'a> {
return Ok(creds);
} else if let Some(username) = username_from_url {
if allowed_types.contains(git2::CredentialType::SSH_KEY) {
// Try to get the SSH key from the agent by default, and report an error
// only if it _seems_ like that's what the user wanted.
//
// Note that the env variables read below are **not** the only way to
// communicate with the agent, which is why we request a key from it no
// matter what.
match git2::Cred::ssh_key_from_agent(username) {
Ok(key) => {
tracing::info!(username, "using ssh_key_from_agent");
return Ok(key);
}
Err(err) => {
if std::env::var("SSH_AUTH_SOCK").is_ok()
|| std::env::var("SSH_AGENT_PID").is_ok()
{
tracing::error!(err = %err);
return Err(err);
}
// There is no agent-related env variable so we
// consider that the user doesn't care about using
// the agent and proceed.
}
// Try to get the SSH key from the agent once. We don't even check if
// $SSH_AUTH_SOCK is set because Windows uses another mechanism.
if !tried_ssh_agent {
tracing::info!(username, "using ssh_key_from_agent");
tried_ssh_agent = true;
return git2::Cred::ssh_key_from_agent(username).map_err(|err| {
tracing::error!(err = %err);
err
});
}
if let Some(ref mut cb) = self.get_ssh_key {