On Tue, Jul 29, 2025 at 08:21:48PM +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.
> This is introduced by a subsequent patch.
>
> 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
> from the PKS and added to dbx list. Both dbx and db lists are introduced by
> a subsequent patch.
>
> Note:-
>
> 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]>
[...]
> +/* Add the certificate into the db/dbx list */
> +static grub_err_t
> +add_certificate (const grub_uint8_t *data, const grub_size_t data_size,
> + struct grub_database *database, const bool is_db)
> +{
> + struct x509_certificate *cert;
> + grub_err_t rc;
> + grub_size_t cert_entries = database->cert_entries;
> +
> + if (data == NULL || data_size == 0)
> + return grub_error (GRUB_ERR_OUT_OF_RANGE, "certificate data or size is
> not available");
> +
> + cert = grub_zalloc (sizeof (struct x509_certificate));
> + if (cert == NULL)
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +
> + rc = parse_x509_certificate (data, data_size, cert);
> + if (rc != GRUB_ERR_NONE)
> + {
> + grub_dprintf ("appendedsig", "skipped %s certificate (%d)\n",
> + ((is_db == true) ? "trusted" : "distrusted"), (int) rc);
The rc value is meaningless for user because it may change from one GRUB
version to another. Just add an error message which makes sense here...
> + grub_free (cert);
> + return rc;
> + }
> +
> + if (is_db == true)
> + {
> + rc = is_dbx_cert (cert);
> + if (rc != GRUB_ERR_NONE)
> + {
> + certificate_release (cert);
> + grub_free (cert);
> + return rc;
> + }
> + }
> +
> + grub_dprintf ("appendedsig", "add a %s certificate CN='%s'\n",
> + ((is_db == true) ? "trusted" : "distrusted"), cert->subject);
grub_dprintf ("add a certificate CN='%s' to %s", ((is_db == true) ? "db" :
"dbx"), cert->subject);
I think the error message above should be changed in similar way, e.g.,
"cannot add a certificate CN='%s' to %s due to an error"...
> + cert_entries++;
> + cert->next = database->certs;
> + database->certs = cert;
> + database->cert_entries = cert_entries;
> +
> + return rc;
> +}
> +
> static grub_err_t
> file_read_whole (grub_file_t file, grub_uint8_t **buf, grub_size_t *len)
> {
> @@ -272,7 +472,7 @@ grub_verify_appended_signature (const grub_uint8_t *buf,
> grub_size_t bufsize)
> struct pkcs7_signerInfo *si;
> int i;
>
> - if (db == NULL)
> + if (!db.cert_entries)
> return grub_error (GRUB_ERR_BAD_SIGNATURE, "no trusted keys to verify
> against");
>
> err = extract_appended_signature (buf, bufsize, &sig);
> @@ -303,7 +503,7 @@ grub_verify_appended_signature (const grub_uint8_t *buf,
> grub_size_t bufsize)
> grub_dprintf ("appendedsig", "data size %" PRIuGRUB_SIZE ", signer %d
> hash %02x%02x%02x%02x...\n",
> datasize, i, hash[0], hash[1], hash[2], hash[3]);
>
> - for (pk = db; pk != NULL; pk = pk->next)
> + for (pk = db.certs; pk != NULL; pk = pk->next)
> {
> err = verify_signature (pk->mpis, si->sig_mpi, si->hash, hash);
> if (err == GRUB_ERR_NONE)
> @@ -359,7 +559,7 @@ is_cert_present_in_db (const struct x509_certificate
> *cert_in)
> {
> struct x509_certificate *cert;
>
> - for (cert = db; cert; cert = cert->next)
> + for (cert = db.certs; cert; cert = cert->next)
...; cert != NULL;...??? If yes then it should be changed in patch which
introduces the file_read_whole() function...
> {
> if (is_cert_match (cert, cert_in) == true)
> return true;
> @@ -374,12 +574,12 @@ is_cert_removed_from_db (const struct x509_certificate
> *cert)
> int i = 1;
> struct x509_certificate *curr_cert, *prev_cert;
>
> - for (curr_cert = prev_cert = db; curr_cert != NULL; curr_cert =
> curr_cert->next)
> + for (curr_cert = prev_cert = db.certs; curr_cert != NULL; curr_cert =
> curr_cert->next)
> {
> if (is_cert_match (curr_cert, cert) == true)
> {
> if (i == 1) /* Match with first certificate in the db list. */
> - db = curr_cert->next;
> + db.certs = curr_cert->next;
> else
> prev_cert->next = curr_cert->next;
>
> @@ -468,8 +668,9 @@ grub_cmd_db_cert (grub_command_t cmd __attribute__
> ((unused)), int argc, char **
>
> grub_dprintf ("appendedsig", "added certificate with CN: %s\n",
> cert->subject);
>
> - cert->next = db;
> - db = cert;
> + cert->next = db.certs;
> + db.certs = cert;
> + db.cert_entries++;
>
> return GRUB_ERR_NONE;
> }
> @@ -517,7 +718,7 @@ grub_cmd_list_db (grub_command_t cmd __attribute__
> ((unused)), int argc __attrib
> int cert_num = 1;
> grub_size_t i;
>
> - for (cert = db; cert != NULL; cert = cert->next)
> + for (cert = db.certs; cert != NULL; cert = cert->next)
... like here...
> {
> grub_printf ("Certificate %d:\n", cert_num);
> grub_printf ("\tSerial: ");
> @@ -609,6 +810,238 @@ static struct grub_fs pseudo_fs = {
>
> static grub_command_t cmd_verify, cmd_list_db, cmd_dbx_cert, cmd_db_cert;
>
> +/* Check the certificate hash presence in the PKS dbx list. */
> +static bool
> +is_dbx_cert_hash (const grub_uint8_t *data, const grub_size_t data_size)
> +{
> + grub_err_t rc;
> + grub_size_t i, cert_hash_size = 0;
> + grub_uint8_t cert_hash[GRUB_MAX_HASH_SIZE] = { 0 };
> +
> + for (i = 0; i < grub_pks_keystore.dbx_entries; i++)
> + {
> + if (grub_pks_keystore.dbx[i].data == NULL ||
> + grub_pks_keystore.dbx[i].data_size == 0)
> + continue;
> +
> + rc = get_hash (&grub_pks_keystore.dbx[i].guid, data, data_size,
> + cert_hash, &cert_hash_size);
> + if (rc != GRUB_ERR_NONE)
> + continue;
> +
> + if (cert_hash_size == grub_pks_keystore.dbx[i].data_size &&
> + grub_memcmp (grub_pks_keystore.dbx[i].data, cert_hash,
> cert_hash_size) == 0)
> + {
> + grub_dprintf ("appendedsig", "a trusted certificate
> (%02x%02x%02x%02x) is ignored "
s/trusted//
> + "because this certificate hash is on the dbx list\n",
> + cert_hash[0], cert_hash[1], cert_hash[2],
> cert_hash[3]);
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +/* Check the binary hash presence in the PKS dbx list. */
> +static bool
> +is_dbx_binary_hash (const grub_uint8_t *binary_hash, const grub_size_t
> binary_hash_size)
> +{
> + grub_size_t i;
> +
> + for (i = 0; i < grub_pks_keystore.dbx_entries; i++)
> + {
> + if (grub_pks_keystore.dbx[i].data == NULL ||
> + grub_pks_keystore.dbx[i].data_size == 0)
> + continue;
> +
> + if (binary_hash_size == grub_pks_keystore.dbx[i].data_size &&
> + grub_memcmp (grub_pks_keystore.dbx[i].data, binary_hash,
> binary_hash_size) == 0)
> + {
> + grub_dprintf ("appendedsig", "a trusted binary hash
> (%02x%02x%02x%02x) is ignored"
s/trusted//
> + " because it is on the dbx list\n", binary_hash[0],
> binary_hash[1],
> + binary_hash[2], binary_hash[3]);
> + return true;
> + }
> + }
> +
> + return false;
> +}
[...]
> GRUB_MOD_INIT (appendedsig)
> {
> int rc;
> @@ -621,7 +1054,6 @@ GRUB_MOD_INIT (appendedsig)
> if (grub_ieee1275_is_secure_boot () == GRUB_SB_ENFORCED)
> check_sigs = true;
>
> - db = NULL;
> grub_register_variable_hook ("check_appended_signatures",
> grub_env_read_sec, grub_env_write_sec);
> grub_env_export ("check_appended_signatures");
>
> @@ -630,38 +1062,54 @@ GRUB_MOD_INIT (appendedsig)
> grub_fatal ("error initing ASN.1 data structures: %d: %s\n", rc,
> asn1_strerror (rc));
>
> /*
> - * If signature verification is enabled,
> - * extract trusted keys from ELF Note and store them in the db.
> + * If signature verification is enabled with static key management mode,
> + * extract trusted keys from ELF Note and store them in the db list.
> */
> - if (check_sigs == true)
> + if (grub_pks_use_keystore == false && check_sigs == true)
> {
> FOR_MODULES (header)
> - {
> - struct grub_file pseudo_file;
> - struct x509_certificate *pk = NULL;
> - grub_err_t err;
> -
> - /* Not an X.509 certificate, skip. */
> - if (header->type != OBJ_TYPE_X509_PUBKEY)
> - continue;
> -
> - grub_memset (&pseudo_file, 0, sizeof (pseudo_file));
> - pseudo_file.fs = &pseudo_fs;
> - pseudo_file.size = header->size - sizeof (struct grub_module_header);
> - pseudo_file.data = (char *) header + sizeof (struct
> grub_module_header);
> -
> - grub_dprintf ("appendedsig", "found an x509 key, size=%"
> PRIuGRUB_UINT64_T "\n",
> - pseudo_file.size);
> -
> - err = read_cert_from_file (&pseudo_file, &pk);
> - if (err != GRUB_ERR_NONE)
> - grub_fatal ("error loading initial key: %s", grub_errmsg);
> -
> - grub_dprintf ("appendedsig", "loaded certificate CN='%s'\n",
> pk->subject);
> -
> - pk->next = db;
> - db = pk;
> - }
> + {
> + /* Not an ELF module, skip. */
> + if (header->type != OBJ_TYPE_X509_PUBKEY)
> + continue;
> + rc = build_static_db_list (header);
> + if (rc != GRUB_ERR_NONE)
> + {
> + free_db_list ();
> + grub_error (rc, "static db list creation failed");
> + }
> + else
> + grub_dprintf ("appendedsig", "the db list now has %"
> PRIuGRUB_SIZE " static keys\n",
> + db.cert_entries);
> + }
> + }
> + /*
> + * If signature verification is enabled with dynamic key management mode,
> + * extract trusted and distrusted keys from PKS and store them in the db
> and dbx list.
> + */
> + else if (grub_pks_use_keystore == true && check_sigs == true)
> + {
> + rc = create_db_list ();
> + if (rc != GRUB_ERR_NONE)
> + {
> + free_db_list ();
I would not call free_db_list() here because even partially populated
list can be useful. Though we should be sure partial or even empty list
does not lead to crashes.
> + grub_error (rc, "db list creation failed");
> + }
> + else
> + {
> + rc = create_dbx_list ();
The dbx should be populated regardless of create_db_list() failure.
> + if (rc != GRUB_ERR_NONE)
> + {
> + free_db_list ();
> + free_dbx_list ();
Again, do not free lists in case of errors.
> + grub_error (rc, "dbx list creation failed");
> + }
> + else
> + grub_dprintf ("appendedsig", "the db list now has %"
> PRIuGRUB_SIZE " keys\n"
> + "the dbx list now has %" PRIuGRUB_SIZE " keys\n",
> + db.signature_entries + db.cert_entries,
> dbx.signature_entries);
> + }
> + grub_pks_free_keystore ();
> }
>
> cmd_db_cert = grub_register_command ("append_add_db_cert",
> grub_cmd_db_cert, N_("X509_CERTIFICATE"),
Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
