[PATCH v3 15/25] mbedtls: add X509 cert parser porting layer

[Ilias Apalodimas]

On Tue, 4 Jun 2024 at 19:05, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Hi Ilias,
>
> On Fri, 31 May 2024 at 07:42, Ilias Apalodimas <ilias.apalodimas at linaro.org> wrote:
>>
>> On Tue, 28 May 2024 at 17:15, Raymond Mao <raymond.mao at linaro.org> wrote:
>> >
>> > Add porting layer for X509 cert parser on top of MbedTLS X509
>> > library.
>> >
>> > Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
>> > ---
>> > Changes in v2
>> > - Move the porting layer to MbedTLS dir.
>> > Changes in v3
>> > - None.
>> >
>> >  lib/mbedtls/Makefile           |   1 +
>> >  lib/mbedtls/x509_cert_parser.c | 497 +++++++++++++++++++++++++++++++++
>> >  2 files changed, 498 insertions(+)
>> >  create mode 100644 lib/mbedtls/x509_cert_parser.c
>> >
>
> [snip]
>>
>> > diff --git a/lib/mbedtls/x509_cert_parser.c b/lib/mbedtls/x509_cert_parser.c
>> > new file mode 100644
>> > index 00000000000..b0867d31047
>> > --- /dev/null
>> > +++ b/lib/mbedtls/x509_cert_parser.c
>>
> [snip]
>>
>> > +static int x509_set_cert_flags(struct x509_certificate *cert)
>> > +{
>> > +       struct public_key_signature *sig = cert->sig;
>> > +
>> > +       if (!sig || !cert->pub) {
>> > +               pr_err("Signature or public key is not initialized\n");
>> > +               return -ENOPKG;
>> > +       }
>> > +
>> > +       if (!cert->pub->pkey_algo)
>> > +               cert->unsupported_key = true;
>> > +
>> > +       if (!sig->pkey_algo)
>> > +               cert->unsupported_sig = true;
>> > +
>> > +       if (!sig->hash_algo)
>> > +               cert->unsupported_sig = true;
>> > +
>> > +       /* TODO: is_hash_blacklisted()? */
>>
>> Is this supported by our current implementation?
>>
> This is not supported currently either. I just copied the TODO mark
> from legacy lib.
>
> [snip]
>>
>> > +               }
>> > +               goto out;
>> > +       }
>> > +
>> > +       pr_devel("Cert Self-signature verified");
>> > +       cert->self_signed = true;
>> > +
>> > +out:
>> > +       return ret;
>> > +
>> > +not_self_signed:
>> > +       return 0;
>> > +}
>>
>> the whole function looks like a copy of lib/crypto/x509_public_key.c.
>> Can you move all the c/p ones to a common file that the existing and
>> mbedTLS implementations can use?
>>
> Per a previous discussion with Tom, eventually we tend to keep only one
> crypto lib, that is the reason I prefer to copy/optimize a few existing
> functions into MbedTLS implementation instead of creating another
> common file.

Regardless of the implementation, the common functions should reside
in a common file which will be used regardless of mbedTLS or the
existing stack.
We do not want to fix bugs twice

Regards
/Ilias

>
> Regards,
> Raymond