strongbox-password-safe / Strongbox

A KeePass/Password Safe Client for iOS and OS X
https://strongboxsafe.com
GNU Affero General Public License v3.0
1.29k stars 100 forks source link

Password history displaying incorrect entries #758

Closed unicorn855 closed 4 months ago

unicorn855 commented 5 months ago

Hi everyone, Since I got my New YubiKeys a few days ago, I took this as a reason to start going through all my password database entries to check/update them. I did this on pc with KeepassXC and decided to periodically sync the DB after changing a few entries when I noticed that in Strongbox (iPhone, I still have to check whether this also happens on the iPad version), some entries where I did not change the password had new password history entries. Has anyone else experienced this as well or am I overlooking something? (The entries still hold the correct password, just the history shows extra entries which shouldn't be there.)

strongbox-mark commented 5 months ago

Hi @unicorn855 - was it KeePassXC that saved this database or was it Strongbox? Strongbox will show whatever history is available, but it sounds like you were making the changes with KeePassXC? Strongbox was just used to view them?

unicorn855 commented 5 months ago

Hi @strongbox-mark , Indeed, I modified the entries on a PC using KeepassXC (but didn't touch the password for the entries in question) and then viewed those entries in Strongbox (because KeepassXC currently still lacks some quality of life features like password history or displaying details on password strength). 🤔 The only new thing I did was adding tags to the entries (with KeepassXC) so I can filter them more easily, but then again I also did this with entries where the password history displays correctly, so idk, perhaps the syntax for tags differs between KeepassXC and Strongbox?

strongbox-mark commented 5 months ago

That is interesting. When you say "password history entries", could you send a screenshot of where you're viewing this (obviously redacted) - I assume you mean on the details screen, when you tap the sort of "clock" icon above the password field?

If so, you see a list of previous passwords right, are you seeing duplicates in that list?

unicorn855 commented 5 months ago

Precisely there, yes after tapping on the clock icon (I'll provide a redacted screenshot in a bit, as I don't have my security keys with me right now, and I configured the virtual hardware key for use with autofill only, for security reasons). I remember seeing duplicate password history entries yes, a handful of them, is this significant? I was in fact wondering why those were listed there :-)

strongbox-mark commented 5 months ago

Yes, definitely shouldn't be duplicates in there, it wouldn't make sense. For example:

Used Until Today at 4pm MyOldPassword Used Until Today at 2pm MyOldPassword

You know, it doesn't make sense, because it should be collapsed into a single historical entry:

Used Until Today at 4pm MyOldPassword

or it should have a different password interleaved:

Used Until Today at 4pm MyOldPassword Used Until Today at 3pm MyOtherOldPassword Used Until Today at 2pm MyOldPassword

it is only supposed to show changes to the password. So that would be a bug, but I'd need to understand your list, so perhaps you could detail more what you're seeing.

unicorn855 commented 5 months ago

image

After some thinking I believe I misconstrued the idea behind the password history feature: it does in fact show the previous password but the dates for those entries appear to be shifted one entry too far into the future for some reason. (Since none of the passwords shown in the screenshot are used currently, I don't think there is a reason to redact them) So, for the example entry, I last changed the password on 31/10/2023 00:07, but as you can see from the screenshot, it uses the very last time I modified the entry instead (27/01/24 5:30pm), except I did not touch the password at that time but only added tags then. Btw, is there a way to see the difference between historical edits for one entry at one glance in Strongbox on iOS?

strongbox-mark commented 5 months ago

That definitely does sound like a bug, but I can't seem to recreate it.

You say:

I last changed the password on 31/10/2023 00:07

How do you know this? Do you have it recorded somewhere? In the screenshot above it say you changed the password on Jan 27th, are you saying you didn't change the password then? or are you saying that that date is incorrect and it should be 31/10/2023 00:07?

What about the other entries? Are they completely incorrect/spurious, you never made these changes? or where did the 30th of October come from?

Sorry, lots of questions but it's very difficult to understand the issue so far. Are you able to reproduce the issue on a fresh database in Strongbox say?

Btw, is there a way to see the difference between historical edits for one entry at one glance in Strongbox on iOS?

I'm afraid there's no easy "diff" but you can view the full history of an item, just scroll down and tap Entry History on that Details screen.

unicorn855 commented 5 months ago

Good morning! :-) I was trying to screenshot the history view in KeepassXC yesterday and realised it won't let me take a screenshot at all (the window turns transparent instead, nice security feature but slightly hindering in this particular case :-D ). I will test a fresh database in strongbox with dummy entries in a little bit and let you know what I find :-) Update: Apparently, there is an 'Allow Screenshot' option in the 'View' menu to temporarily enable screenshotting, so here is the image for the history entries I see in KeepassXC (the dates there are accurate, you can also see what I mean by difference between historical entries, a view like that would be nice to have in Strongbox):

KeepassXC_history

strongbox-mark commented 5 months ago

Thanks, that's super interesting. So you definitely did not change your password on 27th Jan at 5:30pm? Only Custom Data and Tags were changed?

unicorn855 commented 5 months ago

Precisely so, yes. I know this because at that time, I started going through my entries to check which support security keys and to add tags accordingly and I deleted a custom attribute which contained stale hibp info from back when I first set up my database and was still using the original KeePass. Btw, this phenomenon does not seem to be connected to modifying tags either, since I observed the same shifted password history issue with a different database that does not use tags at all.

strongbox-mark commented 5 months ago

Yeah, I'd be very surprised if it had something to do with Tags, it won't be that. It'll be any kind of historical change I would imagine.

So, no password change on 27 Jan at 5:30pm...

The password displayed above as "chw]..." was that a valid password? and what date was it valid until?

unicorn855 commented 5 months ago

All passwords listed in the password history area in Strongbox were once valid, the one you mentioned was valid until oct 31 0:07, if you cross-reference the information in the KeePassXC screenshot I posted, this is what I meant when I said the entries seem shifted by one entry into the future. So, the current password became valid when the change happened on oct 31st at 0:07, replacing the one you are asking about :)

unicorn855 commented 5 months ago

I was playing around with another database in strongbox yesterday and I believe the phenomenon has to do with some inconsistency in the way changes are recorded when saved in KeePassXC. Perhaps we should cross-reference this with the KeePassXC repo so that people over there get notified of it?

strongbox-mark commented 5 months ago

Hi @unicorn855 - before we bring in the KeePassXC guys I'd like to understand what's going on on the Strongbox side. To that end, would you be able to create and send me a dummy database exhibiting this bug? Once I have that I can see what's happening. Perhaps it's just a Strongbox issue, but a database will be a quick way for to figure this out...

unicorn855 commented 5 months ago

Oh sure, I’ll do that but it’ll take a little while because I need to modify that database with KeePassXC after creating it with strongbox. I’ll comment here when that’s done :-)

unicorn855 commented 5 months ago

Here we are, the password to the db is 'test' (without the quotation marks, the zip-file has no password) password_history_issue_test.zip

strongbox-mark commented 5 months ago

Thanks a million will investigate and get back to you

unicorn855 commented 4 months ago

I just installed the latest update, the password history is displaying correctly now from what I can tell, thanks for fixing this, even though it's mostly a cosmetic/QoL thing 👍