KeePassWinHello security issues

[Angelelz]

@sirAndros and @shuffle-c
Technical details of the security issues found in KeePassWinHello implementation of a persistent key:

The plugin creates a persistent key using NCryptCreatePersistedKey method and sets the property NgcCacheType to 1 to make it mandatory to use Windows Hello Security Prompt to be able to use that key for decryption. After that key is created, it is used for all keepass databases to encrypt the composite key data, but the plugin never checks if that key has been replaced!

I used this console app to prove that you can replace the key and the plugin continues to use it, even if it fails to decrypt the data the first time, and then, whenever any database is closed (or locked) the plugin uses AddOrUpdate method from KeyWindowsStorage class to update the credential where the master key data is saved.

Steps to reproduce:

1. Use KeePassWinHello and enable the persistent key option. A key is created. (Optionally, leave KeePass open).
2. Execute this console app and press 'y' when asked for. It will automatically check the KeePassWinHello key property and create another one with that property set to 0.
3. Open Keepass and when trying to decrypt with KeePassWinHello it will fail due to the different key being used. (If a database was open from step 1, skip to step 5)
4. Unlock the database with the regular prompt.
5. Close keepass or lock the database.
6. Execute this console app and press 'y' when asked for. It will automatically decrypt any database with credentials saved by the plugin with no Windows Hello Prompt.

I tried to use the same key changing just the property to 0, but it is not possible after the method NCryptFinalizeKey method is executed, at least that is ok.

I'm sorry I explained everything here, I just did not want to make this public.

As for the solution, I think with just changing VerifyPersistentKeyIntegrity to check if the key has the right NgcCacheType property, and make the method AddOrUpdate use this check before saving the credentials.

Finally, I want to thank you guys for making this plugin, it is really great and I have learn a lot by studying it!

[shuffle-c]

Thank you very-very much!

You did a great research! As you can see, VerifyPersistentKeyIntegrity is intended to prevent exactly this kind of spoofing attack and this is why it's called both in Encrypt and PromptToDecrypt methods. Sadly, I did a stupid mistake when check the value of NgcCacheType property. And I didn't make enough tests for that case. I'm lucky you've found it before someone exploit it to steal passwords (I hope so).
Thank you again! :)

[Angelelz]

I was just studying your code and the way you made it work without using UWP API that are so easy to implement. It is still a puzzle to me how did you guys managed to deduce (reverse engineer?) the property needed so that the key is signed with windows hello. Can you explain that? also, what other properties exists? I've been trying to research about that with not much success. Just 2 month ago I didn't know anything about C#, Cryptography, KeePass plugins or OOP. You can actually see that in my code, I mostly use static methods (functional programming), and by studying your code I've learned a lot about instances of Objects, Properties and Methods. I will try to collaborate on this WindowsHello Library to update it with persistent key and credential storage, If you could take a look a it from time to time, I would be helpful.

Also, thanks for your kind words, they mean a lot for someone like me who's just learning to program. I actually feel like a white hat hacker! 😄

[SeppPenner]

It is still a puzzle to me how did you guys managed to deduce (reverse engineer?) the property needed so that the key is signed with windows hello. Can you explain that? also, what other properties exists? I've been trying to research about that with not much success.

Those (old) Windows native APIs are always a bit of a mystery 😆