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

Show last modification time of an entry's password #4293

Open
OLLI-S opened this issue Feb 5, 2020 · 20 comments
Open

Show last modification time of an entry's password #4293

OLLI-S opened this issue Feb 5, 2020 · 20 comments

Comments

@OLLI-S
Copy link

OLLI-S commented Feb 5, 2020

Summary

For some security features you need to determine the modification date of the password, so you should show this date also in the GUI so users can use this information.

Details

For some security features like Check for breaches based on site/service ( #1083 (comment) ) you need to determine the modification date of the password.
This can be the creation date of the entry (if the password was never changed) but it can also be any other date. The "Modified" date is not always the date where the password was last changed, because there can be other changes in the database (like changes triggered by the KeePassXC browser plugin).
So you need to determine the modification date of the password by analyzing the creation date and checking in the history of the entry when the password was last changed.

This information (when the password was last changed) is also useful for users, so you should make this information transparent to the user.
This is for example useful when the security check tells the user to change the password, so the user can see the reason (when exactly the password was last changed).

The best place would be the page "Properties" (when you edit the entry) because there you already show other related values (like the modification date and the creation date).

But you should also consider adding the column "Password Modified" that is not shown by default and that the user can select to see.
So when searching for all password entries, the user can sort by this column and then see what passwords were last changed and what passwords were not changed for a longer time.

@droidmonkey
Copy link
Member

This would be good information to store in the custom data section of an entry.

@wolframroesler
Copy link
Contributor

This would be good information to store in the custom data section of an entry.

But what it people use different KeePass clients, e. g. on their phone? OK, in this case we could look at the history, but old history items may have been deleted due to restricted history size. Any chance to get the password modification date in a 100% reliable way?

@OLLI-S
Copy link
Author

OLLI-S commented Feb 5, 2020

@droidmonkey By saying "storing": do you mean where to store the information in the Database?

@droidmonkey
Copy link
Member

@OLLI-S yes.

There is no kdbx standard way to represent the date time when the password itself was changed. It doesn't matter what you do, no other client will display the information without modification

@wolframroesler
Copy link
Contributor

Seems like we need something like a "kdbx consortium" to discuss questions like this. For example, if another client decides to also store password modification dates, they should do it in the same way.

@OLLI-S
Copy link
Author

OLLI-S commented Feb 8, 2020

There are many (really many) KeePass applications and to have them all on-board will be hard.
I also don't know the relationship between Dominik to this project (if he is friendly or hostile).

@mmcguill
Copy link

mmcguill commented Feb 8, 2020

Hi, sounds like a useful feature.

I'd be happy to add this to Strongbox too. I would suggest we just add it to the standard "Times" element:

<Times>
    <CreationTime>2018-11-11T09:36:34Z</CreationTime>
    <LastModificationTime>2018-11-11T09:36:47Z</LastModificationTime>
    <LastAccessTime>2018-11-11T09:36:47Z</LastAccessTime>
    <ExpiryTime>2018-11-11T09:35:47Z</ExpiryTime>
    <Expires>False</Expires>
    <UsageCount>1</UsageCount>
    <LocationChanged>2018-11-11T09:36:34Z</LocationChanged>
    ...
    <PasswordChanged>2020-02-08T01:23:45Z</PasswordChanged>
</Times>

That should work, remain compatible with other clients, follow the standard KeePass timestamp model/formats and be easily processed by all clients.
-Mark

@droidmonkey
Copy link
Member

Good call!

@keepassium
Copy link

Mark's proposal nicely fits into the existing format. However, there are two side effects to consider:

  • A non-standard tag will be skipped by most other apps — including Dominik's KeePass, or even earlier versions of KeePassXC itself. All password timestamps will be erased by saving the DB in any of these apps.
  • Adding a non-standard tag would create an important precedent. There are plenty of other features that would benefit from custom tags (access control lists, anyone?). In a year, we all might have to deal with 15 flavors of .kdbx.

In this context, @droidmonkey's idea (to use the custom data of the entry) has an important advantage: the CustomData will be preserved by any kdbx4-compatible app.

Preserved, but not necessarily updated. If the user changes the password in an older version of KeePassXC — the history changes, but the timestamp remains the same.

To address this, the "new" KeePassXC would still have to verify/update password timestamps when loading the DB. That is, to go through entry's history looking for the password modification. If the history has been truncated, and the password modification timestamp is older than the oldest entry in the history — the timestamp becomes invalid/undefined.

So, even if the password modification timestamp is saved in custom data, its value

  1. would have to be derived from the entry's history, and
  2. sometimes won't be available.

So we might as well skip the persistence part and simply calculate the timestamp when loading the DB...

@mmcguill
Copy link

mmcguill commented Feb 9, 2020

Interesting points... Couple of responses:

Non standard apps.
There's no real XSD or comprehensively defined set of elements for the KeePass XML format. Here's an early comment from Dominik on the format.

This means any app supporting KeePass must or at least should handle unknown elements and preserve them.
This may or may not be the case, and I'm sure there are quite a few poorly coded apps that will eat unknown or new elements. However I don't think we can be bound or make important design decisions based on the worst offenders.
Which is why if we were to decide to go with this, I think it's preferable that to be in the standard Times element which I think if we were starting from scratch is where it belongs.

Preserved but not necessarily updated
Again this is going to be the case no matter what, you will have poorly coded apps, bugs, older versions etc. This sort of thing should however diminish over time as people update and more apps come to support it.

Calculating from History
I see the attraction, however some people will have history off, or it'll get trimmed over time.

Regarding your point about KeePassXC doing this anyway, I don't think that's so. This is an optional element. No need to back fill it by calculating from history. Just starting setting it moving forward. It would be a nice to have but not necessary to go to extreme lengths to try guess the password modification date from history.

Basically this isn't a mandatory element, there are very few mandatory elements in the format. So it can be set or not, no need to try to come up with a good value for it if you can't figure one out.

Ultimately password modification date/time is an orthogonal feature to Entry History and it seems a little less than ideal to derive it this way. Basically if you're going to go to the trouble of implementing this feature, might as well go the whole hog and give it it's own element or at least independent existence of some sort...

Custom Tags, XKCD's 15 Formats and the original KeePass application
This is a fair point. I fear the only solution is not a technical one but rather organisational as @wolframroesler has said. Worth reaching out to Dominik and KeePass2Android developers and seeing if they have some input here? Perhaps we could get a lightweight consortium/mailing list going for this sort of KeePass interoperability going, so that all KeePass users can benefit.

@keepassium
Copy link

I'm sure there are quite a few poorly coded apps that will eat unknown or new elements. However I don't think we can be bound or make important design decisions based on the worst offenders.

Ahem... Here's a few apps that skip unknown timestamp elements: Keepass2android, KeePass, KeePassXC :)

@droidmonkey
Copy link
Member

droidmonkey commented Feb 10, 2020

We do not preserve unknown XML elements (outside of custom data and attributes). It would be impossible to preserve them without doing some sort of crazy merge between the XML or dumping all unknown elements into a "errata" storage on each entry/group/db. @keepassium thank you for posting your thoughts, that is a very good writeup of the downsides to introducing a new element anywhere in KDBX.

On a totally unrelated note, we have been tossing around leading a new KDBX5 standard that helps solve a lot of these issues. Don't know if you guys are interested in this at all?

@keepassium
Copy link

On a totally unrelated note, we have been tossing around leading a new KDBX5 standard that helps solve a lot of these issues. Don't know if you guys are interested in this at all?

I guess the first challenge would be to get Dominik on board. (I'm not sure about him, but his support forum guys seem very conservative.) If the new standard is not supported by KeePass, it might as well be called KPXC1.

In either case, I am interested, count me in.

@mmcguill
Copy link

On a totally unrelated note, we have been tossing around leading a new KDBX5 standard that helps solve a lot of these issues. Don't know if you guys are interested in this at all?

Of course if this gained traction, I'd endeavour to support. :)

However... This sort of leads to what Andrei was talking about with his reference to XKCD, i.e. another 'standard'.

I presume the idea to move to this KDBX5/KPXC1 format so that you could add whatever you want without fear of it being overwritten/skipped/incorrectly ignored because basically every other client would not be able to read it.

I worry that this would fragment the KeePass world. KeePass has a good name and is well known and well supported by tons of apps and users. It has a lot of trust built up around it. I'd worry that by starting off a new format, you'd lose a lot of those users who'd prefer to stick to the tried and trusted apps/formats.

I wonder if a better strategy is to try to keep them all on board and try to expand the format by bringing the other apps onboard with good/cool new features (like Password Modification Date) rather than going it alone?

@droidmonkey
Copy link
Member

@mmcguill I absolutely would not go out alone on developing a new KDBX standard. This would have to be done in coordination with all the major KeePass alternatives. KDBX 4 was a missed opportunity since it was developed largely in isolation (my opinion). For example, Argon2 was chosen, but only the argon2d variant is written in. I am going to start a new issue that we can use to discuss our gripes and then present a cohesive message to Dominik.

@droidmonkey droidmonkey mentioned this issue Feb 11, 2020
14 tasks
@0xpr03
Copy link

0xpr03 commented Feb 15, 2020

Just wanted to say that from a user perspective I'm really grateful for having an open standard and multiple implementations. This way you can be sure that even if your current app of choice should go down, there's still another project around to open it with. (And the initial switch from keepass to X, then XC wouldn't have been possible without the way it is currently.)

@OLLI-S
Copy link
Author

OLLI-S commented Feb 16, 2020

What about storing the modification date in a new user-defined field (like the user-defined field for the TOTP)?
But maybe I am not getting all things correctly...

@0xpr03
Copy link

0xpr03 commented Feb 16, 2020

What about storing the modification date in a new user-defined field (like the user-defined field for the TOTP)?
But maybe I am not getting all things correctly...

See previous posts and the linked Standard Discussion.
It wouldn't work across systems, so you won't be able to track the data as soon as your DB is opened by another application. The timestamp gets unreliable.

@droidmonkey droidmonkey changed the title Make the Modification Date of the Password transparent to Users Record and show the last modification date of an entry's password Jun 20, 2020
@droidmonkey
Copy link
Member

I actually like the idea of dynamically determining the last modification time, however this would fail if the maximum history of an entry is less than the last time the password was modified. Take a scenario where history is disabled, then there would be no way to know if the password was modified separate from the entry modification time.

@droidmonkey
Copy link
Member

This should be implemented through the database reports by searching through each entry's history and determining when the password field was last changed. This is made easier in 2.7.0 through the history change function.

@droidmonkey droidmonkey changed the title Record and show the last modification date of an entry's password Show last modification time of an entry's password Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Quick Wins
Development

No branches or pull requests

6 participants