On Mon, Aug 25, 2025 at 04:38:34PM +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 | 7 +
> grub-core/kern/powerpc/ieee1275/ieee1275.c | 139 ++++++++
> .../kern/powerpc/ieee1275/platform_keystore.c | 328 ++++++++++++++++++
> include/grub/ieee1275/ieee1275.h | 3 +
> include/grub/powerpc/ieee1275/ieee1275.h | 20 ++
> .../grub/powerpc/ieee1275/platform_keystore.h | 139 ++++++++
> 9 files changed, 640 insertions(+), 1 deletion(-)
> 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..8577462d5 100644
> --- a/grub-core/Makefile.am
> +++ b/grub-core/Makefile.am
> @@ -246,6 +246,8 @@ 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/ieee1275.h
> +KERNEL_HEADER_FILES +=
> $(top_srcdir)/include/grub/powerpc/ieee1275/platform_keystore.h
> endif
>
> if COND_sparc64_ieee1275
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index d91694de0..16cdbadcb 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -332,6 +332,8 @@ kernel = {
> powerpc_ieee1275 = kern/powerpc/dl.c;
> powerpc_ieee1275 = kern/powerpc/compiler-rt.S;
> powerpc_ieee1275 = kern/lockdown.c;
> + powerpc_ieee1275 = kern/powerpc/ieee1275/ieee1275.c;
> + powerpc_ieee1275 = kern/powerpc/ieee1275/platform_keystore.c;
>
> 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 132d027c7..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;
> @@ -1044,6 +1047,10 @@ grub_ieee1275_get_secure_boot (void)
> }
> else
> grub_dprintf ("ieee1275", "Secure Boot Disabled\n");
> +
> + err = grub_pks_keystore_init ();
> + if (err != GRUB_ERR_NONE)
> + grub_error (err, "initialization of the Platform KeyStore failed\n");
This should be a part of grub_pks_keystore_init().
> }
> #endif /* __powerpc__ */
>
Please drop this empty line.
[...]
> 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..eeb569fd1
> --- /dev/null
> +++ b/grub-core/kern/powerpc/ieee1275/platform_keystore.c
> @@ -0,0 +1,328 @@
> +/*
> + * 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
Is it really needed? If yes it begs for a comment.
> +/* 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)
#define SB_VERSION_KEY_LEN (sizeof (SB_VERSION_KEY_NAME) - 1)
> +/* 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;
> +
> +/*
> + * Platform KeyStore db and dbx, pks_supported flag to indicate the
> availability
> + * of PSK support and use_keystore flag to indicate the key management.
s/key management/key management mode in the GRUB/
> + * pks_supported:
> + * False: No PKS support. This is default.
> + * True: PKS support available.
The pks_supported member does not make a lot of sense for me. It makes
code more complicated for nothing. In real we care about use_keystore
only.
> + * use_keystore:
> + * False: Static key management (use built-in Keys). This is default.
> + * True: Dynamic key management (use Platform KeySotre).
> + */
> +grub_pks_t grub_pks_keystore = { .db = NULL, .dbx = NULL, .db_entries = 0,
> .dbx_entries = 0,
> + .use_keystore = false, .pks_supported =
> false};
> +
> +/*
> + * 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).
It should be made clear it is copied from the platform PKS into the pks_sd
in the GRUB. Otherwise the comment and whole code in this patch is confusing.
> + */
> +static grub_err_t
> +_esl_to_esd (const grub_uint8_t *esl_data, grub_size_t esl_size,
> + 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;
> +}
[...]
> +/*
> + * Test the availability of PKS support. If PKS support is avaialble and
> + * objects present, it reads the secure boot version (SB_VERSION) from PKS.
> + *
> + * 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.
> + */
> +static grub_err_t
> +is_pks_present (void)
This function should return bool.
> +{
> + grub_err_t err;
> + grub_int32_t rc;
> + grub_ieee1275_cell_t missing = 0;
> + grub_uint8_t *data = NULL;
> + grub_uint32_t len = 0, policy = 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");
> + }
> +
> + grub_pks_keystore.pks_supported = true;
We do not care...
> + err = read_sbversion_from_pks (&data, &len, &policy);
> + if (err == GRUB_ERR_NONE && (len != 1 || (*data >= 2)))
> + {
> + grub_free (data);
> + return grub_error (GRUB_ERR_BAD_NUMBER, "found unexpected SB version
> (%d)\n", *data);
> + }
> + else if (*data)
> + grub_pks_keystore.use_keystore = true;
This should happen in the grub_pks_keystore_init().
> + grub_free (data);
> +
> + return err;
> +}
> +
> +/* Free allocated memory. */
> +void
> +grub_pks_free_keystore (void)
> +{
> + grub_size_t i;
> +
> + for (i = 0; i < grub_pks_keystore.db_entries; i++)
> + grub_free (grub_pks_keystore.db[i].data);
> +
> + for (i = 0; i < grub_pks_keystore.dbx_entries; i++)
> + grub_free (grub_pks_keystore.dbx[i].data);
> +
> + grub_free (grub_pks_keystore.db);
> + grub_free (grub_pks_keystore.dbx);
> + grub_memset (&grub_pks_keystore, 0, sizeof (grub_pks_t));
> +}
> +
> +/* Initialization of the Platform KeyStore. */
> +grub_err_t
return void...
> +grub_pks_keystore_init (void)
> +{
> + grub_err_t rc;
> +
> + grub_dprintf ("ieee1275", "trying to load Platform KeyStore\n");
> +
> + rc = is_pks_present ();
if (is_pks_present () == false)
{
grub_dprintf ("ieee1275", "Platform PKS is not available\n");
return;
}
> + if (rc != GRUB_ERR_NONE)
> + {
> + grub_dprintf ("ieee1275", "switch to static key\n");
> + return rc;
> + }
> +
> + if (grub_pks_keystore.pks_supported == true)
> + {
> + grub_memset (&grub_pks_keystore, 0, sizeof (grub_pks_t));
> + /* Read db from PKS. */
> + rc = read_sbvar_from_pks (0, DB, &grub_pks_keystore.db,
> &grub_pks_keystore.db_entries);
> + if (rc == GRUB_ERR_NONE)
> + {
> + /* Read dbx from PKS. */
> + rc = read_sbvar_from_pks (0, DBX, &grub_pks_keystore.dbx,
> &grub_pks_keystore.dbx_entries);
> + if (rc == GRUB_ERR_UNKNOWN_COMMAND)
> + rc = GRUB_ERR_NONE;
> + }
> +
> + if (rc != GRUB_ERR_NONE)
> + grub_pks_free_keystore ();
> + }
grub_pks_keystore.use_keystore = true;
> + return rc;
> +}
[...]
> diff --git a/include/grub/powerpc/ieee1275/platform_keystore.h
> b/include/grub/powerpc/ieee1275/platform_keystore.h
> new file mode 100644
> index 000000000..8b6ba2e5f
> --- /dev/null
> +++ b/include/grub/powerpc/ieee1275/platform_keystore.h
> +#if defined(__powerpc__)
> +
> +/*
> + * Platform KeyStore db and dbx, pks_supported flag to indicate the
> availability
> + * of PSK support and use_keystore flag to indicate the key management.
> + *
> + * pks_supported:
> + * False: No PKS support. This is default.
> + * True: PKS support available.
> + *
> + * use_keystore:
> + * False: Static key management (use built-in Keys). This is default.
> + * True: Dynamic key management (use Platform KeySotre).
> + */
> +extern grub_pks_t EXPORT_VAR (grub_pks_keystore);
> +
> +/* Initialization of the Platform Keystore. */
> +extern grub_err_t
> +grub_pks_keystore_init (void);
> +
> +/* Free allocated memory. */
> +extern void
> +EXPORT_FUNC (grub_pks_free_keystore) (void);
> +
> +#else
> +
> +grub_pks_t grub_pks_keystore;
This clearly shows it should not be a global variable. You should have
a function which returns pointer to PKS or NULL. And even this makes
use_keystore usage questionable...
> +static inline void
> +grub_pks_free_keystore (void)
I am not convinced this function should be called of outside
platform_keystore.c above.
> +{
> +
Please drop this empty line if this function is really needed.
> +}
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
