key is removed from sshagent upon closing keepassxc

[CuredByLaughter]

Unlike with the previous version, keys are removed from the ssh agent upon closing KeePassXC.
At first glance, it seemed the setting "Remove key from agent when database is closed/locked" although unchecked, was being ignored when closing the database by closing KeePassXC itself.

Expected Behavior

I expect the key to still be available in the agent after closing the database and KeePassXC 2.4.0, as was the case with KeePassXC 2.3.4.

Current Behavior

Current settings are as follows:

• on/checked: Add key to agent when database is opened/unlocked
• off/unchecked: Remove key from agent when database is closed/locked
• off/unchecked: Require user confirmation when this key is used
• off/unchecked: Remove key from agent after 600 seconds

The key remains in the agent after closing the database but not after closing KeePassXC. From what I recall, the key remained in the agent when using KeePassXC 2.3.4 after closing KeePassXC.

Possible Solution

Perhaps I was relying on an undefined/undocumented feature when using the previous version.

Steps to Reproduce

1. Have a database with a private key (in my case this is an external file)
2. Configure the SSH Agent for this entry with as above (add key when opened/unlocked and none of the remove required)
3. Close KeePassXC
4. Remove keys from agent ($ ssh-add -D)
5. Start KeePassXC
6. Open/Unlock database
7. Verify that the key has been added to the agent ($ ssh-add -l)
8. Close the database (Database Menu / close database)
9. Verify that the key is still in the agent ($ ssh-add -l)
10. Close KeePassXC (KeePassXC Menu / Quit KeePassXC)
11. Verify that the key is no longer in the agent ($ ssh-add -l)

Context

My workflow previously had been to open the database with KeePassXC to add several keys to the ssh agent. This was followed by closing KeePassXC, opening later if/when needed (for other credentials).
My current workaround is to keep KeePassXC, locking and unlocking the database as needed, so that the keys stay in the ssh agent.

Before creating this issue report I searched for already reported issues and came across the following, which seemed related:
#2902

Debug Info

KeePassXC - Version 2.4.0
Revision: c51752d

Libraries:

• Qt 5.12.2
• libgcrypt 1.8.4

Operating system: macOS Mojave (10.14)
CPU architecture: x86_64
Kernel: darwin 18.5.0

Enabled extensions:

• Auto-Type
• Browser Integration
• SSH Agent
• KeeShare (only unsigned sharing)
• YubiKey
• TouchID

[droidmonkey]

This seems to have been the behavior even in 2.3.4. When SSHAgent was destructed it went through each key and removed the identity from the agent.

keepassxc/src/sshagent/SSHAgent.cpp

Lines 38 to 45 in 6fe821c

	SSHAgent::~SSHAgent()
	{
	for (QSet<OpenSSHKey> keys : m_keys.values()) {
	for (OpenSSHKey key : keys) {
	removeIdentity(key);
	}
	}
	}

I personally think this is the appropriate behavior.

[CuredByLaughter]

I just reinstalled v2.3.4 and performed the steps to reproduce listed. After closing KeePassXC, the keys are still in the agent.
Perhaps that was not the intended behavior with 2.3.4 and thus had a bug (keys were not removed when closing KeePassXC) which was fixed with code changes from 2.3.4 to 2.4.0.

[droidmonkey]

Yes that is my thought as well. The problem with leaving the keys in place when KeePassXC is explicitly shutdown is that we are not cleaning up after ourselves. Perhaps there can be an additional setting of "Keep key in agent after KeePassXC shutdown" which would be available if the "Keep key in agent after database lock" is enabled.

[CuredByLaughter]

cleaning up after ourselves

KeePassXC 2.4.0 removes keys from the ssh agent even if those keys were added to the agent before KeePassXC was started.

[CuredByLaughter]

Browsing the source, it appears as if in 2.3.4 m_keys was being populated only for keys with the option "remove at lock". For 2.4.0 m_addedKeys is being populated also in addIdentity.

See #2503

keepassxc/src/sshagent/SSHAgent.cpp

Line 216 in 62dc4f4

m_addedKeys[keyCopy] = removeOnLock;

[hifi]

If the remove-on-lock option is not set KeePassXC should not remove a key from the agent, even on exit. The user may want to just add keys the first time they open a database and leave them there, that's why remove-on-lock is an option.

@PF93mc8y7erq92qTmTjJBysALa correctly identified m_keys only had keys that had the remove on lock option set and the refactor changed that behavior.

[droidmonkey]

WABAM, your wish is my command

[CuredByLaughter]

@droidmonkey
@hifi
fixed in 2.4.1, thanks!