keepassxreboot / keepassxc

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

KDBX 5 Standard #4317

Closed droidmonkey closed 3 years ago

droidmonkey commented 4 years ago

Placeholder for KeePass Database (KDBX) standard discussion. This post will be edited to include a list of features and upgrades we would like to bring to Dominik for consideration in the next KDBX format update.

Must Haves:

Nice To Haves:

Optional Extensions: None so far

Long form discussions should be held here: #5819

droidmonkey commented 4 years ago

Looping in @keepassium @mmcguill @PhilippC Original issue that spawned this conversation: https://github.com/keepassxreboot/keepassxc/issues/4293#issuecomment-583742873

mmcguill commented 4 years ago

Might be worth adding the idea of "Favourites" or "Pinned" items, items that appear upfront when user opens the database. It's a pretty popular/handy feature in Strongbox and it would be great to have it standardised across the KeePass world.

georgesnow commented 4 years ago

Curious do all the variant apps support tags? If so why not just standardize on a tag? Maybe “Pinned” (case insensitive). And document it that way?

That would mean those apps that want to support it can and it wouldn’t matter to any other platform (or native KeePass, MacPass,KyPass etc)

georgesnow commented 4 years ago

And if someone happens to add it as tag the first time just prompt them explaining the use of the “Pinned” as a tag

droidmonkey commented 4 years ago

@georgesnow that's exactly the reason to make it part of the standard instead of just arbitrarily deciding on a tag and gaining application concurrence. Tags might not be the best choice anyway since they are user defined and can conflict.

georgesnow commented 4 years ago

I am certainly not disputing adding the standard, which is always a better solution for something like this. However, seeing as the native KeePass tends to be conservative in their approach to making such changes (and understandably so).

The suggestion was more of interim solution (should have been clearer about that). That is just based on is there any reference for how long features take to get integrated in the KeePass standard?

The interim solution could be easily revertible or convertible by an app at later time when the new standard is implementated. It provides the benefit to the users now, but doesn't force compliance and break anything in other KeePass ports. I am certainly not saying it's ideal as you said due to user conflict issues. Again was just a thought, but sounds like thinking is not to do an interim solution and just wait, which I get.

droidmonkey commented 4 years ago

Correct, I am trying to avoid implementing a multitude of "minor" interim solutions. That creates significant code bloat with exceptions and workarounds depending on which version of the standard you are on. The myriad of plugins made for KeePass over the years demonstrates why this is not a good thing. They all made their own unique standards.

user858753257 commented 4 years ago

Example from Enpass for tags :

Here they are user defined and the developers from Enpass using a pre defined “Apple Watch” Tag to display all entries on the watch which get tagged with this .

In my eyes “pinning” should be in the standard and tags only for special things a developer decide

funnym0nk3y commented 4 years ago

Is there a plan, when those features could be introduced in the standard and into XC?

droidmonkey commented 4 years ago

This would be a keepassxc 3.0.0 type move. Ideally we'd get Dominik on board with this as a well.

funnym0nk3y commented 4 years ago

Hmm, I guess 3.0.0 is still far away. Thanks anyway! Is somebody in contact with Dominik?

droidmonkey commented 4 years ago

Not yet, want to get our full proposal together first using actual rfc style.

funnym0nk3y commented 4 years ago

If you need help let me know!

ghost commented 3 years ago

Related #398 #3558

KeePassDX, the alternative KeePass client for Android, recently added multiple URL formats that are incompatible with Keepass2Android. The app uses WebDomain and WebDomain[counter] instead of KP2A_URL and KP2A_URL_<counter>. https://github.com/Kunzisoft/KeePassDX/issues/524#issuecomment-708251598

Given the popularity of Keepass2Android, it's reasonable for KeePassXC to support its vendor prefix at this time, but I'd like to see this feature standardized with a common prefix in the future.

strongbox-mark commented 3 years ago

That's a pity, are we too late to ask the KeePassDX team if they change/standardize? Is this gone to Production yet?

droidmonkey commented 3 years ago

I'd just bake a proper one into kdbx 5 at this point.

J-Jamet commented 3 years ago

Hello gentlemen, I read the discussion succinctly, I may have missed some information, please remind me if I am wrong.

That's a pity, are we too late to ask the KeePassDX team if they change/standardize? Is this gone to Production yet?

Indeed, KeePassDX is already in production with the WebDomain label feature. For my part, I just did not use the KP2A nomenclature which clearly indicated that it came from KeePass2Android, prefix ("KP2A_") and preferred used a more generic one.

This functionality also includes fields with ApplicationId[n] label and package name as value which allow opening applications directly from an entry. I just saw that KeePass2Android released its new version 1.08c-r1-nonet with this changelog : "No longer storing package names of Android apps in the URL field". But I don't know what that means yet.

As I indicated in my comment, I am of course ready to harmonize things, which will be much better in the future, and I think custom label fields need to be more appropriate and generic.

strongbox-mark commented 3 years ago

Hi @J-Jamet - Yes, I hear you KP2A seems a little too specific, and I think KeePassXC uses kpxc for some of it's extensions (Audit Exclusions, etc).

Thanks for the extra info on your URLs and Applications - Sounds interesting and it may be possible to use these on iOS too...

Maybe we should just go with "kpext*" or "kdbx5_" or something like that for features we think would be very useful additions?

PhilippC commented 3 years ago

please note that KP2A does not really care about any prefix or so. If it searches for an entry matching a URL, it will first look into the URL field but if nothing matches there, it will search all other fields. If KP2A creates a new field for an android packages name, this is now called AndroidApp[counter], but again, it's just a name which doesn't have a meaning. If a new standard would define new types like URL, AndroidApp etc. that might simplify the matching.

J-Jamet commented 3 years ago

Thank you @PhilippC for the details.

So we can replace the ApplicationId[n] label by AndroidApp[n], which is fine and will allow better compatibility. It does not prepare for IOS apps, maybe it is a good thing if we want to separate the links available for applications which would not exist on Android or IOS. But would not work with the same names of packages (It does not matter to me but maybe important with people who have more than one type of phone). To discuss.

For URLs, we will harmonize with a common prefix. But if the prefix is ​​neither important for KeePassDX, nor for KeePass2Android, the method of visualizing links from KeePassXC is maybe too restrictive and should apply to all custom fields if a link is present whatever the label.

droidmonkey commented 3 years ago

I would prefer to use the existing reserved keyword "Url" and add a common suffix "_N". My upcoming work on entry templates is aiming for that suffix for any repeatable fields. AndroidApp and iOSApp are great reserved prefixes.

phoerious commented 3 years ago

I like FIELDNAME[n]. The User-Interface could generally display fields following this pattern as multi-valued. In fact, for KDBX5 I would write multi-valued fields into the specification, so that we don't have to agree on some weird key nomenclature.

phoerious commented 3 years ago

BTW I added the following suggestions to the list at the top, which been lingering in my head for some time:

droidmonkey commented 3 years ago

Great idea, conversion from single value to multi valued fields can be easy too when upgrading from kdbx4 to kdbx5. One point for multivalue is to preserve order, this can be done through xml order or with explicit "order=#" attribute.

We should also move a lot more stuff into attributes versus fields.

phoerious commented 3 years ago

Or in SQLite by adding an order attribute. I am more and more in favour of using SQLite, especially considering storage of binary blobs.

J-Jamet commented 3 years ago

Multi-valued core and custom fields: I'm ok for this :)

OK, so to resume (for short-term changes, I plan to change the format for the next update):

In any case, [_N] means the first element has no suffix, the following ones start from _1 (ie: URL, URL_1, URL_2). Acronyms can stay with capital letters

Also, does everyone agree that the OTP format adopted by default is the URI otpauth? Because it is a pain to maintain with all the plugins that exist on KeePass (as described above). Is it a problem to capitalize for you or for the current plugins (simply to harmonize)? -> label OTP and not otp.

@phoerious For SQlite, tell me if I'm wrong but are you thinking of using it as external storage tools, not directly in KeePass format? Is it a technical choice of KeePassXC to deal with BLOBs?

Graph instead of tree structure: Are you thinking of including elements in the XML format or is it just at the parser level?

Built-in Quick Unlock Specification: How do you see the quick unlock, because I realize it is a subject with multiple interpretations :

phoerious commented 3 years ago

Is there a reason for prefixing the numbers with an underscore?

SQLite is a consideration for KDBX5 (or KDBS for that matter). It would replace the XML in order to provide a more scalable storage backend.

J-Jamet commented 3 years ago

Is there a reason for prefixing the numbers with an underscore?

Yes, because @droidmonkey prefers. :D Otherwise we don't care

SQLite is a consideration for KDBX5 (or KDBS for that matter). It would replace the XML in order to provide a more scalable storage backend.

Okay, why not, it could be more efficient indeed.

phoerious commented 3 years ago

I believe that was meant to be Url_1, Url_2 etc., not Url[_1], Url[_2]. ;-)

droidmonkey commented 3 years ago

Prefix of _ to me just separates the attribute name from the sequencing. Makes it clearer what the name is especially when looking at the advanced attributes page.

J-Jamet commented 3 years ago

@droidmonkey Tend to agree with this argument

@phoerious I don't understand what you mean. The brackets are just to explain the concept, I was suggesting URL, URL_1, URL_2. But do you want to have only the first letter in uppercase with acronyms and why?

phoerious commented 3 years ago

I guess it should be something that is not too unpleasantly technical when viewed with a non-conforming client, but explicit enough to not cause unwanted effects with existing attributes. For KDBX5, it's a non-issue, since multi-values fields need only one key.

Edit: Ah, I thought the brackets were part of your syntax. I suggested Url[1], Url[2] etc. But I am fine with Url_1, Url_2 as well.

J-Jamet commented 3 years ago

For KDBX5, it's a non-issue, since multi-values fields need only one key.

I understand that KDBX5 or KDBS format is important and I agree with the new standardization. But we also have to think about migration and backward compatibility. Sounds harmless with uppercase and underscore, but not everyone will migrate overnight (KeePass app creators and users who might want to keep an old format for whatever reason). I still have a lot of users on my side who are still on KDB so we have to harmonize as much as possible even old formats so that the migration goes smoothly.

droidmonkey commented 3 years ago

I think we can uniformly agree that kdb needs to be read only (or import only on our case). Kdbx 3 needs to be formally deprecated. I have no interest in letting people cling onto old, potentially less secure, data formats that they don't have any understanding of at all.

J-Jamet commented 3 years ago

I think we can uniformly agree that kdb needs to be read only (or import only on our case). Kdbx 3 needs to be formally deprecated. I have no interest in letting people cling onto old, potentially less secure, data formats that they don't have any understanding of at all.

However KeePass 1.38 is still downloadable and still maintained by Dominik. Personally, I cannot pretend that the format is no longer in use even if I want. Of course, I push to go on the new versions, but either it is completely stopped by everyone or not. If you have a list of security flaws in these older formats, I'm interested in reading it. We could use this argument to stop their uses altogether.

droidmonkey commented 3 years ago

Maintain multiple formats for no reason at all is a security flaw in and of itself. Opportunity for failures abound. Older versions of kdb and kdbx do not support modern KDF such as argon2 and do not have critical HMAC checks to verify data integrity. Not a security flaw, but certainly an issue. Just because someone makes something available doesn't mean you have to support it. KeePassX is still available, we actively discourage its use since it hasn't been updated in 5 years. I doubt keepass 1.x is updated frequently and with the same rigor as keepass 2.x. Things have to be retired, the world changes, I'm in the camp of "move or die".

0xpr03 commented 3 years ago

KeePass 1.38 is still downloadable and still maintained by Dominik

From the Outside: I think it'd make sense to already involve them into this discussion. That way you could get some input what the constraints are for them or for example if there are any deprecation plans for 1.X. Otherwise you may end up doing all the work and getting a no at the end.

J-Jamet commented 3 years ago

Older versions of kdb and kdbx do not support modern KDF such as argon2 and do not have critical HMAC checks to verify data integrity.

I agree on this point but like you said it's not a flaw. It surely deserves that we warn people to migrate, not that they are forced.

Just because someone makes something available doesn't mean you have to support it

Historically, KeePassDroid is compatible with this format. I could have completely abandoned it but I preferred to scale backwards compatibility by redoing the entire architecture of the project. It was clearly a choice! :) I don't recommend using this format (it was only a technical challenge). I am ready to remove it from KeePassDX in the future, that's not the problem.

KeePassX is still available, we actively discourage its use since it hasn't been updated in 5 years

Same, but here KeePass 1.38 was released last year

Things have to be retired, the world changes, I'm in the camp of "move or die".

Indeed I see that, I didn't think the open source world was like this. I'm not as categorical as you on the subject.

Now the fateful question : To return to the main point, does that mean you only want to use the new KDBX5 format when it's in place? RIP KDBX3/KDBX4?

@0xpr03 I completely agree, it's easiest to ask Dominik directly, I'll send him an email.

0xpr03 commented 3 years ago

Indeed I see that, I didn't think the open source world was like this. I'm not as categorical as you on the subject.

(I think it applies even more to OS. People could pay you for supporting totally outdated stuff or maintain it by themselves. Otherwise you're doing the work of a corporation for 0€ which will end up in a burnout.)

J-Jamet commented 3 years ago

It was the sentence to convince! :heart: :dagger: :drop_of_blood:
I just sent Dominik an email regarding the subject. It started with a small observation but an interesting debate.

droidmonkey commented 3 years ago

My plan is to hard deprecated kdbx3 with our 2.7 release (message to user, upgrade now please!) And move kdbx3 to import only when kdbx5 is complete. I think it is more than generous to support the previous major release in addition to current release. Thanks for reaching out to Dominik, I think its time to make decisive moves.

J-Jamet commented 3 years ago

Okay, in this case I am also thinking of warning users for this purpose with major versions of KeePassDX ~to deprecate older versions of databases.~ This may take a little longer than KeePassXC but will be done over time.

Edit: A simple warning message will inform users of a new format.

strongbox-mark commented 3 years ago

Great discussion. I think it would be great if we could get Dominik involved. It's his baby after all. I'm not sure how busy he is, or interested in pushing/extending the format? Please let us know how it goes @J-Jamet - Maybe he could join us here!

J-Jamet commented 3 years ago

@strongbox-mark I already sent him an email with the link to the issue, I'm not going to harass him. I'll tell you if I have an answer.

phoerious commented 3 years ago

I have seen quite a few people who insisted they still needed KDB oder KDBX3 or some shit and their reason has always been "the new format isn't compatible with my mobile app". After I pointed them to the correct apps, they usually switched right away.

I believe it is important to keep general support for the old formats around, but it can be import-only and you have to save in a new format. That is totally up to every single application, though. If you implement KDB[XS]5, nothing keeps you from fully supporting the old formats as well. I would recommend nudging users into the right direction, though. If you never deprecate things, users will forever keep using it. There will never be a point where you can discontinue something because nobody is using it anymore if you are not actively working towards that state.

DReichl commented 3 years ago

The KDBX 4 format does support more features and follows more best practices than the KDBX 3 and KDB formats, but I'm not aware of any real problems with the older formats (especially, I'm not aware of any practical cryptographic attack against them). There are apps that are still using the older formats and aren't maintained actively (which I don't really consider to be a problem as long as there aren't any security issues); deprecating the formats would unnecessarily break compatibility with them. Therefore, I'm not going to do that.

If you want to stop supporting the older formats (e.g. in order to reduce complexity), feel free to do that. For such a format, just show a message that it is unsupported by KeePassXC (calling it "deprecated" would be confusing).

I'll post my thoughts about the other ideas here soon. Thanks!

droidmonkey commented 3 years ago

Good point, we will refrain from using deprecated. We went a bit hard on the key file format, apologies

J-Jamet commented 3 years ago

Thank you @DReichl for these clarifications. As the old formats are not deprecated and do not represent security concerns. I will keep them until the implementation of new elements takes too much work for compatibility or migration.

phoerious commented 3 years ago

In the end it means the same thing and for the average end user it for sure does. Deprecated doesn't mean bad, unsafe or anything, it simply means "unsupported in the future". Perhaps we should clarify that only KeePassXC (and some other apps) will stop supporting the old formats (or at least creating new files in that format).

DReichl commented 3 years ago

Argon2id vs. Argon2d. Of the three Argon2 variants, Argon2d has the best resistance against GPU/ASIC attacks (which is important for database files), and the advantages of Argon2i are far less relevant in our case; see: https://keepass.info/help/base/security.html#secdictprotect Therefore, I think that Argon2d is the best choice in our case.

Is there a point that I'm overlooking?