[PATCH v6 00/28] Integrate MbedTLS v3.6 LTS with U-Boot

[Raymond Mao]

Hi Simon,

On Tue, 10 Sept 2024 at 14:44, Simon Glass <sjg at chromium.org> wrote:

> Hi Raymond,
>
> On Fri, 6 Sept 2024 at 08:50, Raymond Mao <raymond.mao at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Thu, 5 Sept 2024 at 20:43, Simon Glass <sjg at chromium.org> wrote:
> >>
> >> Hi Raymond,
> >>
> >> On Tue, 3 Sept 2024 at 08:59, Raymond Mao <raymond.mao at linaro.org>
> wrote:
> >> >
> >> > Hi Simon,
> >> >
> >> > On Sat, 17 Aug 2024 at 11:58, Simon Glass <sjg at chromium.org> wrote:
> >> >>
> >> >> Hi Raymond,
> >> >>
> >> >> On Fri, 16 Aug 2024 at 15:44, 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_X509` is for replacing original X509, PKCS7, MSCode,
> ASN1,
> >> >> > and Pubkey parser with MbedTLS.
> >> >> > `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.
> >> >>
> >> >> I wonder if we could leave out the SHA stuff? The algorithms are
> >> >> stable and this would seem to avoid much of the size growth, and all
> >> >> the pain of trying to integrate another yet another hashing layer (we
> >> >> already have normal, progressive and h/w acceleration, plus
> >> >> UCLASS_HASH which h/w acceleration should use but that migration
> never
> >> >> happened). I struggle to see any benefit in replacing U-Boot's very
> >> >> solid hashing infra with something else, particularly as this series
> >> >> adds yet another. Better to invest the time to refactor it. I asked
> >> >> about this before and was told that it would happen 'later'. Let's
> >> >> just not change it at all, then it is more likely someone will sort
> it
> >> >> out.
> >> >>
> >> > Unfortunately, MbedTLS depends on its own digest layer. Unless we
> patch MbedTLS
> >> > to allow an external digest library from U-Boot ...
> >>
> >> Yes that sounds best. It looks like only a few call sites, so it
> >> should be a matter of leaving out the MbedTLS code and adding some
> >> static inlines.
> >>
> > Inspired by Ilias's reply to patch #7, though we can use the MbedTLS
> hash alternative options,
> > we still need to convert all U-Boot hash APIs to adapt to the MbedTLS
> style.
> > This will impact all callers in U-Boot and I don't think it worth to do,
> at least now.
>
> Agreed.
>
> > As the first patch set to introduce MbedTLS to U-Boot with turning on
> all necessary features,
> > I think this patch set is in the best way with an overall consideration.
>
> I am not convinced, sorry. Can you update MbedTLS so that its hash
> algo can be changed to call the U-Boot one? Then we can deal with
> hardware acceleration, driver model and avoid yet another layer of
> cruft in common/hash.c
>
> MbedTLS reserves an alternative interface for external algorithms but it
requires to
adapt all args to the ones of MbedTLS which means we have to modify all hash
APIs we have in U-boot and doesn't make too much sense.

But I decided to drop patch #7, and then we won't have any changes in
common/hash.c
from v7.

[snip]

Regards,
Raymond