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
