sbidy / KeePass-KeyManager

A public key manager to manage mutlible x509 certificates for a KeePass password database.
GNU General Public License v3.0
38 stars 6 forks source link

Unable to select certificate with ONLY Key Encipherment key usage (20). #11

Open mfriedemann opened 5 years ago

mfriedemann commented 5 years ago

Hey,

I am trying to use KeePass with my corporate-issued SmartCard, and after trying Bodnar's, and markbott's plugins (which I cannot make work), I tried yours.

Yours fails too, unfortunately, but at least the error message makes a bit of sense. It specifically tells me that the certificate does either not support Key Encipherment or the private key is wrong/missing. The thing is, in https://github.com/sbidy/KeePass-KeyManager/issues/4#issuecomment-403469284 you wrote that you require Key Encipherment (a0), but a0 would actually be a combination of bits.

Now, the cert on my smart card ONLY has the Key Enciperment bit [microsoft calls this Key Encipherment (20)] set, and I assume this might be why it fails. Now, from quickly looking at the MS docs, I haven't found any mention of it requiring specific other bits, so I am unsure if it might be something you can configure in your calls?

Regards, M.

mfriedemann commented 5 years ago

I've found the culprit in CertManager.DecryptMsg(). You are calling EnvelopedCms.Decrypt(X509Certificate2Collection) with a store explicitly initialized for the current user as new X509Store(StoreName.My, StoreLocation.CurrentUser). As detailed in this answer,

if you supply a certificate collection for the Decrypt() function these certificates must contain a private key. The X509Store only grants access to the public key.

I tested just the sequence of generating some random bytes, and called a modified version of DecryptMsg that doesn't open a store and just calls EnvelopedCms.Decrypt() to

search[...] the current user and computer My stores for the appropriate certificate and private key.

This made the snippet work (bytes post and pre encryption are identical).

Maybe you could investigate further if that change has any security implications (I find those C# docs horrible and severly lacking in essential details).

Regards, M.

sbidy commented 5 years ago

Hey, thanks for the research - yes that can be a possible solution. I'll check the EnvelopedCms.Decrypt() to envelope the key. I'll try to release a modified testing version today.

sbidy commented 5 years ago

I've drafted a release for that - after short tests it looks like a possible fix for that problem. Please load the V1.4b from the releases and test again.

mfriedemann commented 5 years ago

I checked that 1.4b and it doesn't work, unfortunately. BUT the strange thing is that even with my small test, it fails more often than it works (it DOES work intermittently, though, and I am sure I use the right pin). I am investigating a bit but it doesn't make any sense. Tried re-inserting the SmartCard to no avail, managed to have my test work a few times, but not consistently.

mfriedemann commented 5 years ago

After taking a peek at the source of EnvelopeCms, I... seriously doubt it is fit for public use. Their "API" seems a mess, the docs are unclear, not to say non-existent, and of course the error handling is poor, as witnessed by the useless exception message/stacktrace Yours Truly is presented with (CrypthographyException "an internal error occured", no inner exception). Anyway, apparently your way of calling it should only add extra certificates that should (hah!) not do any harm (it asks for the PIN in that 80's-style-popup-dialog no matter how it is invoked). The best thing, of course, is that it worked for the first (and second) test I made with the stripped down version, and since then it only worked 2 or 3 more times, without doing anything different. I think I'll investigate BouncyCastle's C# version, worked with the java one in the past and at least that works reliably.

Regards, M.

mfriedemann commented 5 years ago

Success!

After poking some more, it appears to work when changing the encryption algorithm to AES256-CBC OID "2.16.840.1.101.3.4.1.42" (default is DES-EDE3-CBC OID "1.2.840.113549.3.7"). It also works for both variants (with and without the argument to EnvelopedCms.Decrypt()).

At least I am able to encrypt some random bytes and decrypt them to arrive at the same random bytes. If I re-encrypt the decrypted bytes (like you do for generating the KeePass key), the result differs from the first encrypted data (in contrast to when it worked before the few times it did, both ciphertexts where identical). Decrypting this still results in the original random bytes, though, so it may be a consequence of the encryption algorithm, padding or whatever and not an actual issue for this use case.

Regards, M.

sbidy commented 5 years ago

You tried to encrypt random bytes (like the "key" i generate in the plugin) with different cypher modes. So I understand that you can encrypt and decrypt the random bytes with the AES algorithm but not with the default 3DES which is defined as default at the EnvelopedCms class. As in the following post mentioned that a change of the default algorithm is not a good idea if the key is on a smart card - Decrypting EnvelopedCms with non-default AlgorithmIdentifier

So I'v to check what I can change in the code to provide a stable de- and encryption of the random key.

mfriedemann commented 5 years ago

As I said, the EnvelopedCms class appears unfinished and behaves strangely (not to mention the terrible documentation). In contrast to the post you linked (which I had a found as well) for me the problem is that it does NOT work with the default (DES) and the SmartCard (well, it did work about 5 times but I can't reproduce it) but with AES it DOES work.

I tried various other suggestions and none of them made a difference. FWIW, if I set it to AES, it uses OAEP padding (v2 padding), vs. PKCS#1 1.5 padding when the DES default is used. This post suggests one can influence the internal behavior, but it makes no difference and it fails to work either way.

Apart from success in my reduced test, I now compiled the plugin with the algorithm OID set to AES and it works fine, with the SmartCard. Regards, M.

sbidy commented 5 years ago

Thank you for the clarification! 😉 So in my test I can't reproduce this behavior. But I still testing around with the usage of the AES encryption with CBC if nothing breaks here I'll implement this as default cypher mode for the plugin - to avoid problems with other SCs I'll also implement these as configurable option.