Hi Raymond,
Le 09/12/2024 à 17:06, Raymond Mao a écrit :
*This Mail comes from Outside of SoftAtHome: *Do not answer, click links or open attachments unless you recognize the sender and know the content is safe.**
Hi Philippe,
On Mon, 9 Dec 2024 at 04:42, Philippe Reynes philippe.reynes@softathome.com wrote:
Adds the support of the hmac based on sha256. This implementation is based on rfc2104. Signed-off-by: Philippe Reynes <philippe.reynes@softathome.com> --- include/u-boot/sha256.h | 4 ++++ lib/mbedtls/sha256.c | 38 ++++++++++++++++++++++++++++++++++++++ lib/sha256.c | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/include/u-boot/sha256.h b/include/u-boot/sha256.h index 44a9b528b48..2f12275b703 100644 --- a/include/u-boot/sha256.h +++ b/include/u-boot/sha256.h @@ -45,4 +45,8 @@ void sha256_finish(sha256_context * ctx, uint8_t digest[SHA256_SUM_LEN]); void sha256_csum_wd(const unsigned char *input, unsigned int ilen, unsigned char *output, unsigned int chunk_sz); +void sha256_hmac(const unsigned char *key, int keylen, + const unsigned char *input, unsigned int ilen, + unsigned char *output); + #endif /* _SHA256_H */ diff --git a/lib/mbedtls/sha256.c b/lib/mbedtls/sha256.c index 24aa58fa674..1b9fc1a8503 100644 --- a/lib/mbedtls/sha256.c +++ b/lib/mbedtls/sha256.c @@ -8,6 +8,7 @@ #ifndef USE_HOSTCC #include <cyclic.h> #endif /* USE_HOSTCC */ +#include <string.h> #include <u-boot/sha256.h> const u8 sha256_der_prefix[SHA256_DER_LEN] = { @@ -60,3 +61,40 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, sha256_finish(&ctx, output); } + +void sha256_hmac(const unsigned char *key, int keylen, + const unsigned char *input, unsigned int ilen, + unsigned char *output) +{ + int i; + sha256_context ctx; + unsigned char k_ipad[64]; + unsigned char k_opad[64]; + unsigned char tmpbuf[32]; + + memset(k_ipad, 0x36, 64); + memset(k_opad, 0x5C, 64); + + for (i = 0; i < keylen; i++) { + if (i >= 64) + break; + + k_ipad[i] ^= key[i]; + k_opad[i] ^= key[i]; + } +You are assuming that the key length is at most 64 bytes. But according to the HMAC specification: If the key is longer than the hash block size (64 bytes for SHA256), it should be hashed first to produce a shorter key. If the key is shorter than 64 bytes, it should be zero-padded to 64 bytes.
good catch, you're right, I've missed this part. I fix it in v4, and I also have added more test unit. btw: the sha1_hmac has the same issue.
+ sha256_starts(&ctx); + sha256_update(&ctx, k_ipad, sizeof(k_ipad)); + sha256_update(&ctx, input, ilen); + sha256_finish(&ctx, tmpbuf); + + sha256_starts(&ctx); + sha256_update(&ctx, k_opad, sizeof(k_opad)); + sha256_update(&ctx, tmpbuf, sizeof(tmpbuf)); + sha256_finish(&ctx, output); + + memset(k_ipad, 0, sizeof(k_ipad)); + memset(k_opad, 0, sizeof(k_opad)); + memset(tmpbuf, 0, sizeof(tmpbuf)); + memset(&ctx, 0, sizeof(sha256_context)); +} diff --git a/lib/sha256.c b/lib/sha256.c index fb195d988f1..66224c92dd9 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -300,3 +300,40 @@ void sha256_csum_wd(const unsigned char *input, unsigned int ilen, sha256_finish(&ctx, output); } + +void sha256_hmac(const unsigned char *key, int keylen, + const unsigned char *input, unsigned int ilen, + unsigned char *output) +{ + int i; + sha256_context ctx; + unsigned char k_ipad[64]; + unsigned char k_opad[64]; + unsigned char tmpbuf[32]; + + memset(k_ipad, 0x36, 64); + memset(k_opad, 0x5C, 64); + + for (i = 0; i < keylen; i++) { + if (i >= 64) + break; + + k_ipad[i] ^= key[i]; + k_opad[i] ^= key[i]; + } + + sha256_starts(&ctx); + sha256_update(&ctx, k_ipad, sizeof(k_ipad)); + sha256_update(&ctx, input, ilen); + sha256_finish(&ctx, tmpbuf); + + sha256_starts(&ctx); + sha256_update(&ctx, k_opad, sizeof(k_opad)); + sha256_update(&ctx, tmpbuf, sizeof(tmpbuf)); + sha256_finish(&ctx, output); + + memset(k_ipad, 0, sizeof(k_ipad)); + memset(k_opad, 0, sizeof(k_opad)); + memset(tmpbuf, 0, sizeof(tmpbuf)); + memset(&ctx, 0, sizeof(sha256_context)); +}I understand now we have duplicated 'shaX_csum_wd()' under lib and lib/mbedtls. That is because at the time when MbedTLS was added in, _csum_wd() is the only duplicated function and it is a trade-off comparing with creating a new common file for each algorithm. But it is not the case if the number of these duplicated functions is increasing. Do you mind moving them into a common file with the one you created? At least for sha256 with this patch I think.
In the v4, I have moved the function sha256_csum_wd() to the file sha256_common.c
I plan to do another serie for sha1 and sha512.
Regards, Raymond
-- 2.25.1
Regards, Philippe