[PATCH v8 00/27] Integrate MbedTLS v3.6 LTS with U-Boot

[Raymond Mao]

Hi Ilias,

On Wed, 9 Oct 2024 at 05:50, Ilias Apalodimas <ilias.apalodimas at linaro.org>
wrote:

> Hi Simon,
>
> On Wed, 9 Oct 2024 at 04:52, Simon Glass <sjg at chromium.org> wrote:
> >
> > On Thu, 3 Oct 2024 at 15:51, Raymond Mao <raymond.mao at 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