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

Secret Service group is no longer exposed following a merge #9371

Closed
antymat opened this issue Apr 28, 2023 · 13 comments · Fixed by #10452
Closed

Secret Service group is no longer exposed following a merge #9371

antymat opened this issue Apr 28, 2023 · 13 comments · Fixed by #10452

Comments

@antymat
Copy link

antymat commented Apr 28, 2023

I have 2 databases, one on a local machine (A), one of a rsync-mounted drive (B). When I try to merge the A to B, the keepassxc crashes. I run it in terminal, and this was at the end of the output:

QObject::connect(Group, Unknown): invalid nullptr parameter
QObject::connect(FdoSecrets::Item, FdoSecrets::Collection): invalid nullptr parameter
QObject::connect(FdoSecrets::Item, FdoSecrets::Collection): invalid nullptr parameter
Segmentation fault

@antymat
Copy link
Author

antymat commented Apr 28, 2023

KeePassXC 2.7.4
CPU: 24-core AMD Ryzen Threadripper 3960X (-MT MCP-)
speed/min/max: 2199/2200/3800 MHz Kernel: 6.1.21_1 x86_64 Up: 16d 8h 1m
Mem: 62501.3/128692.5 MiB (48.6%) Storage: 13.69 TiB (62.2% used) Procs: 1116
Shell: Bash inxi: 3.3.26

@droidmonkey
Copy link
Member

droidmonkey commented May 3, 2023

Unfortunately I cannot replicate this problem. However, I did notice that when a database is merged into one that is exposing a group via secret service... the secret service is disabled after the merge. Different problem, but it might be related somehow.

PRE-MERGE:
image

POST-MERGE:
image

@droidmonkey droidmonkey changed the title Keepass crashes on merge from another database Secret Service group is no longer exposed following a merge May 3, 2023
@Iizuki
Copy link

Iizuki commented May 29, 2023

I just encountered the exact same error message when trying to merge my databases (which I have done often and it has worked flawlessly so far).

I do use Secret Service but I didn't notice anything especial with regards to it.

Merging the files the other way around worked.

@dryya
Copy link

dryya commented Jul 31, 2023

I also just ran into this issue. Unfortunately all I see is

Entry of Main[b5c5b25b871641d6baa90002b8786996] contains conflicting changes - conflict resolution may lose data!
QObject::connect(Group, Unknown): invalid nullptr parameter
QObject::connect(FdoSecrets::Item, FdoSecrets::Collection): invalid nullptr parameter
QObject::connect(FdoSecrets::Item, FdoSecrets::Collection): invalid nullptr parameter
fish: Job 1, 'keepassxc' terminated by signal SIGSEGV (Address boundary error)

with no core dump. Also use secret service, but the conflicting entries didn't have it enabled. On the other hand, I can open them both separately and keepassxc-cli merge works fine.

@Taijian
Copy link

Taijian commented Mar 18, 2024

OK, is there anything we as affected users can do to help debug this issue so it can get fixed? Because right now my family's workflow is seriously broken, because whenever a shared keyfile gets changed and merged back into my main keyfile, all my Secret Service integrations break, which is quite annoying.

How can I help analyze this issue, @droidmonkey?

@droidmonkey
Copy link
Member

droidmonkey commented Mar 18, 2024

Generally, the root cause of this issue is because keepass kdbx doesn't have a modified time on each key value pair. We actually introduced a last modified entry that we maintain for the entire set of custom data. If you merge a database with newer custom data, and that database doesn't have fdosecrets configured, then you'll lose your own configuration. Easiest solution is to setup fdosecrets on the source database with the settings you desire.

Separately, I found out that keeshare databases were merging their custom data into the main database. This would also cause this issue and I have a fix in place for that: #10452

I think I need to extend that fix to the more general case somehow.

@Taijian
Copy link

Taijian commented Mar 18, 2024

Ah, OK.

So if I understand correctly #10452 would actually solve my problem, because the only merging I'm doing is via KeeShare?
(And yeah, the group(s) that are being KeeShare'd between my wife and I do not have any Secret Service settings, because.... why would they...)

@droidmonkey
Copy link
Member

droidmonkey commented Mar 18, 2024

Yes that PR will solve the keeshare side of this problem. The other side is actually "how keepass works" for better or worse. I forsee solving that problem with a merge options dialog (one option being, don't merge custom data)

See #10173

@Taijian
Copy link

Taijian commented Mar 18, 2024

Thank you for taking the time to both explain and fix (the more readily fixable part of) the problem!

@michaelk83
Copy link

one option being, don't merge custom data

Are custom data and custom attributes the same? I can imagine a scenario where one might want to merge at least some custom attributes, and maybe at least some custom data (if that's not the same thing), but still protect things like the Secret Service settings. The latter is usually local to the machine/database, so should very rarely be merged IMO, if ever.

@droidmonkey
Copy link
Member

That conversation should be moved over to the PR I linked just above

@michaelk83
Copy link

@droidmonkey Do you mean #10452 or #10173 ?

And also on further thought, one might want a separate option to migrate settings (such as the Secret Service stuff) between databases, especially after #2224.

@droidmonkey
Copy link
Member

@Taijian I found another way to protect the FdoSecrets custom data, very simple and should have been implemented before. Will be adding that to the KeeShare fix PR I mentioned above.

@droidmonkey droidmonkey added this to the v2.7.8 milestone Mar 18, 2024
droidmonkey added a commit that referenced this issue Mar 18, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
droidmonkey added a commit that referenced this issue Mar 31, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
droidmonkey added a commit that referenced this issue Apr 20, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
droidmonkey added a commit that referenced this issue Apr 28, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
droidmonkey added a commit that referenced this issue Apr 29, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
droidmonkey added a commit that referenced this issue Apr 29, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
droidmonkey added a commit that referenced this issue Apr 29, 2024
* Fixes #9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
pull bot pushed a commit to shashinma/keepassxc that referenced this issue Apr 30, 2024
* Fixes keepassxreboot#9371 - adds secret service custom data key to the list of protected custom data (will not be overwritten on merge)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants