libebl: recognize FDO Packaging Metadata ELF note

Message ID 20211119003127.466778-1-luca.boccassi@gmail.com
State Superseded
Headers
Series libebl: recognize FDO Packaging Metadata ELF note |

Commit Message

lilydjwg--- via Elfutils-devel Nov. 19, 2021, 12:31 a.m. UTC
  From: Luca Boccassi <bluca@debian.org>

As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
this note will be used starting from Fedora 36. Allow
readelf --notes to pretty print it:

Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
  Owner          Data size  Type
  FDO                   57  FDO_PACKAGING_METADATA
    Packaging Metadata: {"type":"deb","name":"fsverity-utils","version":"1.3-1"}

Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com>
---
 libebl/eblobjnote.c         | 3 +++
 libebl/eblobjnotetypename.c | 3 +++
 libelf/elf.h                | 3 +++
 3 files changed, 9 insertions(+)
  

Comments

Mark Wielaard Nov. 21, 2021, 4:33 p.m. UTC | #1
Hi Luca,

On Fri, Nov 19, 2021 at 12:31:27AM +0000, luca.boccassi--- via Elfutils-devel wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
> this note will be used starting from Fedora 36. Allow
> readelf --notes to pretty print it:
> 
> Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
>   Owner          Data size  Type
>   FDO                   57  FDO_PACKAGING_METADATA
>     Packaging Metadata: {"type":"deb","name":"fsverity-utils","version":"1.3-1"}

Very nice. Thanks,

> diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
> index 36efe275..1f8bcccf 100644
> --- a/libebl/eblobjnote.c
> +++ b/libebl/eblobjnote.c
> @@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const char *name, uint32_t type,
>        if (descsz == 0 && type == NT_VERSION)
>  	return;
>  
> +      if (strcmp ("FDO", name) == 0 && type == FDO_PACKAGING_METADATA && descsz > 0)
> +	printf("    Packaging Metadata: %.*s\n", (int) descsz, desc);
> +

We might want to check that the desc is '\0' terminated (although I
see we also don't do that in other cases, like
NT_GNU_GOLD_VERSION. But it might be good as a robustness check.

> diff --git a/libelf/elf.h b/libelf/elf.h
> index 8e3e618f..633f9f67 100644
> --- a/libelf/elf.h
> +++ b/libelf/elf.h
> @@ -1297,6 +1297,9 @@ typedef struct
>  /* Program property.  */
>  #define NT_GNU_PROPERTY_TYPE_0 5
>  
> +/* Packaging metadata as defined on https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
> +#define FDO_PACKAGING_METADATA 0xcafe1a7e
> +
>  /* Note section name of program property.   */
>  #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"

Would you mind posting the elf.h patch to glibc-alpha@sourceware.org.
We normally sync elf.h with the glibc one. It will also make sure
other users of elf.h also get the new constants.

As a followup I wouldn't mind a minimal testcase.
Especially if it contains a debuginfod url.

We would have to think how to integrate that with libdw
dwfl_build_id_find_elf and dwfl_standard_find_debuginfo which use
debuginfod_find from the debuginfod-client library.

Since the payload of the FDO_PACKAGING_METADATA note are not simply
key/values, but encoded in json, so we will need to add or depend on a
json parser. Any recommendations? It seems a simple enough format to
just write our own (especially if we can simply skip everything except
top-level key/value strings to find the debuginfod-url).

Thanks,

Mark
  
Luca Boccassi Nov. 21, 2021, 7:54 p.m. UTC | #2
On Sun, 2021-11-21 at 17:33 +0100, Mark Wielaard wrote:
> Hi Luca,
> 
> On Fri, Nov 19, 2021 at 12:31:27AM +0000, luca.boccassi--- via
> Elfutils-devel wrote:
> > From: Luca Boccassi <bluca@debian.org>
> > 
> > As defined on: https://systemd.io/COREDUMP_PACKAGE_METADATA/
> > this note will be used starting from Fedora 36. Allow
> > readelf --notes to pretty print it:
> > 
> > Note section [ 3] '.note.package' of 76 bytes at offset 0x2e8:
> >   Owner          Data size  Type
> >   FDO                   57  FDO_PACKAGING_METADATA
> >     Packaging Metadata: {"type":"deb","name":"fsverity-
> > utils","version":"1.3-1"}
> 
> Very nice. Thanks,
> 
> > diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
> > index 36efe275..1f8bcccf 100644
> > --- a/libebl/eblobjnote.c
> > +++ b/libebl/eblobjnote.c
> > @@ -288,6 +288,9 @@ ebl_object_note (Ebl *ebl, uint32_t namesz, const
> > char *name, uint32_t type,
> >        if (descsz == 0 && type == NT_VERSION)
> >         return;
> >  
> > +      if (strcmp ("FDO", name) == 0 && type ==
> > FDO_PACKAGING_METADATA && descsz > 0)
> > +       printf("    Packaging Metadata: %.*s\n", (int) descsz, desc);
> > +
> 
> We might want to check that the desc is '\0' terminated (although I
> see we also don't do that in other cases, like
> NT_GNU_GOLD_VERSION. But it might be good as a robustness check.

No problem, added in v2.

> > diff --git a/libelf/elf.h b/libelf/elf.h
> > index 8e3e618f..633f9f67 100644
> > --- a/libelf/elf.h
> > +++ b/libelf/elf.h
> > @@ -1297,6 +1297,9 @@ typedef struct
> >  /* Program property.  */
> >  #define NT_GNU_PROPERTY_TYPE_0 5
> >  
> > +/* Packaging metadata as defined on
> > https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
> > +#define FDO_PACKAGING_METADATA 0xcafe1a7e
> > +
> >  /* Note section name of program property.   */
> >  #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"
> 
> Would you mind posting the elf.h patch to glibc-alpha@sourceware.org.
> We normally sync elf.h with the glibc one. It will also make sure
> other users of elf.h also get the new constants.

Sure, done:

https://sourceware.org/pipermail/libc-alpha/2021-November/133330.html

> As a followup I wouldn't mind a minimal testcase.
> Especially if it contains a debuginfod url.
> 
> We would have to think how to integrate that with libdw
> dwfl_build_id_find_elf and dwfl_standard_find_debuginfo which use
> debuginfod_find from the debuginfod-client library.
> 
> Since the payload of the FDO_PACKAGING_METADATA note are not simply
> key/values, but encoded in json, so we will need to add or depend on a
> json parser. Any recommendations? It seems a simple enough format to
> just write our own (especially if we can simply skip everything except
> top-level key/value strings to find the debuginfod-url).
> 
> Thanks,
> 
> Mark

Popular C parsers that I know of are json-c and jannson:

https://github.com/json-c/json-c/wiki
https://digip.org/jansson/

json-c seems to be available in slightly more places:

https://repology.org/project/json-c/versions
https://repology.org/project/jansson/versions

Rolling your own full parser can always end up being tricky and a lot
of work, especially for limited usage with no particular requirements.
You need to ensure you've got good fuzzing, etc. If using one of the
above is optional and tied to the debuginfod feature being enabled,
there shouldn't be issues with bootstrapping.
A simple search for the "debugInfoUrl" string, using whatever string is
quoted next as the url would be much simpler of course, if that's all
you need. Up to you of course.
  

Patch

diff --git a/libebl/eblobjnote.c b/libebl/eblobjnote.c
index 36efe275..1f8bcccf 100644
--- a/libebl/eblobjnote.c
+++ b/libebl/eblobjnote.c
@@ -288,6 +288,9 @@  ebl_object_note (Ebl *ebl, uint32_t namesz, const char *name, uint32_t type,
       if (descsz == 0 && type == NT_VERSION)
 	return;
 
+      if (strcmp ("FDO", name) == 0 && type == FDO_PACKAGING_METADATA && descsz > 0)
+	printf("    Packaging Metadata: %.*s\n", (int) descsz, desc);
+
       /* Everything else should have the "GNU" owner name.  */
       if (strcmp ("GNU", name) != 0)
 	return;
diff --git a/libebl/eblobjnotetypename.c b/libebl/eblobjnotetypename.c
index 4662906d..863f20e4 100644
--- a/libebl/eblobjnotetypename.c
+++ b/libebl/eblobjnotetypename.c
@@ -101,6 +101,9 @@  ebl_object_note_type_name (Ebl *ebl, const char *name, uint32_t type,
 	  return buf;
 	}
 
+      if (strcmp (name, "FDO") == 0 && type == FDO_PACKAGING_METADATA)
+	return "FDO_PACKAGING_METADATA";
+
       if (strcmp (name, "GNU") != 0)
 	{
 	  /* NT_VERSION is special, all data is in the name.  */
diff --git a/libelf/elf.h b/libelf/elf.h
index 8e3e618f..633f9f67 100644
--- a/libelf/elf.h
+++ b/libelf/elf.h
@@ -1297,6 +1297,9 @@  typedef struct
 /* Program property.  */
 #define NT_GNU_PROPERTY_TYPE_0 5
 
+/* Packaging metadata as defined on https://systemd.io/COREDUMP_PACKAGE_METADATA/ */
+#define FDO_PACKAGING_METADATA 0xcafe1a7e
+
 /* Note section name of program property.   */
 #define NOTE_GNU_PROPERTY_SECTION_NAME ".note.gnu.property"