On Thu, Aug 21, 2025 at 01:25:04PM +0530, Sudhakar Kuppusamy wrote:
> Enhancing the infrastructure to enable the Platform Keystore (PKS) feature,
> which provides access to the SB_VERSION, db, and dbx secure boot variables
> from PKS.
>
> If PKS is enabled, it will read secure boot variables such as db and dbx
> from PKS and extract EFI Signature List (ESL) from it. The ESLs would be
> saved in the Platform Keystore buffer, and the appendedsig module would
> read it later to extract the certificate's details from ESL.
>
> In the following scenarios, static key management mode will be activated:
> 1. When Secure Boot is enabled with static key management mode
> 2. When SB_VERSION is unavailable but Secure Boot is enabled
> 3. When PKS support is unavailable but Secure Boot is enabled
>
> Note:-
>
> SB_VERSION: Key Management Mode
> 1 - Enable dynamic key management mode. Read the db and dbx variables from
> PKS,
> and use them for signature verification.
> 0 - Enable static key management mode. Read keys from the GRUB ELF Note and
> use it for signature verification.
>
> Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>
> ---
> grub-core/Makefile.am | 2 +
> grub-core/Makefile.core.def | 2 +
> grub-core/kern/ieee1275/ieee1275.c | 1 -
> grub-core/kern/ieee1275/init.c | 14 +-
> grub-core/kern/powerpc/ieee1275/ieee1275.c | 139 +++++++
> .../kern/powerpc/ieee1275/platform_keystore.c | 341 ++++++++++++++++++
> include/grub/ieee1275/ieee1275.h | 3 +
> include/grub/powerpc/ieee1275/ieee1275.h | 20 +
> .../grub/powerpc/ieee1275/platform_keystore.h | 142 ++++++++
> 9 files changed, 662 insertions(+), 2 deletions(-)
> create mode 100644 grub-core/kern/powerpc/ieee1275/ieee1275.c
> create mode 100644 grub-core/kern/powerpc/ieee1275/platform_keystore.c
> create mode 100644 include/grub/powerpc/ieee1275/platform_keystore.h
>
> diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
> index e50db8106..cd6bb7c32 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -241,11 +241,13 @@ KERNEL_HEADER_FILES +=
> $(top_builddir)/include/grub/machine/kernel.h
> endif
>
> if COND_powerpc_ieee1275
> +KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/powerpc/ieee1275/ieee1275.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/ieee1275/ieee1275.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/ieee1275/alloc.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/terminfo.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/extcmd.h
> KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/lib/arg.h
> +KERNEL_HEADER_FILES +=
> $(top_srcdir)/include/grub/powerpc/ieee1275/platform_keystore.h
I am not entirely sure why you cannot put these two changes next to each
other...
> endif
>
> if COND_sparc64_ieee1275
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index d91694de0..05ccbf264 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -328,10 +328,12 @@ kernel = {
> extra_dist = video/sis315_init.c;
> mips_loongson = commands/keylayouts.c;
>
> + powerpc_ieee1275 = kern/powerpc/ieee1275/ieee1275.c;
> powerpc_ieee1275 = kern/powerpc/cache.S;
> powerpc_ieee1275 = kern/powerpc/dl.c;
> powerpc_ieee1275 = kern/powerpc/compiler-rt.S;
> powerpc_ieee1275 = kern/lockdown.c;
> + powerpc_ieee1275 = kern/powerpc/ieee1275/platform_keystore.c;
Ditto...
> sparc64_ieee1275 = kern/sparc64/cache.S;
> sparc64_ieee1275 = kern/sparc64/dl.c;
> diff --git a/grub-core/kern/ieee1275/ieee1275.c
> b/grub-core/kern/ieee1275/ieee1275.c
> index 36ca2dbfc..afa37a9f0 100644
> --- a/grub-core/kern/ieee1275/ieee1275.c
> +++ b/grub-core/kern/ieee1275/ieee1275.c
> @@ -23,7 +23,6 @@
>
> #define IEEE1275_PHANDLE_INVALID ((grub_ieee1275_cell_t) -1)
> #define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_cell_t) 0)
> -#define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1)
>
>
>
> diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> index 89303f33e..46bfaa9a8 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -51,6 +51,8 @@
> #endif
> #if defined(__powerpc__)
> #include <grub/lockdown.h>
> +#include <grub/powerpc/ieee1275/ieee1275.h>
> +#include <grub/powerpc/ieee1275/platform_keystore.h>
> #endif
>
> #ifdef __powerpc__
> @@ -1007,6 +1009,7 @@ grub_parse_cmdline (void)
> static void
> grub_ieee1275_get_secure_boot (void)
> {
> + grub_err_t err;
> grub_ieee1275_phandle_t root;
> grub_uint32_t sb_mode = GRUB_SB_DISABLED;
> int rc;
> @@ -1038,7 +1041,16 @@ grub_ieee1275_get_secure_boot (void)
> * Now, only support disabled and enforced.
> */
> if (sb_mode == GRUB_SB_ENFORCED)
> - grub_lockdown ();
> + {
> + grub_dprintf ("ieee1275", "Secure Boot Enabled\n");
> + grub_lockdown ();
> + }
> + else
> + grub_dprintf ("ieee1275", "Secure Boot Disabled\n");
Sorry, this change does not belong to this patch.
> + err = grub_pks_keystore_init ();
> + if (err != GRUB_ERR_NONE)
> + grub_error (err, "initialization of the Platform KeyStore failed\n");
Only this one...
> }
> #endif /* __powerpc__ */
[...]
> diff --git a/grub-core/kern/powerpc/ieee1275/platform_keystore.c
> b/grub-core/kern/powerpc/ieee1275/platform_keystore.c
> new file mode 100644
> index 000000000..aaf90500f
> --- /dev/null
> +++ b/grub-core/kern/powerpc/ieee1275/platform_keystore.c
> @@ -0,0 +1,341 @@
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2024 Free Software Foundation, Inc.
> + * Copyright (C) 2022, 2023, 2024, 2025 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/mm.h>
> +#include <grub/types.h>
> +#include <grub/misc.h>
> +#include <grub/lockdown.h>
> +#include <grub/ieee1275/ieee1275.h>
> +#include <grub/powerpc/ieee1275/ieee1275.h>
> +#include <grub/powerpc/ieee1275/platform_keystore.h>
> +
> +#if __GNUC__ >= 9
> +#pragma GCC diagnostic ignored "-Waddress-of-packed-member"
> +#endif
> +
> +/* PKS consumer type for firmware. */
> +#define PKS_CONSUMER_FW ((grub_uint8_t) 1)
> +
> +/* The maximum object size interface name for a PKS object. */
> +#define PKS_MAX_OBJ_SIZE ((grub_uint8_t *) "pks-max-object-size")
> +
> +/* PKS read object lable for secure boot version. */
> +#define SB_VERSION_KEY_NAME ((grub_uint8_t *) "SB_VERSION")
> +#define SB_VERSION_KEY_LEN ((grub_size_t) 10)
> +
> +/* PKS read secure boot variable request type for db and dbx. */
> +#define DB ((grub_uint8_t) 1)
> +#define DBX ((grub_uint8_t) 2)
> +
> +static grub_size_t pks_max_object_size;
> +/*
> + * Flag to indicate the key management.
> + * True: Dynamic key management (use Platform KeySotre).
> + * False: Static key management (use built-in Keys).
> + */
> +bool grub_pks_use_keystore = false;
> +/*
> + * Flag to indicate the availability of PSK support.
> + * True: PKS support available.
> + * False: No PKS support.
> + */
> +bool grub_pks_is_support_pks = false;
Why could not we use grub_pks_use_keystore only instead of both
grub_pks_use_keystore and grub_pks_is_support_pks? If you convince me
that both are needed then s/grub_pks_is_support_pks/grub_pks_supported/...
> +/* Platform KeyStore db and dbx. */
> +grub_pks_t grub_pks_keystore = { .db = NULL, .dbx = NULL, .db_entries = 0,
> .dbx_entries = 0 };
> +
> +/*
> + * Import the Globally Unique Identifier (GUID), EFI Signature Database
> (ESD),
> + * and its size into the PKS Signature Database (SD) (i.e pks_sd buffer) and
> pks_sd entries
> + * from the EFI Signature List (ESL).
> + */
> +static grub_err_t
> +esd_from_esl (const grub_uint8_t *esl_data, grub_size_t esl_size,
s/esd_from_esl/__esl_to_esd/
> + const grub_size_t signature_size, const grub_packed_guid_t
> *guid,
> + grub_pks_sd_t **pks_sd, grub_uint32_t *pks_sd_entries)
> +{
> + grub_esd_t *esd;
> + grub_pks_sd_t *signature = *pks_sd;
> + grub_uint32_t entries = *pks_sd_entries;
> + grub_size_t data_size, offset = 0;
> +
> + /* Reads the ESD from ESL. */
> + while (esl_size > 0)
> + {
> + esd = (grub_esd_t *) (esl_data + offset);
> + data_size = signature_size - sizeof (grub_esd_t);
> +
> + signature = grub_realloc (signature, (entries + 1) * sizeof
> (grub_pks_sd_t));
> + if (signature == NULL)
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +
> + signature[entries].data = grub_malloc (data_size * sizeof
> (grub_uint8_t));
> + if (signature[entries].data == NULL)
> + {
> + /*
> + * Allocated memory will be freed by
> + * grub_free_platform_keystore.
> + */
> + *pks_sd = signature;
> + *pks_sd_entries = entries + 1;
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> + }
> +
> + grub_memcpy (signature[entries].data, esd->signature_data, data_size);
> + signature[entries].data_size = data_size;
> + signature[entries].guid = *guid;
> + entries++;
> + esl_size -= signature_size;
> + offset += signature_size;
> + }
> +
> + *pks_sd = signature;
> + *pks_sd_entries = entries;
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +/* Extract the ESD after removing the ESL header from ESL. */
> +static grub_err_t
> +esl_to_esd (const grub_uint8_t *esl_data, grub_size_t *next_esl,
> + grub_pks_sd_t **pks_sd, grub_uint32_t *pks_sd_entries)
> +{
> + grub_packed_guid_t guid;
> + grub_esl_t *esl;
> + grub_size_t offset, esl_size,
> + signature_size, signature_header_size;
You do not need to wrap this line. I am OK with lines (a bit) longer
than 80 characters...
> + /* Convert the ESL data into the ESL. */
> + esl = (grub_esl_t *) esl_data;
> + if (*next_esl < sizeof (grub_esl_t) || esl == NULL)
> + return grub_error (GRUB_ERR_BUG, "invalid ESL");
> +
> + esl_size = grub_le_to_cpu32 (esl->signature_list_size);
> + signature_header_size = grub_le_to_cpu32 (esl->signature_header_size);
> + signature_size = grub_le_to_cpu32 (esl->signature_size);
> + grub_memcpy (&guid, &esl->signature_type, sizeof (grub_packed_guid_t));
> +
> + if (esl_size < sizeof (grub_esl_t) || esl_size > *next_esl)
> + return grub_error (GRUB_ERR_BUG, "invalid ESL size (%u)\n", esl_size);
> +
> + *next_esl = esl_size;
> + offset = sizeof (grub_esl_t) + signature_header_size;
> + esl_size = esl_size - offset;
> +
> + return esd_from_esl (esl_data + offset, esl_size, signature_size, &guid,
> + pks_sd, pks_sd_entries);
> +}
> +
> +/*
> + * Import the EFI Signature Database (ESD) and the number of ESD from the ESL
> + * into the pks_sd buffer and pks_sd entries.
> + */
> +static grub_err_t
> +pks_sd_from_esl (const grub_uint8_t *esl_data, grub_size_t esl_size,
> + grub_pks_sd_t **pks_sd, grub_uint32_t *pks_sd_entries)
> +{
> + grub_err_t rc;
> + grub_size_t next_esl = esl_size;
> +
> + do
> + {
> + rc = esl_to_esd (esl_data, &next_esl, pks_sd, pks_sd_entries);
> + if (rc != GRUB_ERR_NONE)
> + break;
> +
> + esl_data += next_esl;
> + esl_size -= next_esl;
> + next_esl = esl_size;
> + }
> + while (esl_size > 0);
> +
> + return rc;
> +}
> +
> +/*
> + * Read the secure boot version from PKS as an object.
> + * Caller must free result.
> + */
> +static grub_err_t
> +read_sbversion_from_pks (grub_uint8_t **out, grub_uint32_t *outlen,
> grub_uint32_t *policy)
> +{
> + grub_int32_t rc;
> +
> + *out = grub_malloc (pks_max_object_size);
> + if (*out == NULL)
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +
> + rc = grub_ieee1275_pks_read_object (PKS_CONSUMER_FW, SB_VERSION_KEY_NAME,
> + SB_VERSION_KEY_LEN,
> pks_max_object_size, *out,
> + outlen, policy);
> + if (rc < 0)
> + return grub_error (GRUB_ERR_READ_ERROR, "SB version read failed (%d)\n",
> rc);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +/*
> + * Reads the secure boot variable from PKS.
> + * Caller must free result.
> + */
> +static grub_err_t
> +read_sbvar_from_pks (const grub_uint8_t sbvarflags, const grub_uint8_t
> sbvartype,
> + grub_uint8_t **out, grub_size_t *outlen)
> +{
> + grub_int32_t rc;
> +
> + *out = grub_malloc (pks_max_object_size);
> + if (*out == NULL)
> + return grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +
> + rc = grub_ieee1275_pks_read_sbvar (sbvarflags, sbvartype,
> pks_max_object_size, *out, outlen);
> + if (rc == IEEE1275_CELL_NOT_FOUND)
> + return grub_error (GRUB_ERR_UNKNOWN_COMMAND, "secure boot variable %s
> not found (%d)",
> + (sbvartype == DB ? "db" : "dbx"), rc);
> + else if (rc < 0)
> + return grub_error (GRUB_ERR_READ_ERROR, "secure boot variable %s reading
> (%d)",
> + (sbvartype == DB ? "db" : "dbx"), rc);
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +/* Test the availability of PKS support. */
> +static grub_err_t
> +is_support_pks (void)
s/is_support_pks/is_pks_present/
> +{
> + grub_int32_t rc;
> + grub_ieee1275_cell_t missing = 0;
> +
> + rc = grub_ieee1275_test (PKS_MAX_OBJ_SIZE, &missing);
> + if (rc < 0 || missing == IEEE1275_CELL_INVALID)
> + return grub_error (GRUB_ERR_BAD_FIRMWARE, "firmware doesn't have PKS
> support\n");
> + else
> + {
> + rc = grub_ieee1275_pks_max_object_size (&pks_max_object_size);
> + if (rc < 0)
> + return grub_error (GRUB_ERR_BAD_NUMBER, "PKS support is there but
> it has zero objects\n");
> + }
> +
> + return GRUB_ERR_NONE;
> +}
> +
> +/*
> + * Retrieve the secure boot variable from PKS, unpacks it, read the ESD
> + * from ESL, and store the information in the pks_sd buffer.
> + */
> +static grub_err_t
> +read_secure_boot_variables (const grub_uint8_t sbvarflags, const
> grub_uint8_t sbvartype,
> + grub_pks_sd_t **pks_sd, grub_uint32_t
> *pks_sd_entries)
> +{
> + grub_err_t rc;
> + grub_uint8_t *esl_data = NULL;
> + grub_size_t esl_data_size = 0;
> +
> + rc = read_sbvar_from_pks (sbvarflags, sbvartype, &esl_data,
> &esl_data_size);
> + if (rc == GRUB_ERR_NONE && esl_data_size != 0)
> + rc = pks_sd_from_esl ((const grub_uint8_t *) esl_data, esl_data_size,
> + pks_sd, pks_sd_entries);
> + grub_free (esl_data);
> +
> + return rc;
> +}
> +
> +/*
> + * Reads secure boot version (SB_VERSION) and it supports following.
> + * SB_VERSION:
> + * 1 - Enable dynamic key management mode. Read the db and dbx variables
> from PKS,
> + and use them for signature verification.
> + * 0 - Enable static key management mode. Read keys from the GRUB ELF Note
> and
> + * use it for signature verification.
> + */
> +static void
> +get_secure_boot_version (void)
s/get_secure_boot_version/is_pks_present/
And it seems to me the get_secure_boot_version() should be merged with
this function.
> +{
> + grub_err_t rc;
> + grub_uint8_t *data = NULL;
> + grub_uint32_t len = 0, policy = 0;
> +
> + rc = read_sbversion_from_pks (&data, &len, &policy);
> + if (rc == GRUB_ERR_NONE && (len != 1 || (*data >= 2)))
> + grub_error (GRUB_ERR_BAD_NUMBER, "found unexpected SB version (%d)\n",
> *data);
> +
> + if (rc != GRUB_ERR_NONE)
> + grub_dprintf ("ieee1275", "switch to static key\n");
> + else if (*data)
> + grub_pks_use_keystore = true;
> +
> + grub_free (data);
> +}
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
