Hi Ilias, On Fri, 30 Aug 2024 at 03:37, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Simon, > > On Thu, 29 Aug 2024 at 18:01, Simon Glass <s...@chromium.org> wrote: > > > > Hi Raymond, > > > > On Fri, 16 Aug 2024 at 15:47, Raymond Mao <raymond....@linaro.org> wrote: > > > > > > Integrate common/hash.c on the hash shim layer so that hash APIs > > > from mbedtls can be leveraged by boot/image and efi_loader. > > > > > > Signed-off-by: Raymond Mao <raymond....@linaro.org> > > > --- > > > Changes in v2 > > > - Use the original head files instead of creating new ones. > > > Changes in v3 > > > - Add handle checkers for malloc. > > > Changes in v4 > > > - None. > > > Changes in v5 > > > - Add __maybe_unused to solve linker errors in some platforms. > > > - replace malloc with calloc. > > > Changes in v6 > > > - None. > > > > > > common/hash.c | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 146 insertions(+) > > > > I am not seeing the benefit of replacing U-Boot's hashing algorithms. > > They work well and don't change. Also it seems to be making the code a > > lot uglier, with an uncertain timeline for clean-up. > > A lot uglier where? It adds a few wrappers that fit into the current > design and callbacks. > I don't think what you are asking is possible. To do assymetric > crypto, signatures etc -- and in the future add TLS support in wget > mbedTLS relies on its internal hashing functions for the cipher suites > it supports. So what you are asking would just make the code even > larger. Raymond can you please double check?
It's really just a case of dropping the hash calls. It should not cause any other problems, so far as I can see, but I have not dug in in detail. Re TLS is relying on its internal hashing functions, is this what you are talking about? $ git grep mbedtls_sha1_free common/hash.c: mbedtls_sha1_free(ctx); common/hash.c: mbedtls_sha1_free((mbedtls_sha1_context *)ctx); lib/mbedtls/external/mbedtls/include/mbedtls/sha1.h:void mbedtls_sha1_free(mbedtls_sha1_context *ctx); lib/mbedtls/external/mbedtls/library/md.c: mbedtls_sha1_free(ctx->md_ctx); lib/mbedtls/external/mbedtls/library/psa_crypto_hash.c: mbedtls_sha1_free(&operation->ctx.sha1); lib/mbedtls/external/mbedtls/library/sha1.c:void mbedtls_sha1_free(mbedtls_sha1_context *ctx) lib/mbedtls/external/mbedtls/library/sha1.c: mbedtls_sha1_free(ctx); lib/mbedtls/external/mbedtls/library/sha1.c: mbedtls_sha1_free(&ctx); lib/mbedtls/external/mbedtls/library/sha1.c: mbedtls_sha1_free(&ctx); lib/mbedtls/sha1.c: mbedtls_sha1_free(ctx); I see this in psa_crypto_hash.c (not sure what that is though). > > Can you do the rest of the integration first? I believe this is the best approach. We need to permit using crypto acceleration too (via driver model), which is obviously impossible if mbed algorithms are using built-in hashing. The biggest challenge here is that common/hash.c needs some love, as I mentioned in an earlier version. Regards, Simon
