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

Crash on Windows 11 Pro with IdentityAgent in ssh config #13933

Closed
3b0b opened this issue Dec 2, 2022 · 2 comments · Fixed by #13951 or #14029
Closed

Crash on Windows 11 Pro with IdentityAgent in ssh config #13933

3b0b opened this issue Dec 2, 2022 · 2 comments · Fixed by #13951 or #14029
Assignees
Labels
bug sftp SFTP Protocol Implementation
Milestone

Comments

@3b0b
Copy link

3b0b commented Dec 2, 2022

Describe the bug
Since version 8.3.0 in April, Cyberduck crashes on launch with my ssh config file. I originally thought my problem was #13672, but after realizing the fix for that was supposed to have been released already, I actually checked my crash reports and narrowed this down today. As of 8.4.5.38423 and 8.5.1.38745, the problem goes away if I comment out my IdentityAgent line. Fortunately, I don't think I actually need it - I don't use the SSH agent, so the following ProxyCommand line, which doesn't cause me any problems, is the important one.

To Reproduce
Steps to reproduce the behavior:

  1. Have an ssh config with a line that starts with IdentityAgent //
  2. Try to launch Cyberduck

If the next character of the IdentityAgent is a period, the program crashes. If it is anything else, it seems to hang indefinitely with a process running but the UI never appears.

The reason I had the configuration in question was because Krypton Authenticator adds the following (path redacted):

# Added by Krypton
Host *
	IdentityAgent \\.\pipe\krd-agent
	ProxyCommand path\to\krssh.exe %h %p

but I had a different tool - unfortunately I've forgotten what it was; it may have been a Visual Studio Code plugin - which worked if I changed the backslashes in the IdentityAgent line to forward slashes but not with the configuration as above.

Since I have a workaround, I've submitted this mostly in case it's a simple parsing issue that (A) is easy to fix and (2) might trip people up in a less obscure case. I may even come back later and try to whip up a pull request. My assumption is that the problem is in handling that kind of string in LocalFactory.get(), but I ran out of time to investigate this today before I figured out where LocalFactory lives in the codebase.

Expected behavior
Cyberduck working; if necessary, a convenient and documented way to tell Cyberduck not to try to parse my ssh config or to point it at a different one (which might already exist and I failed to find it...).

Desktop (please complete the following information):

  • OS: Windows 11 Pro and Windows 11 Pro Insider Preview
  • Version: all attempts from at least 10.0.22593 through 10.0.25247

Log Files
638055899217769141.txt

@dkocher dkocher added sftp SFTP Protocol Implementation bug labels Dec 3, 2022
@AliveDevil
Copy link
Contributor

Thanks for reporting, until now we only support the default built-in ssh-agent pipe (was quite the effort to get that working).
The ssh agent pipe is hardcoded1 to \\.\pipe\openssh-ssh-agent.

@dkocher We need a AgentAuthenticatorFactory, where we can override e.g. UnixOpenSshAgentAuthenticator and WindowsOpenSshAgentAuthenticator in client-code.

Footnotes

  1. https://github.com/iterate-ch/cyberduck/blob/master/ssh/src/main/java/ch/cyberduck/core/sftp/openssh/WindowsOpenSSHAgentAuthenticator.java#L30

@AliveDevil
Copy link
Contributor

Root-issue for this lies in #5330, where our path sanitization just straight up bails out and fails hard.

@AliveDevil AliveDevil linked a pull request Dec 6, 2022 that will close this issue
@dkocher dkocher modified the milestones: 8.5.3, 8.5.4 Dec 6, 2022
dkocher added a commit that referenced this issue Dec 23, 2022
@dkocher dkocher linked a pull request Jan 2, 2023 that will close this issue
AliveDevil added a commit that referenced this issue Jan 2, 2023
dkocher added a commit that referenced this issue Jan 3, 2023
ylangisc added a commit that referenced this issue Jan 4, 2023
Handle rare case of shared bookmarks including macOS-paths on Windows
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug sftp SFTP Protocol Implementation
Projects
None yet
3 participants