On Tue, 2 Jul 2024 at 21:29, Raymond Mao <raymond....@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....@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 (dhowe...@redhat.com)
> + *
> + * Copyright (c) 2024 Linaro Limited
> + * Author: Raymond Mao <raymond....@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.apalodi...@linaro.org>
