mlaily / KeePass-CertificateShortcutProvider

A KeePass plugin allowing you to open your database using either a master password OR an X509 certificate.
MIT License
20 stars 6 forks source link

Move initialization to the Options menu #2

Closed frankmorgner closed 4 years ago

frankmorgner commented 4 years ago

Creating the key file should not be done during the composite key creation, this is rather confusing to the user. I've added a menu entry to create the keyfile instead.

As a sidenote: I still find the process of switching between password and smartcard rather cumbersome: The user needs to check password and uncheck key provider to use only the master password or he needs to check key provider and uncheck the password. This should work automatically without any clicks. Think of the Windows login process: If a smart card is phyisically detected, then the Smart card PIN is prompted, if no card is detected then the user password is prompted. This doesn't need any other user interaction. Would that be possible within KeePass2?

mlaily commented 4 years ago

Thanks for your PR!

I only have minor mostly style-related reservations.

For example, why would you want to explicitly support C# 6? You should probably just update your tools...

Anyway, thanks to you, I now know what secure desktop is :)

Regarding your side note, well, I agree with you it's cumbersome to use, but keep in mind this plugin abuses the design of KeePass composite keys...

I don't think it's possible to do better from within a plugin.

Ideally, KeePass would natively allow composite keys with optional parts, but I don't expect this any time soon...

frankmorgner commented 4 years ago

Feel free to remove the C#6 commit. However, I see no reason why backward compatibility is harmful.

Maybe it would be better to add some service on top of KeePass for decrypting the password. This service could then use IPC to unlock the DB instead of directly integrating into the KeePass...

mlaily commented 4 years ago

Mmh, this might work in theory, but I'm not sure it's a good idea, given the sensitive nature of the task.

I'd rather trust KeePass to do the right thing security-wise regarding its db.

mlaily commented 4 years ago

(Merged, but GitHub did not detect it properly)