From edc34323389db76204ffbc7e40a3fe3d89406572 Mon Sep 17 00:00:00 2001
From: Richard Hughes <richard@hughsie.com>
Date: Thu, 13 Aug 2020 20:46:05 +0100
Subject: [PATCH] dbxtool: List the checksums correctly for multiple
 EFI_SIGNATURE_LISTs

Fixes https://github.com/fwupd/fwupd/issues/2319
---
 plugins/uefi-dbx/fu-dbxtool.c              | 46 +++++++++++++---------
 plugins/uefi-dbx/fu-efi-signature-common.c | 37 +++++++++++++++++
 plugins/uefi-dbx/fu-efi-signature-common.h | 12 ++++++
 plugins/uefi-dbx/fu-efi-signature-list.c   | 12 ------
 plugins/uefi-dbx/fu-efi-signature-list.h   |  2 -
 plugins/uefi-dbx/fu-efi-signature-parser.c | 29 +-------------
 plugins/uefi-dbx/fu-efi-signature-parser.h |  6 +--
 plugins/uefi-dbx/fu-efi-signature.c        |  9 ++++-
 plugins/uefi-dbx/fu-efi-signature.h        |  1 +
 plugins/uefi-dbx/fu-fuzzer.c               |  2 +-
 plugins/uefi-dbx/fu-plugin-uefi-dbx.c      | 24 +++--------
 plugins/uefi-dbx/fu-self-test.c            |  2 +-
 plugins/uefi-dbx/meson.build               |  4 ++
 13 files changed, 100 insertions(+), 86 deletions(-)
 create mode 100644 plugins/uefi-dbx/fu-efi-signature-common.c
 create mode 100644 plugins/uefi-dbx/fu-efi-signature-common.h

diff --git a/plugins/uefi-dbx/fu-dbxtool.c b/plugins/uefi-dbx/fu-dbxtool.c
index 1b93b5c66..819c47a6c 100644
--- a/plugins/uefi-dbx/fu-dbxtool.c
+++ b/plugins/uefi-dbx/fu-dbxtool.c
@@ -16,6 +16,7 @@
 #include "fu-common.h"
 #include "fu-efivar.h"
 #include "fu-uefi-dbx-common.h"
+#include "fu-efi-signature-common.h"
 #include "fu-efi-signature-parser.h"
 
 /* custom return code */
@@ -35,7 +36,7 @@ fu_dbxtool_convert_guid (const gchar *guid)
 	return guid;
 }
 
-static FuEfiSignatureList *
+static GPtrArray *
 fu_dbxtool_get_siglist_system (GError **error)
 {
 	gsize bufsz = 0;
@@ -43,19 +44,19 @@ fu_dbxtool_get_siglist_system (GError **error)
 	if (!fu_efivar_get_data (FU_EFIVAR_GUID_SECURITY_DATABASE, "dbx",
 				 &buf, &bufsz, NULL, error))
 		return FALSE;
-	return fu_efi_signature_parser_one (buf, bufsz,
+	return fu_efi_signature_parser_new (buf, bufsz,
 					    FU_EFI_SIGNATURE_PARSER_FLAGS_NONE,
 					    error);
 }
 
-static FuEfiSignatureList *
+static GPtrArray *
 fu_dbxtool_get_siglist_local (const gchar *filename, GError **error)
 {
 	gsize bufsz = 0;
 	g_autofree guint8 *buf = NULL;
 	if (!g_file_get_contents (filename, (gchar **) &buf, &bufsz, error))
 		return FALSE;
-	return fu_efi_signature_parser_one (buf, bufsz,
+	return fu_efi_signature_parser_new (buf, bufsz,
 					    FU_EFI_SIGNATURE_PARSER_FLAGS_IGNORE_HEADER,
 					    error);
 }
@@ -122,8 +123,8 @@ main (int argc, char *argv[])
 
 	/* list contents, either of the existing system, or an update */
 	if (action_list) {
-		GPtrArray *sigs;
-		g_autoptr(FuEfiSignatureList) dbx = NULL;
+		guint cnt = 1;
+		g_autoptr(GPtrArray) dbx = NULL;
 		if (dbxfile != NULL) {
 			dbx = fu_dbxtool_get_siglist_local (dbxfile, &error);
 			if (dbx == NULL) {
@@ -139,13 +140,17 @@ main (int argc, char *argv[])
 				return EXIT_FAILURE;
 			}
 		}
-		sigs = fu_efi_signature_list_get_all (dbx);
-		for (guint i = 0; i < sigs->len; i++) {
-			FuEfiSignature *sig = g_ptr_array_index (sigs, i);
-			g_print ("%4u: {%s} {%s} %s\n", i + 1,
-				 fu_dbxtool_convert_guid (fu_efi_signature_get_owner (sig)),
-				 fu_efi_signature_kind_to_string (fu_efi_signature_list_get_kind (dbx)),
-				 fu_efi_signature_get_checksum (sig));
+		for (guint j = 0; j < dbx->len; j++) {
+			FuEfiSignatureList *siglist = g_ptr_array_index (dbx, j);
+			GPtrArray *sigs = fu_efi_signature_list_get_all (siglist);
+			for (guint i = 0; i < sigs->len; i++) {
+				FuEfiSignature *sig = g_ptr_array_index (sigs, i);
+				g_print ("%4u: {%s} {%s} %s\n",
+					 cnt++,
+					 fu_dbxtool_convert_guid (fu_efi_signature_get_owner (sig)),
+					 fu_efi_signature_kind_to_string (fu_efi_signature_get_kind (sig)),
+					 fu_efi_signature_get_checksum (sig));
+			}
 		}
 		return EXIT_SUCCESS;
 	}
@@ -162,8 +167,8 @@ main (int argc, char *argv[])
 	if (action_apply) {
 		gsize bufsz = 0;
 		g_autofree guint8 *buf = NULL;
-		g_autoptr(FuEfiSignatureList) dbx_system = NULL;
-		g_autoptr(FuEfiSignatureList) dbx_update = NULL;
+		g_autoptr(GPtrArray) dbx_system = NULL;
+		g_autoptr(GPtrArray) dbx_update = NULL;
 
 		if (dbxfile == NULL) {
 			/* TRANSLATORS: user did not include a filename parameter */
@@ -187,7 +192,7 @@ main (int argc, char *argv[])
 			g_printerr ("%s: %s\n", _("Failed to load local dbx"), error->message);
 			return EXIT_FAILURE;
 		}
-		dbx_update = fu_efi_signature_parser_one (buf, bufsz,
+		dbx_update = fu_efi_signature_parser_new (buf, bufsz,
 							  FU_EFI_SIGNATURE_PARSER_FLAGS_IGNORE_HEADER,
 							  &error);
 		if (dbx_update == NULL) {
@@ -195,11 +200,16 @@ main (int argc, char *argv[])
 			g_printerr ("%s: %s\n", _("Failed to parse local dbx"), error->message);
 			return EXIT_FAILURE;
 		}
+		if (dbx_update->len != 1) {
+			/* TRANSLATORS: could not parse file */
+			g_printerr ("%s: %s\n", _("Failed to extract local dbx "), error->message);
+			return EXIT_FAILURE;
+		}
 
 		/* check this is a newer dbx update */
-		if (!force && fu_efi_signature_list_are_inclusive (dbx_system, dbx_update)) {
+		if (!force && fu_efi_signature_list_array_inclusive (dbx_system, dbx_update)) {
 			/* TRANSLATORS: same or newer update already applied */
-			g_printerr ("%s\n", _("Cannot apply update as this dbx update has already been applied."));
+			g_printerr ("%s\n", _("Cannot apply as dbx update has already been applied."));
 			return EXIT_FAILURE;
 		}
 
diff --git a/plugins/uefi-dbx/fu-efi-signature-common.c b/plugins/uefi-dbx/fu-efi-signature-common.c
new file mode 100644
index 000000000..1c70d7ea5
--- /dev/null
+++ b/plugins/uefi-dbx/fu-efi-signature-common.c
@@ -0,0 +1,37 @@
+/*
+ * Copyright (C) 2020 Richard Hughes <richard@hughsie.com>
+ *
+ * SPDX-License-Identifier: LGPL-2.1+
+ */
+
+#include "config.h"
+
+#include "fu-efi-signature-common.h"
+#include "fu-efi-signature-list.h"
+
+static gboolean
+fu_efi_signature_list_array_has_checksum (GPtrArray *siglists, const gchar *checksum)
+{
+	for (guint i = 0; i < siglists->len; i++) {
+		FuEfiSignatureList *siglist = g_ptr_array_index (siglists, i);
+		if (fu_efi_signature_list_has_checksum (siglist, checksum))
+			return TRUE;
+	}
+	return FALSE;
+}
+
+gboolean
+fu_efi_signature_list_array_inclusive (GPtrArray *outer, GPtrArray *inner)
+{
+	for (guint j = 0; j < inner->len; j++) {
+		FuEfiSignatureList *siglist = g_ptr_array_index (inner, j);
+		GPtrArray *items = fu_efi_signature_list_get_all (siglist);
+		for (guint i = 0; i < items->len; i++) {
+			FuEfiSignature *sig = g_ptr_array_index (items, i);
+			const gchar *checksum = fu_efi_signature_get_checksum (sig);
+			if (!fu_efi_signature_list_array_has_checksum (outer, checksum))
+				return FALSE;
+		}
+	}
+	return TRUE;
+}
diff --git a/plugins/uefi-dbx/fu-efi-signature-common.h b/plugins/uefi-dbx/fu-efi-signature-common.h
new file mode 100644
index 000000000..b9761110e
--- /dev/null
+++ b/plugins/uefi-dbx/fu-efi-signature-common.h
@@ -0,0 +1,12 @@
+/*
+ * Copyright (C) 2020 Richard Hughes <richard@hughsie.com>
+ *
+ * SPDX-License-Identifier: LGPL-2.1+
+ */
+
+#pragma once
+
+#include "fu-efi-signature.h"
+
+gboolean	 fu_efi_signature_list_array_inclusive	(GPtrArray	*outer,
+							 GPtrArray	*inner);
diff --git a/plugins/uefi-dbx/fu-efi-signature-list.c b/plugins/uefi-dbx/fu-efi-signature-list.c
index 19fec96a8..422a0010e 100644
--- a/plugins/uefi-dbx/fu-efi-signature-list.c
+++ b/plugins/uefi-dbx/fu-efi-signature-list.c
@@ -57,18 +57,6 @@ fu_efi_signature_list_has_checksum (FuEfiSignatureList *self, const gchar *check
 	return FALSE;
 }
 
-gboolean
-fu_efi_signature_list_are_inclusive (FuEfiSignatureList *self, FuEfiSignatureList *other)
-{
-	g_return_val_if_fail (FU_IS_EFI_SIGNATURE_LIST (self), FALSE);
-	for (guint i = 0; i < other->items->len; i++) {
-		FuEfiSignature *sig = g_ptr_array_index (other->items, i);
-		if (!fu_efi_signature_list_has_checksum (self, fu_efi_signature_get_checksum (sig)))
-			return FALSE;
-	}
-	return TRUE;
-}
-
 static void
 fu_efi_signature_list_finalize (GObject *obj)
 {
diff --git a/plugins/uefi-dbx/fu-efi-signature-list.h b/plugins/uefi-dbx/fu-efi-signature-list.h
index 75582ecc1..688f2ee84 100644
--- a/plugins/uefi-dbx/fu-efi-signature-list.h
+++ b/plugins/uefi-dbx/fu-efi-signature-list.h
@@ -19,5 +19,3 @@ void		 fu_efi_signature_list_add		(FuEfiSignatureList	*self,
 GPtrArray	*fu_efi_signature_list_get_all		(FuEfiSignatureList	*self);
 gboolean	 fu_efi_signature_list_has_checksum	(FuEfiSignatureList	*self,
 							 const gchar		*checksum);
-gboolean	 fu_efi_signature_list_are_inclusive	(FuEfiSignatureList	*self,
-							 FuEfiSignatureList	*other);
diff --git a/plugins/uefi-dbx/fu-efi-signature-parser.c b/plugins/uefi-dbx/fu-efi-signature-parser.c
index df8571bab..3754064c8 100644
--- a/plugins/uefi-dbx/fu-efi-signature-parser.c
+++ b/plugins/uefi-dbx/fu-efi-signature-parser.c
@@ -128,7 +128,7 @@ fu_efi_signature_list_parse_list (GPtrArray *siglists,
 }
 
 GPtrArray *
-fu_efi_signature_parser_all (const guint8 *buf, gsize bufsz,
+fu_efi_signature_parser_new (const guint8 *buf, gsize bufsz,
 			     FuEfiSignatureParserFlags flags,
 			     GError **error)
 {
@@ -157,30 +157,3 @@ fu_efi_signature_parser_all (const guint8 *buf, gsize bufsz,
 	/* success */
 	return g_steal_pointer (&siglists);
 }
-
-FuEfiSignatureList *
-fu_efi_signature_parser_one (const guint8 *buf, gsize bufsz,
-			     FuEfiSignatureParserFlags flags,
-			     GError **error)
-{
-	g_autoptr(GPtrArray) siglists = NULL;
-
-	siglists = fu_efi_signature_parser_all (buf, bufsz, flags, error);
-	if (siglists == NULL)
-		return FALSE;
-	if (siglists->len == 0) {
-		g_set_error (error,
-			     G_IO_ERROR,
-			     G_IO_ERROR_FAILED,
-			     "no EFI_SIGNATURE_LIST items");
-		return FALSE;
-	}
-	if (siglists->len > 1) {
-		g_set_error (error,
-			     G_IO_ERROR,
-			     G_IO_ERROR_FAILED,
-			     "more than one EFI_SIGNATURE_LIST items");
-		return FALSE;
-	}
-	return g_object_ref (g_ptr_array_index (siglists, 0));
-}
diff --git a/plugins/uefi-dbx/fu-efi-signature-parser.h b/plugins/uefi-dbx/fu-efi-signature-parser.h
index 956088a69..d5a138c06 100644
--- a/plugins/uefi-dbx/fu-efi-signature-parser.h
+++ b/plugins/uefi-dbx/fu-efi-signature-parser.h
@@ -13,11 +13,7 @@ typedef enum {
 	FU_EFI_SIGNATURE_PARSER_FLAGS_IGNORE_HEADER	= 1 << 0,
 } FuEfiSignatureParserFlags;
 
-GPtrArray	*fu_efi_signature_parser_all	(const guint8	*buf,
-						 gsize		 bufsz,
-						 FuEfiSignatureParserFlags flags,
-						 GError		**error);
-FuEfiSignatureList* fu_efi_signature_parser_one	(const guint8	*buf,
+GPtrArray	*fu_efi_signature_parser_new	(const guint8	*buf,
 						 gsize		 bufsz,
 						 FuEfiSignatureParserFlags flags,
 						 GError		**error);
diff --git a/plugins/uefi-dbx/fu-efi-signature.c b/plugins/uefi-dbx/fu-efi-signature.c
index e2e8494c5..bc33bbc0d 100644
--- a/plugins/uefi-dbx/fu-efi-signature.c
+++ b/plugins/uefi-dbx/fu-efi-signature.c
@@ -25,7 +25,7 @@ fu_efi_signature_kind_to_string (FuEfiSignatureKind kind)
 	if (kind == FU_EFI_SIGNATURE_KIND_SHA256)
 		return "sha256";
 	if (kind == FU_EFI_SIGNATURE_KIND_X509)
-		return "x509";
+		return "x509_cert";
 	return "unknown";
 }
 
@@ -39,6 +39,13 @@ fu_efi_signature_new (FuEfiSignatureKind kind, const gchar *owner, GBytes *data)
 	return g_steal_pointer (&self);
 }
 
+FuEfiSignatureKind
+fu_efi_signature_get_kind (FuEfiSignature *self)
+{
+	g_return_val_if_fail (FU_IS_EFI_SIGNATURE (self), 0);
+	return self->kind;
+}
+
 const gchar *
 fu_efi_signature_get_owner (FuEfiSignature *self)
 {
diff --git a/plugins/uefi-dbx/fu-efi-signature.h b/plugins/uefi-dbx/fu-efi-signature.h
index 14926de74..9aebf0abd 100644
--- a/plugins/uefi-dbx/fu-efi-signature.h
+++ b/plugins/uefi-dbx/fu-efi-signature.h
@@ -23,6 +23,7 @@ const gchar	*fu_efi_signature_kind_to_string	(FuEfiSignatureKind kind);
 FuEfiSignature	*fu_efi_signature_new			(FuEfiSignatureKind kind,
 							 const gchar	*owner,
 							 GBytes		*data);
+FuEfiSignatureKind fu_efi_signature_get_kind		(FuEfiSignature	*self);
 GBytes		*fu_efi_signature_get_data		(FuEfiSignature	*self);
 const gchar	*fu_efi_signature_get_checksum		(FuEfiSignature	*self);
 const gchar 	*fu_efi_signature_get_owner		(FuEfiSignature	*self);
diff --git a/plugins/uefi-dbx/fu-fuzzer.c b/plugins/uefi-dbx/fu-fuzzer.c
index 374d6e2cc..a147c6a7d 100644
--- a/plugins/uefi-dbx/fu-fuzzer.c
+++ b/plugins/uefi-dbx/fu-fuzzer.c
@@ -26,7 +26,7 @@ main (int argc, char *argv[])
 		g_printerr ("Failed to load %s: %s\n", argv[1], error->message);
 		return EXIT_FAILURE;
 	}
-	siglists = fu_efi_signature_parser_all (buf, bufsz,
+	siglists = fu_efi_signature_parser_new (buf, bufsz,
 						FU_EFI_SIGNATURE_PARSER_FLAGS_IGNORE_HEADER,
 						&error);
 	if (siglists == NULL) {
diff --git a/plugins/uefi-dbx/fu-plugin-uefi-dbx.c b/plugins/uefi-dbx/fu-plugin-uefi-dbx.c
index 15444df90..9217a1602 100644
--- a/plugins/uefi-dbx/fu-plugin-uefi-dbx.c
+++ b/plugins/uefi-dbx/fu-plugin-uefi-dbx.c
@@ -10,6 +10,7 @@
 #include "fu-efivar.h"
 #include "fu-hash.h"
 #include "fu-uefi-dbx-common.h"
+#include "fu-efi-signature-common.h"
 #include "fu-efi-signature-parser.h"
 
 struct FuPluginData {
@@ -45,13 +46,11 @@ void
 fu_plugin_add_security_attrs (FuPlugin *plugin, FuSecurityAttrs *attrs)
 {
 	FuPluginData *data = fu_plugin_get_data (plugin);
-	GPtrArray *items;
 	gsize bufsz = 0;
-	guint missing_cnt = 0;
 	g_autofree guint8 *buf_system = NULL;
 	g_autofree guint8 *buf_update = NULL;
-	g_autoptr(FuEfiSignatureList) dbx_system = NULL;
-	g_autoptr(FuEfiSignatureList) dbx_update = NULL;
+	g_autoptr(GPtrArray) dbx_system = NULL;
+	g_autoptr(GPtrArray) dbx_update = NULL;
 	g_autoptr(FwupdSecurityAttr) attr = NULL;
 	g_autoptr(GError) error_local = NULL;
 
@@ -74,7 +73,7 @@ fu_plugin_add_security_attrs (FuPlugin *plugin, FuSecurityAttrs *attrs)
 		fwupd_security_attr_set_result (attr, FWUPD_SECURITY_ATTR_RESULT_NOT_VALID);
 		return;
 	}
-	dbx_update = fu_efi_signature_parser_one (buf_update, bufsz,
+	dbx_update = fu_efi_signature_parser_new (buf_update, bufsz,
 						  FU_EFI_SIGNATURE_PARSER_FLAGS_IGNORE_HEADER,
 						  &error_local);
 	if (dbx_update == NULL) {
@@ -90,7 +89,7 @@ fu_plugin_add_security_attrs (FuPlugin *plugin, FuSecurityAttrs *attrs)
 		fwupd_security_attr_set_result (attr, FWUPD_SECURITY_ATTR_RESULT_NOT_VALID);
 		return;
 	}
-	dbx_system = fu_efi_signature_parser_one (buf_system, bufsz,
+	dbx_system = fu_efi_signature_parser_new (buf_system, bufsz,
 						  FU_EFI_SIGNATURE_PARSER_FLAGS_NONE,
 						  &error_local);
 	if (dbx_system == NULL) {
@@ -100,18 +99,7 @@ fu_plugin_add_security_attrs (FuPlugin *plugin, FuSecurityAttrs *attrs)
 	}
 
 	/* look for each checksum in the update in the system version */
-	items = fu_efi_signature_list_get_all (dbx_update);
-	for (guint i = 0; i < items->len; i++) {
-		FuEfiSignature *item = g_ptr_array_index (items, i);
-		if (!fu_efi_signature_list_has_checksum (dbx_system, fu_efi_signature_get_checksum (item))) {
-			g_debug ("%s missing from the system DBX",
-				 fu_efi_signature_get_checksum (item));
-			missing_cnt += 1;
-		}
-	}
-
-	/* add security attribute */
-	if (missing_cnt > 0) {
+	if (!fu_efi_signature_list_array_inclusive (dbx_system, dbx_update)) {
 		fwupd_security_attr_set_result (attr, FWUPD_SECURITY_ATTR_RESULT_NOT_FOUND);
 		return;
 	}
diff --git a/plugins/uefi-dbx/fu-self-test.c b/plugins/uefi-dbx/fu-self-test.c
index 4a98c42b8..de8657b55 100644
--- a/plugins/uefi-dbx/fu-self-test.c
+++ b/plugins/uefi-dbx/fu-self-test.c
@@ -33,7 +33,7 @@ fu_efi_signature_list_parse_func (void)
 	g_assert_true (ret);
 
 	/* parse the update */
-	siglists = fu_efi_signature_parser_all (buf, bufsz,
+	siglists = fu_efi_signature_parser_new (buf, bufsz,
 						FU_EFI_SIGNATURE_PARSER_FLAGS_IGNORE_HEADER,
 						&error);
 	g_assert_no_error (error);
diff --git a/plugins/uefi-dbx/meson.build b/plugins/uefi-dbx/meson.build
index 14d66d635..8a8eb07a5 100644
--- a/plugins/uefi-dbx/meson.build
+++ b/plugins/uefi-dbx/meson.build
@@ -6,6 +6,7 @@ shared_module('fu_plugin_uefi_dbx',
     'fu-plugin-uefi-dbx.c',
     'fu-uefi-dbx-common.c',
     'fu-efi-signature.c',
+    'fu-efi-signature-common.c',
     'fu-efi-signature-list.c',
     'fu-efi-signature-parser.c',
   ],
@@ -34,6 +35,7 @@ if get_option('tests')
       'fu-self-test.c',
       'fu-uefi-dbx-common.c',
       'fu-efi-signature.c',
+      'fu-efi-signature-common.c',
       'fu-efi-signature-list.c',
       'fu-efi-signature-parser.c',
     ],
@@ -58,6 +60,7 @@ uefi_dbx_fuzzer = executable(
   sources : [
     'fu-fuzzer.c',
     'fu-efi-signature.c',
+    'fu-efi-signature-common.c',
     'fu-efi-signature-list.c',
     'fu-efi-signature-parser.c',
   ],
@@ -81,6 +84,7 @@ dbxtool = executable(
     'fu-dbxtool.c',
     'fu-uefi-dbx-common.c',
     'fu-efi-signature.c',
+    'fu-efi-signature-common.c',
     'fu-efi-signature-list.c',
     'fu-efi-signature-parser.c',
   ],