Skip to content

Commit 537f718

Browse files
alexcrichtonalexcrichton
committed
Correctly attempt multiple usernames with libgit2
This commit corrects the logic for attempting multiple usernames with libgit2. There is a restriction that for each authentication seession you can only authenticate with one ssh username, but we were attempting multiple as part of the same sesssion. Instead restart authentication whenever we try a new username. Closes #2399
1 parent 88e3081 commit 537f718

File tree

2 files changed

+97
-82
lines changed

2 files changed

+97
-82
lines changed

‎src/cargo/sources/git/utils.rs

+94-65
Original file line numberDiff line numberDiff line change
@@ -388,29 +388,18 @@ impl<'a> GitCheckout<'a> {
388388
/// attempted and we don't try the same ones again.
389389
fn with_authentication<T, F>(url: &str, cfg: &git2::Config, mut f: F)
390390
-> CargoResult<T>
391-
where F: FnMut(&mut git2::Credentials) -> CargoResult<T>
391+
where F: FnMut(&mut git2::Credentials) -> Result<T, git2::Error>
392392
{
393393
let mut cred_helper = git2::CredentialHelper::new(url);
394394
cred_helper.config(cfg);
395395

396-
let mut attempted = git2::CredentialType::empty();
397-
let mut failed_cred_helper = false;
398-
399-
// We try a couple of different user names when cloning via ssh as there's a
400-
// few possibilities if one isn't mentioned, and these are used to keep
401-
// track of that.
402-
enum UsernameAttempt {
403-
Arg,
404-
CredHelper,
405-
Local,
406-
Git,
407-
}
408-
let mut username_attempt = UsernameAttempt::Arg;
409-
let mut username_attempts = Vec::new();
410-
411-
let res = f(&mut |url, username, allowed| {
412-
let allowed = allowed & !attempted;
396+
let mut ssh_username_requested = false;
397+
let mut cred_helper_bad = None;
398+
let mut ssh_agent_attempts = Vec::new();
399+
let mut any_attempts = false;
413400

401+
let mut res = f(&mut |url, username, allowed| {
402+
any_attempts = true;
414403
// libgit2's "USERNAME" authentication actually means that it's just
415404
// asking us for a username to keep going. This is currently only really
416405
// used for SSH authentication and isn't really an authentication type.
@@ -422,51 +411,32 @@ fn with_authentication<T, F>(url: &str, cfg: &git2::Config, mut f: F)
422411
//
423412
// callback(SSH_KEY, user, ...)
424413
//
425-
// So if we have a USERNAME request we just pass it either `username` or
426-
// a fallback of "git". We'll do some more principled attempts later on.
414+
// So if we're being called here then we know that (a) we're using ssh
415+
// authentication and (b) no username was specified in the URL that
416+
// we're trying to clone. We need to guess an appropriate username here,
417+
// but that may involve a few attempts. Unfortunately we can't switch
418+
// usernames during one authentication session with libgit2, so to
419+
// handle this we bail out of this authentication session after setting
420+
// the flag `ssh_username_requested`, and then we handle this below.
427421
if allowed.contains(git2::USERNAME) {
428-
attempted = attempted | git2::USERNAME;
429-
return git2::Cred::username(username.unwrap_or("git"))
422+
debug_assert!(username.is_none());
423+
ssh_username_requested = true;
424+
return Err(git2::Error::from_str("gonna try usernames later"))
430425
}
431426

432427
// An "SSH_KEY" authentication indicates that we need some sort of SSH
433428
// authentication. This can currently either come from the ssh-agent
434429
// process or from a raw in-memory SSH key. Cargo only supports using
435430
// ssh-agent currently.
436431
//
437-
// We try a few different usernames here, including:
438-
//
439-
// 1. The `username` argument, if provided. This will cover cases where
440-
// the user was passed in the URL, for example.
441-
// 2. The global credential helper's username, if any is configured
442-
// 3. The local account's username (if present)
443-
// 4. Finally, "git" as it's a common fallback (e.g. with github)
432+
// If we get called with this then the only way that should be possible
433+
// is if a username is specified in the URL itself (e.g. `username` is
434+
// Some), hence the unwrap() here. We try custom usernames down below.
444435
if allowed.contains(git2::SSH_KEY) {
445-
loop {
446-
let name = match username_attempt {
447-
UsernameAttempt::Arg => {
448-
username_attempt = UsernameAttempt::CredHelper;
449-
username.map(|s| s.to_string())
450-
}
451-
UsernameAttempt::CredHelper => {
452-
username_attempt = UsernameAttempt::Local;
453-
cred_helper.username.clone()
454-
}
455-
UsernameAttempt::Local => {
456-
username_attempt = UsernameAttempt::Git;
457-
env::var("USER").or_else(|_| env::var("USERNAME")).ok()
458-
}
459-
UsernameAttempt::Git => {
460-
attempted = attempted | git2::SSH_KEY;
461-
Some("git".to_string())
462-
}
463-
};
464-
if let Some(name) = name {
465-
let ret = git2::Cred::ssh_key_from_agent(&name);
466-
username_attempts.push(name);
467-
return ret
468-
}
469-
}
436+
let username = username.unwrap();
437+
debug_assert!(!ssh_username_requested);
438+
ssh_agent_attempts.push(username.to_string());
439+
return git2::Cred::ssh_key_from_agent(&username)
470440
}
471441

472442
// Sometimes libgit2 will ask for a username/password in plaintext. This
@@ -475,25 +445,84 @@ fn with_authentication<T, F>(url: &str, cfg: &git2::Config, mut f: F)
475445
// plaintext password is through the `credential.helper` support, so
476446
// fetch that here.
477447
if allowed.contains(git2::USER_PASS_PLAINTEXT) {
478-
attempted = attempted | git2::USER_PASS_PLAINTEXT;
479448
let r = git2::Cred::credential_helper(cfg, url, username);
480-
failed_cred_helper = r.is_err();
449+
cred_helper_bad = Some(r.is_err());
481450
return r
482451
}
483452

484453
// I'm... not sure what the DEFAULT kind of authentication is, but seems
485454
// easy to support?
486455
if allowed.contains(git2::DEFAULT) {
487-
attempted = attempted | git2::DEFAULT;
488456
return git2::Cred::default()
489457
}
490458

491459
// Whelp, we tried our best
492460
Err(git2::Error::from_str("no authentication available"))
493461
});
494462

495-
if attempted.bits() == 0 || res.is_ok() {
496-
return res
463+
// Ok, so if it looks like we're going to be doing ssh authentication, we
464+
// want to try a few different usernames as one wasn't specified in the URL
465+
// for us to use. In order, we'll try:
466+
//
467+
// * A credential helper's username for this URL, if available.
468+
// * This account's username.
469+
// * "git"
470+
//
471+
// We have to restart the authentication session each time (due to
472+
// constraints in libssh2 I guess? maybe this is inherent to ssh?), so we
473+
// call our callback, `f`, in a loop here.
474+
if ssh_username_requested {
475+
debug_assert!(res.is_err());
476+
let mut attempts = Vec::new();
477+
attempts.push("git".to_string());
478+
if let Ok(s) = env::var("USER").or_else(|_| env::var("USERNAME")) {
479+
attempts.push(s);
480+
}
481+
if let Some(ref s) = cred_helper.username {
482+
attempts.push(s.clone());
483+
}
484+
485+
while let Some(s) = attempts.pop() {
486+
// We should get `USERNAME` first, where we just return our attempt,
487+
// and then after that we should get `SSH_KEY`. If the first attempt
488+
// fails we'll get called again, but we don't have another option so
489+
// we bail out.
490+
let mut attempts = 0;
491+
res = f(&mut |_url, username, allowed| {
492+
if allowed.contains(git2::USERNAME) {
493+
return git2::Cred::username(&s);
494+
}
495+
if allowed.contains(git2::SSH_KEY) {
496+
debug_assert_eq!(Some(&s[..]), username);
497+
attempts += 1;
498+
if attempts == 1 {
499+
ssh_agent_attempts.push(s.to_string());
500+
return git2::Cred::ssh_key_from_agent(&s)
501+
}
502+
}
503+
Err(git2::Error::from_str("no authentication available"))
504+
});
505+
506+
// If we made two attempts then that means:
507+
//
508+
// 1. A username was requested, we returned `s`.
509+
// 2. An ssh key was requested, we returned to look up `s` in the
510+
// ssh agent.
511+
// 3. For whatever reason that lookup failed, so we were asked again
512+
// for another mode of authentication.
513+
//
514+
// Essentially, if `attempts == 2` then in theory the only error was
515+
// that this username failed to authenticate (e.g. no other network
516+
// errors happened). Otherwise something else is funny so we bail
517+
// out.
518+
if attempts != 2 {
519+
break
520+
}
521+
}
522+
}
523+
524+
if res.is_ok() || !any_attempts {
525+
return res.map_err(From::from)
497526
}
498527

499528
// In the case of an authentication failure (where we tried something) then
@@ -502,15 +531,15 @@ fn with_authentication<T, F>(url: &str, cfg: &git2::Config, mut f: F)
502531
res.chain_error(|| {
503532
let mut msg = "failed to authenticate when downloading \
504533
repository".to_string();
505-
if attempted.contains(git2::SSH_KEY) {
506-
let names = username_attempts.iter()
507-
.map(|s| format!("`{}`", s))
508-
.collect::<Vec<_>>()
509-
.join(", ");
534+
if ssh_agent_attempts.len() > 0 {
535+
let names = ssh_agent_attempts.iter()
536+
.map(|s| format!("`{}`", s))
537+
.collect::<Vec<_>>()
538+
.join(", ");
510539
msg.push_str(&format!("\nattempted ssh-agent authentication, but \
511540
none of the usernames {} succeeded", names));
512541
}
513-
if attempted.contains(git2::USER_PASS_PLAINTEXT) {
542+
if let Some(failed_cred_helper) = cred_helper_bad {
514543
if failed_cred_helper {
515544
msg.push_str("\nattempted to find username/password via \
516545
git's `credential.helper` support, but failed");

‎tests/test_cargo_build_auth.rs

+3-17
Original file line numberDiff line numberDiff line change
@@ -145,17 +145,10 @@ test!(https_something_happens {
145145
updating = UPDATING,
146146
addr = addr,
147147
))
148-
.with_stderr(&format!("\
149-
{error} Unable to update https://{addr}/foo/bar
150-
151-
Caused by:
152-
failed to clone into: [..]
153-
148+
.with_stderr_contains(&format!("\
154149
Caused by:
155150
{errmsg}
156151
",
157-
addr = addr,
158-
error = ERROR,
159152
errmsg = if cfg!(windows) {
160153
"[[..]] failed to send request: [..]\n"
161154
} else if cfg!(target_os = "macos") {
@@ -197,16 +190,9 @@ test!(ssh_something_happens {
197190
updating = UPDATING,
198191
addr = addr,
199192
))
200-
.with_stderr(&format!("\
201-
{error} Unable to update ssh://{addr}/foo/bar
202-
203-
Caused by:
204-
failed to clone into: [..]
205-
193+
.with_stderr_contains("\
206194
Caused by:
207195
[[..]] Failed to start SSH session: Failed getting banner
208-
",
209-
addr = addr,
210-
error = ERROR)));
196+
"));
211197
t.join().ok().unwrap();
212198
});

0 commit comments

Comments
 (0)