On Mon, Sep 22, 2025 at 02:57:53PM +0530, Sudhakar Kuppusamy wrote:
> Introducing the appended signature key management environment variable.
> It is automatically set to either "static" or "dynamic" based on the
> Platform KeyStore.
>
> "static": Enforce static key management signature verification.
> This is the default. When the GRUB is locked down,
> user cannot change the value by setting the appendedsig_key_mgmt
> variable back to "dynamic".
>
> "dynamic": Enforce dynamic key management signature verification.
> When the GRUB is locked down, user cannot change the value
> by setting the appendedsig_key_mgmt variable back to "static".
>
> Signed-off-by: Sudhakar Kuppusamy <[email protected]>
> ---
> grub-core/commands/appendedsig/appendedsig.c | 75 ++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/grub-core/commands/appendedsig/appendedsig.c
> b/grub-core/commands/appendedsig/appendedsig.c
> index 288b7339a..30d453007 100644
> --- a/grub-core/commands/appendedsig/appendedsig.c
> +++ b/grub-core/commands/appendedsig/appendedsig.c
> @@ -33,6 +33,7 @@
> #include <libtasn1.h>
> #include <grub/env.h>
> #include <grub/lockdown.h>
> +#include <grub/powerpc/ieee1275/platform_keystore.h>
>
> #include "appendedsig.h"
>
> @@ -94,6 +95,16 @@ static sb_database_t db = {.certs = NULL, .cert_entries =
> 0};
> */
> static bool check_sigs = false;
>
> +/*
> + * append_key_mgmt: Key Management Modes
> + * False: Static key management (use built-in Keys). This is default.
> + * True: Dynamic key management (use Platform KeySotre).
> + */
> +static bool append_key_mgmt = false;
> +
> +/* Platform KeyStore db and dbx. */
> +static grub_pks_t *pks_keystore;
> +
> static grub_ssize_t
> pseudo_read (struct grub_file *file, char *buf, grub_size_t len)
> {
> @@ -475,6 +486,46 @@ grub_env_write_sec (struct grub_env_var *var
> __attribute__ ((unused)), const cha
> return ret;
> }
>
> +static const char *
> +grub_env_read_key_mgmt (struct grub_env_var *var __attribute__ ((unused)),
> + const char *val __attribute__ ((unused)))
> +{
> + if (append_key_mgmt == true)
> + return "dynamic";
> +
> + return "static";
> +}
> +
> +static char *
> +grub_env_write_key_mgmt (struct grub_env_var *var __attribute__ ((unused)),
> const char *val)
> +{
> + char *ret;
> +
> + /*
> + * Do not allow the value to be changed if signature verification is
> + * (check_sigs is set to enforce) enabled and GRUB is locked down.
s/(check_sigs is set to enforce) enabled/enabled (check_sigs is set to true)/
> + */
> + if (check_sigs == true && grub_is_lockdown () == GRUB_LOCKDOWN_ENABLED)
> + {
> + ret = grub_strdup (grub_env_read_key_mgmt (NULL, NULL));
> + if (ret == NULL)
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +
> + return ret;
> + }
> +
> + if ((*val == '1') || (*val == 'd'))
> + append_key_mgmt = true;
> + else if ((*val == '0') || (*val == 's'))
> + append_key_mgmt = false;
I would be consistent and expect "dynamic" or "static" only.
And I hope these values are documented in the GRUB docs.
> + ret = grub_strdup (grub_env_read_key_mgmt (NULL, NULL));
> + if (ret == NULL)
> + grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
> +
> + return ret;
> +}
Otherwise LGTM...
If you fix these two minor issues you can add my RB to this patch.
Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
