PruteanuVlad opened a new pull request, #18138: URL: https://github.com/apache/nuttx/pull/18138
Hi all, I discovered a bug within the HMAC-SHA implementation, here is my proposed fix. However, I have a couple of questions that need some clarification, apologies if this isn't the proper channel. ## Summary Using HMAC-SHA with longer keys results in a failed test. This can be seen using the changes I made in [this PR](https://github.com/apache/nuttx-apps/pull/3376) The changes from the PR are also used to validate my fix. The issue stems from the code explicitly refusing keys larger than `auth_hash.keysize`. In reality, the RFC standard doesn't mention any key size limit. Instead, if a key is larger than the algorithm's block size, the key should first be hashed before using. ([2.Definition of HMAC](https://datatracker.ietf.org/doc/html/rfc2104#section-2)) The fix is fairly straightforward, it adds a key context (kctx) to be used along ictx and octx. When needed, the keys are hashed first. Updated values for `auth_hash.keysize` are also included. Modified files: crypto/cryptosoft.c - adds operation for hashing keys that are too long include/crypto/cryptosoft.h - adds kctx field crypto/xform.c - fixes keysize ## Impact The bug affects the software implementation as well as the hardware ESP32 implementation. I haven't checked the other hardware implementations. Currently, the PR only fixes the software implementation, although I plan to update it with the fix for ESP32 as well. Here, the first question appears: **As far as I can tell, the fix for ESP32 will be identical with the software one. Since I also have the hardware available I will also test the fix. If there are other platforms affected by this, and if the fix for them would be the same, should I also port the fix there, even if I can't test it on real hardware?** My second question is: **When searching for loose ends, I stumbled across [this file](https://github.com/apache/nuttx/blob/master/crypto/hmac.c). Best I can tell the functions inside aren't used anywhere? The init functions appear to be processing this edge case correctly, but as I've said, they aren't used. So, what to do about this file?** ## Testing Development was done using ESP32 DevkitC. Building was done on Ubuntu 24.04 VM. For testing, I ran the official crypto HMAC test, to which I added cases for this specific case from the RFC doc. ``` nsh> hmac hmac md5 success hmac md5 success hmac md5 success hmac md5 success hmac md5 success hmac sha1 success hmac sha1 success hmac sha1 success hmac sha1 success hmac sha1 success hmac sha256 success hmac sha256 success hmac sha256 success hmac sha256 success hmac sha256 success ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
