andrew-schofield / keepass2-haveibeenpwned

Simple Have I Been Pwned checker for KeePass
MIT License
425 stars 25 forks source link

Doesn't list pwned passwords #28

Closed przemhb closed 6 years ago

przemhb commented 7 years ago

Hi,

at first thank you very much for the plug-in. Ever since I knew HIBP I thought of such a plug.

I have installed it and tried to use it. Unfortunately despite the fact there are pwned passwords in my database the plug did not list them at all. Does it check passwords or accounts/e-mails only?

Have a nice day!

regards p.h.

andrew-schofield commented 7 years ago

It doesn't directly check the passwords, no.

It goes through each entry and checks the domain in the URL field against the HIBP breach list.

The ability to check for duplicate/similar passwords already exists in KeePass, so I'd recommend that if the plugin detects a breached entry, use the built-in functionality to see if that password is shared with other entries.

przemhb commented 7 years ago

The problem is a user can have passwords that are widely known as insecure, despite the fact they never breached. Let's say Top5 popular password. The plugin will not tell him/her the password is not secure, which will eventually lead to false sense of security, as all passwords were checked by the plug...

I do have some passwords which HIBP has on a list, but never breached. I have created secure HIBP wrapper website, which uses javascript to hash user entered password to SHA-1 before sending it to HIBP for being checked. I guess Your plugin could check passwords in a similar way.

What do You think?

hharsunen commented 7 years ago

Would be nice feature but I think this should perhaps support only a offline operation, so user (manually) or plugin downloads the password lists from https://haveibeenpwned.com/Passwords and then you could check all your passwords against the offline list. The manual method would be sufficient enough at least for me.

It's not that haveibeenpwned.com would be unsecure but the fact that end users should not be checking their real passwords online, check out https://www.troyhunt.com/introducing-306-million-freely-downloadable-pwned-passwords/

andrew-schofield commented 7 years ago

I'll admit I wasn't aware that Troy had published that page, however as you say, submitting live passwords to it would be a bad idea anyway.

It's trivial to create SHA1 hashes of password that are stored in keepass, which could then be compared against the HIBP password lists however there are 2 technical issues that could/would cause an issue here:

  1. The HIBP API has a rate limit of 1500ms between requests, which would make live-checking quite slow if you have many passwords (300 passwords, assuming no duplicates, would take at least 7.5 minutes to check)
  2. The offline file is massive (>11GB plain text uncompressed) which itself poses issues parsing and processing (I'm assuming Troy has just got these loaded into a SQL DB which will find the results very quickly).
hharsunen commented 7 years ago

How about importing those text-files to local NoSQL db and quering that for faster search results? Or some hashlib for C# At least this for python is very fast https://gist.github.com/alexanderzobnin/09db0c9f74754d32d3c2538d4d6a3b0d

przemhb commented 7 years ago

Ad. 1. There is no need to achieve live-checking. It could be implemented as background process and take as much time as needed obeying API's limit. Please note it would be required only once - after plugin installation. Just for the first, initial scan. From then on only changed or new passwords would be checked. Ad 2. One can download such a huge file once, but he/she would have to check from time to time if the file is up to date and download it once again in case it wasn't. Very inconvenient and unnecessary.

andrew-schofield commented 7 years ago

I've just released v1.2 which adds a simple (no background checking, marking of already checked entries) version of both username and password checking.

przemhb commented 6 years ago

Thanks a lot! Works fine for me. The only thing is I am wondering if having the passwords marked as expired wouldn't be a good idea? (In addition to changing expiry date to current day).

andrew-schofield commented 6 years ago

Sorry, that's a bug. It should actually expire the entry as well.

andrew-schofield commented 6 years ago

Well, I just checked and if you have "expire breached entries" checked on the settings dialog, it does in fact set the expire flag on any breached entries.