Yubico / yubihsm-shell

yubihsm-shell and libyubihsm
https://developers.yubico.com/yubihsm-shell/
Apache License 2.0
90 stars 54 forks source link

New Questions #258

Closed Fplyth0ner-Combie closed 2 years ago

Fplyth0ner-Combie commented 2 years ago

I noticed that your code for generating shared secrets didn't work properly, in BCRYPT_ECDSA_PUBLIC_P256_MAGIC and BCryptImportKeyPair. There is no problem importing the private key, but if the public key comes from a third party, you must use BCRYPT_NO_KEY_VALIDATION to succeed. At first, I thought BCRYPT_NO_KEY_VALIDATION was only required for third-party public key imports, but it turns out that BCRYPT_NO_KEY_VALIDATION is still required even for public key imports generated from the current private key. I don't understand.

Fplyth0ner-Combie commented 2 years ago

Also, BCRYPT_ECDSA_P256_ALGORITHM doesn't seem to work properly in BCryptSecretAgreement, I'm very upset...

qpernil commented 2 years ago

Could you elaborate on what Windows version this is occurring ? This code has worked in my tests on Windows 10.

qpernil commented 2 years ago

The reason we use BCRYPT_NO_KEY_VALIDATION is for cases where we don't have a public key, so we just set it to all-zeroes. It turns out that Windows will ignore the public key anyway with that flag (and if memory serves possibly even without that flag). We even use this fact to calculate the public key, by importing a key pair with the public key as all-zeroes, end then export the public key. (For EC keys you can always calculate the public key from the private, but Windows has no direct API support for that, so this is a bit of a trick to achieve that). Please note that our api assumes uncompressed points (65 bytes, the first one always a 4) while the blob format doesn't include the first byte, and hence expects 64 bytes.

Fplyth0ner-Combie commented 2 years ago

My test results: When using BCryptImportKeyPair to Import the ECDH public key (BCRYPT_ECCKEY_BLOB), if the public key does not match private key, it fails. I messed up a few things, maybe.

Fplyth0ner-Combie commented 2 years ago

I can understand public key, but not private key.

Fplyth0ner-Combie commented 2 years ago

In fact, I'm working on a job: using python's ecdsa library to generate two ECDH public-private key pairs, the server keeps one private key with the other public key, and the client gets the rest (PEM). Then I had trouble on Windows, I tried to convert public and private keys in PEM format to CERT_PUBLIC_KEY_INFO and CRYPT_ECC_PRIVATE_KEY_INFO using CryptStringToBinaryA and CryptDecodeObjectEx. It is then converted to BCRYPT_ECCKEY_BLOB (containing a mismatched pair of public and private keys). Then get the respective BCRYPT_KEY_HANDLE by calling BCryptImportKeyPair (disregarding the discussion above), and use BCryptSecretAgreement. Incredibly. No matter which combination of public and private key pairs I use, the shared secret generated on Windows is always different. This is impossible. I'm mad.

Fplyth0ner-Combie commented 2 years ago

I'm still looking for the source of the problem, very hard.

Fplyth0ner-Combie commented 2 years ago

secp256k1

qpernil commented 2 years ago

It's hard to tell exactly what you mean by just these descriptions, would you be able to provide some sample code ? And it would be great to know what Windows version you are using, as it would not surprise me if different versions behave differently. Thx.

Fplyth0ner-Combie commented 2 years ago

You are right, I did mess up something.... The reason for my doubt comes from my wrong use of SECP256k1 in BCRYPT_ECDH_P256_ALGORITHM (SECP256r1, NIST P256). Now, my problem is solved, I learned a lot from your project, thank you for your help!

Fplyth0ner-Combie commented 2 years ago

Hi. I have some other questions I'd like to ask you. Does the import of elliptic curve BLOBs be supported for Windows NT versions prior to 10.0? use bcryptimportkeypair

Fplyth0ner-Combie commented 2 years ago

Hi. I have some other questions I'd like to ask you. Does the import of elliptic curve BLOBs be supported for Windows NT versions prior to 10.0? use bcryptimportkeypair

I Solved It..... It is amazing.... I did some tests on Windows 10 and Windows 8.1 hhh

Fuck Microsoft....

qpernil commented 2 years ago

Again, it's very difficult to say anything without sample code. Can you provide ?

Fplyth0ner-Combie commented 2 years ago

What I'm describing is the following code snippet

https://github.com/Yubico/yubihsm-shell/blob/a8b888f351eea1d92c64d5790e75188a63addf5a/common/ecdh.c#L122

...... memset(buf + sizeof(BCRYPT_ECCKEY_BLOB), 0, 2 * cb_privkey); memcpy(buf + sizeof(BCRYPT_ECCKEY_BLOB) + 2 * cb_privkey, privkey, cb_privkey); BCRYPT_KEY_HANDLE priv; NTSTATUS status = BCryptImportKeyPair(curves[curve], NULL, BCRYPT_ECCPRIVATE_BLOB, &priv, buf, sizeof(BCRYPT_ECCKEY_BLOB) + 3 * cb_privkey, BCRYPT_NO_KEY_VALIDATION); if (BCRYPT_SUCCESS(status)) { blob->dwMagic = BCRYPT_ECDH_PUBLIC_P256_MAGIC; blob->cbKey = cb_privkey; memcpy(buf + sizeof(BCRYPT_ECCKEY_BLOB), pubkey + 1, cb_pubkey - 1); BCRYPT_KEY_HANDLE pub; status = BCryptImportKeyPair(curves[curve], NULL, BCRYPT_ECCPUBLIC_BLOB, &pub, buf, sizeof(BCRYPT_ECCKEY_BLOB) + 2 * cb_privkey, 0); ......

Your amazing implementation here, I have verified this many times on different versions of Windows.

Fplyth0ner-Combie commented 2 years ago

I haven't seen this usage documented anywhere, but it works. If I follow MSDN completely, I may not be able to solve it, thank you.

qpernil commented 2 years ago

So did the code in ecdh.c not work on some Windows version ? Im a little confused what worked and what didn't. Thx.

qpernil commented 2 years ago

Also to explain the code in ecdh.c - ecdh_calculate_secret; It imports a private key blob with a zeroed-out public part as a BCRYPT_ECCPRIVATE_BLOB to get a handle for it's own private key. By setting BCRYPT_NO_KEY_VALIDATION we get it to ignore the public part. (I seem to recall it did anyway, i.e. without that flag, but better safe than sorry). This can work because the public part can always be calculated from the private part, so it strictly doesn't need the public key, other than possibly to check validity. Then it imports the other party's public key as a BCRYPT_ECCPUBLIC_BLOB to get a handle for the public key to use. Then it performs the ecdh calculation and extracts the raw secret value. The two blob types have different contents. See documentation for the layout for each of them. They are both defined in terms of the same fixed header followed by different variable-length fields (so they can't really be described as structs in C).

Fplyth0ner-Combie commented 2 years ago

Yes!!

It imports a private key blob _with a zeroed-out public part as a BCRYPT_ECCPRIVATE_BLOB_ to get a handle for it's own private key. By setting BCRYPT_NO_KEY_VALIDATION we get it to ignore the public part.

The key part that annoys me is the marked sentence.

It works fine if you fill it with ZERO; otherwise, it only works on Windows 10.

qpernil commented 2 years ago

So the code as-is works ? On what OS:es have you tried it ? And does that mean we can close this issue ? Also, out of curiosity, could you provide a code snippet for how you did it when it didn't work ?

qpernil commented 2 years ago

I'm closing this issue now as problem seems to be solved