search

keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
21.34k stars 1.47k forks source link

Hang when Putty agent support is enabled #7919

Open luzat opened 2 years ago

luzat commented 2 years ago

Overview

KeePassXC 2.7.1 (maybe older, too) on Windows hangs when Putty Agent SSH support is enabled and not Putty Agent is not running.

Steps to Reproduce

  1. Start KeePassXC with Putty Agent support enabled and Putty Agent not running 2a. Either: Try to unlock database (enter password, unlock) 2b. Or: Try to go into Tools > Settings

Expected Behavior

No hang and possibly a notice that Putty Agent is not available.

Actual Behavior

KeePassXC completely hangs.

Editing KeePassXC.ini (UsePageant=false) seems to be the only way to fix it. Doesn't happen with OpenSSH agent support if the OpenSSH agent is disabled.

Context

Does happen on 2,7.1 on Windows 11, not tested on 2.7 or earlier.

KeePassXC - Version 2.7.1 Revision: 5916a8f

Qt 5.15.3 Debugging mode is disabled.

Operating system: Windows 10 Version 2009 CPU architecture: x86_64 Kernel: winnt 10.0.22000

Enabled extensions:

Cryptographic libraries:

hifi commented 2 years ago

This isn't the first time this was reported so something is up with 2.7.x and Pageant on Windows. I'll try to get this reproduced at some point but when I did 2.7.1 fixes for Windows I'm pretty sure I tested having Pageant enabled but not running and it didn't hang on Windows 10.

luzat commented 2 years ago

Sorry, didn't see other bugs and figured it was introduced recently. I might try to debug it myself if I find time.

ducksys commented 2 years ago

Haven't been able to use 2.7.1 with pageant running since it was released, I use https://github.com/ndbeals/winssh-pageant, and 2.7.1 hangs forever after entering the password, 2.7.0 still works as expected on Windows 11 x64 22621.105

droidmonkey commented 2 years ago

I can't reproduce this, what exact settings do you have and what type of keys are you trying to load?

ducksys commented 2 years ago

I think that I figured out what was happening in my case:

When upgrading from 2.7.0 to 2.7.1 the "Enable SSH Agent integration" option changed from individual checkboxes to a radio button group with 3 options: pageant, openssh, both. For me, after upgading the "both" setting got selected.

winssh-pageant is a bridge to the windows openssh agent for pageant-only clients, when this bridge is running, you're supposed to add your keys to the windows openssh agent, and they'll be available for pageant-only programs like putty.

The main takeaway is that you can't add keys to the bridge via the pageant protocol, and when someone selects the "pageant" or "both" options in keepass-xc, it will try to do exactly that, and freeze / wait forever for completion, this is why I never could open my database with 2.7.1.

So, for this weird case, perhaps keepass-xc could implement a timeout for pageant operations, and perhaps winssh-pageant could send fake responses to avoid these situations.

hifi commented 2 years ago

Okay, that makes sense. The virtual agent should refuse identities like a real agent would because what's happening is that the Pageant window exists but it apparently never replies to the message.

We should also add a fast timeout as suggested to prevent it from hanging.

ducksys commented 2 years ago

Thanks, I also started a discussion about it over at winssh-pageant https://github.com/ndbeals/winssh-pageant/discussions/20

droidmonkey commented 2 years ago

Mine fails immediately and says agent protocol error. Are you using the most up to date winssh-pageant?

ducksys commented 2 years ago

edited:

winssh-pageant: I tried both the latest release and built the latest master, both hang for me.

winssh-agent: been using OpenSSH_for_Windows_8.6p1, LibreSSL 3.3.3 but now updated to the latest release, and I still get hangs.

ndbeals commented 1 year ago

Hello, I'm the author of winssh-pageant. My tool should be able to support this (or anything your ssh-agent does) All it does is listen for a message from one place, and then relay that identical message to a different place. It shouldn't matter what the message is, as long as both the sender and receiver understand it, this application should be transparent between them.

hifi commented 1 year ago

If winssh-pageant exposes a Pageant window that looks like the real Pageant KeePassXC will try to communicate with it. From what I understand from this issue is that it doesn't support the add key protocol which it doesn't even need to implement which in turn will cause KeePassXC to hang when it tries to communicate with it directly.

Both applications are at fault if that's the case - KeePassXC doesn't have a proper timeout for Windows IPC communication and winssh-pageant doesn't implement the complete Pageant protocol - even just so that it would refuse the add key operation.

ndbeals commented 1 year ago

There's no Pageant protocol to implement, it's just the ssh-agent protocol being communicated over a win32 api WM_COPYDATA window event instead of a named pipe. The only thing my tool does is relay the data from a WM_COPYDATA event to the named pipe.

That being said that's only true of the most recent version of my tool, as I didn't really understand the protocol before.

There was a hang bug fixed (added a timeout deadline for named pipe communication) in winssh-pageant that was related to waiting for responses from the openssh ssh-agent, which may help here too.

hifi commented 1 year ago

Here's where we detect if Pageant is running, your tool likely hits this check so we think it's Pageant:

https://github.com/keepassxreboot/keepassxc/blob/develop/src/sshagent/SSHAgent.cpp#L149

Here's where we do WM_COPYDATA:

https://github.com/keepassxreboot/keepassxc/blob/develop/src/sshagent/SSHAgent.cpp#L242

From my understanding this SendMessageA hangs. So I guess the reason why it would hang is that your window isn't exactly handling the message. I'm not a Windows developer and all Windows support was done as best effort at the time so the implementation in KeePassXC is likely also bad as it doesn't have any timeout and just hopes the target window handles the SendMessage call.

ndbeals commented 1 year ago

Yes, your guess appears to be correct. I have fixed that bug in my application but from your end you should consider adding a timeout to that SendMessageA call because of possibly poorly behaving applications like mine lol. If you do add a timeout, make sure it's long enough that a user could resonably interact with a Windows Hello prompt.

Though I'd like to hear from the OP if this issue is still happening after upgrading winssh-pageant

droidmonkey commented 1 year ago

I'll look to convert over to https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-sendmessagetimeouta

ndbeals commented 1 year ago

Looking more into what the Pageant integration in KeePassXC does, I believe using the keepassxc pageant integration with winssh-pageant is redundant.

KeePassXC can already directly talk to the Microsoft OpenSSH agent, since winssh-pageant is just a translator between pageant messages and ssh-agent, having KeePassXC communicate with winssh-pageant seemingly has no extra purpose.

Not to say it'd be useless in all scenarios, I can see it being desired if you're running two separate agents (both the real pageant and ssh-agent). This is more of a heads up for users.

sventek-s commented 1 year ago

I am still facing app freezes when I enable use both agents in KeepassXC, what can one do to make the app to stop freezing?