On Mon, Sep 22, 2025 at 02:57:54PM +0530, Sudhakar Kuppusamy wrote: > If secure boot is enabled with static key management mode, the trusted > certificates will be extracted from the GRUB ELF Note and added to db list. > > If secure boot is enabled with dynamic key management mode, the trusted > certificates and certificate/binary hash will be extracted from the PKS > and added to db list. The distrusted certificates, certificate/binary hash > are read from the PKS and added to dbx list. Both dbx and db lists are > introduced by > a subsequent patch.
I think you mean "Both dbx and db lists usage is added by a subsequent patch"... > Note:- s/-// > If the certificate or the certificate hash exists in the dbx list, then > do not add that certificate/certificate hash to the db list. > > Signed-off-by: Sudhakar Kuppusamy <[email protected]> > Reviewed-by: Stefan Berger <[email protected]> > Reviewed-by: Avnish Chouhan <[email protected]> > --- > grub-core/commands/appendedsig/appendedsig.c | 398 ++++++++++++++++++- > include/grub/efi/pks.h | 112 ++++++ > include/grub/types.h | 4 + > 3 files changed, 497 insertions(+), 17 deletions(-) > create mode 100644 include/grub/efi/pks.h > > diff --git a/grub-core/commands/appendedsig/appendedsig.c > b/grub-core/commands/appendedsig/appendedsig.c > index 30d453007..dd820b35a 100644 > --- a/grub-core/commands/appendedsig/appendedsig.c > +++ b/grub-core/commands/appendedsig/appendedsig.c > @@ -34,6 +34,7 @@ > #include <grub/env.h> > #include <grub/lockdown.h> > #include <grub/powerpc/ieee1275/platform_keystore.h> > +#include <grub/efi/pks.h> > > #include "appendedsig.h" > > @@ -46,6 +47,11 @@ GRUB_MOD_LICENSE ("GPLv3+"); > #define SIG_MAGIC "~Module signature appended~\n" > #define SIG_MAGIC_SIZE ((sizeof(SIG_MAGIC) - 1)) > > +/* SHA256, SHA384 and SHA512 hash sizes. */ > +#define SHA256_HASH_LEN 32 > +#define SHA384_HASH_LEN 48 > +#define SHA512_HASH_LEN 64 s/_LEN/_SIZE to be consistent with the rest of the code... > /* > * This structure is extracted from scripts/sign-file.c in the linux kernel > * source. It was licensed as LGPLv2.1+, which is GPLv3+ compatible. > @@ -79,11 +85,22 @@ struct sb_database > { > grub_x509_cert_t *certs; /* Certificates. */ > grub_uint32_t cert_entries; /* Number of certificates. */ > + grub_uint8_t **hashes; /* Certificate/binary hashes. */ > + grub_size_t *hash_sizes; /* Size of certificate/binary hashes. */ s/Size of/Sizes of/ > + grub_uint32_t hash_entries; /* Number of certificate/binary hashes. */ > + bool is_db; /* Flag to indicate the db/dbx list. */ > }; > typedef struct sb_database sb_database_t; > > /* The db list is used to validate appended signatures. */ > -static sb_database_t db = {.certs = NULL, .cert_entries = 0}; > +static sb_database_t db = {.certs = NULL, .cert_entries = 0, .hashes = NULL, > + .hash_sizes = NULL, .hash_entries = 0, .is_db = > true}; > +/* > + * The dbx list is used to ensure that the distrusted certificates or GRUB > modules/ > + * kernel binaries are rejected during appended signatures/hashes validation. > + */ > +static sb_database_t dbx = {.certs = NULL, .cert_entries = 0, .hashes = NULL, > + .hash_sizes = NULL, .hash_entries = 0, .is_db = > false}; > > /* > * Signature verification flag (check_sigs). > @@ -118,6 +135,172 @@ static struct grub_fs pseudo_fs = { > .fs_read = pseudo_read > }; > > +/* > + * GUID can be used to determine the hashing function and > + * generate the hash using determined hashing function. > + */ > +static grub_err_t > +get_hash (const grub_packed_guid_t *guid, const grub_uint8_t *data, const > grub_size_t data_size, > + grub_uint8_t *hash, grub_size_t *hash_size) > +{ > + gcry_md_spec_t *hash_func = NULL; > + > + if (guid == NULL) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "GUID is not available"); > + > + if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA256_GUID, GRUB_PACKED_GUID_SIZE) > == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA256_GUID, > GRUB_PACKED_GUID_SIZE) == 0) > + hash_func = &_gcry_digest_spec_sha256; > + else if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA384_GUID, > GRUB_PACKED_GUID_SIZE) == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA384_GUID, > GRUB_PACKED_GUID_SIZE) == 0) > + hash_func = &_gcry_digest_spec_sha384; > + else if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA512_GUID, > GRUB_PACKED_GUID_SIZE) == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA512_GUID, > GRUB_PACKED_GUID_SIZE) == 0) > + hash_func = &_gcry_digest_spec_sha512; > + else > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "unsupported GUID hash"); > + > + grub_crypto_hash (hash_func, hash, data, data_size); > + *hash_size = hash_func->mdlen; s/ = / = / Redundant space... > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +generate_cert_hash (const grub_size_t cert_hash_size, const grub_uint8_t > *data, > + const grub_size_t data_size, grub_uint8_t *hash, > grub_size_t *hash_size) > +{ > + grub_packed_guid_t guid = { 0 }; > + > + /* support SHA256, SHA384 and SHA512 for certificate hash */ > + if (cert_hash_size == SHA256_HASH_LEN) > + grub_memcpy (&guid, &GRUB_PKS_CERT_X509_SHA256_GUID, > GRUB_PACKED_GUID_SIZE); > + else if (cert_hash_size == SHA384_HASH_LEN) > + grub_memcpy (&guid, &GRUB_PKS_CERT_X509_SHA384_GUID, > GRUB_PACKED_GUID_SIZE); > + else if (cert_hash_size == SHA512_HASH_LEN) > + grub_memcpy (&guid, &GRUB_PKS_CERT_X509_SHA512_GUID, > GRUB_PACKED_GUID_SIZE); > + else > + { > + grub_dprintf ("appendedsig", "unsupported hash type (%" PRIuGRUB_SIZE > ") and " > + "skipped\n", cert_hash_size); > + return GRUB_ERR_UNKNOWN_COMMAND; > + } > + > + return get_hash (&guid, data, data_size, hash, hash_size); > +} > + > +/* Check the hash presence in the db/dbx list. */ > +static bool > +check_hash_presence (grub_uint8_t *const hash, const grub_size_t hash_size, > + const sb_database_t *sb_database) > +{ > + grub_uint32_t i; > + > + for (i = 0; i < sb_database->hash_entries; i++) > + { > + if (sb_database->hashes[i] == NULL) > + continue; > + > + if (hash_size == sb_database->hash_sizes[i] && > + grub_memcmp (sb_database->hashes[i], hash, hash_size) == 0) > + return true; > + } > + > + return false; > +} > + > +/* Add the certificate/binary hash into the db/dbx list. */ > +static grub_err_t > +add_hash (grub_uint8_t *const data, const grub_size_t data_size, > sb_database_t *sb_database) > +{ > + grub_uint8_t **hashes; > + grub_size_t *hash_sizes; > + > + if (data == NULL || data_size == 0) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "certificate/binary-hash data > or size is not available"); > + > + if (sb_database->is_db == true) > + { > + if (check_hash_presence (data, data_size, &dbx) == true) > + { > + grub_dprintf ("appendedsig", > + "cannot add a hash (%02x%02x%02x%02x), as it is > present in the dbx list\n", > + data[0], data[1], data[2], data[3]); > + return GRUB_ERR_ACCESS_DENIED; > + } > + } > + > + if (check_hash_presence (data, data_size, sb_database) == true) > + { > + grub_dprintf ("appendedsig", > + "cannot add a hash (%02x%02x%02x%02x), as it is present > in the %s list\n", > + data[0], data[1], data[2], data[3], ((sb_database->is_db > == true) ? "db" : "dbx")); > + return GRUB_ERR_EXISTS; > + } > + > + hashes = grub_realloc (sb_database->hashes, sizeof (grub_uint8_t *) * > (sb_database->hash_entries + 1)); > + if (hashes == NULL) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory"); > + > + hash_sizes = grub_realloc (sb_database->hash_sizes, sizeof (grub_size_t) * > (sb_database->hash_entries + 1)); > + if (hash_sizes == NULL) > + { > + /* > + * Allocated memory will be freed by > + * free_db_list/free_dbx_list. s#free_db_list()/free_dbx_list()#free_db_list()/free_dbx_list()# > + */ > + hashes[sb_database->hash_entries] = NULL; > + sb_database->hashes = hashes; > + sb_database->hash_entries++; > + > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory"); > + } > + > + hashes[sb_database->hash_entries] = grub_malloc (data_size); > + if (hashes[sb_database->hash_entries] == NULL) > + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory"); > + > + grub_dprintf ("appendedsig", > + "added the hash %02x%02x%02x%02x... with size of %" > PRIuGRUB_SIZE " to the %s list\n", > + data[0], data[1], data[2], data[3], data_size, > + ((sb_database->is_db == true) ? "db" : "dbx")); > + > + grub_memcpy (hashes[sb_database->hash_entries], data, data_size); > + hash_sizes[sb_database->hash_entries] = data_size; > + sb_database->hash_sizes = hash_sizes; > + sb_database->hashes = hashes; > + sb_database->hash_entries++; > + > + return GRUB_ERR_NONE; > +} > + > +static bool > +is_hash (const grub_packed_guid_t *guid) > +{ > + /* GUID type of the binary hash. */ > + if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA256_GUID, GRUB_PACKED_GUID_SIZE) > == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_SHA384_GUID, GRUB_PACKED_GUID_SIZE) > == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_SHA512_GUID, GRUB_PACKED_GUID_SIZE) > == 0) > + return true; > + > + /* GUID type of the certificate hash. */ > + if (grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA256_GUID, > GRUB_PACKED_GUID_SIZE) == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA384_GUID, > GRUB_PACKED_GUID_SIZE) == 0 || > + grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA512_GUID, > GRUB_PACKED_GUID_SIZE) == 0) > + return true; > + > + return false; > +} > + > +static bool > +is_x509 (const grub_packed_guid_t *guid) > +{ > + if (grub_memcmp (guid, &GRUB_PKS_CERT_X509_GUID, GRUB_PACKED_GUID_SIZE) == > 0) > + return true; > + > + return false; > +} > + > static bool > is_cert_match (const grub_x509_cert_t *cert1, const grub_x509_cert_t *cert2) > { > @@ -136,7 +319,33 @@ is_cert_match (const grub_x509_cert_t *cert1, const > grub_x509_cert_t *cert2) > return false; > } > > -/* Check the certificate presence in the db list. */ > +/* Check the certificate hash presence in the dbx list. */ > +static bool > +is_cert_hash_present_in_dbx (const grub_uint8_t *data, const grub_size_t > data_size) > +{ > + grub_err_t rc; > + grub_uint32_t i; > + grub_size_t cert_hash_size = 0; > + grub_uint8_t cert_hash[GRUB_MAX_HASH_LEN] = { 0 }; > + > + for (i = 0; i < dbx.hash_entries; i++) > + { > + if (dbx.hashes[i] == NULL) > + continue; > + > + rc = generate_cert_hash (dbx.hash_sizes[i], data, data_size, > cert_hash, &cert_hash_size); > + if (rc != GRUB_ERR_NONE) > + continue; > + > + if (cert_hash_size == dbx.hash_sizes[i] && > + grub_memcmp (dbx.hashes[i], cert_hash, cert_hash_size) == 0) > + return true; > + } > + > + return false; > +} > + > +/* Check the certificate presence in the db/dbx list. */ > static bool > check_cert_presence (const grub_x509_cert_t *cert_in, const sb_database_t > *sb_database) > { > @@ -149,7 +358,10 @@ check_cert_presence (const grub_x509_cert_t *cert_in, > const sb_database_t *sb_da > return false; > } > > -/* Add the certificate into the db list */ > +/* > + * Add the certificate into the db list if it is not present in the dbx and > db list > + * when is_db is true. Add the certificate into the dbx list when is_db is > false. > + */ > static grub_err_t > add_certificate (const grub_uint8_t *data, const grub_size_t data_size, > sb_database_t *sb_database) > @@ -167,30 +379,54 @@ add_certificate (const grub_uint8_t *data, const > grub_size_t data_size, > rc = grub_x509_cert_parse (data, data_size, cert); > if (rc != GRUB_ERR_NONE) > { > - grub_dprintf ("appendedsig", "cannot add a certificate CN='%s' to the > db list\n", > - cert->subject); > + grub_dprintf ("appendedsig", "cannot add a certificate CN='%s' to the > %s list\n", > + cert->subject, ((sb_database->is_db == true) ? "db" : > "dbx")); You can drop one "()" pair around ternary operator. It is redundant. Otherwise patch LGTM... If you fix these minor issues you can add my RB to the patch. Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
