Fix SSH agent losing track of keys after a database reload - #13705
Closed
underclockeddev wants to merge 1 commit into
Closed
underclockeddev wants to merge 1 commit into
underclockeddev wants to merge 1 commit into
Conversation
Reloading a database from disk replaces its Database object, and every Database has its own uuid. SSHAgent records key ownership by that uuid, so after a reload, locking no longer removed the keys and unlocking was refused as an ownership conflict. Hand ownership over to the replacement in DatabaseWidget::replaceDatabase(). Fixes keepassxreboot#13704 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
varjolintu
added
the
pr: ai-assisted
Member
|
As posted in the linked issue, this appears to be an issue on your end with your database files. The database uuid should not change on file reload. If it does, then you are actually loading a different database. This fix is a decent safe guard for this happening, but it shouldn't happen under normal operating conditions. |
Member
|
I was mistaken, the stable uuid is the root group's which should have been used in the ssh code instead of database uuid which is a made up one for each database object created. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #13704
DatabaseWidget::replaceDatabase()now calls a newSSHAgent::databaseReplaced(oldDb, newDb), which moves ownership of any keys recorded under the old database's uuid to the new one. Lock and unlock then find them as before.The investigation, patch and tests were written with Claude Code (Claude Opus 5.5) and reviewed by me before submitting.
Screenshots
N/A
Testing strategy
TestSSHAgent:testRemoveOnLockAfterReloadandtestReaddOnUnlockAfterReload. Both fail ondevelop(the second with "Key identity ownership conflict. Refusing to add.") and pass with this change.ctestsuite (non-GUI, 41 tests) passes.keepassxcandkeepassxc-cli: unlock a database holding an SSH key, edit the file withkeepassxc-cli, lock, checkssh-add -l, then clear the agent and unlock again. Ondevelopthe key stays after lock and is not re-added on unlock. With this change both work. Controls without the external edit behave the same on both builds.Script: https://gist.github.com/underclockeddev/4051fb85ea881fcdecc2841eacc692d1
Type of change