On Mon, Jul 14, 2025 at 11:05:03PM +0530, Sudhakar Kuppusamy wrote:
> Building on the parsers and the ability to embed x509 certificates, as
> well as the existing gcrypt functionality, add a module for verifying
> appended signatures.
>
> This includes a verifier that requires that Linux kernels and grub modules

s/grub/GRUB/ this is not fixed in many places in many patches. Please
fix that everywhere...

> have appended signatures, and commands to manage the list of trusted
> certificates for verification.
>
> Verification must be enabled by setting check_appended_signatures. If
> GRUB is locked down when the module is loaded, verification will be
> enabled and locked automatically.
>
> As with the PGP verifier, it is not a complete secure-boot solution:
> other mechanisms, such as a password or lockdown, must be used to ensure
> that a user cannot drop to the GRUB shell and disable verification.
>
> Signed-off-by: Daniel Axtens <d...@axtens.net>
> Signed-off-by: Sudhakar Kuppusamy <sudha...@linux.ibm.com>
> Reviewed-by: Stefan Berger <stef...@linux.ibm.com>
> Reviewed-by: Avnish Chouhan <avn...@linux.ibm.com>
> ---
>  grub-core/Makefile.core.def                  |  14 +
>  grub-core/commands/appendedsig/appendedsig.c | 608 +++++++++++++++++++
>  include/grub/file.h                          |   2 +
>  3 files changed, 624 insertions(+)
>  create mode 100644 grub-core/commands/appendedsig/appendedsig.c
>
> diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
> index b3f71196a..7520296a6 100644
> --- a/grub-core/Makefile.core.def
> +++ b/grub-core/Makefile.core.def
> @@ -979,6 +979,20 @@ module = {
>    cppflags = '$(CPPFLAGS_GCRY)';
>  };
>
> +module = {
> +  name = appendedsig;
> +  common = commands/appendedsig/appendedsig.c;
> +  common = commands/appendedsig/x509.c;
> +  common = commands/appendedsig/pkcs7.c;
> +  common = commands/appendedsig/asn1util.c;
> +  common = commands/appendedsig/gnutls_asn1_tab.c;
> +  common = commands/appendedsig/pkix_asn1_tab.c;
> +  enable = powerpc_ieee1275;
> +  cflags = '$(CFLAGS_GCRY) -Wno-redundant-decls';
> +  cppflags = '$(CPPFLAGS_GCRY) -I$(srcdir)/lib/libtasn1-grub';
> +  depends = crypto, gcry_rsa, gcry_sha256, gcry_sha512, mpi, asn1;
> +};
> +
>  module = {
>    name = hdparm;
>    common = commands/hdparm.c;
> diff --git a/grub-core/commands/appendedsig/appendedsig.c 
> b/grub-core/commands/appendedsig/appendedsig.c
> new file mode 100644
> index 000000000..d1149695b
> --- /dev/null
> +++ b/grub-core/commands/appendedsig/appendedsig.c
> @@ -0,0 +1,608 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2020, 2021, 2022 Free Software Foundation, Inc.
> + *  Copyright (C) 2020, 2021, 2022, 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/types.h>
> +#include <grub/misc.h>
> +#include <grub/mm.h>
> +#include <grub/err.h>
> +#include <grub/dl.h>
> +#include <grub/file.h>
> +#include <grub/command.h>
> +#include <grub/crypto.h>
> +#include <grub/i18n.h>
> +#include <grub/gcrypt/gcrypt.h>
> +#include <grub/kernel.h>
> +#include <grub/extcmd.h>
> +#include <grub/verify.h>
> +#include <libtasn1.h>
> +#include <grub/env.h>
> +#include <grub/lockdown.h>
> +
> +#include "appendedsig.h"
> +
> +GRUB_MOD_LICENSE ("GPLv3+");
> +
> +/* Public key type. */
> +#define GRUB_PKEY_ID_PKCS7 2
> +
> +/* Appended signature magic string. */
> +static const char magic[] = "~Module signature appended~\n";
> +
> +/*
> + * This structure is extracted from scripts/sign-file.c in the linux kernel
> + * source. It was licensed as LGPLv2.1+, which is GPLv3+ compatible.
> + */
> +struct module_signature
> +{
> +  grub_uint8_t algo;       /* Public-key crypto algorithm [0]. */
> +  grub_uint8_t hash;       /* Digest algorithm [0]. */
> +  grub_uint8_t id_type;    /* Key identifier type [GRUB_PKEY_ID_PKCS7]. */
> +  grub_uint8_t signer_len; /* Length of signer's name [0]. */
> +  grub_uint8_t key_id_len; /* Length of key identifier [0]. */
> +  grub_uint8_t __pad[3];
> +  grub_uint32_t sig_len;   /* Length of signature data. */
> +} GRUB_PACKED;
> +
> +/* This represents an entire, parsed, appended signature. */
> +struct grub_appended_signature
> +{
> +  grub_size_t signature_len;            /* Length of PKCS#7 data + metadata 
> + magic. */
> +  struct module_signature sig_metadata; /* Module signature metadata. */
> +  struct pkcs7_signedData pkcs7;        /* Parsed PKCS#7 data. */
> +};
> +
> +/* Trusted certificates for verifying appended signatures. */
> +struct x509_certificate *db;
> +
> +static enum
> +{
> +  CHECK_SIGS_NO = 0,
> +  CHECK_SIGS_ENFORCE = 1,
> +  CHECK_SIGS_FORCED = 2

Is there any particular reason to have values assigned to constants?
If not please drop all of them.

And I think difference between CHECK_SIGS_ENFORCE and CHECK_SIGS_FORCED
should be explained. If they are really different...

> +} check_sigs = CHECK_SIGS_NO;
> +
> +static const char *
> +grub_env_read_sec (struct grub_env_var *var __attribute__ ((unused)),
> +                   const char *val __attribute__ ((unused)))
> +{
> +  if (check_sigs == CHECK_SIGS_FORCED)
> +    return "forced";
> +  else if (check_sigs == CHECK_SIGS_ENFORCE)

You can drop "else" here...

> +    return "enforce";
> +
> +  return "no";

I have an impression that these values are not fully documented. And
they should be...

> +}
> +
> +static char *
> +grub_env_write_sec (struct grub_env_var *var __attribute__ ((unused)), const 
> char *val)
> +{
> +  char *ret;
> +
> +  /* Do not allow the value to be changed if set to forced. */
> +  if (check_sigs == CHECK_SIGS_FORCED)
> +    {
> +      ret = grub_strdup ("forced");
> +      if (ret == NULL)
> +        grub_error (GRUB_ERR_OUT_OF_MEMORY, "could not duplicate a string 
> forced.");

s/."/"/ here and everywhere...

> +      return ret;
> +    }
> +
> +  if ((*val == '2') || (*val == 'f'))
> +    check_sigs = CHECK_SIGS_FORCED;
> +  else if ((*val == '1') || (*val == 'e'))
> +    check_sigs = CHECK_SIGS_ENFORCE;
> +  else if ((*val == '0') || (*val == 'n'))
> +    check_sigs = CHECK_SIGS_NO;
> +
> +  ret = grub_strdup (grub_env_read_sec (NULL, NULL));
> +  if (ret == NULL)
> +    grub_error (GRUB_ERR_OUT_OF_MEMORY, "could not duplicate a string %s",
> +                grub_env_read_sec (NULL, NULL));
> +
> +  return ret;
> +}
> +
> +static grub_err_t
> +file_read_all (grub_file_t file, grub_uint8_t **buf, grub_size_t *len)

s/file_read_all/file_read_whole/

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to