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

Integrated Password Analyzer and Health Check [$170] #551

Closed
andkopp opened this issue May 7, 2017 · 97 comments
Closed

Integrated Password Analyzer and Health Check [$170] #551

andkopp opened this issue May 7, 2017 · 97 comments

Comments

@andkopp
Copy link

andkopp commented May 7, 2017

Another point for the wish list: I am looking for an integrated password analysis tool that evaluates the passwords in my database both for their equality or similarity among themselves, their password strength as well as their age.

Regarding the implementation, I can imagine several variants. It could be a separate menu point, over which a test procedure can be started. At the end of the check, the result per database entry is displayed in a list. The list should be filterable according to problem type and criticality.
Another possibility would be to always check the passwords in the background. For each entry, a kind of traffic light (eg green, yellow, red) and, if necessary, a warning message is displayed which indicates how secure the password of an entry is.

Examples of warning messages:

  • The password is used several times in the same or similar form (the entries x, y, z... use a related password)
  • The password is not strong.
  • The password is quite old (the older a password, the higher the probability that it could be stolen in an attack on the vendor).

Edit: The following plugins for the original Keepass try to achieve the same or similiar goals.

@droidmonkey
Copy link
Member

Also add support for displaying expired passwords.

@tycho
Copy link
Contributor

tycho commented Feb 28, 2018

Was about to file an issue requesting this feature. The password strength estimation stuff is already in there, this seems like it would be mostly UI work.

This could be partially solved by adding a new Entropy column to the list of entries. It could probably be a combination of numeric entropy value and the visual indicator used in the password generator. To find weakest/strongest passwords, I could just sort by that column.

To find old passwords we can already sort by the Modified date column, although that is potentially very misleading, depending on how you manage your entries. It changes with -any- modification, including changes that don't touch the password field at all (e.g. adding a TOTP or icon would count as "modification", so you might think that a recent modification timestamp means the password was changed recently but this isn't necessarily true). The history of each entry is tracked though, so it should be possible to discover the last time the Password field was modified and have that as another column ("Last Password Change", "Password Modified"?).

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Feb 28, 2018

I like the Entropy column approach.

For expired password I think it's for passwords that are explicitly tagged as expired, this will be easier to develop

@tycho
Copy link
Contributor

tycho commented Feb 28, 2018

Well in my case I haven't ever touched the "expiration" field, but I probably should start using it. It would be nice to just have it track how old the Password field is so that I could slowly work through the list of the oldest passwords and start changing them.

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Feb 28, 2018

@tycho I think it needs more effort but indeed it's doable.

@droidmonkey
Copy link
Member

Just sort by modified time, not perfect because any modification changes it, but better than nothing.

@tycho
Copy link
Contributor

tycho commented Mar 1, 2018

Already talked about that. I don't think sorting by Modified is at all useful for the purpose of identifying old passwords. See my comments above.

@droidmonkey
Copy link
Member

There is no built-in mechanism in KDBX to store the last modified time of just the password field so there is nothing useful to use unfortunately. Expires is the best method without diving into custom data/attributes.

@tycho
Copy link
Contributor

tycho commented Mar 1, 2018

What about the modification audit log attached to every entry?

@droidmonkey
Copy link
Member

Are you referring to the history?

@tycho
Copy link
Contributor

tycho commented Mar 1, 2018

Yes?

@droidmonkey
Copy link
Member

That is not meant for determining individual changes, it is a literal copy of the entry in case of an error/typo/whatever. If you want to continue this please lets take it to the keepassxc-dev IRC channel. Thanks!

@tycho
Copy link
Contributor

tycho commented Mar 1, 2018

I get that it's not deltafied, but my point is that the data is there. You can walk history entries to figure out when the password value last changed.

@Skycoder42
Copy link

Skycoder42 commented Mar 1, 2018

I think sticking to the expired field would be the best approach, too. Using the history would propably get way to complicated. Sorting by generelly last modified (the Entry, not the PW field) however seems to be a good idea. For most cases that's close enough to the "last time password changed", I think.

@ba32107
Copy link
Contributor

ba32107 commented Jan 19, 2020

Hi @wolframroesler, thanks for your work, this feature looks very promising. One question, does your PR include the password expiry warning? If not, which one of the above issues would incorporate this? None of them mention expired passwords explicitly.

@wolframroesler
Copy link
Contributor

Thanks for the thumbs-up @ba32107 :) the expiry warning is already included in my PR.

droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Jan 20, 2020
Introduce a password health check to the application that
evaluates every entry in a database. Entries that fail 
various tests are listed for user review and action. Also
moves the statistics panel to the new Database -> Reports 
widget. Recycled entries are excluded from the results.

Tests include passwords that are expired, re-used, and weak.

* Closes keepassxreboot#551

* Move zxcvbn usage to a centralized class (PasswordHealth) 
and replace its usages across the application to ensure 
standardized interpretation of entropy calculations.

* Add new icons for the database reports view

* Updated the demo database to show off the reports
@rugk
Copy link

rugk commented Jan 20, 2020

Maybe you could introduce a new tag for all these features related to that whole thing? (Or a GitHub projects page??)

@LightTemplar
Copy link

So, regarding the current issue, is there anything left I need to add to the implementation before it can be merged?

Yep, it's a finding and merging of duplicate entries.
Issue #750, which was included here.

@wolframroesler
Copy link
Contributor

@LightTemplar Health Check already reports duplicated passwords, but I think #750 is about entries that are duplicated as a whole, or very similar to other entries (say, same URL and user name but different password, because an entry was cloned, the clone was modified, and the original was forgotten). While I consider that an interesting feature, I don't think it belongs into Health Check because it doesn't affect password quality or security. Maybe #750 should be re-opened, and a definition of what it means that two entries are "similar" should be supplied.

@droidmonkey
Copy link
Member

droidmonkey commented Jan 25, 2020

I don't think such a feature should exist. It is near impossible to tell if an entry is a duplicate or intentionally the same. You might have only 1 field different, is that a duplicate entry? It doesn't make sense to automate that process.

wolframroesler added a commit to wolframroesler/keepassxc that referenced this issue Jan 25, 2020
Introduce a password health check to the application that
evaluates every entry in a database. Entries that fail 
various tests are listed for user review and action. Also
moves the statistics panel to the new Database -> Reports 
widget. Recycled entries are excluded from the results.

Tests include passwords that are expired, re-used, and weak.

* Closes keepassxreboot#551

* Move zxcvbn usage to a centralized class (PasswordHealth) 
and replace its usages across the application to ensure 
standardized interpretation of entropy calculations.

* Add new icons for the database reports view

* Updated the demo database to show off the reports
@LightTemplar
Copy link

@wolframroesler, I don't mind, if #750 will be reopened.

@droidmonkey, I think, the app should let user decide, what values should be same, to count entries as duplicate. I personally need username and second level domain name, sometimes even without first level, e.g. aliexpress.de and aliexpress.com - they should be in one entry, but now they are in different.

@droidmonkey
Copy link
Member

Letting the user decide exponentially complicates the feature and implementation. From a cost benefit viewpoint it's not worth implementing. You can easily search for * and then sort the results by the various columns to find, and fix, duplicates in your database.

@LightTemplar
Copy link

@droidmonkey, I have 824 entries in the database. I don't think, that the option to sort them and go through all of them is the best solution.

@OLLI-S
Copy link

OLLI-S commented Jan 26, 2020

KeePass offers a feature to "Delete Duplicate Entries":

image

I have 1002 entries in KeePasXC and if I search for duplicate entries in KeePass, the 2 entries are found/deleted (I did not save the database in KeePass).
Finding these 2 duplicate entries out of 1002 entries manually is much work, so I think #750 should be re-opened. Defining what duplicate means is a total different discussion (that should be made on #750) but I would re-open it.

My plan - besides world domination - is to have some more database-maintenance tools (I will create separate issues for my suggestions).
So maybe you rename the title of #750 to "Database-Maintenance - Find/merge duplicated password entries"

@rugk
Copy link

rugk commented Jan 26, 2020

I don't think such a feature should exist. It is near impossible to tell if an entry is a duplicate or intentionally the same.

Well… it is?
Could not you then just offer a list of duplicates and the user can choose:

  • to either delete one entry
  • or to "link" them (the thing you can do when you clone an entry and check the box "link username" and/or "link password"

This way, they are either marked as unintentional (duplicate deleted) or as intentional (then properly link them).

Use case
E.g. I have the problem with Stackexchange. Their 1000 domains are driving me crazy, as I of course want to have password-autofill on each page, but with same username+password.

--> I anyway wanted to have a solution for that problem too, because the current UX is not that good when you need to clone each entry again just to link them. (Because I still have old entries in there I need to delete.)

(maybe better to put that into a new issue, is not it? Although "duplicate finding" is exactly the problem/process I have here.)

@Techman
Copy link

Techman commented Jan 27, 2020

I personally think that this feature should be implemented. Is there a special string that I can copy-paste into the search box to find duplicate entries?

If KeePassXC can report non-unique passwords and maximum password reuse in the statistics area of database settings, there should be a way to deal with that.

@droidmonkey
Copy link
Member

droidmonkey commented Jan 27, 2020

What is duplicate to you is not duplicate to the next user. That means YOU need to decide what to search for to find duplicates. If you want to consolidate stackexchange entries, search for stackexchange. If you want to find entries with a similar field (Title, URL, etc) then search for * which shows ALL entries in your database. Then click the header you want to sort by and look for entries that are grouped together.

Aside from finding EXACT duplicates, automating this process is fruitless since it depends on how you define a duplicate between entries you are concerned with.

@OLLI-S
Copy link

OLLI-S commented Jan 27, 2020

You should add a feature called "Find Duplicates" and show a list with possible duplicate entries.
In the first column you show the title of the entry, in the second column you show a percentage of equality (100% means all fields are equal, 90% means that all fields except one field are equal).
You also should show the columns Username, Password (masked) and URL and above the table a checkbox to show the password in clear text.

In this form (below the table) you should also show a link to a documentation that explains how to avoid duplicate entries and how linking entries works (and what the benefit of linking is).

@droidmonkey I suggest that we continue the discussion in #750 and that you reopen it. Maybe you also change the title of #750 to "**Database-Maintenance - **Find/merge duplicated password entries"

@droidmonkey
Copy link
Member

Fair enough, I reopened #750

droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Jan 28, 2020
Introduce a password health check to the application that
evaluates every entry in a database. Entries that fail 
various tests are listed for user review and action. Also
moves the statistics panel to the new Database -> Reports 
widget. Recycled entries are excluded from the results.

Tests include passwords that are expired, re-used, and weak.

* Closes keepassxreboot#551

* Move zxcvbn usage to a centralized class (PasswordHealth) 
and replace its usages across the application to ensure 
standardized interpretation of entropy calculations.

* Add new icons for the database reports view

* Updated the demo database to show off the reports
wolframroesler added a commit to wolframroesler/keepassxc that referenced this issue Jan 30, 2020
The way the class is currently being used, the cache never does
anything (because evaluate is never invoked twice for the same
entry), so according to YAGNI it has to go.

Fixes keepassxreboot#551
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Feb 1, 2020
Introduce a password health check to the application that evaluates every entry in a database. Entries that fail  various tests are listed for user review and action. Also moves the statistics panel to the new Database -> Reports  widget. Recycled entries are excluded from the results.

We now have two classes, PasswordHealth to deal with a single password and HealthChecker to deal with all passwords of a database.

Tests include passwords that are expired, re-used, and weak.

* Closes keepassxreboot#551

* Move zxcvbn usage to a centralized class (PasswordHealth)  and replace its usages across the application to ensure standardized interpretation of entropy calculations.

* Add new icons for the database reports view

* Updated the demo database to show off the reports
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Feb 1, 2020
The way the class is currently being used, the cache never does
anything (because evaluate is never invoked twice for the same
entry), so according to YAGNI it has to go.

Fixes keepassxreboot#551
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Feb 1, 2020
Introduce a password health check to the application that evaluates every entry in a database. Entries that fail  various tests are listed for user review and action. Also moves the statistics panel to the new Database -> Reports  widget. Recycled entries are excluded from the results.

We now have two classes, PasswordHealth to deal with a single password and HealthChecker to deal with all passwords of a database.

Tests include passwords that are expired, re-used, and weak.

* Closes keepassxreboot#551

* Move zxcvbn usage to a centralized class (PasswordHealth)  and replace its usages across the application to ensure standardized interpretation of entropy calculations.

* Add new icons for the database reports view

* Updated the demo database to show off the reports
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Feb 1, 2020
The way the class is currently being used, the cache never does
anything (because evaluate is never invoked twice for the same
entry), so according to YAGNI it has to go.

Fixes keepassxreboot#551
droidmonkey pushed a commit that referenced this issue Feb 1, 2020
The way the class is currently being used, the cache never does
anything (because evaluate is never invoked twice for the same
entry), so according to YAGNI it has to go.

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

No branches or pull requests