Hi Raymond,
On Tue, 7 May 2024 at 20:55, Raymond Mao raymond.mao@linaro.org wrote:
Implement digest shim layer on top of MbedTLS crypto library.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Split the shim layer into separated files and use the original head files instead of creating new ones.
lib/mbedtls/Makefile | 7 +++ lib/mbedtls/md5.c | 68 ++++++++++++++++++++++++++ lib/mbedtls/sha1.c | 111 +++++++++++++++++++++++++++++++++++++++++++ lib/mbedtls/sha256.c | 65 +++++++++++++++++++++++++ lib/mbedtls/sha512.c | 96 +++++++++++++++++++++++++++++++++++++ 5 files changed, 347 insertions(+) create mode 100644 lib/mbedtls/md5.c create mode 100644 lib/mbedtls/sha1.c create mode 100644 lib/mbedtls/sha256.c create mode 100644 lib/mbedtls/sha512.c
diff --git a/lib/mbedtls/Makefile b/lib/mbedtls/Makefile index 85f0a3cfd07..b8eda9638f4 100644 --- a/lib/mbedtls/Makefile +++ b/lib/mbedtls/Makefile @@ -14,6 +14,13 @@ ccflags-y += \ -I$(src)/external/mbedtls/library \ # This line is intentionally left blank
+# shim layer for hash +obj-$(CONFIG_MBEDTLS_LIB_CRYPTO) += hash_mbedtls.o +hash_mbedtls-$(CONFIG_$(SPL_)MD5) += md5.o +hash_mbedtls-$(CONFIG_$(SPL_)SHA1) += sha1.o +hash_mbedtls-$(CONFIG_$(SPL_)SHA256) += sha256.o +hash_mbedtls-$(CONFIG_$(SPL_)SHA512) += sha512.o
obj-$(CONFIG_MBEDTLS_LIB_CRYPTO) += mbedtls_lib_crypto.o mbedtls_lib_crypto-y := \ $(MBEDTLS_LIB_DIR)/aes.o \ diff --git a/lib/mbedtls/md5.c b/lib/mbedtls/md5.c new file mode 100644 index 00000000000..2488d4f5603 --- /dev/null +++ b/lib/mbedtls/md5.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Hash shim layer on MbedTLS Crypto library
- Copyright (c) 2023 Linaro Limited
- Author: Raymond Mao raymond.mao@linaro.org
- */
+#include "compiler.h"
+#ifndef USE_HOSTCC +#include <watchdog.h> +#endif /* USE_HOSTCC */ +#include <u-boot/md5.h>
+void MD5Init(MD5Context *ctx) +{
mbedtls_md5_init(ctx);mbedtls_md5_starts(ctx);+}
+void MD5Update(MD5Context *ctx, unsigned char const *buf, unsigned int len) +{
mbedtls_md5_update(ctx, buf, len);+}
+void MD5Final(unsigned char digest[16], MD5Context *ctx) +{
mbedtls_md5_finish(ctx, digest);mbedtls_md5_free(ctx);+}
+void md5(unsigned char *input, int len, unsigned char output[16]) +{
MD5Context context;MD5Init(&context);MD5Update(&context, input, len);MD5Final(output, &context);+}
+void md5_wd(const unsigned char *input, unsigned int len,
unsigned char output[16], unsigned int chunk_sz)+{
MD5Context context;+#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
const unsigned char *end, *curr;int chunk;+#endif
MD5Init(&context);+#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
curr = input;end = input + len;while (curr < end) {chunk = end - curr;if (chunk > chunk_sz)chunk = chunk_sz;MD5Update(&context, curr, chunk);curr += chunk;schedule();}+#else
MD5Update(&context, input, len);+#endif
MD5Final(output, &context);+}
For all having functions you have a watchdog and non-watchdog variant. Doing a quick grep, only board/gdsys/a38x/hre.c uses the non-watchdog variant directly which seems wrong. Instead of defining 2 functions why don't we define the watchdog one only? We could then save enough space and make the gap smaller.
+void sha1_hmac(const unsigned char *key, int keylen,
const unsigned char *input, unsigned int ilen,unsigned char *output)+{
int i;sha1_context ctx;unsigned char k_ipad[64];unsigned char k_opad[64];unsigned char tmpbuf[20];memset(k_ipad, 0x36, 64);memset(k_opad, 0x5C, 64);
0x36 and 0x5c need a define that explains the magic values. Also, don't use lengths directly, memset(k_ipad, 0x36, sizeof(k_ipad)) is more readable.
for (i = 0; i < keylen; i++) {if (i >= 64)break;
This check needs to be outside the loop and report an error in the keylen is bigger than expected
k_ipad[i] ^= key[i];k_opad[i] ^= key[i];}sha1_starts(&ctx);sha1_update(&ctx, k_ipad, 64);
Same here don't use lengths directly
sha1_update(&ctx, input, ilen);sha1_finish(&ctx, tmpbuf);sha1_starts(&ctx);sha1_update(&ctx, k_opad, 64);sha1_update(&ctx, tmpbuf, 20);sha1_finish(&ctx, output);memset(k_ipad, 0, 64);memset(k_opad, 0, 64);memset(tmpbuf, 0, 20);
and here etc...
[...]
Thanks /Ilias