On Thu, Mar 27, 2025 at 01:02:30AM +0530, Sudhakar Kuppusamy wrote: > From: Daniel Axtens <d...@axtens.net> > > This code allows us to parse: > > - PKCS#7 signedData messages. Only a single signerInfo is supported, > which is all that the Linux sign-file utility supports creating > out-of-the-box. Only RSA, SHA-256 and SHA-512 are supported. > Any certificate embedded in the PKCS#7 message will be ignored. > > - X.509 certificates: at least enough to verify the signatures on the > PKCS#7 messages. We expect that the certificates embedded in grub will
s/grub/GRUB/ > be leaf certificates, not CA certificates. The parser enforces this. > > - X.509 certificates support the Extended Key Usage extension and handle > it by verifying that the certificate has a Code Signing usage. Could not you split this patch to 2 or 3 logical parts? > Signed-off-by: Javier Martinez Canillas <javi...@redhat.com> # EKU support > Reported-by: Michal Suchanek <msucha...@suse.com> # key usage issue > Signed-off-by: Daniel Axtens <d...@axtens.net> > Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com> > Reviewed-by: Stefan Berger <stef...@linux.ibm.com> > Reviewed-by: Avnish Chouhan <avn...@linux.ibm.com> > --- > grub-core/commands/appendedsig/appendedsig.h | 110 +++ > grub-core/commands/appendedsig/asn1util.c | 98 ++ > grub-core/commands/appendedsig/pkcs7.c | 460 +++++++++ > grub-core/commands/appendedsig/x509.c | 953 +++++++++++++++++++ > 4 files changed, 1621 insertions(+) > create mode 100644 grub-core/commands/appendedsig/appendedsig.h > create mode 100644 grub-core/commands/appendedsig/asn1util.c > create mode 100644 grub-core/commands/appendedsig/pkcs7.c > create mode 100644 grub-core/commands/appendedsig/x509.c > > diff --git a/grub-core/commands/appendedsig/appendedsig.h > b/grub-core/commands/appendedsig/appendedsig.h > new file mode 100644 > index 000000000..3f4d7002e > --- /dev/null > +++ b/grub-core/commands/appendedsig/appendedsig.h > @@ -0,0 +1,110 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2020, 2022 Free Software Foundation, Inc. > + * Copyright (C) 2020, 2022 IBM Corporation s/2020, 2022/2020, 2022, 2025/ everywhere please... > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <grub/crypto.h> > +#include <grub/libtasn1.h> > + > +extern asn1_node _gnutls_gnutls_asn; s/_gnutls_gnutls_asn/grub_gnutls_gnutls_asn/ > +extern asn1_node _gnutls_pkix_asn; s/_gnutls_pkix_asn/grub_gnutls_pkix_asn/ > +#define MAX_OID_LEN 32 > + > +/* > + * One or more x509 certificates. > + * We do limited parsing: extracting only the serial, CN and RSA public key. > + */ > +struct x509_certificate > +{ > + struct x509_certificate *next; > + grub_uint8_t *serial; > + grub_size_t serial_len; > + char *subject; > + grub_size_t subject_len; > + /* We only support RSA public keys. This encodes [modulus, publicExponent] > */ > + gcry_mpi_t mpis[2]; > +}; > + > +/* > + * A PKCS#7 signedData signerInfo. > + */ > +struct pkcs7_signerInfo > +{ > + const gcry_md_spec_t *hash; > + gcry_mpi_t sig_mpi; > +}; > + > +/* > + * A PKCS#7 signedData message. > + * We make no attempt to match intelligently, so we don't save any info about > + * the signer. > + */ > +struct pkcs7_signedData > +{ > + int signerInfo_count; > + struct pkcs7_signerInfo *signerInfos; > +}; > + > +/* Do libtasn1 init */ > +int > +asn1_init (void); > + > +/* > + * Import a DER-encoded certificate at 'data', of size 'size'. > + * Place the results into 'results', which must be already allocated. > + */ > +grub_err_t > +parse_x509_certificate (const void *data, grub_size_t size, struct > x509_certificate *results); > + > +/* > + * Release all the storage associated with the x509 certificate. > + * If the caller dynamically allocated the certificate, it must free it. > + * The caller is also responsible for maintenance of the linked list. > + */ > +void > +certificate_release (struct x509_certificate *cert); > + > +/* > + * Parse a PKCS#7 message, which must be a signedData message. > + * The message must be in 'sigbuf' and of size 'data_size'. The result is > + * placed in 'msg', which must already be allocated. > + */ > +grub_err_t > +parse_pkcs7_signedData (const void *sigbuf, grub_size_t data_size, struct > pkcs7_signedData *msg); > + > +/* > + * Release all the storage associated with the PKCS#7 message. > + * If the caller dynamically allocated the message, it must free it. > + */ > +void > +pkcs7_signedData_release (struct pkcs7_signedData *msg); > + > +/* > + * Read a value from an ASN1 node, allocating memory to store it. > + * It will work for anything where the size libtasn1 returns is right: > + * - Integers > + * - Octet strings > + * - DER encoding of other structures > + * It will _not_ work for things where libtasn1 size requires adjustment: > + * - Strings that require an extra null byte at the end > + * - Bit strings because libtasn1 returns the length in bits, not bytes. > + * > + * If the function returns a non-NULL value, the caller must free it. > + */ > +void * > +grub_asn1_allocate_and_read (asn1_node node, const char *name, const char > *friendly_name, int *content_size); > diff --git a/grub-core/commands/appendedsig/asn1util.c > b/grub-core/commands/appendedsig/asn1util.c > new file mode 100644 > index 000000000..06c3b61c1 > --- /dev/null > +++ b/grub-core/commands/appendedsig/asn1util.c > @@ -0,0 +1,98 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2020, 2022 Free Software Foundation, Inc. > + * Copyright (C) 2020, 2022 IBM Corporation > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <grub/libtasn1.h> > +#include <grub/types.h> > +#include <grub/err.h> > +#include <grub/mm.h> > +#include <grub/crypto.h> > +#include <grub/misc.h> > +#include <grub/gcrypt/gcrypt.h> > + > +#include "appendedsig.h" > + > +asn1_node _gnutls_gnutls_asn = NULL; > +asn1_node _gnutls_pkix_asn = NULL; Global variables should be prefixed with grub_... > +extern const asn1_static_node gnutls_asn1_tab[]; > +extern const asn1_static_node pkix_asn1_tab[]; Ditto. > +/* > + * Read a value from an ASN1 node, allocating memory to store it. > + * It will work for anything where the size libtasn1 returns is right: > + * - Integers > + * - Octet strings > + * - DER encoding of other structures > + * It will _not_ work for things where libtasn1 size requires adjustment: > + * - Strings that require an extra NULL byte at the end > + * - Bit strings because libtasn1 returns the length in bits, not bytes. > + * If the function returns a non-NULL value, the caller must free it. > + */ > +void * > +grub_asn1_allocate_and_read (asn1_node node, const char *name, const char > *friendly_name, int *content_size) > +{ > + int result; > + grub_uint8_t *tmpstr = NULL; > + int tmpstr_size = 0; > + > + result = asn1_read_value (node, name, NULL, &tmpstr_size); > + if (result != ASN1_MEM_ERROR) > + { > + grub_snprintf (grub_errmsg, sizeof (grub_errmsg), > + _("Reading size of %s did not return expected status: > %s"), Please drop _(). > + friendly_name, asn1_strerror (result)); > + grub_errno = GRUB_ERR_BAD_FILE_TYPE; Why do not you use grub_error() here? > + return NULL; > + } > + > + tmpstr = grub_malloc (tmpstr_size); > + if (tmpstr == NULL) > + { > + grub_snprintf (grub_errmsg, sizeof (grub_errmsg), > + "Could not allocate memory to store %s", friendly_name); > + grub_errno = GRUB_ERR_OUT_OF_MEMORY; Ditto. > + return NULL; > + } > + > + result = asn1_read_value (node, name, tmpstr, &tmpstr_size); > + if (result != ASN1_SUCCESS) > + { > + grub_free (tmpstr); > + grub_snprintf (grub_errmsg, sizeof (grub_errmsg), "Error reading %s: > %s", > + friendly_name, asn1_strerror (result)); > + grub_errno = GRUB_ERR_BAD_FILE_TYPE; Ditto. > + return NULL; > + } > + > + *content_size = tmpstr_size; > + > + return tmpstr; > +} > + > +int > +asn1_init (void) > +{ > + int res; Missing empty line. > + res = asn1_array2tree (gnutls_asn1_tab, &_gnutls_gnutls_asn, NULL); > + if (res != ASN1_SUCCESS) > + return res; > + > + res = asn1_array2tree (pkix_asn1_tab, &_gnutls_pkix_asn, NULL); > + return res; > +} > diff --git a/grub-core/commands/appendedsig/pkcs7.c > b/grub-core/commands/appendedsig/pkcs7.c > new file mode 100644 > index 000000000..31a5eab6a > --- /dev/null > +++ b/grub-core/commands/appendedsig/pkcs7.c > @@ -0,0 +1,460 @@ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2020, 2022 Free Software Foundation, Inc. > + * Copyright (C) 2020, 2022 IBM Corporation > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include "appendedsig.h" > +#include <grub/misc.h> > +#include <grub/crypto.h> > +#include <grub/gcrypt/gcrypt.h> > +#include <sys/types.h> > + > +static char asn1_error[ASN1_MAX_ERROR_DESCRIPTION_SIZE]; > + > +/* > + * RFC 5652 s 5.1 > + */ Please put this comment in one line and s/s/section/? > +static const char *signedData_oid = "1.2.840.113549.1.7.2"; > + > +/* > + * RFC 4055 s 2.1 > + */ Ditto. Please fix similar problems everywhere... > +static const char *sha256_oid = "2.16.840.1.101.3.4.2.1"; > +static const char *sha512_oid = "2.16.840.1.101.3.4.2.3"; > + > +static grub_err_t > +process_content (grub_uint8_t *content, int size, struct pkcs7_signedData > *msg) > +{ > + int res; > + asn1_node signed_part; > + grub_err_t err = GRUB_ERR_NONE; > + char algo_oid[MAX_OID_LEN]; > + int algo_oid_size = sizeof (algo_oid); > + int algo_count; > + int signer_count; > + int i; > + char version; > + int version_size = sizeof (version); > + grub_uint8_t *result_buf; > + int result_size = 0; > + int crls_size = 0; > + gcry_error_t gcry_err; > + bool sha256_in_da, sha256_in_si, sha512_in_da, sha512_in_si; > + char *da_path; > + char *si_sig_path; > + char *si_da_path; > + > + res = asn1_create_element (_gnutls_pkix_asn, "PKIX1.pkcs-7-SignedData", > &signed_part); > + if (res != ASN1_SUCCESS) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, > + "Could not create ASN.1 structure for PKCS#7 signed > part."); All error messages in the core GRUB code start with lowercase and do not have full stop at the end. Please fix it in all patches. > + res = asn1_der_decoding2 (&signed_part, content, &size, > + ASN1_DECODE_FLAG_STRICT_DER, asn1_error); > + if (res != ASN1_SUCCESS) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, > + "Error reading PKCS#7 signed data: %s", asn1_error); > + goto cleanup_signed_part; > + } > + > + /* > + * SignedData ::= SEQUENCE { > + * version CMSVersion, > + * digestAlgorithms DigestAlgorithmIdentifiers, > + * encapContentInfo EncapsulatedContentInfo, > + * certificates [0] IMPLICIT CertificateSet OPTIONAL, > + * crls [1] IMPLICIT RevocationInfoChoices OPTIONAL, > + * signerInfos SignerInfos } > + */ > + > + /* version per the algo in 5.1, must be 1 */ Could you expand this comment? It has to be clear. > + res = asn1_read_value (signed_part, "version", &version, &version_size); > + if (res != ASN1_SUCCESS) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, "Error reading signedData > version: %s", > + asn1_strerror (res)); > + goto cleanup_signed_part; > + } > + > + if (version != 1) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, > + "Unexpected signature version v%d, only v1 > supported", version); > + goto cleanup_signed_part; > + } > + > + /* > + * digestAlgorithms DigestAlgorithmIdentifiers > + * > + * DigestAlgorithmIdentifiers ::= SET OF DigestAlgorithmIdentifier > + * DigestAlgorithmIdentifer is an X.509 AlgorithmIdentifier (10.1.1) > + * > + * RFC 4055 s 2.1: > + * sha256Identifier AlgorithmIdentifier ::= { id-sha256, NULL } > + * sha512Identifier AlgorithmIdentifier ::= { id-sha512, NULL } > + * > + * We only support 1 element in the set, and we do not check parameters > atm. > + */ > + res = asn1_number_of_elements (signed_part, "digestAlgorithms", > &algo_count); > + if (res != ASN1_SUCCESS) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, "Error counting number of > digest algorithms: %s", > + asn1_strerror (res)); > + goto cleanup_signed_part; > + } > + > + if (algo_count <= 0) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, > + "A minimum of 1 digest algorithm is required"); > + goto cleanup_signed_part; > + } > + > + if (algo_count > 2) > + { > + err = grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, > + "A maximum of 2 digest algorithms is supported"); > + goto cleanup_signed_part; > + } > + > + sha256_in_da = false; > + sha512_in_da = false; > + > + for (i = 0; i < algo_count; i++) > + { > + da_path = grub_xasprintf ("digestAlgorithms.?%d.algorithm", i + 1); > + if (!da_path) > + { > + err = grub_error (GRUB_ERR_OUT_OF_MEMORY, > + "Could not allocate path for digest algorithm " > + "parsing path"); > + goto cleanup_signed_part; > + } > + > + algo_oid_size = sizeof (algo_oid); > + res = asn1_read_value (signed_part, da_path, algo_oid, &algo_oid_size); > + if (res != ASN1_SUCCESS) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, "Error reading digest > algorithm: %s", > + asn1_strerror (res)); > + grub_free (da_path); > + goto cleanup_signed_part; > + } > + > + if (grub_strncmp (sha512_oid, algo_oid, algo_oid_size) == 0) > + { > + if (!sha512_in_da) sha512_in_da == false (for bools) please everywhere... > + sha512_in_da = true; > + else > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, > + "SHA-512 specified twice in digest algorithm > list"); > + grub_free (da_path); > + goto cleanup_signed_part; > + } > + } > + else if (grub_strncmp (sha256_oid, algo_oid, algo_oid_size) == 0) > + { > + if (!sha256_in_da) > + sha256_in_da = true; > + else > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, > + "SHA-256 specified twice in digest algorithm > list"); > + grub_free (da_path); > + goto cleanup_signed_part; > + } > + } > + else > + { > + err = grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, "Only SHA-256 and > SHA-512 hashes are supported, found OID %s", > + algo_oid); > + grub_free (da_path); > + goto cleanup_signed_part; > + } > + > + grub_free (da_path); > + } > + > + /* at this point, at least one of sha{256,512}_in_da must be true */ > + > + /* > + * We ignore the certificates, but we don't permit CRLs. > + * A CRL entry might be revoking the certificate we're using, and we have > + * no way of dealing with that at the moment. > + */ > + res = asn1_read_value (signed_part, "crls", NULL, &crls_size); > + if (res != ASN1_ELEMENT_NOT_FOUND) > + { > + err = grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, > + "PKCS#7 messages with embedded CRLs are not > supported"); > + goto cleanup_signed_part; > + } > + > + /* read the signatures */ > + > + res = asn1_number_of_elements (signed_part, "signerInfos", &signer_count); > + if (res != ASN1_SUCCESS) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, "Error counting number of > signers: %s", > + asn1_strerror (res)); > + goto cleanup_signed_part; > + } > + > + if (signer_count <= 0) > + { > + err = grub_error (GRUB_ERR_BAD_SIGNATURE, "A minimum of 1 signer is > required"); > + goto cleanup_signed_part; > + } > + > + msg->signerInfos = grub_calloc (signer_count, sizeof (struct > pkcs7_signerInfo)); > + if (!msg->signerInfos) msg->signerInfos == NULL (for pointers) everywhere please... Additionally, all (goto) labels should be prefixed with one space. Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
