Hi Philippe, On Fri, 6 Dec 2024 at 08:11, Philippe REYNES <philippe.rey...@softathome.com> wrote:
> Hi Raymond, > Le 05/12/2024 à 18:11, 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 Wed, 4 Dec 2024 at 12:54, Philippe Reynes < > philippe.rey...@softathome.com> wrote: > >> Adds the support of the hmac based on sha256. >> This implementation is based on rfc2104. >> >> Signed-off-by: Philippe Reynes <philippe.rey...@softathome.com> >> --- >> include/u-boot/sha256.h | 4 ++++ >> lib/mbedtls/sha256.c | 38 ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 42 insertions(+) >> >> diff --git a/include/u-boot/sha256.h b/include/u-boot/sha256.h >> index b58d5b58d39..77dbcd4553a 100644 >> --- a/include/u-boot/sha256.h >> +++ b/include/u-boot/sha256.h >> @@ -44,4 +44,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); >> + >> > Shall we have an inline function for the case if mbedtls is not enabled? > > > you're right, I should manage the case where mbedtls is not enabled. > But with you're feedback below, the function hmac is always defined (with > or without mbedtls). > > > > >> #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]; >> + } >> + >> + 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)); >> +} >> > > This function seems to be generic and should be located in lib instead of > lib/mbedtls. > > > I have looked the function sha256_csum_wd and I have seen that this > function > is defined twice, in the file lib/sha256.c and in lib/mbedtls/sha256.c. I > have done > the same to have the support of sha256_hmac with or without mbedtls. > But I think that we should avoid to duplicate code, and we should clean it. > Yes, common functions like sha256_csum_wd and sha256_hmac should be moved to a new file (e.g. sha256_common.c?) that can be used both enabling/ disabling mbedtls. Regards, Raymond