Hi Ilias, On Wed, 9 Oct 2024 at 05:50, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote:
> Hi Simon, > > On Wed, 9 Oct 2024 at 04:52, Simon Glass <s...@chromium.org> wrote: > > > > On Thu, 3 Oct 2024 at 15:51, Raymond Mao <raymond....@linaro.org> wrote: > > > > > > Integrate MbedTLS v3.6 LTS (currently v3.6.0) with U-Boot. > > > > > > Motivations: > > > ------------ > > > > > > 1. MbedTLS is well maintained with LTS versions. > > > 2. LWIP is integrated with MbedTLS and easily to enable HTTPS. > > > 3. MbedTLS recently switched license back to GPLv2. > > > > > > Prerequisite: > > > ------------- > > > > > > This patch series requires mbedtls git repo to be added as a > > > subtree to the main U-Boot repo via: > > > $ git subtree add --prefix lib/mbedtls/external/mbedtls \ > > > https://github.com/Mbed-TLS/mbedtls.git \ > > > v3.6.0 --squash > > > Moreover, due to the Windows-style files from mbedtls git repo, > > > we need to convert the CRLF endings to LF and do a commit manually: > > > $ git add --renormalize . > > > $ git commit > > > > > > New Kconfig options: > > > -------------------- > > > > > > `MBEDTLS_LIB` is for MbedTLS general switch. > > > `MBEDTLS_LIB_CRYPTO` is for replacing original digest and crypto libs > with > > > MbedTLS. > > > `MBEDTLS_LIB_CRYPTO_ALT` is for using original U-Boot crypto libs as > > > MbedTLS crypto alternatives. > > > `MBEDTLS_LIB_X509` is for replacing original X509, PKCS7, MSCode, ASN1, > > > and Pubkey parser with MbedTLS. > > > By default `MBEDTLS_LIB_CRYPTO_ALT` and `MBEDTLS_LIB_X509` are selected > > > when `MBEDTLS_LIB` is enabled. > > > `LEGACY_CRYPTO` is introduced as a main switch for legacy crypto > library. > > > `LEGACY_CRYPTO_BASIC` is for the basic crypto functionalities and > > > `LEGACY_CRYPTO_CERT` is for the certificate related functionalities. > > > For each of the algorithm, a pair of `<alg>_LEGACY` and `<alg>_MBEDTLS` > > > Kconfig options are introduced. Meanwhile, `SPL_` Kconfig options are > > > introduced. > > > > > > In this patch set, MBEDTLS_LIB, MBEDTLS_LIB_CRYPTO and MBEDTLS_LIB_X509 > > > are by default enabled in qemu_arm64_defconfig and sandbox_defconfig > > > for testing purpose. > > > > > > Patches for external MbedTLS project: > > > ------------------------------------- > > > > > > Since U-Boot uses Microsoft Authentication Code to verify PE/COFFs > > > executables which is not supported by MbedTLS at the moment, > > > addtional patches for MbedTLS are created to adapt with the EFI loader: > > > 1. Decoding of Microsoft Authentication Code. > > > 2. Decoding of PKCS#9 Authenticate Attributes. > > > 3. Extending MbedTLS PKCS#7 lib to support multiple signer's > certificates. > > > 4. MbedTLS native test suites for PKCS#7 signer's info. > > > > > > All above 4 patches (tagged with `mbedtls/external`) are submitted to > > > MbedTLS project and being reviewed, eventually they should be part of > > > MbedTLS LTS release. > > > But before that, please merge them into U-Boot, otherwise the building > > > will be broken when MBEDTLS_LIB_X509 is enabled. > > > > > > See below PR link for the reference: > > > https://github.com/Mbed-TLS/mbedtls/pull/9001 > > > > > > Miscellaneous: > > > -------------- > > > > > > Optimized MbedTLS library size by tailoring the config file > > > and disabling all unnecessary features for EFI loader. > > > From v2, original libs (rsa, asn1_decoder, rsa_helper, md5, sha1, > sha256, > > > sha512) are completely replaced when MbedTLS is enabled. > > > From v3, the size-growth is slightly reduced by refactoring Hash > functions. > > > From v6, smaller implementations for SHA256 and SHA512 are enabled and > > > target size reduce significantly. > > > Target(QEMU arm64) size-growth when enabling MbedTLS: > > > v1: 6.03% > > > v2: 4.66% > > > v3 - v5: 4.55% > > > v6: 2.90% > > > > > > Please see the latest output from buildman for size-growth on QEMU > arm64, > > > Sandbox and Nanopi A64. [1] > > > > > > Tests done: > > > ----------- > > > > > > EFI Secure Boot test (EFI variables loading and verifying, EFI signed > image > > > verifying and booting) via U-Boot console. > > > EFI Secure Boot and Capsule sandbox test passed. > > > > > > Known issues: > > > ------------- > > > > > > None. > > > > > > [1]: buildman output for size comparison (With both `MBEDTLS_LIB` and > > > `MBEDTLS_LIB_CRYPTO` selected) > > > (qemu_arm64, sandbox and nanopi_a64) > > > ``` > > > aarch64: (for 2/2 boards) all -1568.0 bss -8.0 data -64.0 rodata > +200.0 text -1696.0 > > > qemu_arm64 : all +4472 bss -16 data -64 rodata +200 > text +4352 > > > u-boot: add: 29/-14, grow: 6/-13 bytes: 12812/-8084 > (4728) > > > function old > new delta > > > mbedtls_internal_sha1_process - > 4540 +4540 > > > > I am not going to review this version as others are on top of this. It > > looks reasonable to me. We do need to tidy up the hashing in > > common/hash.c at some point but this series doesn't add to the pain > > there. > > I don't have time to review those things in depth. OTOH we have enough > testing on the CI to make sure cryptography is working and I do like > the state of the patches as well. > > The current code leave mbedTLS hashing algorithms as a choice, but set > the existing hashing algos as the default, since they are smaller and > work with offloading. Since using new algorithms from mbedTLS might > have valid use cases, I suggest we pull this and clean up the hash > subsystem, to include all 4 options > - mbedTLS in sw > - U-Boot hashes in sw > - Mix of mbedTLS & hardware offloading > - Mix of U-Boot hashing & hardware offloading (already works) > > The patch set has these already with combinations of kconfigs: - mbedTLS in sw (set CONFIG_MBEDTLS_LIB=y && MBEDTLS_LIB_CRYPTO=y, tested via github CI) - U-Boot hashes in sw (default when setting CONFIG_MBEDTLS_LIB=y only, tested via github CI) - Mix of mbedTLS & hardware offloading (I didn't test this but it should work as it was when SHA_HW_ACCEL=y) Regards, Raymond