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

Keeshare: imported database is missing entries #4199

Closed
jeyca opened this issue Jan 18, 2020 · 27 comments
Closed

Keeshare: imported database is missing entries #4199

jeyca opened this issue Jan 18, 2020 · 27 comments
Labels

Comments

@jeyca
Copy link

jeyca commented Jan 18, 2020

I import a second kdbx file using keeshare:

I created a new folder, put in a name, chose keeshare, chose type: import, chose the path to the kdbx (I had to switch to show all files) and put in the password. It seems to work, but some entries are just missing. I opened the file as a new database and it works, it is the same file that is imported and is opened as new database, but some files in the imported folder are just missing.

I tried to find out what is the difference between the entries that are shown and the missing ones, but could not find any yet.

Operating system: Xubuntu 18.04
Keepassxc: 2.5.2

Enabled extensions:

  • no extensions enabled
@jeyca jeyca added the bug label Jan 18, 2020
@droidmonkey
Copy link
Member

Are these databases related in any way? Do they share the same exact Entry

@jeyca
Copy link
Author

jeyca commented Jan 18, 2020

no not related, in what way they could be related except the keeshare?

Do they share the same exact Entry

what exactly do you mean?

@droidmonkey
Copy link
Member

KeeShare works at the entry level, if two entries in different databases have the same UUID then they will merge with each other no matter what folder they are in.

@jeyca
Copy link
Author

jeyca commented Jan 18, 2020

ok. But when I search for the title of a missing entry, I cannot find anything in the first database. But it might be possible that I "copied" these entries some time before. That means, because I was not able to copy entries from one database to another, I copied the file, opened it as new database and then deleted all entries except one folder. But I also deleted all entries from that folder in the original database (you could say that I did a cut and paste operation). There is also nothing left in the rescycle bin of these old entries.

@jeyca
Copy link
Author

jeyca commented Jan 18, 2020

ok I could solve it by cloning the missing entry in the second database, then it shows up. But because I have to this with a lot of entries, I wonder if there is a better solution for all entries at once of that second database? I thought about export/import, but then it looks like I would lose all the autotype settings..

@jeyca
Copy link
Author

jeyca commented Jan 18, 2020

There's really something fishy about the keeshare function, I don't recommend using that, it keeps deleting arbitrary entries in my primary database. I am lucky that I had good secondary backups, because it also removed the passwords of my encrypted backup hard disks...

I don't think this is connected to the old "cut and paste operation" when I copied the file and deleted everything except one folder. I created completely new database files which I synced with a folder in my primary database.

What happens: I have synced a secondary database file using keeshare type sync. Then, all entries from folder "harddisks" (and others, which are completely unrelated to the synced folder) are removed. If I merge in a backup file of my primary database, I can see how the entries come back, then the green export/import banner shows up (I guess thats the sync operation) and the entries are removed again.

The same happens when I choose "import" instead of "sync" type, but then only after closing and reopening the primary database file.

@jeyca
Copy link
Author

jeyca commented Jan 18, 2020

opposed to what was said in #4198 I found that the keeshare feature is documented here:

https://github.com/keepassxreboot/keepassxc/blob/develop/docs/QUICKSTART.md#using-sharing

and it is also stated that even the entries outside of a shared group are affected.

Please note, that the import currently is not restricted to the configured group. Every entry which was imported and moved outside the import group will be updated regardless of it's location!

That is unfortunate, but it might explain why it is even possible that entries get lost. But it still does not explain why these specific unrelated entries are removed. Is it possible to look up any kind of logging to see what actually happens here?

@droidmonkey
Copy link
Member

If you can setup a simple example we can replicate while debugging. I have found KeeShare to do some surprising operations as well.

@ckieschnick
Copy link
Contributor

@jeyca Are you using a "normal" database as import container for KeeShare? If you cloned your database before - and started to remove single entries, those entries are entered into a deleted entries list. This list - especially when using a real database as container - will be used by KeeShare to delete obsolete entries from the target database. For the intended setup, this list contains only entries which originated from the source database and contains only entries which the user of the source database doesn't want to share anymore. When you are using a standard database - especially a clone, this means that KeeShare will try to replicate all "newer" changes from the source database - aka your real source - into the target database.

That's why we didn't recommend to use real database for import/synchronization.

Another important point is - if you are using a real database as sharing container in sync it may get replaced when you are changing the target database since sync means to copy changes back - it doesn't sync those two databases. This is a separate feature which has nothing to do with KeeShare!

@mstarke
Copy link
Contributor

mstarke commented Jan 24, 2020

I think we should consider making it non-obvious that keeshare uses a standard database as exchange container. Maybe by just adding a different file extension or something similarly simple. Even more so, we should add a special marker in the database that allows keeshare to show a prominent warning that there is something unexpected going to happen if that marke is missing.

@droidmonkey
Copy link
Member

I am not against that, KeeShare should always use its own generated container. It was my mistake to assume how KeeShare worked before knowing so and allowing the kdbx unsigned version. We could universally use the .share extension and just detect if it is a zip file or not. There are options here, perhaps on another issue ticket for discussion.

@mstarke
Copy link
Contributor

mstarke commented Jan 24, 2020

Maybe we can just come up with a PR that switches to using .share with autodetection on the format and include the marker as well as the warning signs.

@droidmonkey
Copy link
Member

We would also want to prompt a user to "upgrade" their .kdbx share container to .share on load.

@tysonclugg
Copy link

tysonclugg commented Apr 25, 2021

I'm experiencing the same issue, and I've got an idea on what is happening.

I created fresh databases to split up my old monolithic database. I merged the old database into the new databases and deleted groups of passwords in each of the fresh databases, leaving each new database with non-overlapping subsets of the old database. Importantly this means every password was deleted in at least one database.

I think the sync code synchronises these deleted password tombstone entries in a way that doesn't allow for the entry to appear in a different database (ie: entries can't be "moved" between databases). Looking at the output of keepass-cli export ... on various databases shows a large number of <DeletedObject> items that seems in support of my theory.

@droidmonkey
Copy link
Member

This is true and a very good observation!

@tysonclugg
Copy link

Having tombstones decoupled from the entries has brought about this issue, moving forward may require a new database format. My suggestion is to abandon the current tombstone implementation and instead add a Deleted timestamp as an attribute of every entry. When moving an entry, mark the old entry as Deleted and updated 1 second prior, and clear the Deleted attribute on the new entry with the correct (current, not 1 second prior) updated timestamp. Alternately, use the correct timestamp for both and have secondary ordering of sync (after Updated) by Deleted timestamps.

@droidmonkey
Copy link
Member

We are severely limited to the kdbx specifications.

@tysonclugg
Copy link

What about changing the kdbx.share format instead?

@droidmonkey
Copy link
Member

That is a zip file with a kdbx database and a signature file. That is going away in the next major version.

@tysonclugg
Copy link

What if moving/merging an entry ALWAYS updates entry timestamps (on both sides if moving between databases is ever supported), and sync code NEVER syncs tombstones when timestamps match?

I know that this could cause a lost-update if both a password change and a move occur on different machines which then sync, but the history would contain the new password in any case, so it's far better than the current scenario where passwords are automatically deleted forever after the tombstone expires...

@tysonclugg
Copy link

Silly me, I just saw that tombstones have timestamps. As long as the Updated/Deleted times are compared, and the most recent change wins, or in the case of them being the same the update wins, then this problem almost goes away. The only remaining issue is splitting databases using merge and delete (like I did) which leaves the delete as the most recent change. Implementing move between databases with matching update/delete timestamps would fix this.

Also, the features listed below would help:

  1. Adding a way to touch entries in a database and update their timestamp.
  2. Clearing tombstones via a menu item or via keepassxc-cli.

@tysonclugg
Copy link

@droidmonkey Was this actually fixed, or is this now documented behaviour?

@droidmonkey
Copy link
Member

This was not fixed yet. I needed more time to develop the final solution.

@tysonclugg
Copy link

This was not fixed yet. I needed more time to develop the final solution.

Cool, can this issue be opened (or a new issue) to ensure this isn't forgotten please?

@droidmonkey
Copy link
Member

It's a duplicate

@tysonclugg
Copy link

@droidmonkey I'm not sure how this is a duplicate, can you nominate which issue is the original please? Alternately, would you be OK with me logging a new issue to track all data loss related issues in a coherent manner?

All of the open issues that seem to be directly related to this issue were logged after this issue, suggesting this is the original issue:

I also found couple of directly related but closed issues linked from the above:

I also looked through other semi-related open issues of some relevance to this issue (mostly KeeShare/sync/deleted item related):

I didn't look through the list of closed issues.

@droidmonkey
Copy link
Member

droidmonkey commented Apr 12, 2022

We don't necessarily keep the first issue reported. The first issue you referenced (#6477) has a better description of the problem at hand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants