On Tue, Sep 30, 2025 at 05:10:05PM +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 <[email protected]>
> ---
> grub-core/Makefile.am | 2 +
> grub-core/Makefile.core.def | 2 +
> grub-core/kern/ieee1275/ieee1275.c | 1 -
> grub-core/kern/ieee1275/init.c | 4 +
> grub-core/kern/powerpc/ieee1275/ieee1275.c | 137 +++++++
> .../kern/powerpc/ieee1275/platform_keystore.c | 344 ++++++++++++++++++
> include/grub/ieee1275/ieee1275.h | 3 +
> include/grub/powerpc/ieee1275/ieee1275.h | 18 +
> .../grub/powerpc/ieee1275/platform_keystore.h | 122 +++++++
> 9 files changed, 632 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 6824a0ee4..853d879a4 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -334,6 +334,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 a81a36f57..c46624f39 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__
> @@ -1043,6 +1045,8 @@ grub_ieee1275_get_secure_boot (void)
> }
> else
> grub_dprintf ("ieee1275", "Secure Boot Disabled\n");
> +
> + grub_pks_keystore_init ();
> }
> #endif /* __powerpc__ */
> grub_addr_t grub_modbase;
> diff --git a/grub-core/kern/powerpc/ieee1275/ieee1275.c
> b/grub-core/kern/powerpc/ieee1275/ieee1275.c
> new file mode 100644
> index 000000000..38cfe4d8d
> --- /dev/null
> +++ b/grub-core/kern/powerpc/ieee1275/ieee1275.c
> @@ -0,0 +1,137 @@
> +/* ieee1275.c - Access the Open Firmware client interface. */
> +/*
> + * GRUB -- GRand Unified Bootloader
> + * Copyright (C) 2003,2004,2005,2007,2008,2009 Free Software Foundation,
> Inc.
> + * Copyright (C) 2020, 2021, 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/ieee1275/ieee1275.h>
> +#include <grub/powerpc/ieee1275/ieee1275.h>
> +#include <grub/misc.h>
> +
> +grub_int32_t
> +grub_ieee1275_test (const char *name)
> +{
> + struct test_args
> + {
> + struct grub_ieee1275_common_hdr common;/* The header information like
> interface name, number of inputs and outputs. */
> + grub_ieee1275_cell_t name; /* The interface name. */
> + grub_ieee1275_cell_t missing;
> + } args;
> +
> + INIT_IEEE1275_COMMON (&args.common, "test", 1, 1);
> + args.name = (grub_ieee1275_cell_t) name;
> +
> + if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> + return -1;
> +
> + if (args.missing == IEEE1275_CELL_INVALID)
> + return -1;
> +
> + return 0;
> +}
> +
> +grub_int32_t
> +grub_ieee1275_pks_max_object_size (grub_uint32_t *result)
> +{
> + struct mos_args
> + {
> + struct grub_ieee1275_common_hdr common;/* The header information like
> interface name, number of inputs and outputs. */
> + grub_ieee1275_cell_t size; /* The maximum object size for a
> PKS object. */
> + } args;
> +
> + INIT_IEEE1275_COMMON (&args.common, "pks-max-object-size", 0, 1);
> +
> + if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> + return -1;
> +
> + if (args.size == IEEE1275_CELL_INVALID)
> + return -1;
> +
> + *result = args.size;
I can imagine the buggy firmware may return 0 here and later you will
call grub_malloc() with this value. And grub_malloc() may not return
NULL in such case... So, I think it would be good if you identify
a minimal value which should be returned by this function. Smaller values
should be treated as an error and the function should return -1.
> + return 0;
> +}
> +
> +grub_int32_t
> +grub_ieee1275_pks_read_object (const grub_uint32_t consumer, const char
> *label,
> + const grub_uint32_t label_len, const
> grub_uint32_t buffer_len,
> + grub_uint8_t *buffer, grub_uint32_t *data_len,
> + grub_uint32_t *policies)
> +{
> + struct pks_read_args
> + {
> + struct grub_ieee1275_common_hdr common; /* The header information like
> interface name, number of inputs and outputs. */
> + grub_ieee1275_cell_t consumer; /* The object belonging to
> consumer with the label. */
> + grub_ieee1275_cell_t label; /* Object label buffer logical
> real address. */
> + grub_ieee1275_cell_t label_len; /* The byte length of the object
> label. */
> + grub_ieee1275_cell_t buffer; /* Output buffer logical real
> address. */
> + grub_ieee1275_cell_t buffer_len; /* Length of the output buffer.
> */
> + grub_ieee1275_cell_t data_len; /* The number of bytes copied to
> the output buffer. */
> + grub_ieee1275_cell_t policies; /* The object policies. */
> + grub_int32_t rc; /* The return code. */
> + } args;
> +
> + INIT_IEEE1275_COMMON (&args.common, "pks-read-object", 5, 3);
> + args.consumer = consumer;
> + args.label_len = label_len;
> + args.buffer_len = buffer_len;
> + args.label = (grub_ieee1275_cell_t) label;
> + args.buffer = (grub_ieee1275_cell_t) buffer;
> +
> + if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> + return -1;
> +
> + if (args.data_len == IEEE1275_CELL_INVALID)
> + return -1;
> +
> + *data_len = args.data_len;
> + *policies = args.policies;
> +
> + return args.rc;
> +}
> +
> +grub_int32_t
> +grub_ieee1275_pks_read_sbvar (const grub_uint32_t sbvar_flags, const
> grub_uint32_t sbvar_type,
> + const grub_uint32_t buffer_len, grub_uint8_t
> *buffer,
> + grub_size_t *data_len)
> +{
> + struct pks_read_sbvar_args
> + {
> + struct grub_ieee1275_common_hdr common; /* The header information like
> interface name, number of inputs and outputs. */
> + grub_ieee1275_cell_t sbvar_flags; /* The sbvar operation flags. */
> + grub_ieee1275_cell_t sbvar_type; /* The sbvar being requested. */
> + grub_ieee1275_cell_t buffer; /* Output buffer logical real
> address. */
> + grub_ieee1275_cell_t buffer_len; /* Length of the Output buffer.
> */
> + grub_ieee1275_cell_t data_len; /* The number of bytes copied to
> the output buffer. */
> + grub_int32_t rc; /* The return code. */
> + } args;
> +
> + INIT_IEEE1275_COMMON (&args.common, "pks-read-sbvar", 4, 2);
> + args.sbvar_flags = sbvar_flags;
> + args.sbvar_type = sbvar_type;
> + args.buffer_len = buffer_len;
> + args.buffer = (grub_ieee1275_cell_t) buffer;
> +
> + if (IEEE1275_CALL_ENTRY_FN (&args) == -1)
> + return -1;
> +
> + if (args.data_len == IEEE1275_CELL_INVALID)
> + return -1;
> +
> + *data_len = args.data_len;
> +
> + return args.rc;
> +}
> 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..715587cde
> --- /dev/null
> +++ b/grub-core/kern/powerpc/ieee1275/platform_keystore.c
> @@ -0,0 +1,344 @@
> +/*
> + * 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>
> +
> +/* PKS consumer type for firmware. */
> +#define PKS_CONSUMER_FW ((grub_uint32_t) 1)
> +
> +/* The maximum object size interface name for a PKS object. */
> +#define PKS_MAX_OBJ_SIZE "pks-max-object-size"
You define a constant here and you use it partially in this patch.
I think at least some of these constants above and below should
be moved to a header which declares functions using them.
> +/* PKS read object label for secure boot version. */
> +#define SB_VERSION_KEY_NAME "SB_VERSION"
> +#define SB_VERSION_KEY_LEN (sizeof (SB_VERSION_KEY_NAME) - 1)
> +
> +/* PKS read secure boot variable request type for db and dbx. */
> +#define PKS_SBVAR_DB ((grub_uint32_t) 1)
> +#define PKS_SBVAR_DBX ((grub_uint32_t) 2)
When you fix these two issues you can add my RB to this patch.
Additionally, as I can see many RBs left as is in many patches
regardless of significant amount of changes to them. So, I need
confirmation from all folks which gave RBs earlier before I will
push the patches as such...
Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
