Feedback Request: Implement Fuzzers and Add to OSS-Fuzz

[Andrew Hamilton]

Hello,

I’m looking for feedback on whether there would be project interest /
support on me creating an initial fuzz test suite for some core GRUB
functions and then integrating these fuzzers into the OSS-Fuzz project that
would run them and automate bug reporting to project owners / maintainers
on some predefined periodic basis.
https://github.com/google/oss-fuzz

Fuzzing helps find program correctness issues like out of bounds accesses,
use after free, crashes, etc. that can sometimes be used for malicious
purposes.

To get a sense of what a fuzzer suite looks like you could reference an
existing project integrated into oss-fuzz like GNUTLS:
https://gitlab.com/gnutls/gnutls/-/tree/master/fuzz?ref_type=heads

There is some project setup as well in the oss-fuzz repo similar to:
https://github.com/google/oss-fuzz/tree/master/projects/gnutls

If there is favorable feedback I would perform the following general steps:
1. Identify core GRUB functions to fuzz first (likely those usable in
lockdown and seen as most likely to be widely used at first, and that are
seen as a possible source of untrusted input).
2. In a private fork of GRUB (with core GRUB maintainers invited):
  a. Get a fuzz target added to the build system.
  b. Implement fuzzers for the core features.
  c. Fix issues identified by the fuzzers, if any.
  d. Create docs around how to build the fuzzers, run them, and locally
troubleshoot issues
   e. Submit patches to the maintainers and (if not sensitive due to
uncovered vulnerabilities) to this mailing list for review.
   f. With maintainers support, get the fuzzers merged to GRUB mainline.
3. Open a new project request in the oss-fuzz repo following their
documentation:
 a.
https://github.com/google/oss-fuzz/blob/master/docs/getting-started/accepting_new_projects.md
 b.
https://google.github.io/oss-fuzz/getting-started/new-project-guide/
4. If the project approves I’d be willing to serve as the maintainer of the
fuzzing and oss-fuzz components to ensure build failures are corrected and
newly identified issues are worked and triaged.
5. Over time we can work to improve code coverage via the fuzzers.


Pros:
1. Increase the code robustness
2. Quickly find robustness issues caused by future changes
Cons:
1. Requires up front development to get going- I’m happy to do as much of
this as I can but it will require support from the busy project maintainers
for reviews and such
2. Requires staying on top of new fuzz failures prior to the oss-fuzz
disclosure deadline (I’m happy to support this as much as I can) - ref
https://google.github.io/oss-fuzz/getting-started/bug-disclosure-guidelines/


Please let me know your thoughts.

Thanks,
Andrew
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v4 5/6] commands/ls: Print full paths for file args

[Daniel Kiper]

On Thu, Feb 27, 2025 at 12:28:31AM -0600, Glenn Washburn wrote:
> On Mon, 24 Feb 2025 18:14:59 +0100 Daniel Kiper  wrote:
> > On Mon, Jan 06, 2025 at 01:02:43AM -0600, Glenn Washburn wrote:
> > > For arguments that are paths to files, print the full path of the file.
> > >
> > > Signed-off-by: Glenn Washburn 
> > > ---
> > >  grub-core/commands/ls.c | 8 +---
> > >  1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/grub-core/commands/ls.c b/grub-core/commands/ls.c
> > > index e33c16158d63..384d3e3cede8 100644
> > > --- a/grub-core/commands/ls.c
> > > +++ b/grub-core/commands/ls.c
> > > @@ -98,6 +98,7 @@ static int
> > >  print_file (const char *filename, const struct grub_dirhook_info *info,
> > > void *data)
> > >  {
> > > +  char *pathname = NULL;
> > >struct grub_ls_list_files_ctx *ctx = data;
> > >
> > >if ((! ctx->all) && (filename[0] == '.'))
> > > @@ -117,7 +118,6 @@ print_file (const char *filename, const struct 
> > > grub_dirhook_info *info,
> > >if (! info->dir)
> > >  {
> > >grub_file_t file;
> > > -  char *pathname;
> > >
> > >if (ctx->dirname[grub_strlen (ctx->dirname) - 1] == '/')
> > >   pathname = grub_xasprintf ("%s%s", ctx->dirname, filename);
> > > @@ -143,7 +143,6 @@ print_file (const char *filename, const struct 
> > > grub_dirhook_info *info,
> > >else
> > >   grub_xputs ("");
> > >
> > > -  grub_free (pathname);
> > >grub_errno = GRUB_ERR_NONE;
> > >  }
> > >else
> > > @@ -165,7 +164,10 @@ print_file (const char *filename, const struct 
> > > grub_dirhook_info *info,
> > >datetime.day, datetime.hour,
> > >datetime.minute, datetime.second);
> > >  }
> > > -  grub_printf ("%s%s\n", filename, info->dir ? "/" : "");
> > > +  grub_printf ("%s%s\n", (ctx->filename) ? pathname : filename,
> >
> > "(pathname != NULL) ? pathname : filename" would not be more 
> > natural/correct?
>
> I think this is equivalent. Yes, this does seem more intuitive. Do you
> want me to update and send a v5?

Yes, please! And feel free to add my RB to all patches...

Daniel

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

Re: [PATCH 3/3] blsuki: Add uki command to load Unified Kernel Image entries

[Ross Philipson via Grub-devel]

On 2/14/25 8:40 AM, Alec Brown wrote:

A Unified Kernel Image is a single UEFI PE file that combines a UEFI boot stub,
a Linux kernel image, an initrd, and further resources. The uki command will
locate where the uki file is and create a GRUB menu entry to load it.


I chatted with Alec yesterday and we went over this patch. There are a 
few things that need attention here, see below.




Signed-off-by: Alec Brown 
---
  docs/grub.texi  |  26 ++
  grub-core/commands/blsuki.c | 712 +++-
  include/grub/menu.h |   2 +
  3 files changed, 562 insertions(+), 178 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index a8465fc0b..ecf261717 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -4403,6 +4403,7 @@ you forget a command, you can run the command 
@command{help}
  * test::Check file types and compare values
  * true::Do nothing, successfully
  * trust::   Add public key to list of trusted keys
+* uki:: Load Unified Kernel Image menu entries
  * unset::   Unset an environment variable
  @comment * vbeinfo:: List available video modes
  * verify_detached:: Verify detached digital signature
@@ -5904,6 +5905,31 @@ Unset the environment variable @var{envvar}.
  @end deffn
  
  
+@node uki

+@subsection uki
+
+@deffn Command uki [@option{--path} dir] [@option{--show-default}] 
[@option{--show-non-default}] [@option{--entry} file]
+Load Unified Kernel Image entries into the GRUB menu.
+
+The @option{--path} option overrides the default path to the directory 
containing
+the UKI entries. If this option isn't used, the default location is
+/EFI/Linux in the EFI system partition.
+
+The @option{--show-default} option allows the default boot entry to be added 
to the
+GRUB menu from the UKI entries.
+
+The @option{--show-non-default} option allows non-default boot entries to be 
added to
+the GRUB menu from the UKI entries.
+
+The @option{--entry} option allows specific boot entries to be added to the 
GRUB menu
+from the UKI entries.
+
+The @option{--entry}, @option{--show-default}, and @option{--show-non-default} 
options
+are used to filter which UKI entries are added to the GRUB menu. If none are
+used, all entries in the default location or the location specified by 
@option{--path}
+will be added to the GRUB menu.
+@end deffn
+
  @ignore
  @node vbeinfo
  @subsection vbeinfo
diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c
index 8a3c5818f..3c26f2a56 100644
--- a/grub-core/commands/blsuki.c
+++ b/grub-core/commands/blsuki.c
@@ -39,11 +39,23 @@
  #define GRUB_BOOT_DEVICE "/"
  #endif
  
+#ifdef GRUB_MACHINE_EFI

+#include 
+#include 
+#include 
+#endif
+
  GRUB_MOD_LICENSE ("GPLv3+");
  
  #define GRUB_BLS_CONFIG_PATH "/loader/entries/"

+#define GRUB_UKI_CONFIG_PATH "/EFI/Linux"
  
-static const struct grub_arg_option opt[] =

+#define GRUB_BLS_CMD 1
+#define GRUB_UKI_CMD 2
+
+static int cmd_type = 0;
+
+static const struct grub_arg_option bls_opt[] =
{
  {"path", 'p', 0, N_("Specify path to find BLS entries."), N_("DIR"), 
ARG_TYPE_PATHNAME},
  {"show-default", 'd', 0, N_("Allow the default BLS entry to be added to the 
GRUB menu."), 0, ARG_TYPE_NONE},
@@ -52,6 +64,17 @@ static const struct grub_arg_option opt[] =
  {0, 0, 0, 0, 0, 0}
};
  
+#ifdef GRUB_MACHINE_EFI

+static const struct grub_arg_option uki_opt[] =
+  {
+{"path", 'p', 0, N_("Specify path to find UKI entries."), N_("DIR"), 
ARG_TYPE_PATHNAME},
+{"show-default", 'd', 0, N_("Allow the default UKI entry to be added to the 
GRUB menu."), 0, ARG_TYPE_NONE},
+{"show-non-default", 'n', 0, N_("Allow the non-default UKI entries to be added 
to the GRUB menu."), 0, ARG_TYPE_NONE},
+{"entry", 'e', 0, N_("Allow specificUKII entries to be added to the GRUB menu."), 
N_("FILE"), ARG_TYPE_FILE},
+{0, 0, 0, 0, 0, 0}
+  };
+#endif
+
  struct keyval
  {
const char *key;
@@ -270,7 +293,7 @@ bls_read_entry (grub_file_t f, grub_blsuki_entry_t *entry)
  break;
}
  
-   separator[0] = '\0';

+  separator[0] = '\0';
  
do

{
@@ -287,6 +310,183 @@ bls_read_entry (grub_file_t f, grub_blsuki_entry_t *entry)
return rc;
  }
  
+#ifdef GRUB_MACHINE_EFI


This function and the bls_read_entry() both just return 1 or 0 to 
indicate failure or success. These functions could just be changed to 
return a bool because of this.



+static int
+uki_read_entry (grub_file_t f, grub_blsuki_entry_t *entry)
+{
+  struct grub_msdos_image_header *dos;
+  struct grub_pe_image_header *pe;
+  grub_off_t section_offset = 0;
+  struct grub_pe32_section_table *section;
+  struct grub_pe32_coff_header *coff_header;
+
+  dos = grub_zalloc (sizeof (*dos));
+  if (dos == NULL)
+return 1;
+  if (grub_file_read (f, dos, sizeof (*dos)) < (grub_ssize_t) sizeof (*dos))
+{
+  grub_dpri

Re: [PATCH 0/6 v13] LVM Cachevol and Integrity volumes break entire LVM VG

[Patrick Plenefisch]

I just rebased them with no changes except for line numbers shifting,
and pushed to https://github.com/byteit101/grub2/commits/grub-lvmintegrity/
and attached them as patches with --thread. I unfortunately don't have
time to figure out how to get send-email to work for the next few
weeks as I'm travelling until late March.

Patrick

On Wed, Feb 26, 2025 at 3:09 PM Daniel Kiper  wrote:
>
> On Thu, Feb 20, 2025 at 06:52:36PM -0500, Patrick Plenefisch wrote:
> > Awesome! What do I do now?
>
> Sadly your patches conflict with current master... :-( Could you rebase
> them? And if you could use "git send-email" to send patches that would
> be perfect...
>
> Sorry for inconvenience...
>
> Daniel
From c4e94fb6b30af415b5b0e089a8b6461c4b1a6698 Mon Sep 17 00:00:00 2001
Message-Id: 
In-Reply-To: 
References: 
From: Patrick Plenefisch 
Date: Sat, 4 Jan 2025 15:02:54 -0500
Subject: [PATCH 4/6] lvm: Add support for integrity lv

lv matching must be done after processing the ignored feature
indirections, as integrity volumes & caches may have several levels
of indirection that the segments must be shifted through.

Signed-off-by: Patrick Plenefisch 
Reviewed-by: Daniel Kiper 
---
 grub-core/disk/lvm.c | 70 
 1 file changed, 38 insertions(+), 32 deletions(-)

diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index f17b43655..8f84642bc 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -817,8 +817,14 @@ grub_lvm_detect (grub_disk_t disk,
 			  seg->nodes[seg->node_count - 1].name = tmp;
 			}
 		}
-		  else if (grub_memcmp (p, "cache\"",
-   sizeof ("cache\"") - 1) == 0)
+		  /*
+		   * Cache and integrity LVs have extra parts that
+		   * we can ignore for our read-only access
+		   */
+		  else if (grub_strncmp (p, "cache\"",
+   sizeof ("cache\"") - 1) == 0
+   || grub_strncmp (p, "integrity\"",
+   sizeof ("integrity\"") - 1) == 0)
 		{
 		  struct ignored_feature_lv *ignored_feature = NULL;
 
@@ -948,36 +954,6 @@ grub_lvm_detect (grub_disk_t disk,
 	}
 	}
 
-  /* Match lvs.  */
-  {
-	struct grub_diskfilter_lv *lv1;
-	struct grub_diskfilter_lv *lv2;
-	for (lv1 = vg->lvs; lv1; lv1 = lv1->next)
-	  for (i = 0; i < lv1->segment_count; i++)
-	for (j = 0; j < lv1->segments[i].node_count; j++)
-	  {
-		if (vg->pvs)
-		  for (pv = vg->pvs; pv; pv = pv->next)
-		{
-		  if (! grub_strcmp (pv->name,
-	 lv1->segments[i].nodes[j].name))
-			{
-			  lv1->segments[i].nodes[j].pv = pv;
-			  break;
-			}
-		}
-		if (lv1->segments[i].nodes[j].pv == NULL)
-		  for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
-		{
-		  if (lv1 == lv2)
-		continue;
-		  if (grub_strcmp (lv2->name,
-   lv1->segments[i].nodes[j].name) == 0)
-			lv1->segments[i].nodes[j].lv = lv2;
-		}
-	  }
-
-  }
 
   {
 	struct ignored_feature_lv *ignored_feature;
@@ -1029,6 +1005,36 @@ grub_lvm_detect (grub_disk_t disk,
 	  }
   }
 
+  /* Match lvs. Must be done after cache and integrity are found  */
+  {
+	struct grub_diskfilter_lv *lv1;
+	struct grub_diskfilter_lv *lv2;
+	for (lv1 = vg->lvs; lv1; lv1 = lv1->next)
+	  for (i = 0; i < lv1->segment_count; i++)
+	for (j = 0; j < lv1->segments[i].node_count; j++)
+	  {
+		if (vg->pvs)
+		  for (pv = vg->pvs; pv; pv = pv->next)
+		{
+		  if (! grub_strcmp (pv->name,
+	 lv1->segments[i].nodes[j].name))
+			{
+			  lv1->segments[i].nodes[j].pv = pv;
+			  break;
+			}
+		}
+		if (lv1->segments[i].nodes[j].pv == NULL)
+		  for (lv2 = vg->lvs; lv2; lv2 = lv2->next)
+		{
+		  if (lv1 == lv2)
+		continue;
+		  if (grub_strcmp (lv2->name,
+   lv1->segments[i].nodes[j].name) == 0)
+			lv1->segments[i].nodes[j].lv = lv2;
+		}
+	  }
+  }
+
 	  grub_lvm_free_ignored_feature_lvs (ignored_feature_lvs);
   if (grub_diskfilter_vg_register (vg))
 	goto fail4;
-- 
2.39.5

From 0c194e569bcf4d273a1f3c7e749259f66ec932a4 Mon Sep 17 00:00:00 2001
Message-Id: <0c194e569bcf4d273a1f3c7e749259f66ec932a4.1740687079.git.simonp...@gmail.com>
In-Reply-To: 
References: 
From: Patrick Plenefisch 
Date: Tue, 13 Aug 2024 20:15:37 -0400
Subject: [PATCH 2/6] disk/lvm: Remove unused cache_pool

cache_pool is never read or used, remove it

Signed-off-by: Patrick Plenefisch 
Reviewed-by: Daniel Kiper 
---
 grub-core/disk/lvm.c | 25 -
 1 file changed, 25 deletions(-)

diff --git a/grub-core/disk/lvm.c b/grub-core/disk/lvm.c
index 97932fe9a..f17b43655 100644
--- a/grub-core/disk/lvm.c
+++ b/grub-core/disk/lvm.c
@@ -37,7 +37,6 @@ GRUB_MOD_LICENSE ("GPLv3+");
 struct ignored_feature_lv
 {
   struct grub_diskfilter_lv *lv;
-  char *cache_pool;
   char *origin;
   struct ignored_feature_lv *next;
 };
@@ -127,7 +126,6 @@ grub_lvm_free_ignored_feature_lvs (struct ignored_feature_lv *ignored_feature_lv
 	}
   grub_free (ignored_feature->lv);
   grub_free (ignored_feature->origin);
-  grub_free 

Re: [SECURITY PATCH 00/73] GRUB2 vulnerabilities - 2025/02/18

[Christian Hesse]

Daniel Kiper via Grub-devel  on Mon, 2025/02/24 15:34:
> > [...]
> > The current situation is just insane.  
> 
> I can understand your frustration but I am afraid we are not able to do
> much about it at this point. Sorry... We have problems with finding
> people doing security patches, forward porting, reviews, tests, etc.
> So, simply we do not have resources to maintain point releases either.
> Though if somebody wants step up and make it I am happy with that...

Well, that is... unfortunate.
But I can understand that, my time is limited as well.

Anyway... Any chance for better communication? Would be nice to have
information and access to the changes in advance (under embargo). That way we
could at least test and evaluation without pressure before pushing anything.
Thanks!
-- 
main(a){char*c=/*Schoene Gruesse */"B?IJj;MEH"
"CX:;",b;for(a/*Best regards my address:*/=0;b=c[a++];)
putchar(b-1/(/*Chriscc -ox -xc - && ./x*/b/42*2-3)*42);}


pgpwcX4hmYzp7.pgp
Description: OpenPGP digital signature
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v1 19/21] appendedsig: Reads the default DB keys from ELF Note

[sudhakar]

On 2025-01-02 18:49, Stefan Berger wrote:

On 12/18/24 9:56 AM, Sudhakar Kuppusamy wrote:

if secure boot enabled with PKS and set use_static_keys flag, it


If Secure Boot is enabled with PKS and the use_static_keys flag is
set, then read the DB default keys from the ELF note and store them in
the trusted list buffer.

reads the DB default keys from ELF Note and store it in trusted list 
buffer.


Signed-off-by: Sudhakar Kuppusamy 
---
  grub-core/commands/appendedsig/appendedsig.c | 58 
++--

  1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/grub-core/commands/appendedsig/appendedsig.c 
b/grub-core/commands/appendedsig/appendedsig.c

index 8b084087e..9a9f4ef1c 100644
--- a/grub-core/commands/appendedsig/appendedsig.c
+++ b/grub-core/commands/appendedsig/appendedsig.c
@@ -1082,7 +1082,7 @@ grub_create_distrusted_list (void)
   * parses it, and adds it to the trusted list.
   */
  static grub_err_t
-grub_build_static_trusted_list (const struct grub_module_header 
*header)
+grub_build_static_trusted_list (const struct grub_module_header 
*header, const grub_bool_t mode)


A more meaningful variable name than 'mode' would be good. mode = true
or false doesn't mean much.


  {
grub_err_t err = GRUB_ERR_NONE;
struct grub_file pseudo_file;
@@ -1101,7 +1101,14 @@ grub_build_static_trusted_list (const struct 
grub_module_header *header)

if (err != GRUB_ERR_NONE)
  return err;
  -  err = grub_add_certificate (cert_data, cert_data_size, &grub_db, 
1);

+  if (mode)
+{
+  err = grub_is_distrusted_cert_hash (cert_data, cert_data_size);
+  if (err != GRUB_ERR_NONE)
+return err;
+}
+
+  err = grub_add_certificate (cert_data, cert_data_size, &grub_db, 
mode);

if (cert_data != NULL)
  grub_free (cert_data);
  @@ -1154,6 +1161,20 @@ grub_release_distrusted_list (void)
grub_memset (&grub_dbx, 0x00, sizeof (grub_dbx));
  }
  +static grub_err_t
+grub_load_static_keys (const struct grub_module_header *header, const 
grub_bool_t mode)

+{
+  int rc = GRUB_ERR_NONE;
+  FOR_MODULES (header)
+{
+  /* Not an ELF module, skip.  */
+  if (header->type != OBJ_TYPE_X509_PUBKEY)
+continue;
+  rc = grub_build_static_trusted_list (header, mode);


Do you have to check rc at this point?


+}
+  return rc;
+}
+
  GRUB_MOD_INIT (appendedsig)
  {
int rc;
@@ -1172,26 +1193,29 @@ GRUB_MOD_INIT (appendedsig)
  if (!grub_use_platform_keystore && check_sigs == 
check_sigs_forced)

  {
-  FOR_MODULES (header)
+  rc = grub_load_static_keys (header, 0);
+  if (rc != GRUB_ERR_NONE)
  {
-  /* Not an ELF module, skip.  */
-  if (header->type != OBJ_TYPE_X509_PUBKEY)
-continue;
-
-  rc = grub_build_static_trusted_list (header);
-  if (rc != GRUB_ERR_NONE)
-{
-  grub_release_trusted_list ();
-  grub_error (rc, "static trusted list creation failed");
-}
-  else
-grub_printf ("appendedsig: the trusted list now has %" 
PRIuGRUB_SIZE " static keys\n",

- grub_db.key_entries);
+  grub_release_trusted_list ();
+  grub_error (rc, "static trusted list creation failed");
  }
+  else
+grub_printf ("appendedsig: the trusted list now has %" 
PRIuGRUB_SIZE " static keys\n",

+ grub_db.key_entries);
+
  }
else if (grub_use_platform_keystore && check_sigs == 
check_sigs_forced)

  {
-  rc = grub_create_trusted_list ();
+
+  if (grub_platform_keystore.use_static_keys == 1)


if (grub_platform_keystore.use_static_keys)


+{
+  grub_printf ("Warning: db variable is not available at PKS 
and using a static keys "

+   "as a default key in trusted list\n");
+  rc = grub_load_static_keys (header, 1);
+}
+  else
+rc = grub_create_trusted_list ();
+
if (rc != GRUB_ERR_NONE)
  {
grub_release_trusted_list ();



addressed all the comments. thank you Stefan.

Thanks,
Sudhakar Kuppusamy

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

Re: [PATCH v1 16/21] appendedsig: The creation of trusted and distrusted lists

[sudhakar]

On 2024-12-31 22:51, Stefan Berger wrote:

On 12/18/24 9:56 AM, Sudhakar Kuppusamy wrote:
The trusted certificates and binary hashes, distrusted certificates 
and
binary/certificate hashes will be extracted from the platform keystore 
buffer

if Secure Boot is enabled with PKS.
In order to verify the integerity of the kernel, the extracted data


integrity


would be stored in the buffer db and dbx.


needs to be stored?



The trusted certificates will be extracted from the grub ELFNOTE if 
Secure Boot is
enabled with static key. In order to verify the integerity of the 
kernel,


integrity


the extracted data would be stored in the buffer db.


needs to be stored?



Note:-

if the trusted certificate nor binary hash exists in the distrusted 
list (DBX),


If neither the ... nor ...


rejected it while extracting it from the platform keystore buffer.


what is 'it' in this context



Signed-off-by: Sudhakar Kuppusamy 
---
  grub-core/commands/appendedsig/appendedsig.c | 636 
+--

  1 file changed, 592 insertions(+), 44 deletions(-)

diff --git a/grub-core/commands/appendedsig/appendedsig.c 
b/grub-core/commands/appendedsig/appendedsig.c

index 5c82b96a4..31649e800 100644
--- a/grub-core/commands/appendedsig/appendedsig.c
+++ b/grub-core/commands/appendedsig/appendedsig.c
@@ -34,7 +34,7 @@
  #include 
  #include 
  #include 
-
+#include 
  #include "appendedsig.h"
GRUB_MOD_LICENSE ("GPLv3+");
@@ -64,9 +64,23 @@ struct grub_appended_signature
struct pkcs7_signedData pkcs7;/* Parsed PKCS#7 data */
  };
  -/* Trusted certificates for verifying appended signatures */
-struct x509_certificate *grub_trusted_key;
+/* This represents a trusted/distrusted list*/
+struct grub_database
+{
+  struct x509_certificate *keys; /* Certificates */
+  grub_size_t key_entries;   /* Number of certificates */
+  grub_uint8_t **signatures; /* Certificate/binary hashes */
+  grub_size_t *signature_size;   /* Size of certificate/binary hashes 
*/
+  grub_size_t signature_entries; /* Number of certificate/binary 
hashes */

+};
+
+/* Trusted list */
+struct grub_database grub_db = {.keys = NULL, .key_entries = 0, 
.signatures = NULL,
+.signature_size = NULL, 
.signature_entries = 0};

  +/* Distrusted list */
+struct grub_database grub_dbx = {.signatures = NULL, .signature_size 
= NULL,

+ .signature_entries = 0};
  /*
   * Force gcry_rsa to be a module dependency.
   *
@@ -87,6 +101,13 @@ struct x509_certificate *grub_trusted_key;
   * also resolves our concerns about loading from the filesystem.
   */
  extern gcry_pk_spec_t _gcry_pubkey_spec_rsa;
+extern gcry_md_spec_t _gcry_digest_spec_sha224;
+extern gcry_md_spec_t _gcry_digest_spec_sha384;
+
+/* releasing trusted list memory */


Typically these functions start with 'free'. Free trusted list memory.


+static void grub_release_trusted_list (void);
+/* releasing distrusted list memory */
+static void grub_release_distrusted_list (void);


I am not sure whether local functions and variables should be prefixed
with grub_ since typically those prefixed wit grub_ are global
functions. This comment applies to all functions in this patch and
series.


static enum
  {
@@ -95,6 +116,248 @@ static enum
check_sigs_forced = 2
  } check_sigs = check_sigs_no;
  +/*
+ * GUID can be used to determine the hashing function and
+ * generate the hash using determined hashing function.
+ */
+static grub_err_t
+grub_get_hash (const grub_uuid_t *guid, const grub_uint8_t *data, 
const grub_size_t data_size,

+   grub_uint8_t *hash, grub_size_t *hash_size)
+{
+  gcry_md_spec_t *hash_func = NULL;
+
+  if (guid == NULL)
+return grub_error (GRUB_ERR_OUT_OF_RANGE, "GUID is null");
+
+  if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA256_GUID, GRUB_UUID_SIZE) 
== 0 ||
+   grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA256_GUID, 
GRUB_UUID_SIZE) == 0)

+hash_func = &_gcry_digest_spec_sha256;
+  else if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA384_GUID, 
GRUB_UUID_SIZE) == 0 ||
+   grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA384_GUID, 
GRUB_UUID_SIZE) == 0)

+hash_func = &_gcry_digest_spec_sha384;
+  else if (grub_memcmp (guid, &GRUB_PKS_CERT_SHA512_GUID, 
GRUB_UUID_SIZE) == 0 ||
+   grub_memcmp (guid, &GRUB_PKS_CERT_X509_SHA512_GUID, 
GRUB_UUID_SIZE) == 0)

+hash_func = &_gcry_digest_spec_sha512;
+  else
+return GRUB_ERR_UNKNOWN_COMMAND;


return grub_error (GRUB_ERR_OUT_OF_RANGE, "Unsupported GUID for hash")


+
+  grub_memset (hash, 0x00, GRUB_MAX_HASH_SIZE);
+  grub_crypto_hash (hash_func, hash, data, data_size);
+  *hash_size =  hash_func->mdlen;


grub_memset(hash, 0, *hash_size);


+
+  return GRUB_ERR_NONE;
+}
+
+/* adding the certificate/binary hash into the trusted/distrusted 
list */


Add the ...


+static grub_err_t
+grub_add_hash (const grub_uint8_t **data, const grub_size_t 
data_size,
+   grub_uint8_t ***signature_list, grub_siz

Re: [PATCH v1 17/21] appendedsig: While verifying the kernel, use trusted and distrusted lists

[sudhakar]

On 2024-12-31 23:07, Stefan Berger wrote:

On 12/18/24 9:56 AM, Sudhakar Kuppusamy wrote:
To verify the kernel's: verify the kernel binary against list of 
binary hashes


To verify the kernel's signature?

against lists of binary hashes

that are distrusted and trusted. If it is not listed in both trusted 
and distrusted,


that are either distrusted or trusted.

If it is not list in either trusted or distrusted hashes list then the
trusted keys from the trusted key list are used to verify the
signature.


the trusted keys from trusted key list used to verify the signature.

Signed-off-by: Sudhakar Kuppusamy 
---
  grub-core/commands/appendedsig/appendedsig.c | 188 
+--

  1 file changed, 133 insertions(+), 55 deletions(-)

diff --git a/grub-core/commands/appendedsig/appendedsig.c 
b/grub-core/commands/appendedsig/appendedsig.c

index 31649e800..8b084087e 100644
--- a/grub-core/commands/appendedsig/appendedsig.c
+++ b/grub-core/commands/appendedsig/appendedsig.c
@@ -497,6 +497,81 @@ extract_appended_signature (const grub_uint8_t 
*buf, grub_size_t bufsize,
return parse_pkcs7_signedData (appsigdata, pkcs7_size, 
&sig->pkcs7);

  }
  +static grub_err_t
+grub_get_binary_hash (const grub_size_t binary_hash_size, const 
grub_uint8_t *data,
+  const grub_size_t data_size, grub_uint8_t 
*hash, grub_size_t *hash_size)

+{
+  grub_uuid_t guid = { 0 };
+
+  /* support SHA256, SHA384 and SHA512 for binary hash */
+  if (binary_hash_size == 32)
+grub_memcpy (&guid, &GRUB_PKS_CERT_SHA256_GUID, GRUB_UUID_SIZE);
+  else if (binary_hash_size == 48)
+grub_memcpy (&guid, &GRUB_PKS_CERT_SHA384_GUID, GRUB_UUID_SIZE);
+  else if (binary_hash_size == 64)
+grub_memcpy (&guid, &GRUB_PKS_CERT_SHA512_GUID, GRUB_UUID_SIZE);
+  else
+{
+  grub_dprintf ("appendedsig", "unsupported hash type (%" 
PRIuGRUB_SIZE ") and skipping binary hash\n",

+binary_hash_size);
+  return GRUB_ERR_UNKNOWN_COMMAND;
+}
+
+  return grub_get_hash (&guid, data, data_size, hash, hash_size);
+}
+
+/*
+ * verify binary hash against the list of binary hashes that are 
distrusted

Verify a binary hash

+ * and trusted.


The following errors can occur:
- GRUB_ERR_BAD_SIGNATURE: indicates that the hash is distrusted.
- GRUB_ERR_NONE: the hash is trusted, since it was found in the
trusted hashes list
- GRUB_ERR_EOF: the hash could not be found in the hashes list


+ */
+static grub_err_t
+grub_verify_binary_hash (const grub_uint8_t *data, const grub_size_t 
data_size)

+{
+  grub_err_t rc = GRUB_ERR_NONE;
+  grub_size_t i = 0, hash_size = 0;
+  grub_uint8_t hash[GRUB_MAX_HASH_SIZE] = { 0 };
+
+  for (i = 0; i < grub_dbx.signature_entries; i++)
+{
+  rc = grub_get_binary_hash (grub_dbx.signature_size[i], data, 
data_size,

+ hash, &hash_size);
+  if (rc != GRUB_ERR_NONE)
+continue;
+
+  if (hash_size == grub_dbx.signature_size[i] &&
+  grub_memcmp (grub_dbx.signatures[i], hash, hash_size) == 0)
+{
+  grub_dprintf ("appendedsig", "the binary hash 
(%02x%02x%02x%02x) was listed "
+"as distrusted\n", hash[0], hash[1], hash[2], 
hash[3]);


merge the error string into one


+  return GRUB_ERR_BAD_SIGNATURE;
+}
+}
+
+  for (i = 0; i < grub_db.signature_entries; i++)
+{
+  rc = grub_get_binary_hash (grub_db.signature_size[i], data, 
data_size,

+ hash, &hash_size);
+  if (rc != GRUB_ERR_NONE)
+continue;
+
+  if (hash_size == grub_db.signature_size[i] &&
+  grub_memcmp (grub_db.signatures[i], hash, hash_size) == 0)
+{
+  grub_dprintf ("appendedsig", "verified with a trusted 
binary hash "
+"(%02x%02x%02x%02x)\n", hash[0], hash[1], 
hash[2], hash[3]);

+  return GRUB_ERR_NONE;
+}
+}
+
+  return GRUB_ERR_EOF;
+}
+
+
+/*
+ * verify the kernel's integrity, the trusted key will be used from
+ * the trusted key list. If it fails, verify it against the list of 
binary hashes

+ * that are distrusted and trusted.
+ */
  static grub_err_t
  grub_verify_appended_signature (const grub_uint8_t *buf, grub_size_t 
bufsize)

  {
@@ -506,12 +581,12 @@ grub_verify_appended_signature (const 
grub_uint8_t *buf, grub_size_t bufsize)

unsigned char *hash;
gcry_mpi_t hashmpi;
gcry_err_code_t rc;
-  struct x509_certificate *pk;
+  struct x509_certificate *cert;
struct grub_appended_signature sig;
struct pkcs7_signerInfo *si;
int i;
  -  if (!grub_db.key_entries)
+  if (!grub_db.key_entries && !grub_db.signature_entries)
  return grub_error (GRUB_ERR_BAD_SIGNATURE, N_("No trusted keys 
to verify against"));

  err = extract_appended_signature (buf, bufsize, &sig);
@@ -520,68 +595,71 @@ grub_verify_appended_signature (const 
grub_uint8_t *buf, grub_size_t bufsize)

  datasize = bufsize - sig.signature_len;
  -  for (i = 0; i < si

Re: [PATCH v1 18/21] ieee1275: set use_static_keys flag

[sudhakar]

On 2025-01-02 18:52, Stefan Berger wrote:

On 12/18/24 9:56 AM, Sudhakar Kuppusamy wrote:

if secure boot enabled with PKS, it set the use_static_keys flag


I was not sure at this point what the patch actually does so I
reformulated it a bit. I would start the patch description with the
reason why you are introducing the use_static_key:

Introduce the use_static_keys flag to indicate that static keys are to
be used rather than keys from the PKS storage's DB variable. This
variable is set when Secure Boot is enabled with PKS but the DB
variable is not present in the PKS storage. The appendedsig module
would use this variable to extract the default DB keys from the ELF
note and store the keys found there in the trustedlist... or something
like this.


when DB variable is not present in PKS storage and the appendedsig 
(module)

would use it later to extract the default DB key's from ELF Note and
store it in trustedlist.

Signed-off-by: Sudhakar Kuppusamy 
---
  grub-core/kern/ieee1275/platform_keystore.c | 15 ++-
  include/grub/platform_keystore.h| 12 +++-
  2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/grub-core/kern/ieee1275/platform_keystore.c 
b/grub-core/kern/ieee1275/platform_keystore.c

index 1c564d5da..ddc11afd9 100644
--- a/grub-core/kern/ieee1275/platform_keystore.c
+++ b/grub-core/kern/ieee1275/platform_keystore.c
@@ -34,7 +34,11 @@
  /* Platform Keystore */
  static grub_size_t pks_max_object_size;
  grub_uint8_t grub_use_platform_keystore = 0;
-grub_pks_t grub_platform_keystore = { .db = NULL, .dbx = NULL, 
.db_entries = 0, .dbx_entries = 0 };

+grub_pks_t grub_platform_keystore = { .db = NULL,
+  .dbx = NULL,
+  .db_entries = 0,
+  .dbx_entries = 0,
+  .use_static_keys = 0 };


Use false


/* converts the esl data into the ESL */
  static grub_esl_t *
@@ -316,6 +320,15 @@ grub_platform_keystore_init (void)
/* DB */
rc = grub_read_secure_boot_variables (0, DB, 
&grub_platform_keystore.db,
  
&grub_platform_keystore.db_entries);

+  if (rc == PKS_OBJECT_NOT_FOUND)
+{
+  rc = GRUB_ERR_NONE;
+  /*
+   * DB variable won't be available by default in PKS.


The DB variable.


+   * So, it will loads the Default Keys from ELF Note */


s/loads/load
-> If it is not available load the default keys from the ELF node (?)



+  grub_platform_keystore.use_static_keys = 1;
+}
+
if (rc == GRUB_ERR_NONE)
  {
/* DBX */
diff --git a/include/grub/platform_keystore.h 
b/include/grub/platform_keystore.h

index 7a7378926..b333db7fd 100644
--- a/include/grub/platform_keystore.h
+++ b/include/grub/platform_keystore.h
@@ -49,6 +49,7 @@
  #define GRUB_UUID_SIZE 16
  #define GRUB_MAX_HASH_SIZE 64
  +typedef grub_uint8_t grub_bool_t;


This would be a candidate for types.h. term/tparam.c already defines
it as char. bool also already exists and is also used in some places.


  typedef struct grub_uuid grub_uuid_t;
  typedef struct grub_esd grub_esd_t;
  typedef struct grub_esl grub_esl_t;
@@ -207,10 +208,11 @@ struct grub_pks_sd
  /* The structure of a PKS.*/
  struct grub_pks
  {
-  grub_pks_sd_t *db;/* signature database */
-  grub_pks_sd_t *dbx;   /* forbidden signature database */
-  grub_size_t db_entries;   /* size of signature database */
-  grub_size_t dbx_entries;  /* size of forbidden signature database 
*/

+  grub_pks_sd_t *db;  /* signature database */
+  grub_pks_sd_t *dbx; /* forbidden signature database */
+  grub_size_t db_entries; /* size of signature database */
+  grub_size_t dbx_entries;/* size of forbidden signature database 
*/
+  grub_bool_t use_static_keys;/* flag to indicate use of static keys 
*/

  } GRUB_PACKED;
#ifdef __powerpc__
@@ -225,7 +227,7 @@ extern grub_pks_t 
EXPORT_VAR(grub_platform_keystore);

  #else
#define grub_use_platform_keystore  0
-grub_pks_t grub_platform_keystore = {NULL, NULL, 0, 0};
+grub_pks_t grub_platform_keystore = {NULL, NULL, 0, 0, 0};


Use false


  void grub_release_platform_keystore (void);
#endif



Addressed all the comments. Thank you Stefan.

Thanks,
Sudhakar Kuppusamy

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

Re: [PATCH v1 05/21] pgp: factor out rsa_pad

[sudhakar]

On 2025-01-05 00:10, Vladimir 'phcoder' Serbinenko wrote:

rsa_pad will be removed when we update libgcrypt (see pending patch).
Can we accommodate for this?

On Wed, Dec 18, 2024 at 5:58 PM Sudhakar Kuppusamy
 wrote:


From: Daniel Axtens 

rsa_pad does the PKCS#1 v1.5 padding for the RSA signature scheme.
We want to use it in other RSA signature verification applications.

I considered and rejected putting it in lib/crypto.c. That file 
doesn't

currently require any MPI functions, but rsa_pad does. That's not so
much of a problem for the grub kernel and modules, but crypto.c also
gets built into all the grub utilities. So - despite the utils not
using any asymmetric ciphers -  we would need to built the entire MPI
infrastructure in to them.

A better and simpler solution is just to spin rsa_pad out into its own
PKCS#1 v1.5 module.

Signed-off-by: Daniel Axtens 
Signed-off-by: Sudhakar Kuppusamy 
---
 grub-core/Makefile.core.def |  8 +
 grub-core/commands/pgp.c| 28 ++
 grub-core/lib/pkcs1_v15.c   | 59 
+

 include/grub/pkcs1_v15.h| 27 +
 4 files changed, 96 insertions(+), 26 deletions(-)
 create mode 100644 grub-core/lib/pkcs1_v15.c
 create mode 100644 include/grub/pkcs1_v15.h

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index f70e02e69..60db2adc5 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2540,6 +2540,14 @@ module = {
   cppflags = '$(CPPFLAGS_GCRY)';
 };

+module = {
+  name = pkcs1_v15;
+  common = lib/pkcs1_v15.c;
+
+  cflags = '$(CFLAGS_GCRY) -Wno-redundant-decls -Wno-sign-compare';
+  cppflags = '$(CPPFLAGS_GCRY)';
+};
+
 module = {
   name = all_video;
   common = lib/fake_module.c;
diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
index c6766f044..b084dc9a2 100644
--- a/grub-core/commands/pgp.c
+++ b/grub-core/commands/pgp.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -411,32 +412,7 @@ static int
 rsa_pad (gcry_mpi_t *hmpi, grub_uint8_t *hval,
 const gcry_md_spec_t *hash, struct grub_public_subkey *sk)
 {
-  grub_size_t tlen, emlen, fflen;
-  grub_uint8_t *em, *emptr;
-  unsigned nbits = gcry_mpi_get_nbits (sk->mpis[0]);
-  int ret;
-  tlen = hash->mdlen + hash->asnlen;
-  emlen = (nbits + 7) / 8;
-  if (emlen < tlen + 11)
-return 1;
-
-  em = grub_malloc (emlen);
-  if (!em)
-return 1;
-
-  em[0] = 0x00;
-  em[1] = 0x01;
-  fflen = emlen - tlen - 3;
-  for (emptr = em + 2; emptr < em + 2 + fflen; emptr++)
-*emptr = 0xff;
-  *emptr++ = 0x00;
-  grub_memcpy (emptr, hash->asnoid, hash->asnlen);
-  emptr += hash->asnlen;
-  grub_memcpy (emptr, hval, hash->mdlen);
-
-  ret = gcry_mpi_scan (hmpi, GCRYMPI_FMT_USG, em, emlen, 0);
-  grub_free (em);
-  return ret;
+  return grub_crypto_rsa_pad(hmpi, hval, hash, sk->mpis[0]);
 }

 struct grub_pubkey_context
diff --git a/grub-core/lib/pkcs1_v15.c b/grub-core/lib/pkcs1_v15.c
new file mode 100644
index 0..dbacd563d
--- /dev/null
+++ b/grub-core/lib/pkcs1_v15.c
@@ -0,0 +1,59 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013  Free Software Foundation, Inc.
+ *
+ *  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 .
+ */
+
+#include 
+#include 
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+/*
+ * Given a hash value 'hval', of hash specification 'hash', perform
+ * the EMSA-PKCS1-v1_5 padding suitable for a key with modulus 'mod'
+ * (see RFC 8017 s 9.2) and place the result in 'hmpi'.
+ */
+gcry_err_code_t
+grub_crypto_rsa_pad (gcry_mpi_t * hmpi, grub_uint8_t * hval,
+const gcry_md_spec_t * hash, gcry_mpi_t mod)
+{
+  grub_size_t tlen, emlen, fflen;
+  grub_uint8_t *em, *emptr;
+  unsigned nbits = gcry_mpi_get_nbits (mod);
+  int ret;
+  tlen = hash->mdlen + hash->asnlen;
+  emlen = (nbits + 7) / 8;
+  if (emlen < tlen + 11)
+return GPG_ERR_TOO_SHORT;
+
+  em = grub_malloc (emlen);
+  if (!em)
+return 1;
+
+  em[0] = 0x00;
+  em[1] = 0x01;
+  fflen = emlen - tlen - 3;
+  for (emptr = em + 2; emptr < em + 2 + fflen; emptr++)
+*emptr = 0xff;
+  *emptr++ = 0x00;
+  grub_memcpy (emptr, hash->asnoid, hash->asnlen);
+  emptr += hash->asnlen;
+  grub_memcpy (emptr, hval, hash->mdlen);
+
+  ret = gcry_mpi_scan (hmpi, GCRYMPI_FMT_USG, em, emlen, 0);
+  grub_free (em);
+  return ret;
+}
diff --git a/include/grub/pk

Re: [PATCH v1 05/21] pgp: factor out rsa_pad

[sudhakar]

On 2025-01-24 16:10, Avnish Chouhan wrote:

Indentation looks off in couple of places. Please fix it.

Reviewed-by: Avnish Chouhan 

On 2024-12-18 20:26, Sudhakar Kuppusamy wrote:

From: Daniel Axtens 

rsa_pad does the PKCS#1 v1.5 padding for the RSA signature scheme.
We want to use it in other RSA signature verification applications.

I considered and rejected putting it in lib/crypto.c. That file 
doesn't

currently require any MPI functions, but rsa_pad does. That's not so
much of a problem for the grub kernel and modules, but crypto.c also
gets built into all the grub utilities. So - despite the utils not
using any asymmetric ciphers -  we would need to built the entire MPI
infrastructure in to them.

A better and simpler solution is just to spin rsa_pad out into its own
PKCS#1 v1.5 module.

Signed-off-by: Daniel Axtens 
Signed-off-by: Sudhakar Kuppusamy 
---
 grub-core/Makefile.core.def |  8 +
 grub-core/commands/pgp.c| 28 ++
 grub-core/lib/pkcs1_v15.c   | 59 
+

 include/grub/pkcs1_v15.h| 27 +
 4 files changed, 96 insertions(+), 26 deletions(-)
 create mode 100644 grub-core/lib/pkcs1_v15.c
 create mode 100644 include/grub/pkcs1_v15.h

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index f70e02e69..60db2adc5 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2540,6 +2540,14 @@ module = {
   cppflags = '$(CPPFLAGS_GCRY)';
 };

+module = {
+  name = pkcs1_v15;
+  common = lib/pkcs1_v15.c;
+
+  cflags = '$(CFLAGS_GCRY) -Wno-redundant-decls -Wno-sign-compare';
+  cppflags = '$(CPPFLAGS_GCRY)';
+};
+
 module = {
   name = all_video;
   common = lib/fake_module.c;
diff --git a/grub-core/commands/pgp.c b/grub-core/commands/pgp.c
index c6766f044..b084dc9a2 100644
--- a/grub-core/commands/pgp.c
+++ b/grub-core/commands/pgp.c
@@ -24,6 +24,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -411,32 +412,7 @@ static int
 rsa_pad (gcry_mpi_t *hmpi, grub_uint8_t *hval,
 const gcry_md_spec_t *hash, struct grub_public_subkey *sk)
 {
-  grub_size_t tlen, emlen, fflen;
-  grub_uint8_t *em, *emptr;
-  unsigned nbits = gcry_mpi_get_nbits (sk->mpis[0]);
-  int ret;
-  tlen = hash->mdlen + hash->asnlen;
-  emlen = (nbits + 7) / 8;
-  if (emlen < tlen + 11)
-return 1;
-
-  em = grub_malloc (emlen);
-  if (!em)
-return 1;
-
-  em[0] = 0x00;
-  em[1] = 0x01;
-  fflen = emlen - tlen - 3;
-  for (emptr = em + 2; emptr < em + 2 + fflen; emptr++)
-*emptr = 0xff;
-  *emptr++ = 0x00;
-  grub_memcpy (emptr, hash->asnoid, hash->asnlen);
-  emptr += hash->asnlen;
-  grub_memcpy (emptr, hval, hash->mdlen);
-
-  ret = gcry_mpi_scan (hmpi, GCRYMPI_FMT_USG, em, emlen, 0);
-  grub_free (em);
-  return ret;
+  return grub_crypto_rsa_pad(hmpi, hval, hash, sk->mpis[0]);
 }

 struct grub_pubkey_context
diff --git a/grub-core/lib/pkcs1_v15.c b/grub-core/lib/pkcs1_v15.c
new file mode 100644
index 0..dbacd563d
--- /dev/null
+++ b/grub-core/lib/pkcs1_v15.c
@@ -0,0 +1,59 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2013  Free Software Foundation, Inc.
+ *
+ *  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 .
+ */
+
+#include 
+#include 
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+/*
+ * Given a hash value 'hval', of hash specification 'hash', perform
+ * the EMSA-PKCS1-v1_5 padding suitable for a key with modulus 'mod'
+ * (see RFC 8017 s 9.2) and place the result in 'hmpi'.
+ */
+gcry_err_code_t
+grub_crypto_rsa_pad (gcry_mpi_t * hmpi, grub_uint8_t * hval,
+const gcry_md_spec_t * hash, gcry_mpi_t mod)
+{
+  grub_size_t tlen, emlen, fflen;
+  grub_uint8_t *em, *emptr;
+  unsigned nbits = gcry_mpi_get_nbits (mod);
+  int ret;
+  tlen = hash->mdlen + hash->asnlen;
+  emlen = (nbits + 7) / 8;
+  if (emlen < tlen + 11)
+return GPG_ERR_TOO_SHORT;
+
+  em = grub_malloc (emlen);
+  if (!em)
+return 1;
+
+  em[0] = 0x00;
+  em[1] = 0x01;
+  fflen = emlen - tlen - 3;
+  for (emptr = em + 2; emptr < em + 2 + fflen; emptr++)
+*emptr = 0xff;
+  *emptr++ = 0x00;
+  grub_memcpy (emptr, hash->asnoid, hash->asnlen);
+  emptr += hash->asnlen;
+  grub_memcpy (emptr, hval, hash->mdlen);
+
+  ret = gcry_mpi_scan (hmpi, GCRYMPI_FMT_USG, em, emlen, 0);
+  grub_free (em);
+  return ret;
+}
diff --git a/include/grub/pkcs1_v15.h b/include/grub/pkcs1_v15.h
ne

Re: [PATCH v1 09/21] appended signatures: parse PKCS#7 signedData and X.509 certificates

[sudhakar]

On 2025-01-24 16:40, Avnish Chouhan wrote:

Suggestion : It will be good if we can remove the brackets in one
liner if conditions and loops!

Reviewed-by: Avnish Chouhan 

On 2024-12-18 20:26, Sudhakar Kuppusamy wrote:

From: Daniel Axtens 

This code allows us to parse:

 - PKCS#7 signedData messages. Only a single signerInfo is supported,
   which is all that the Linux sign-file utility supports creating
   out-of-the-box. Only RSA, SHA-256 and SHA-512 are supported.
   Any certificate embedded in the PKCS#7 message will be ignored.

 - X.509 certificates: at least enough to verify the signatures on the
   PKCS#7 messages. We expect that the certificates embedded in grub 
will
   be leaf certificates, not CA certificates. The parser enforces 
this.


 - X.509 certificates support the Extended Key Usage extension and 
handle
   it by verifying that the certificate has a single purpose, that is 
code
   signing. This is required because Red Hat certificates have both 
Key

   Usage and Extended Key Usage extensions present.

Signed-off-by: Javier Martinez Canillas  # EKU 
support

Reported-by: Michal Suchanek  # key usage issue
Signed-off-by: Daniel Axtens 
Signed-off-by: Sudhakar Kuppusamy 
---
 grub-core/commands/appendedsig/appendedsig.h | 110 +++
 grub-core/commands/appendedsig/asn1util.c|  99 ++
 grub-core/commands/appendedsig/pkcs7.c   | 473 +
 grub-core/commands/appendedsig/x509.c| 981 
+++

 4 files changed, 1663 insertions(+)
 create mode 100644 grub-core/commands/appendedsig/appendedsig.h
 create mode 100644 grub-core/commands/appendedsig/asn1util.c
 create mode 100644 grub-core/commands/appendedsig/pkcs7.c
 create mode 100644 grub-core/commands/appendedsig/x509.c

diff --git a/grub-core/commands/appendedsig/appendedsig.h
b/grub-core/commands/appendedsig/appendedsig.h
new file mode 100644
index 0..fa59302c8
--- /dev/null
+++ b/grub-core/commands/appendedsig/appendedsig.h
@@ -0,0 +1,110 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2020, 2022 Free Software Foundation, Inc.
+ *  Copyright (C) 2020, 2022 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 .
+ */
+
+#include 
+#include 
+
+extern asn1_node _gnutls_gnutls_asn;
+extern asn1_node _gnutls_pkix_asn;
+
+#define MAX_OID_LEN 32
+
+/*
+ * One or more x509 certificates.
+ * We do limited parsing: extracting only the serial, CN and RSA 
public key.

+ */
+struct x509_certificate
+{
+  struct x509_certificate *next;
+  grub_uint8_t *serial;
+  grub_size_t serial_len;
+  char *subject;
+  grub_size_t subject_len;
+  /* We only support RSA public keys. This encodes [modulus, 
publicExponent] */

+  gcry_mpi_t mpis[2];
+};
+
+/*
+ * A PKCS#7 signedData signerInfo.
+ */
+struct pkcs7_signerInfo
+{
+  const gcry_md_spec_t *hash;
+  gcry_mpi_t sig_mpi;
+};
+
+/*
+ * A PKCS#7 signedData message.
+ * We make no attempt to match intelligently, so we don't save any 
info about

+ * the signer.
+ */
+struct pkcs7_signedData
+{
+  int signerInfo_count;
+  struct pkcs7_signerInfo *signerInfos;
+};
+
+/* Do libtasn1 init */
+int
+asn1_init (void);
+
+/*
+ * Import a DER-encoded certificate at 'data', of size 'size'.
+ * Place the results into 'results', which must be already allocated.
+ */
+grub_err_t
+parse_x509_certificate (const void *data, grub_size_t size, struct
x509_certificate *results);
+
+/*
+ * Release all the storage associated with the x509 certificate.
+ * If the caller dynamically allocated the certificate, it must free 
it.

+ * The caller is also responsible for maintenance of the linked list.
+ */
+void
+certificate_release (struct x509_certificate *cert);
+
+/*
+ * Parse a PKCS#7 message, which must be a signedData message.
+ * The message must be in 'sigbuf' and of size 'data_size'. The 
result is

+ * placed in 'msg', which must already be allocated.
+ */
+grub_err_t
+parse_pkcs7_signedData (const void *sigbuf, grub_size_t data_size,
struct pkcs7_signedData *msg);
+
+/*
+ * Release all the storage associated with the PKCS#7 message.
+ * If the caller dynamically allocated the message, it must free it.
+ */
+void
+pkcs7_signedData_release (struct pkcs7_signedData *msg);
+
+/*
+ * Read a value from an ASN1 node, allocating memory to store it.
+ * It will work for anything where the size libtasn1 returns is 
right:

+ *  - Integers
+ *  - Octet strings
+ *  - DER encoding 

Re: [PATCH v1 10/21] appended signatures: support verifying appended signatures

[sudhakar]

On 2025-02-06 11:40, Avnish Chouhan wrote:

On 2024-12-18 20:26, Sudhakar Kuppusamy wrote:

From: Daniel Axtens 

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

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 
Signed-off-by: Sudhakar Kuppusamy 
---
 grub-core/Makefile.core.def   |  14 +
 grub-core/commands/appendedsig/appendedsig.c  | 620 
++

 grub-core/commands/appendedsig/appendedsig.h  |   2 +-
 grub-core/commands/appendedsig/asn1util.c |   2 +-
 .../commands/appendedsig/gnutls_asn1_tab.c|   2 +-
 .../commands/appendedsig/pkix_asn1_tab.c  |   2 +-
 grub-core/commands/appendedsig/x509.c |   2 +-
 include/grub/file.h   |   2 +
 8 files changed, 641 insertions(+), 5 deletions(-)
 create mode 100644 grub-core/commands/appendedsig/appendedsig.c

diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 60db2adc5..d693986c6 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -979,6 +979,20 @@ module = {
   cppflags = '-I$(srcdir)/lib/posix_wrap';
 };

+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;
+
+  // posix wrapper required for gcry to get sys/types.h
+  cflags = '$(CFLAGS_POSIX)';
+  cppflags = '-I$(srcdir)/lib/posix_wrap 
-I$(srcdir)/lib/libtasn1-grub';

+};
+
 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 0..5c82b96a4
--- /dev/null
+++ b/grub-core/commands/appendedsig/appendedsig.c
@@ -0,0 +1,620 @@
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2020, 2021, 2022 Free Software Foundation, Inc.
+ *  Copyright (C) 2020, 2021, 2022 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 .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "appendedsig.h"
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+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 [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 *grub_trusted_key;
+
+/*
+ * Force gcry_rsa to be a module dependency.
+ *
+ * If we use grub_crypto_pk_rsa, then then the gcry_rsa module won't 
be built
+ * in if you add 'appendedsig' to grub-install --modules. You would 
need to
+ * add 'gcry_rsa' too. That's confusing and seems suboptimal, 
especially when

+ * we 

Re: [PATCH v1 13/21] ieee1275: enter lockdown based on /ibm,secure-boot

[sudhakar]

On 2025-02-06 11:53, Avnish Chouhan wrote:

On 2024-12-18 20:26, Sudhakar Kuppusamy wrote:

From: Daniel Axtens 

If the 'ibm,secure-boot' property of the root node is 2 or greater,
enter lockdown.

Signed-off-by: Daniel Axtens 
Signed-off-by: Sudhakar Kuppusamy 
---
 docs/grub.texi |  4 ++--
 grub-core/Makefile.core.def|  1 +
 grub-core/kern/ieee1275/init.c | 28 
 include/grub/lockdown.h|  3 ++-
 4 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/docs/grub.texi b/docs/grub.texi
index f71ce9ffc..6b634f111 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -8959,8 +8959,8 @@ platforms.
 @section Lockdown when booting on a secure setup

 The GRUB can be locked down when booted on a secure boot environment,
for example
-if the UEFI secure boot is enabled. On a locked down configuration,
the GRUB will
-be restricted and some operations/commands cannot be executed.
+if UEFI or Power secure boot is enabled. On a locked down 
configuration, the
+GRUB will be restricted and some operations/commands cannot be 
executed.


 The @samp{lockdown} variable is set to @samp{y} when the GRUB is 
locked down.

 Otherwise it does not exit.
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 9cf4a6009..1ed55b0e3 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -331,6 +331,7 @@ kernel = {
   powerpc_ieee1275 = kern/powerpc/cache.S;
   powerpc_ieee1275 = kern/powerpc/dl.c;
   powerpc_ieee1275 = kern/powerpc/compiler-rt.S;
+  powerpc_ieee1275 = kern/lockdown.c;

   sparc64_ieee1275 = kern/sparc64/cache.S;
   sparc64_ieee1275 = kern/sparc64/dl.c;
diff --git a/grub-core/kern/ieee1275/init.c 
b/grub-core/kern/ieee1275/init.c

index dfbd0b899..59984b605 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -49,6 +49,7 @@
 #if defined(__powerpc__) || defined(__i386__)
 #include 
 #endif
+#include 

 /* The maximum heap size we're going to claim at boot. Not used by 
sparc. */

 #ifdef __i386__
@@ -953,6 +954,31 @@ grub_parse_cmdline (void)
 }
 }

+static void
+grub_get_ieee1275_secure_boot (void)
+{
+  grub_ieee1275_phandle_t root;
+  int rc;
+  grub_uint32_t is_sb;
+
+  grub_ieee1275_finddevice ("/", &root);


Hi,

Failure check condition is missing!

Thank you,
Avnish Chouhan


+
+  rc = grub_ieee1275_get_integer_property (root, "ibm,secure-boot", 
&is_sb,

+   sizeof (is_sb), 0);
+
+  /*
+   * ibm,secure-boot:
+   * 0 - disabled
+   * 1 - audit
+   * 2 - enforce
+   * 3 - enforce + OS-specific behaviour
+   *
+   * We only support enforce.
+   */
+  if (rc >= 0 && is_sb >= 2)
+grub_lockdown ();
+}
+
 grub_addr_t grub_modbase;

 void
@@ -978,6 +1004,8 @@ grub_machine_init (void)
 #else
   grub_install_get_time_ms (grub_rtc_get_time_ms);
 #endif
+
+  grub_get_ieee1275_secure_boot ();
 }

 void
diff --git a/include/grub/lockdown.h b/include/grub/lockdown.h
index 40531fa82..ebfee4bf0 100644
--- a/include/grub/lockdown.h
+++ b/include/grub/lockdown.h
@@ -24,7 +24,8 @@
 #define GRUB_LOCKDOWN_DISABLED   0
 #define GRUB_LOCKDOWN_ENABLED1

-#ifdef GRUB_MACHINE_EFI
+#if defined(GRUB_MACHINE_EFI) || \
+(defined(__powerpc__) && defined(GRUB_MACHINE_IEEE1275))
 extern void
 EXPORT_FUNC (grub_lockdown) (void);
 extern int



Addressed above comments. Thank you Avnish Chouhan.

Thanks,
Sudhakar Kuppusamy

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