Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

KDBX 5 Standard #4317

Closed
14 tasks
droidmonkey opened this issue Feb 11, 2020 · 124 comments
Closed
14 tasks

KDBX 5 Standard #4317

droidmonkey opened this issue Feb 11, 2020 · 124 comments

Comments

@droidmonkey
Copy link
Member

droidmonkey commented Feb 11, 2020

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:

  • Standardize on Argon2id instead of Argon2d
  • Add Password Modification Datetime
  • Database/Entry Custom Data modification time
  • Built-in Entry Layout Specification (Add support for customizable entry layouts #863 (comment))
  • Built-in TOTP Specification (otp attribute)
  • Built-in Hardware Key Specification
  • Built-in Quick Unlock Specification
  • Built-in favorites/pinned items
  • Only support straight binary key files
  • Multi-valued core and custom fields
  • Proper storage format for and deduplication of BLOBs (SQLite can handle this)
  • Transactional edits/global history on database level, not entry level (e.g., to undo deletes or moves and to ensure overall consistency)

Nice To Haves:

  • Custom icon naming
  • Graph instead of tree structure (one item can belong to multiple groups, makes tagging somewhat redundant)

Optional Extensions:
None so far

Long form discussions should be held here: #5819

@droidmonkey
Copy link
Member Author

Looping in @keepassium @mmcguill @PhilippC
Original issue that spawned this conversation: #4293 (comment)

@mmcguill
Copy link

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
Copy link

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
Copy link

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
Copy link
Member Author

@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
Copy link

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
Copy link
Member Author

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
Copy link

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
Copy link

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

@droidmonkey
Copy link
Member Author

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

@funnym0nk3y
Copy link

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

@droidmonkey
Copy link
Member Author

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

@funnym0nk3y
Copy link

If you need help let me know!

@ghost
Copy link

ghost commented Nov 14, 2020

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>.
Kunzisoft/KeePassDX#524 (comment)

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
Copy link

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

@droidmonkey
Copy link
Member Author

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

@J-Jamet
Copy link

J-Jamet commented Nov 16, 2020

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
Copy link

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 "kp_ext_*" or "kdbx5_" or something like that for features we think would be very useful additions?

@PhilippC
Copy link

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
Copy link

J-Jamet commented Nov 16, 2020

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
Copy link
Member Author

droidmonkey commented Nov 16, 2020

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
Copy link
Member

phoerious commented Nov 16, 2020

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
Copy link
Member

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

  • Multi-valued core and custom fields
  • Proper storage format for and deduplication of BLOBs (SQLite can handle this)
  • Graph instead of tree structure (one item can belong to multiple groups, makes tagging somewhat redundant)

@droidmonkey
Copy link
Member Author

droidmonkey commented Nov 16, 2020

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
Copy link
Member

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

@DReichl
Copy link

DReichl commented Feb 8, 2021

History entries are saved, because the old data may be useful in the future, also months/years later. The history entries of one entry don't have anything to do with the ones of a different entry. The database options for limiting entry histories reflect this: there are options for limiting an entry history based on the number of history entries and based on the estimated size, but not based on the time. There is a command for a time-based history deletion ('Tools' → 'Database Tools' → 'Database Maintenance'), but this is a manual operation that is only useful in special situations.

In contrast to that, you seem to have a different feature in mind. As far as I see it, you basically want to record all operations (including ones that don't change important data, e.g. moving or renaming groups), show them in a log and possibly allow to undo sequences of operations. Limiting such a log based on the time or the number of operations (entry-independent) makes sense here.

So, what we currently have is similar to a backup/storage software keeping older versions of files. Your idea is similar to an undo function instead, with the undo information being stored in the database.

Combining these two features into one is problematic. From a functionality point of view, the latter is a superset of the former (one could collect/create history entries for a specific entry from the log). However, there also are disadvantages. Recording all operations would require a huge amount of space; too much in my opinion. Maintaining such a log (e.g. erasing non-consecutive parts of it in certain situations like an export) would be difficult and error-prone (easily leading to security problems). It's questionable whether the functionality justifies the space and complexity problems; I don't think that there are many users who want to see/undo sequences of operations that happened a long time ago.

The ability to see/undo some of the most recent operations would certainly be useful though. If you want to add such a function, my suggestion would be to keep the log/undo information in the process memory only, until the database or the application is closed. This is how most other applications do it, and I don't see why our case should be an exception. It would possible to see/undo the most recent operations, and important information (in history entries) continues to be saved in the database file (usually for a longer time); I think this would be a good solution.

If you disagree and really want to store log/undo information in the database file, you can store it in custom data items (e.g. with time suffixes in the item names in order to ensure that KeePass merges them as intended during a synchronization, maybe encrypting values with entry-/group-dependent keys in order to avoid security problems). KeePass won't use these; I'll instead maybe add an undo function in the future that stores the undo information in the process memory.

@droidmonkey
Copy link
Member Author

droidmonkey commented Feb 8, 2021

Memory based undo/redo is my only preference. Storage in the database would be for an audit log only, and is particularly useful in shared usage scenarios or keeshare based databases. IMO this would be an opt-in feature for those users or corporate environment that requires such tracking. It has been requested several times on our board.

This could, of course, be achieved using custom data at the database level. I find that to be a solution, but not a desired solution due to the merging and exporting concerns mentioned.

@DReichl
Copy link

DReichl commented Feb 15, 2021

Proper storage format for and deduplication of BLOBs (SQLite can handle this)

In KDBX 4, the contents of files attached to entries are stored in the inner header (compressed and encrypted, without Base64 encoding) and a deduplication is performed: each content is stored only once and can be referenced multiple times in the XML part.

All other data stored in a database file is significantly smaller. A deduplication wouldn't save much space here, especially considering that a part of the compression (GZip/Deflate, turned on by default) is to detect repetitions and store back-references instead (typically in a 32 KB sliding window only, which should be sufficient for "normal" databases though; history entries are stored near the actual entry).

So, I doubt that there is a different format that would require significantly less space. Especially, I haven't found any feature in the SQLite database file format that could result in considerably smaller files. Furthermore, one also needs to consider other factors (security, complexity, extensibility, ...). In our case, I don't think that a SQLite-based format would be more suitable than the current KDBX format.

Some users try to attach many large files (GBs) to entries. I consider this to be out of the scope of a password manager and recommend to use a specialized file encryption software (e.g. VeraCrypt) for this task instead (the password manager can store the password for the encrypted file container).

@droidmonkey
Copy link
Member Author

droidmonkey commented Feb 15, 2021

There is a significant challenge with the attachments being in the inner header, when exporting XML you lose your attachments. We could keep them in the inner header, but when XML export you create attachment keys in the xml with the attachments as base64 encoded. Upon import of the xml they would be placed back into the inner header of the new database. I'm not sure if there is a significant difference between compressed attachments in binary form or compressed attachments in base64. The base64 would compressed again with the xml. It would certainly make reading the database a little easier.

Agree that attachments over 10 MB in general are not scoped for kdbx and should be in an encrypted file system. We warn users when they try to add large attachments.

@Fraetor
Copy link

Fraetor commented Feb 15, 2021

Would increasing the size of the sliding window have a positive effect? The databases I tend to work with are around 500KB in size, though I suspect most of that is in favicons.

@phoerious
Copy link
Member

The biggest issue with the current format is that BLOBs are indexed by order of occurrence. At least they should get a unique ID to ensure consistency . I have seen databases where the XML got out of sync with the header BLOB. Using a BLOB's hash as ID would be a neat trick that would ensure consistency of both the content and the referencing mechanism.

@DReichl
Copy link

DReichl commented Feb 16, 2021

The XML documents in a KDBX file and in an exported XML file differ slightly. In a KDBX 4 file, attachments are stored in the inner header. An exported XML file doesn't have such a header, thus attachments are stored in the 'Meta/Binaries' node of the XML document instead (compressed and Base64-encoded, with deduplication). So, storing attachments is supported in both cases.

When storing attachments in the inner header, the database file typically is a bit smaller, but the difference isn't that large (when using a good GZip/Deflate library with appropriate settings, Base64 data is compressed pretty well). Another advantage is that fewer data needs to be processed (no Base64 encoding, the XML serializer doesn't need to write all the Base64 data, and consequently fewer data is fed into the compression algorithm), resulting in a better performance. Furthermore, depending on the used XML serializer and the Base64 encoder, there may be implementation advantages (e.g. no large strings in the process memory, possibly allowing a better process memory encryption). In my opinion, the additional complexity required for storing attachments in the inner header isn't that huge. Therefore, I currently don't want to remove this feature.

Would increasing the size of the sliding window have a positive effect?

RFC 1951 (which specifies the Deflate format) says: 'The representation used in the "deflate" format limits distances to 32K bytes ...'. So, increasing the size of the sliding window would be incompatible with standard Deflate implementations.

The Deflate64 variant uses a 64 KB sliding window. Unfortunately, it is incompatible with the Deflate format, and not all libraries support it (e.g. zlib does not). For testing, I compressed a 4.3 MB database XML file (containing attachments as well as many entries) using Deflate and Deflate64, resulting in 589 KB and 584 KB files respectively; not an impressive improvement.

I'd rather add support for a different compression algorithm, e.g. LZMA/LZMA2. The reason why I haven't added support for another algorithm yet is that I'm not sure whether this would really be worth it. The compression wouldn't be that much better (for example, the 4.3 MB database XML file mentioned above compresses to a 555 KB file using LZMA2) and a compression library would be required (LZMA(2) is rarely supported directly by frameworks), increasing the complexity. Currently, I'm happy with GZip/Deflate and prefer to wait; maybe another compression algorithm becomes more widely supported in the future.

The databases I tend to work with are around 500KB in size, though I suspect most of that is in favicons.

Custom icons are stored in the PNG format (for various reasons), which uses Deflate compression internally. Trying to compress already compressed data usually saves only very few space, even when the second compression algorithm is better (w.r.t. size).

I have seen databases where the XML got out of sync with the header BLOB.

This is a bug in the application that saved the database file, not a problem of the file format.

The ID of a content item is specific to one database file. While loading a database file, entries may reference contents by their IDs; when loading has finished, the IDs are meaningless and should not be remembered. While saving a database file, every content gets a new ID. When implementing it like this, IDs cannot get out of sync.

One could make the IDs persistent (e.g. UUID or hash of the content), but I don't see any real advantage. A disadvantage would be that the IDs would be longer.

@phoerious
Copy link
Member

phoerious commented Feb 16, 2021

This is a bug in the application that saved the database file, not a problem of the file format.

Yes and no. The file format makes errors like these possible without any means of catching them. And once your database is in such a state, there isn't even a way to ever fix it without manually going through all attachments.

One could make the IDs persistent (e.g. UUID or hash of the content), but I don't see any real advantage. A disadvantage would be that the IDs would be longer.

That's exactly what I proposed and you don't even need to save a full SHA256 hash.

@DReichl
Copy link

DReichl commented Feb 24, 2021

If the consecutive IDs would be replaced by UUIDs/hashes, some developers might think that it's a good idea to store the binary contents and the entries separately, which could easily result in synchronization problems though (this time in the sense that there are binary contents that aren't referenced by any entry anymore, or entries referencing binary contents that have accidently been deleted already).

UUIDs/hashes are unnecessary here. With consecutive IDs, it's clearer that they are specific to one database file.

Built-in TOTP Specification

This exists in the meanwhile; KeePass 2.47 has introduced support for a {TIMEOTP} placeholder:
https://keepass.info/help/base/placeholders.html#otp

@droidmonkey
Copy link
Member Author

droidmonkey commented Feb 24, 2021

TIMEOTP? Why not TOTP. We already settled on TOTP placeholder. Either way we won't be supporting that specification, the key-uri syntax is far easier to deal with and condenses everything into one attribute (otp).

@DReichl
Copy link

DReichl commented Feb 24, 2021

The name {TIMEOTP} (short for 'Time-based One-Time Password') is consistent with the name {HMACOTP} (short for 'HMAC-based One-Time Password'), which exists since 2010.

@0xpr03
Copy link

0xpr03 commented Feb 24, 2021

Yeah but I've never heard of timeotp only TOTP as short version, see also wikipedia. And why use the longer version if the shorter is a standard name for users?

@Fraetor
Copy link

Fraetor commented Feb 24, 2021

Existing implementations such as in KeePassXC use the otp attribute for TOTPs, with the specific method being specified in the content of the attribute in the key URI syntax.

otpauth://totp/AccountName?secret=AAAAAAAAAAAAAA&period=30&digits=6&issuer=ServiceName

What are the advantages of breaking it up into specific fields?

@J-Jamet
Copy link

J-Jamet commented Feb 24, 2021

It's just that KeePass and KeePassXC have taken different development paths.
There is no need to manage others TOTPs in desktop applications because few users use more than one app on the same machine (and they are already in production). So no more need to argue at this level.
The problem is now to manage these different formats in secondary applications (those on other devices).
I made the choice to manage already existing TOTP formats to satisfy users but there are too many of them (especially with the list of historical plugins) and it's horrible to define them consistently in a same shared base.

That's why this thread exists for new features. I don't want the problem to happen again, it would be a shame, and we all share this same philosophy of sharing. ;)

@DReichl
Copy link

DReichl commented Feb 24, 2021

When adding support for the {TIMEOTP} placeholder, I did consider using the otpauth format for the parameters, but decided against it, for the following reasons:

  • An otpauth URI contains a label, consisting of an account name and possibly an issuer name. For the purpose that the format has been designed (namely to transfer OTP generator information through a QR code), this is perfectly reasonable. However, in the case of KeePass, these names would duplicate the user name and the title of the owning entry (with possibility of desynchronization, etc.). One could use an empty label, but this would make the otpauth URI unsuitable for a transfer, which could confuse users. I think it's better to separate the storage format and otpauth URIs. For generating/importing proper otpauth URIs (suitable for transfers), commands can be provided.
  • The otpauth format supports Base32 secrets only. This restriction is not present in RFC 6238, and thus I'd like to support additional encodings (UTF-8, hexadecimal, Base64).
  • Entering/managing information in separate fields is clearer and easier for users.
  • It makes sense to make specifying parameters for the {TIMEOTP} placeholder analogous to specifying parameters for the {HMACOTP} placeholder.

{TOTP} doesn't collide with {TIMEOTP}. So, if you want to continue supporting your solution, I don't see a problem with that.

@DReichl
Copy link

DReichl commented Mar 3, 2021

Custom icon naming

Good idea; I've added this now (and last modification/deletion times, for synchronizations):
https://keepass.info/help/kb/kdbx_4.1.html

@phoerious
Copy link
Member

That's a quite useful. Although overall, I'd be happy if we could somehow sketch out what a KDBX 4.1 and/or 5 should look like together. I don't want to add too much bureaucracy to the process, but Dominik just picking or rejecting things and informing us about it, seems kinda one-way (no offence in any way, I appreciate the communication).

@0xpr03
Copy link

0xpr03 commented Mar 3, 2021

[..] names would duplicate the user name and the title of the owning entry (with possibility of desynchronization, etc.). [..] otpauth format supports Base32 secrets only. This restriction is not present in RFC 6238, and thus I'd like to support additional encodings (UTF-8, hexadecimal, Base64).

That sounds like a good reason to maybe migrate keepassxc towards the timeotp format ?

@Fraetor
Copy link

Fraetor commented Mar 3, 2021

If it was made a part of the format standard that sounds like a good idea. The point of standards is to standardise on them. The transition path does need to be carefully considered however, and it may be worth having both the otp and timeotp attributes for a time while other clients move away from otp (such as mobile clients).

@mstarke
Copy link
Contributor

mstarke commented Mar 3, 2021

@droidmonkey TIMEOTP? Why not TOTP. We already settled on TOTP placeholder. Either way we won't be supporting that specification, the key-uri syntax is far easier to deal with and condenses everything into one attribute (otp).

I know it's a hassle to implement a multitude of different formats but that's the deal when using a shared format. Might I urge you to reconsider not supporting another format? This would lead to KeePassXC special behaviour that other clients have to support as well. Regarding the otpauth field. You seem to have chosen to use a non-standard encoder field to add support for steam. Or is this a supported way of expressing different TOTP flavours?

@droidmonkey
Copy link
Member Author

droidmonkey commented Mar 3, 2021

To be fair, keepass2 is late to the game with totp. We are compatible with both previous keepass2 totp plugins and all mobile apps. Stream is a niche use case and not meant to be a standard

@droidmonkey
Copy link
Member Author

droidmonkey commented Mar 3, 2021

@0xpr03 neither of those points are valid. The username and title can very easily be placeholders that are dynamically filled when polled. Base32, Hex, and whatever else are encoding schemes. You can convert to any of them at any time. Standardizing on Base32 actually makes everything easier because you don't need to guess what encoding scheme is being used and have extra bloat code to deal with that.

@DReichl
Copy link

DReichl commented Mar 4, 2021

Dominik just picking or rejecting things and informing us about it, seems kinda one-way

My posts weren't intended to only inform you. I'm not mentioning it in every post/e-mail, but as stated in red on the top of the KDBX 4.1 page and in my initial e-mail about KDBX 4.1 to everyone, nothing is final yet and all feedback/discussion is welcome.

In the cases where I added something, your suggestions were sufficiently clear to me and the implementation was straightforward (for example, the last modification time elements should be called 'LastModificationTime' for consistency with already existing 'LastModificationTime' elements, they should store the time in the same format, etc.). In the cases where I didn't want to add something, I tried to explain why. If you want something to be implemented differently or believe that I didn't think about something, then please don't hesitate to write that. All of the things mentioned on the KDBX 4.1 page are open for discussion, and until it's final, I have no problem at all with changing them.

The username and title can very easily be placeholders that are dynamically filled when polled.

As the user name and the title would need to be URI-encoded, a minimal otpauth URI (assuming default values for the length, period and algorithm) would look like this:

otpauth://totp/{T-CONV:/{TITLE}/Uri/}:{T-CONV:/{USERNAME}/Uri/}?secret=...

or

otpauth://totp/{T-CONV:/{USERNAME}/Uri/}?secret=...&issuer={T-CONV:/{TITLE}/Uri/}

This would indeed avoid a desynchronization of the title and the user name.

I prefer the solution of the {TIMEOTP} placeholder, but like I wrote previously, there should be no problem if you want to continue supporting your solution.

Standardizing on Base32 actually makes everything easier because you don't need to guess what encoding scheme is being used and have extra bloat code to deal with that.

There is no need to guess the encoding of a {TIMEOTP} or {HMACOTP} secret. The name of the string storing the secret ends with the encoding ('-Hex', '-Base32', '-Base64', no suffix for UTF-8), i.e. there is no ambiguity.

One of the advantages of making the parameters for {TIMEOTP} analogous to the ones for {HMACOTP} is that code can be reused. KeePass uses the same method for loading the secret (with the first part of the string name as parameter), i.e. no extra code is necessary.

@Fraetor
Copy link

Fraetor commented Mar 4, 2021

Thanks for the clarification Dominik, the way the page was in the "help" section of the KeePass website, and its inclusion in 2.48 made it seem a bit more final. Perhaps the Introduction section could be changed as it currently reads (to me at least) as if KDBX 4.1 is the default, where it is really that a KDBX 4.1 database will not be downgraded if opened.

Maximising code reused is a sensible goal, but there are lots of implementations which have different code to be reused. Do we know what is the most commonly adopted method? Is {HMACOTP} widely implemented? If it is, then the code for that should be reusable (or rewritable into reusability), while if otpauth has wider implementation across KDBX compatible software, it may make sense to minimise disruption to their codebases. I'm not personally sure of how pervasive either these are in the wider ecosystem however.

From a user perspective otpauth should probably be supported as an import/export format, as it is the most common way secrets are given out (typically in QR codes), but that is out of scope for the database format, and can be easily reconstructed from individual fields if required.

With niche things like Steam, are the details stored in the same otp field as regular TOTPs?

@droidmonkey
Copy link
Member Author

It is my understanding that HMACOTP has no support in any alt-client, mobile or desktop. KeePassXC obviously does not support it. I have no problem supporting the TIMEOTP placeholder that actually fills the calculated TOTP value, my problem is with the extra attributes defining the parameters. We settled on key-uri for the exact reason of QR code standardization and it also supports HOTP if we decide to implement that. I don't see us moving off that as our default when creating new OTP settings for an entry. So by that virtue, we won't be compatible with KeePass unless you implement the otp parameter with key-uri. That is why I am worked up about this.

@J-Jamet
Copy link

J-Jamet commented Mar 4, 2021

From the beginning of unique codes implementation in KeePassDX, the app correctly manages the HmacOTP parameters of KeePass2, but not the associated placeholders {HMACOTP} / {TIMEOTP} (not useful because there is an internal display that standardizes this views elements, it needs to be improved). Also, I already naturally managed the otpauth://hotp links as well to be consistent.
I let you debate this part because you have different opinions but just to say these settings were already managed before.

@phoerious
Copy link
Member

phoerious commented Mar 5, 2021

We settled on key-uri for the exact reason of QR code standardization and it also supports HOTP if we decide to implement that.

That. Of course, we could wrap some magic around it to let the user copy it as a URL for export, but why should we? The raw field value should be usable as is and a {TIMEOTP} value makes no sense outside the application unless I manually reencode it and assemble it to a URL. Also having it as a URL allows us to parse the value using standard off-the-shelf tools.

Regarding "we are free to implement it differently", sure, I know that. And we may. But everybody implementing things differently, because we cannot agree on anything is what drives open source apart. Having ten incompatible implementations in the end is the worse outcome for everybody, developers and users alike; even if each implementation has its own unique benefit. "Linux on the desktop" is a long history of exactly that kind of failure over and over again.

@phoerious
Copy link
Member

I'm moving this thread to discussions.

@keepassxreboot keepassxreboot locked and limited conversation to collaborators Mar 5, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →