SECP256K1 implementation in cecies

[navinreddy23]

Hi,

What

I am trying to modify the encrypt, decrypt and keygen modules to support SECP256K1 and AES-GCM 256, but the decryption fails.

Why

Since most of the publicly available implementations of ECIES support this curve, I wanted to give it a try.
I was able to modify the example to generate keys.

• Public key is of 65 bytes in length and private key is 32 bytes in length.
• Encryption succeeds, and all the verification of public-private pairs succeed.
• Decryption is OK until the GCM tag verification. But fails to authenticate the tag.

I did some debugging and found out that the AES key derived from HKDF is different for encrypt and decrypt.
But for the Curve448 example01, the keys are the same in encrypt and decrypt.

I was not able to figure out why I was getting a different result. Can anyone from @GlitchedPolygons help me out?

[GlitchedPolygons]

@navinreddy23

Hmm, at first glance: no clue, sorry :/

Can you maybe post your code so I can investigate it as soon as I have some time for this?

Cheers :)

[navinreddy23]

@GlitchedPolygons

As I was cleaning up the code to share it with you, it magically started working 🪄 . I might have made some key size mistakes, earlier. Thank you for your timely response!

If you would still like to review it, how may I submit the code? I tried pushing it from my local branch, but I don't have sufficient rights. Let me know if I can share it any way.

[GlitchedPolygons]

@navinreddy23 aww, I love it when things start working magically instead of stop :D

You may fork it and submit a pull request: I'll look at the diff and integrate your changes in the next release :)

[navinreddy23]

@GlitchedPolygons Sure, I will update the pull request with additional changes. I am still not able to verify against the Python ecies implementation. They've not used salt there, and the order of the ephemeral keys, tag and IV are different from cecies.

I will make the cecies compatible with the Python implementation. It will give users a way to cross-verify.

[GlitchedPolygons]

@navinreddy23 thanks! :)

Hm, a python impl. compat. was never a requirement for CECIES.

Using a random salt is always recommended.

If you contribute to CECIES I highly appreciate that :D

Please leave the existing implementations alone, and instead of replacing current behavior (which is already live in production in multiple projects), please add new, additional functions with a prefix of some sort (e.g. cecies_py_) or something like that.

I'll think of an elegant integration at a later point 😃

[navinreddy23]

@GlitchedPolygons

I made some changes to match the python implementation. But I was unable to verify against it. The symmetric key that is derived for AES-GCM is different in both the cases. I checked the IV and Tag bytes, they are identical. After the GCM decrypt is done, python throws an error saying MAC checked failed. It is obviously due to the key mismatch.

Is there a way you could help me verify it? I can provide the python implementation…

For cecies, I made a few macros to calculate offset of the input and output buffer. I have added a compile flag to disable salt usage. Could you please review #7?

PS: I have not broken any test :-P

[GlitchedPolygons]

@navinreddy23 Thanks for the PR! I need some time to look into this :)

I'll check it out soon

[navinreddy23]

@GlitchedPolygons
Thank you! Please let me know if there can be improvements made.

[GlitchedPolygons]

@navinreddy23 PR looks good at first glance, I need more time to go into detail though: I really can't find a reason for the python incompatibility other than some sort of MbedTLS-related format mismatch 🤔

Concerning the newly introduced secp256k1 curve: did you test if and how decryption of a curve448/curve25519 fails against the new secp256k1 function and vice-versa? It must not succeed, not crash, and instead fail gracefully (via error code perhaps)?

Cheers, and thanks again so much for your contribution :)

[navinreddy23]

@GlitchedPolygons,

PR looks good at first glance

Thank you!

I really can't find a reason for the python incompatibility other than some sort of MbedTLS-related format mismatch

I did some research and found out that the HKDF functionality is different, hence it generates a different symmetric key for a given pair of ephemeral-shared secret. I guess these links support the argument. #5 and Mbed-TLS/mbedtls#2335.

Concerning the newly introduced secp256k1 curve: did you test if and how decryption of a curve448/curve25519 fails against the new secp256k1 function and vice-versa?

Haven't tested this scenario. Should we update the test cases?

Cheers, and thanks again so much for your contribution :)

Happy to contribute!

[GlitchedPolygons]

@navinreddy23

I did some research and found out that the HKDF functionality is different, hence it generates a different symmetric key for a given pair of ephemeral-shared secret. I guess these links support the argument. #5 and Mbed-TLS/mbedtls#2335.

Kudos, good find!

It's too bad that Mbed-TLS/mbedtls#2335 is co-responsible for the phenomenon, because now we have two options that both suck:

• We either ignore it and proceed with the integration, which will forever remain incompatible with other implementations (e.g. the python impl. that you mentioned)
• • This is bad because if and when Feature Request: ANSI-X9.63-KDF Mbed-TLS/mbedtls#2335 is going to be addressed by the ARM MbedTLS team, their fix will be a breaking change for us, since we'd have to increase the major version yet again, and previously encrypted ciphertexts would no longer be compatible with the future fixed version
• We wait until the MbedTLS team fixes Feature Request: ANSI-X9.63-KDF Mbed-TLS/mbedtls#2335
• • This sucks because we have to wait for an undefined amount of time before we can proceed

I'm not really happy with either of those two :/

It makes me wanna leave CECIES exactly as it right now, and I'm so sorry for that. I'm sure you understand :S

Or do you have any other idea?

[navinreddy23]

@GlitchedPolygons

Not sure what way to take!

One other PR tried to implement ECIES, but I can't find it in the submodule we are using.

We wait until the MbedTLS team fixes Mbed-TLS/mbedtls#2335

I think we should wait for them to fix it. But it already has been open for 3 years now!

Shouldn't the major version be upped regardless of when mbedtls releases this feature, and we update the submodule?

Lets leave the PR as is... We can resume when mbedtls makes HKDF compatible. No problem!

[GlitchedPolygons]

I agree. Thanks for your help, and let's hope for the best 👍

[diatel]

@navinreddy23 @GlitchedPolygons FYI, hkdf seems to work okay in the latest 3.5.0 release

[GlitchedPolygons]

@navinreddy23 @GlitchedPolygons FYI, hkdf seems to work okay in the latest 3.5.0 release

Good to know! Thanks for the info. Will look into this as soon as possible then :)

[rtmtree]

Can anyone point out is there any constraint/rule when choosing the keypair?

The thing is when I use the keypair from the test file, it works.

But when I generate keypair from somewhere else and just put it in the code, it fails with sementation fails when decrypting.

It both happen in the curve25519 in main and the secp256k1 in the opening PR.

[GlitchedPolygons]

I'd like to know that too, if anybody knows more don't hesitate pls, I unfortunately don't have time to investigate any of this currently :/