[PATCH v4 15/29] x509: move common functions to x509 helper

[Ilias Apalodimas]

On Tue, 2 Jul 2024 at 21:29, Raymond Mao <raymond.mao at linaro.org> wrote:
>
> Move x509_check_for_self_signed as a common helper function
> that can be shared by legacy crypto lib and MbedTLS implementation.
>
> Signed-off-by: Raymond Mao <raymond.mao at linaro.org>
> ---
> Changes in v4
> - Initial patch.
>
>  lib/crypto/Makefile          |  1 +
>  lib/crypto/x509_helper.c     | 67 ++++++++++++++++++++++++++++++++++++
>  lib/crypto/x509_public_key.c | 56 +-----------------------------
>  3 files changed, 69 insertions(+), 55 deletions(-)
>  create mode 100644 lib/crypto/x509_helper.c
>
> diff --git a/lib/crypto/Makefile b/lib/crypto/Makefile
> index 4ad1849040d..946cc3a7b59 100644
> --- a/lib/crypto/Makefile
> +++ b/lib/crypto/Makefile
> @@ -37,6 +37,7 @@ x509_key_parser-y := \
>         x509.asn1.o \
>         x509_akid.asn1.o \
>         x509_cert_parser.o \
> +       x509_helper.o \
>         x509_public_key.o
>
>  $(obj)/x509_cert_parser.o: \
> diff --git a/lib/crypto/x509_helper.c b/lib/crypto/x509_helper.c
> new file mode 100644
> index 00000000000..d0c80907ec3
> --- /dev/null
> +++ b/lib/crypto/x509_helper.c
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * X509 helper functions
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells at redhat.com)
> + *
> + * Copyright (c) 2024 Linaro Limited
> + * Author: Raymond Mao <raymond.mao at linaro.org>

Same comment with your other patches moving code around. You haven't
contributed a substantial amount of code here to justify the copyright
and authorship


> + */
> +#include <linux/err.h>
> +#include <crypto/public_key.h>
> +#include <crypto/x509_parser.h>
> +
> +/*
> + * Check for self-signedness in an X.509 cert and if found, check the signature
> + * immediately if we can.
> + */
> +int x509_check_for_self_signed(struct x509_certificate *cert)
> +{
> +       int ret = 0;
> +
> +       if (cert->raw_subject_size != cert->raw_issuer_size ||
> +           memcmp(cert->raw_subject, cert->raw_issuer,
> +                  cert->raw_issuer_size))
> +               goto not_self_signed;
> +
> +       if (cert->sig->auth_ids[0] || cert->sig->auth_ids[1]) {
> +               /*
> +                * If the AKID is present it may have one or two parts. If
> +                * both are supplied, both must match.
> +                */
> +               bool a = asymmetric_key_id_same(cert->skid,
> +                                               cert->sig->auth_ids[1]);
> +               bool b = asymmetric_key_id_same(cert->id,
> +                                               cert->sig->auth_ids[0]);
> +
> +               if (!a && !b)
> +                       goto not_self_signed;
> +
> +               ret = -EKEYREJECTED;
> +               if (((a && !b) || (b && !a)) &&
> +                   cert->sig->auth_ids[0] && cert->sig->auth_ids[1])
> +                       goto out;
> +       }
> +
> +       ret = -EKEYREJECTED;
> +       if (strcmp(cert->pub->pkey_algo, cert->sig->pkey_algo))
> +               goto out;
> +
> +       ret = public_key_verify_signature(cert->pub, cert->sig);
> +       if (ret == -ENOPKG) {
> +               cert->unsupported_sig = true;
> +               goto not_self_signed;
> +       }
> +       if (ret < 0)
> +               goto out;
> +
> +       pr_devel("Cert Self-signature verified");
> +       cert->self_signed = true;
> +
> +out:
> +       return ret;
> +
> +not_self_signed:
> +       return 0;
> +}
> diff --git a/lib/crypto/x509_public_key.c b/lib/crypto/x509_public_key.c
> index a10145a7cdc..4ba13c1adc3 100644
> --- a/lib/crypto/x509_public_key.c
> +++ b/lib/crypto/x509_public_key.c
> @@ -139,61 +139,7 @@ error:
>         return ret;
>  }
>
> -/*
> - * Check for self-signedness in an X.509 cert and if found, check the signature
> - * immediately if we can.
> - */
> -int x509_check_for_self_signed(struct x509_certificate *cert)
> -{
> -       int ret = 0;
> -
> -       pr_devel("==>%s()\n", __func__);
> -
> -       if (cert->raw_subject_size != cert->raw_issuer_size ||
> -           memcmp(cert->raw_subject, cert->raw_issuer,
> -                  cert->raw_issuer_size) != 0)
> -               goto not_self_signed;
> -
> -       if (cert->sig->auth_ids[0] || cert->sig->auth_ids[1]) {
> -               /* If the AKID is present it may have one or two parts.  If
> -                * both are supplied, both must match.
> -                */
> -               bool a = asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]);
> -               bool b = asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0]);
> -
> -               if (!a && !b)
> -                       goto not_self_signed;
> -
> -               ret = -EKEYREJECTED;
> -               if (((a && !b) || (b && !a)) &&
> -                   cert->sig->auth_ids[0] && cert->sig->auth_ids[1])
> -                       goto out;
> -       }
> -
> -       ret = -EKEYREJECTED;
> -       if (strcmp(cert->pub->pkey_algo, cert->sig->pkey_algo) != 0)
> -               goto out;
> -
> -       ret = public_key_verify_signature(cert->pub, cert->sig);
> -       if (ret < 0) {
> -               if (ret == -ENOPKG) {
> -                       cert->unsupported_sig = true;
> -                       ret = 0;
> -               }
> -               goto out;
> -       }
> -
> -       pr_devel("Cert Self-signature verified");
> -       cert->self_signed = true;
> -
> -out:
> -       pr_devel("<==%s() = %d\n", __func__, ret);
> -       return ret;
> -
> -not_self_signed:
> -       pr_devel("<==%s() = 0 [not]\n", __func__);
> -       return 0;
> -}
> +#endif /* !CONFIG_IS_ENABLED(MBEDTLS_LIB_X509) */
>
>  #ifndef __UBOOT__
>  /*
> --
> 2.25.1
>

with the above fixed
Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>