Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3 to 5 prompts for ssh hardware key authorization #5784

Open
cen1 opened this issue May 31, 2024 · 7 comments
Open

3 to 5 prompts for ssh hardware key authorization #5784

cen1 opened this issue May 31, 2024 · 7 comments
Labels
auth enhancement patches-welcome The core team will accept patches for this feature, but is not planning to implement it themselves.

Comments

@cen1
Copy link

cen1 commented May 31, 2024

Describe the bug
When using Yubikey PIV for git ssh configured with touch-only authorization, every push to a lfs enabled repo requires multiple (3-5) confirmations. Enabling PIN would make the experience even worse.

3 prompts if there are no new objects, 5 prompts if there are additional lfs objects.

Same behavior when using password protected ssh key.

~/.ssh/config

Host github.com
    Hostname github.com
    User git
    IdentityFile /home/me/.ssh/yubikey1

Related: #3318

To Reproduce
Steps to reproduce the behavior:

  1. Setup Yubikey PIV with touch authorization or touch/PIN
  2. Configure ssh to use Yubikey
  3. Push to any lfs enabled repo

Expected behavior
A single confirmation

System environment
Debian KDE

Additional context
Afaik credentials helper is not an option due to presence requirement.

@chrisd8088
Copy link
Member

Hey, thanks for the report! If there's a way to reproduce the issue without a YubiKey, that would be great, and even better if it can be spelled out step-by-step. That would likely speed up anyone who is able to take a look at the issue, and perhaps more importantly, would allow us to set up automated CI tests that can demonstrate that any changes are effective in addressing the problem.

And I should say, patches are always very welcome as a contribution to the project!

@cen1
Copy link
Author

cen1 commented Jun 3, 2024

I think the issue is specifically about a hardware key being used because if you just take a regular password protected key, you could say the solution is to use credentials manager/caching. Because each key access requires physical interaction with the hardware key, credential manager does not work as a cache in this case, at least with my testing.
Without Yubikey, the issue could probably be translated to reducing password prompts for password protected ssh keys without a credentials helper.

@JohnnyJayJay
Copy link

I set up LFS for the first time and I'm experiencing the same annoyance. I'm using an ED25519-SK ssh key (stored as a FIDO2 resident credential on my Yubikey) and I get prompted 5 times to enter my pin and touch the device when pushing to a remote (once by Git itself, 4 times by Git LFS).

This details my setup.

@cfstras
Copy link

cfstras commented Jul 12, 2024

Also affected.
I'm using Secretive to protect my keys with MacBook Touch ID.

This is with an active SSH ControlMaster set, so for a regular git push I normally only have to use the SSH key once every 4 hours.

Here's an excerpt from GIT_TRACE=1 git push. Even though my configured controlmaster is already active, I'm still getting 2 SSH key verification popups.
I'm guessing this is because git-lfs overrides the ControlPath for these. This is really annoying in a general workflow.

Is there any way I configure git-lfs to reuse the already existing ControlMaster connection instead of making a new one not only once, but two times per push?

15:42:42.988958 trace git-lfs: exec: ssh '-oControlMaster=yes' '-oControlPath=/tmp/sock-1797698617/lfs.sock' '[email protected]' 'git-lfs-transfer my-org/my-repo upload'

15:42:48.018428 trace git-lfs: exec: ssh '-oControlMaster=yes' '-oControlPath=/tmp/sock-1706239699/lfs.sock' '[email protected]' 'git-lfs-transfer my-org/my-repo upload'

15:42:52.308155 trace git-lfs: exec: ssh '[email protected]' 'git-lfs-authenticate my-org/myrepo upload'

Git config:

Host github.com
    AddKeysToAgent yes # These seem to have no effect
    UseKeychain yes

	# This only works to reduce from 3 -> 2 SSH key taps :/
    ControlPath ~/.ssh/controlmaster-%C.sock
    ControlMaster auto
    ControlPersist 4h

@chrisd8088
Copy link
Member

chrisd8088 commented Jul 19, 2024

Is there any way I configure git-lfs to reuse the already existing ControlMaster connection instead of making a new one

@cfstras -- Good question, and thanks for the lines from a trace log! Looking over the code, particularly that from the PRs #4446, #5223, and #5537, I notice a couple of things which may be going on here.

When the ssh.NewSSHTransfer() function runs, it receives the ControlMaster socket path it will use from a call to GetLFSExeAndArgs(), which ultimately uses getControlDir() to generate a unique, temporary path.

So, if that function is called more than once, it's going to use a separate socket path each time. And it appears that during a Git push operation, the git-lfs-pre-push command ends up invoking this function twice, first from within the newUploadContext() call and then within the uploadForRefUpdates() call. (I've put the call stack traces below, mostly for future reference.)

Call Stacks
commands.newUploadContext()
commands.newLockVerifier()
tq.(*lazyManifest).IsStandaloneTransfer()
tq.(*lazyManifest).Upgrade()
tq.newConcreteManifest()
lfsapi.(*Client).SSHTransfer()
commands.uploadForRefUpdates()
commands.verifyLocksForUpdates()
commands.(*lockVerifier).Verify()
locking.(*Client).SearchLocksVerifiable()
locking.(*genericLockClient).SearchVerifiable()
locking.(*genericLockClient).getClient()
lfsapi.(*Client).SSHTransfer()

With some refactoring, we might be able to avoid the second call altogether, or at least reuse the same socket control path.

Then, separately, if the git-lfs-transfer commands indicate that the remote doesn't support the pure SSH Git LFS transfer option, we fall back to running the git-lfs-authenticate command over SSH. That's started by a third call to ssh.GetLFSExeAndArgs() from within the sshAuthClient.Resolve() method in our lfshttp package, and in that call we pass false for the multiplexDesired flag and an empty string for the multiplexControlPath.

Again, with some refactoring, perhaps we could reuse the same socket control path here, if one had been established already.

There's also the question of whether we could add a lfs.ssh.multiplex.controlPath configuration setting that a user could set to match their SSH configuration's ControlPath, so we could reuse any existing socket in that path. I think that's going to be challenging, as we'd have to parse all of OpenSSH's tokens, like %C, and interpolate the same values into them. It might almost be better to see if we could unwind Git LFS's attempts to supply -oControlMaster and -oControlPath arguments, which were introduced in PR #4446, and defer to SSH's own configuration. But that would also require a bit of thought, as I'm not immediately sure what the consequences of that would be.

Some time and attention will be required to get this refactoring done, along with an improved and revised test suite, so with apologies, I won't make any guesses as to when it might be completed. (Contributions are always very much appreciated, of course.)

@chrisd8088
Copy link
Member

@cfstras (or others who've had a version of this problem) --

Out of interest, what happens if you set lfs.ssh.autoMultiplex=false in your Git (not SSH) configuration? Something like this in $HOME/.gitconfig, for example:

[lfs "ssh"]
        autoMultiplex = false

But leave your SSH configuration file with the settings you gave above, particularly something like:

Host github.com
    ControlPath ~/.ssh/controlmaster-%C.sock
    ControlMaster auto
    ControlPersist 4h

Does that help at all? With lfs.ssh.autoMultiplex=false Git LFS should skip setting either -oControlMaster or -oControlPath, which I think should mean we defer to OpenSSH's own settings.

For me, that appears to mean the Git LFS client just runs the same multiple SSH commands, but without any extra multiplexing arguments, and SSH appears to then use the control socket established by the first one (given an SSH configuration like the one above).

trace git-lfs: exec: ssh '[email protected]' 'git-lfs-transfer my-org/my-repo.git upload'
trace git-lfs: exec: ssh '[email protected]' 'git-lfs-transfer my-org/my-repo.git upload'
trace git-lfs: exec: ssh '[email protected]' 'git-lfs-authenticate my-org/my-repo.git upload'

Do you see something similar?

@nicholasbishop
Copy link

Confirmed that works for me, thank you! No additional security key presses needed in an LFS repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth enhancement patches-welcome The core team will accept patches for this feature, but is not planning to implement it themselves.
Projects
Status: Enhancements
Development

No branches or pull requests

5 participants