On Tue, 4 Jun 2024 at 19:05, Raymond Mao <raymond....@linaro.org> wrote:
>
> Hi Ilias,
>
> On Fri, 31 May 2024 at 07:42, Ilias Apalodimas <ilias.apalodi...@linaro.org>
> wrote:
>>
>> On Tue, 28 May 2024 at 17:15, Raymond Mao <raymond....@linaro.org> wrote:
>> >
>> > Add porting layer for X509 cert parser on top of MbedTLS X509
>> > library.
>> >
>> > Signed-off-by: Raymond Mao <raymond....@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
