nabijaczleweli / cargo-update

A cargo subcommand for checking and applying updates to installed executables
MIT License
1.22k stars 42 forks source link

Updating using keepassxc ssh-agent #110

Closed nycex closed 5 years ago

nycex commented 5 years ago

I'm using the keepassxc ssh-agent function and it works with ssh, git and cargo install, but with cargo install-update i get the following error when doing cargo install-update -ga:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: -1, klass: 0, message: "
no authentication available" }', src/libcore/result.rs:999:5

backtrace:

stack backtrace:
   0:     0x55d264fbc7eb - backtrace::backtrace::libunwind::trace::hfe5db90796807973
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1:     0x55d264fbc7eb - backtrace::backtrace::trace_unsynchronized::h34b865a835594335
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2:     0x55d264fbc7eb - std::sys_common::backtrace::_print::h527254ae44989167
                               at src/libstd/sys_common/backtrace.rs:47
   3:     0x55d264fbc7eb - std::sys_common::backtrace::print::he85dd5ddddf46503
                               at src/libstd/sys_common/backtrace.rs:36
   4:     0x55d264fbc7eb - std::panicking::default_hook::{{closure}}::h847a2eb38b396f14
                               at src/libstd/panicking.rs:200
   5:     0x55d264fbc4c7 - std::panicking::default_hook::h2ca0f9a30a0e206b
                               at src/libstd/panicking.rs:214
   6:     0x55d264fbcf60 - std::panicking::rust_panic_with_hook::hffcefc09751839d1
                               at src/libstd/panicking.rs:477
   7:     0x55d264fbcae2 - std::panicking::continue_panic_fmt::hc0f142c930c846fc
                               at src/libstd/panicking.rs:384
   8:     0x55d264fbc9c6 - rust_begin_unwind
                               at src/libstd/panicking.rs:311
   9:     0x55d264fdb97d - core::panicking::panic_fmt::h2daf88b2616ca2b2
                               at src/libcore/panicking.rs:85
  10:     0x55d264d7c1be - core::result::unwrap_failed::h834722eb63104a22
  11:     0x55d264d8fff3 - cargo_update::ops::with_authentication::{{closure}}::h3ce01db401f9020a
  12:     0x55d264db75f1 - cargo_update::ops::GitRepoPackage::pull_version_impl::{{closure}}::{{closure}}::heb1dbb48c1005603
  13:     0x55d264e38b25 - std::panicking::try::do_call::he3f7290af8589bcd
  14:     0x55d264fc3b7a - __rust_maybe_catch_panic
                               at src/libpanic_unwind/lib.rs:82
  15:     0x55d264e37423 - git2::panic::wrap::h7893a882ee8fb1b6
  16:     0x55d264e3bd3e - git2::remote_callbacks::credentials_cb::h76b2c3b8a1e220e0
  17:     0x55d264ebd9df - request_creds.isra.0
  18:     0x55d264ebe90e - _git_ssh_setup_conn
  19:     0x55d264edcca0 - git_smart__connect
  20:     0x55d264e9cb00 - git_remote__connect
  21:     0x55d264e9e1eb - git_remote_fetch
  22:     0x55d264e31c3c - git2::remote::Remote::fetch::h25706383f7087232
  23:     0x55d264d8b6b4 - cargo_update::ops::GitRepoPackage::pull_version_impl::h1ea7995748ddfb1d
  24:     0x55d264d74789 - cargo_install_update::actual_main::h3a05c3cde10e6a7a
  25:     0x55d264d72a66 - cargo_install_update::main::h56b00bac1584b9e8
  26:     0x55d264d6e6e3 - std::rt::lang_start::{{closure}}::hc3c6f18673592973
  27:     0x55d264fbc963 - std::rt::lang_start_internal::{{closure}}::h447d8812e3ee306d
                               at src/libstd/rt.rs:49
  28:     0x55d264fbc963 - std::panicking::try::do_call::h4a61cb372364c745
                               at src/libstd/panicking.rs:296
  29:     0x55d264fc3b7a - __rust_maybe_catch_panic
                               at src/libpanic_unwind/lib.rs:82
  30:     0x55d264fbd46d - std::panicking::try::hdf71f938885bca42
                               at src/libstd/panicking.rs:275
  31:     0x55d264fbd46d - std::panic::catch_unwind::h7e85dbf162b1611a
                               at src/libstd/panic.rs:394
  32:     0x55d264fbd46d - std::rt::lang_start_internal::h1e06cc26b9fc25ea
                               at src/libstd/rt.rs:48
  33:     0x55d264d76072 - main
  34:     0x7f7974277ee3 - __libc_start_main
  35:     0x55d264d6b28e - _start
  36:                0x0 - <unknown>
thread 'main' panicked at 'Box<Any>', /home/bernhard/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/git2-0.10.1/src/panic.rs:27:9
stack backtrace:
   0:     0x55d264fbc7eb - backtrace::backtrace::libunwind::trace::hfe5db90796807973
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1:     0x55d264fbc7eb - backtrace::backtrace::trace_unsynchronized::h34b865a835594335
                               at /cargo/registry/src/github.com-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2:     0x55d264fbc7eb - std::sys_common::backtrace::_print::h527254ae44989167
                               at src/libstd/sys_common/backtrace.rs:47
   3:     0x55d264fbc7eb - std::sys_common::backtrace::print::he85dd5ddddf46503
                               at src/libstd/sys_common/backtrace.rs:36
   4:     0x55d264fbc7eb - std::panicking::default_hook::{{closure}}::h847a2eb38b396f14
                               at src/libstd/panicking.rs:200
   5:     0x55d264fbc4c7 - std::panicking::default_hook::h2ca0f9a30a0e206b
                               at src/libstd/panicking.rs:214
   6:     0x55d264fbcf60 - std::panicking::rust_panic_with_hook::hffcefc09751839d1
                               at src/libstd/panicking.rs:477
   7:     0x55d264e38533 - std::panicking::begin_panic::h81f6d2791698b7e4
   8:     0x55d264e31e85 - git2::remote::Remote::fetch::h25706383f7087232
   9:     0x55d264d8b6b4 - cargo_update::ops::GitRepoPackage::pull_version_impl::h1ea7995748ddfb1d
  10:     0x55d264d74789 - cargo_install_update::actual_main::h3a05c3cde10e6a7a
  11:     0x55d264d72a66 - cargo_install_update::main::h56b00bac1584b9e8
  12:     0x55d264d6e6e3 - std::rt::lang_start::{{closure}}::hc3c6f18673592973
  13:     0x55d264fbc963 - std::rt::lang_start_internal::{{closure}}::h447d8812e3ee306d
                               at src/libstd/rt.rs:49
  14:     0x55d264fbc963 - std::panicking::try::do_call::h4a61cb372364c745
                               at src/libstd/panicking.rs:296
  15:     0x55d264fc3b7a - __rust_maybe_catch_panic
                               at src/libpanic_unwind/lib.rs:82
  16:     0x55d264fbd46d - std::panicking::try::hdf71f938885bca42
                               at src/libstd/panicking.rs:275
  17:     0x55d264fbd46d - std::panic::catch_unwind::h7e85dbf162b1611a
                               at src/libstd/panic.rs:394
  18:     0x55d264fbd46d - std::rt::lang_start_internal::h1e06cc26b9fc25ea
                               at src/libstd/rt.rs:48
  19:     0x55d264d76072 - main
  20:     0x7f7974277ee3 - __libc_start_main
  21:     0x55d264d6b28e - _start
  22:                0x0 - <unknown>

Edit: I'm using cargo install --git ssh://gitlab.com/nycex/someprivateproject --force to install the packages

nabijaczleweli commented 5 years ago

ugh, that's what I get for stealing code from old cargo, I guess. I'll see about porting the auth code from current cargo, but, for the time being, could you be a love and run an update with this patch to see which auth type git is trying to use?

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index aa11e32d4..72e7a78fd 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -825,17 +825,27 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> T

     let mut cred_helper = git2::CredentialHelper::new(url);
     cred_helper.config(&cfg);
-    f(&mut |url, username, allowed| if allowed.contains(CredentialType::SSH_KEY) {
-        let user = username.map(|s| s.to_string())
-            .or_else(|| cred_helper.username.clone())
-            .unwrap_or("git".to_string());
-        GitCred::ssh_key_from_agent(&user)
-    } else if allowed.contains(CredentialType::USER_PASS_PLAINTEXT) {
-        GitCred::credential_helper(&cfg, url, username)
-    } else if allowed.contains(CredentialType::DEFAULT) {
-        GitCred::default()
-    } else {
-        Err(GitError::from_str("no authentication available")).unwrap()
+    f(&mut |url, username, allowed| {
+        println!("user_pass_plaintext?={}", allowed.is_user_pass_plaintext());
+        println!("ssh_key?={}", allowed.is_ssh_key());
+        println!("ssh_memory?={}", allowed.is_ssh_memory());
+        println!("ssh_custom?={}", allowed.is_ssh_custom());
+        println!("default?={}", allowed.is_default());
+        println!("ssh_interactive?={}", allowed.is_ssh_interactive());
+        println!("username?={}", allowed.is_username());
+
+        if allowed.contains(CredentialType::SSH_KEY) {
+            let user = username.map(|s| s.to_string())
+                .or_else(|| cred_helper.username.clone())
+                .unwrap_or("git".to_string());
+            GitCred::ssh_key_from_agent(&user)
+        } else if allowed.contains(CredentialType::USER_PASS_PLAINTEXT) {
+            GitCred::credential_helper(&cfg, url, username)
+        } else if allowed.contains(CredentialType::DEFAULT) {
+            GitCred::default()
+        } else {
+            Err(GitError::from_str("no authentication available")).unwrap()
+        }
     })
 }
nycex commented 5 years ago

Ran it with the patch, it outputs the following before the error:

user_pass_plaintext?=false
ssh_key?=false
ssh_memory?=false
ssh_custom?=false
default?=false
ssh_interactive?=false
username?=true
nabijaczleweli commented 5 years ago

Yep, that's as expected, then. The newest commit referenced above should resolve the issue by having ported over the auth code from the current Cargo master, so if you would be so kind as to try it out?

nycex commented 5 years ago

It still doesn't work, as i can see from the error message it tries to use my username of my machine instead of git username or ssh key

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: -1, klass: 0, message: "no authentication available: failed to authenticate when downloading repository ssh://gitlab.com/nycex/someprivateproject (tried ssh-agent, but none of the following usernames worked: \"mylocalpcusername\")" }', src/libcore/result.rs:999:5
nabijaczleweli commented 5 years ago

That's... certainly interesting? Can you try with this diff, which should either fix some ported cargo logic or at least provide some diagnostics?

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index 1b20528b7..122d8e0d2 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -840,12 +840,14 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>

         if allowed.contains(CredentialType::USERNAME) {
             ssh_username_requested = true;
+            println!("CredentialType::USERNAME");

             Err(GitError::from_str("username to be tried later"))
         } else if allowed.contains(CredentialType::SSH_KEY) && !tried_ssh_key {
             tried_ssh_key = true;

             let username = username.unwrap();
+            println!("first round: {}", username);
             ssh_agent_attempts.push(username.to_string());

             GitCred::ssh_key_from_agent(username)
@@ -861,10 +863,27 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>
     });

     if ssh_username_requested {
-        for uname in cred_helper.username.into_iter().chain(["USER", "USERNAME"].into_iter().flat_map(env::var)).chain(Some("git".to_string())) {
+        let mut attempts = Vec::new();
+        attempts.push("git".to_string());
+        if let Ok(s) = env::var("USER").or_else(|_| env::var("USERNAME")) {
+            println!("e: {}", s);
+            attempts.push(s);
+        }
+        if let Some(ref s) = cred_helper.username {
+            println!("ch: {}", s);
+            attempts.push(s.clone());
+        }
+        println!("{:?}", attempts);
+
+        while let Some(uname) = attempts.pop() {
             let mut ssh_attempts = 0;

             res = f(&mut |_, _, allowed| {
+                println!("{}; is_u?={}; is_sk?={}",
+                         ssh_attempts,
+                         allowed.contains(CredentialType::USERNAME),
+                         allowed.contains(CredentialType::SSH_KEY));
+
                 if allowed.contains(CredentialType::USERNAME) {
                     return GitCred::username(&uname);
                 } else if allowed.contains(CredentialType::SSH_KEY) {
@@ -875,7 +894,7 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>
                     }
                 }

-                Err(GitError::from_str("no authentication available"))
+                Err(GitError::from_str("no authentication available (2)"))
             });

             if ssh_attempts == 2 {
nycex commented 5 years ago

It doesn't fix it, but i get this output now:

CredentialType::USERNAME
e: mylocalpcusername
["git", "mylocalpcusername"]
0; is_u?=true; is_sk?=false
0; is_u?=false; is_sk?=true
1; is_u?=false; is_sk?=true
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: -1, klass: 0, message: "no authentication available (2): failed to authenticate when downloading repository ssh://gitlab.com/nycex/someprivateproject (tried ssh-agent, but none of the following usernames worked: \"mylocalpcusername\")" }', src/libcore/result.rs:999:5
nycex commented 5 years ago

I don't even know why it gets my local pc username, i have nycex set as my user.name in my gitconfig.

nabijaczleweli commented 5 years ago

That'd mean that, somehow, cargo manages to pick up that variable but we don't?..

The following diff will display whatever we do manage to pick up, and maybe that'll hold some clues, but I'm stumped, quite frankly

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index 1b20528b7..f6a4bac47 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -825,9 +825,13 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>
     where F: FnMut(&mut git2::Credentials) -> Result<T, GitError>
 {
     let cfg = GitConfig::open_default().unwrap();
+    for ent in cfg.entries(None).unwrap().flatten() {
+        println!("{:?} {:?} {:?}", ent.name(), ent.value(), ent.level());
+    }

     let mut cred_helper = git2::CredentialHelper::new(url);
     cred_helper.config(&cfg);
+    println!("{:?}", cred_helper.username);

     let mut ssh_username_requested = false;
     let mut cred_helper_bad = None;
nycex commented 5 years ago

The GitConfig does manage to pick it up, but the CredentialHelper doesn't as i see it:

Some("user.email") Some("nycex@protonmail.com") XDG
Some("user.name") Some("nycex") XDG
Some("user.signingkey") Some("censored") XDG
Some("color.ui") Some("true") XDG
Some("core.editor") Some("nvim") XDG
Some("alias.exclude") Some("update-index --assume-unchanged") XDG
Some("alias.include") Some("update-index --no-assume-unchanged") XDG
Some("filter.lfs.clean") Some("git-lfs clean -- %f") XDG
Some("filter.lfs.smudge") Some("git-lfs smudge -- %f") XDG
Some("filter.lfs.process") Some("git-lfs filter-process") XDG
Some("filter.lfs.required") Some("true") XDG
Some("commit.gpgsign") Some("true") XDG
None
nabijaczleweli commented 5 years ago

Oh, it looks like CredentialHelper just doesn't use that key? Interesting how cargo's updater manages to work, then.

Anyway, I added an explicit attempt over the user.name key in the commit above, so it'd be great if you could try with that.

If it doesn't work, then the following diff will show some diagnostics:

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index 4fc18030c..7fffe6a3b 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -861,6 +861,16 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>
     });

     if ssh_username_requested {
+        println!("{:?}", cfg.get_string("user.name"));
+        println!("{:#?}",
+                 cred_helper.username
+                     .clone()
+                     .into_iter()
+                     .chain(cfg.get_string("user.name"))
+                     .chain(["USERNAME", "USER"].into_iter().flat_map(env::var))
+                     .chain(Some("git").into_iter().map(str::to_string))
+                     .collect::<Vec<_>>());
+
         // NOTE: this is the only divergence from the original cargo code: we also try cfg["user.name"]
         //       see https://github.com/nabijaczleweli/cargo-update/issues/110#issuecomment-533091965 for explanation
         for uname in cred_helper.username
@@ -871,6 +881,12 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>
             let mut ssh_attempts = 0;

             res = f(&mut |_, _, allowed| {
+                println!("{} {} is_u?={} is_sk?={}",
+                         ssh_attempts,
+                         uname,
+                         allowed.contains(CredentialType::USERNAME),
+                         allowed.contains(CredentialType::SSH_KEY));
+
                 if allowed.contains(CredentialType::USERNAME) {
                     return GitCred::username(&uname);
                 } else if allowed.contains(CredentialType::SSH_KEY) {
nycex commented 5 years ago

It does pick up the correct username now, but it still doesn't use the ssh-agent key. That's the output with the last two patches applied, if that helps:

Some("user.email") Some("nycex@protonmail.com") XDG
Some("user.name") Some("nycex") XDG
Some("user.signingkey") Some("censored") XDG
Some("color.ui") Some("true") XDG
Some("core.editor") Some("nvim") XDG
Some("alias.exclude") Some("update-index --assume-unchanged") XDG
Some("alias.include") Some("update-index --no-assume-unchanged") XDG
Some("filter.lfs.clean") Some("git-lfs clean -- %f") XDG
Some("filter.lfs.smudge") Some("git-lfs smudge -- %f") XDG
Some("filter.lfs.process") Some("git-lfs filter-process") XDG
Some("filter.lfs.required") Some("true") XDG
Some("commit.gpgsign") Some("true") XDG
None
Ok("nycex")
[
    "nycex",
    "localpcusername",
    "git",
]
0 nycex is_u?=true is_sk?=false
0 nycex is_u?=false is_sk?=true
1 nycex is_u?=false is_sk?=true
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { code: -1, klass: 0, message: "no authentication available: failed to authenticate when downloading repository ssh://gitlab.com/nycex/someprivateproject (tried ssh-agent, but none of the following usernames worked: \"nycex\")" }', src/libcore/result.rs:999:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
nabijaczleweli commented 5 years ago

Maybe ssh_key_from_agent() is failing somehow? This patch might show why, but I'm grasping at straws, quite frankly

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index 4fc18030c..a71ed6c9e 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -877,7 +877,10 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>
                     ssh_attempts += 1;
                     if ssh_attempts == 1 {
                         ssh_agent_attempts.push(uname.to_string());
-                        return git2::Cred::ssh_key_from_agent(&uname);
+                        let ret = git2::Cred::ssh_key_from_agent(&uname);
+                        println!("agent type: {:?}", ret.as_ref().ok().map(|c| c.credtype()));
+                        println!("agent err: {:?}", ret.as_ref().err());
+                        return ret;
                     }
                 }
nycex commented 5 years ago

well it should be working

agent type: Some(2)
agent err: None
nabijaczleweli commented 5 years ago

oh damn you're right, I fucked up copying the condition, so with the commit it should work?

And if it doesn't then this patch should?

diff --git a/src/ops/mod.rs b/src/ops/mod.rs
index 4c9b52f8d..57821e0ce 100644
--- a/src/ops/mod.rs
+++ b/src/ops/mod.rs
@@ -883,8 +883,9 @@ fn with_authentication<T, F>(url: &str, mut f: F) -> Result<T, GitError>

                 Err(GitError::from_str("no authentication available"))
             });
+            println!("{:?}", res.as_ref().err());

-            if ssh_attempts != 2 {
+            if res.is_ok() || ssh_attempts != 2 {
                 break;
             }
         }
nycex commented 5 years ago

Now it works as intended. Thank you (the patch is not needed)

nabijaczleweli commented 5 years ago

Thanks for sticking with me through this.

Released in v1.8.3.