Do not expect the INF ClassGuid to be the ESRT GUID

I misunderstood the role of the ClassGuid for UEFI firmware. It is not supposed
to be the ESRT GUID, but instead a generic 'firmware' GUID set by Microsoft.
The actual ESRT is set as a FirmwareID registry key only, so learn how to parse
the HK section and fix up all the unit tests.

As there are .cab files in the wild (cough, ColorHug, cough) that set the
device ID as the ClassGuid, fall back to this to maintain compatibility with
either method.

Many thanks to Mario Limonciello for helping to debug this.

See https://msdn.microsoft.com/en-us/library/windows/hardware/ff547502%28v=vs.85%29.aspx
for more information.

8 files changed, 111 insertions, 21 deletions

diff --git a/data/tests/example.inf b/data/tests/example.inf
index c6e2a87..38b32f7 100644
--- a/data/tests/example.inf
+++ b/data/tests/example.inf
@@ -3,7 +3,7 @@
 [Version]
 Signature="$WINDOWS NT$"
 Class=Firmware
-ClassGuid={84f40464-9272-4ef7-9399-cd95f12da696}
+ClassGuid={F2E7DD72-6468-4E36-B6f1-6488F42C1B52}
 Provider=%Provider%
 DriverVer=03/03/2015,2.0.2
 PnpLockdown=1
@@ -30,8 +30,8 @@ AddReg = Firmware_AddReg
 
 [Firmware_AddReg]
 HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da695}
-HKR,,FirmwareVersion,%REG_DWORD%,0x0000000
-HKR,,FirmwareFilename,,firmware.bin
+HKR,,FirmwareVersion,%REG_DWORD%, 0x0000000
+HKR , , FirmwareFilename , , "firmware.bin"
 
 [SourceDisksNames]
 1 = %DiskName%
diff --git a/data/tests/firmware/2_0_0/firmware.inf b/data/tests/firmware/2_0_0/firmware.inf
index 19cfecf..6299370 100644
--- a/data/tests/firmware/2_0_0/firmware.inf
+++ b/data/tests/firmware/2_0_0/firmware.inf
@@ -2,7 +2,7 @@
 
 [Version]
 Class=Firmware
-ClassGuid=84f40464-9272-4ef7-9399-cd95f12da696
+ClassGuid={f2e7dd72-6468-4e36-b6f1-6488f42c1b52}
 DriverVer=03/01/2015,2.0.0
 
 ; this is a nonstandard section
@@ -12,6 +12,11 @@ URLs=http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.0.cab
 [Firmware_CopyFiles]
 firmware.bin
 
+[Firmware_AddReg]
+HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da696}
+HKR,,FirmwareVersion,%REG_DWORD%,0x0000000
+HKR,,FirmwareFilename,,firmware.bin
+
 [Strings]
 Provider     = "Hughski"
 MfgName      = "Hughski Limited"
diff --git a/data/tests/firmware/2_0_1/firmware.inf b/data/tests/firmware/2_0_1/firmware.inf
index 3b38181..61b6947 100644
--- a/data/tests/firmware/2_0_1/firmware.inf
+++ b/data/tests/firmware/2_0_1/firmware.inf
@@ -2,12 +2,17 @@
 
 [Version]
 Class=Firmware
-ClassGuid=84f40464-9272-4ef7-9399-cd95f12da696
+ClassGuid={f2e7dd72-6468-4e36-b6f1-6488f42c1b52}
 DriverVer=03/01/2015,2.0.1
 
 [Firmware_CopyFiles]
 firmware.bin
 
+[Firmware_AddReg]
+HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da696}
+HKR,,FirmwareVersion,%REG_DWORD%,0x0000000
+HKR,,FirmwareFilename,,firmware.bin
+
 [Strings]
 Provider     = "Hughski"
 MfgName      = "Hughski Limited"
diff --git a/data/tests/firmware/2_0_2/firmware.inf b/data/tests/firmware/2_0_2/firmware.inf
index 07b0f7d..fe604b7 100644
--- a/data/tests/firmware/2_0_2/firmware.inf
+++ b/data/tests/firmware/2_0_2/firmware.inf
@@ -2,12 +2,17 @@
 
 [Version]
 Class=Firmware
-ClassGuid=84f40464-9272-4ef7-9399-cd95f12da696
+ClassGuid={f2e7dd72-6468-4e36-b6f1-6488f42c1b52}
 DriverVer=03/03/2015,2.0.2
 
 [Firmware_CopyFiles]
 firmware.bin
 
+[Firmware_AddReg]
+HKR,,FirmwareId,,{84f40464-9272-4ef7-9399-cd95f12da696}
+HKR,,FirmwareVersion,%REG_DWORD%,0x0000000
+HKR,,FirmwareFilename,,firmware.bin
+
 [Strings]
 Provider     = "Hughski"
 MfgName      = "Hughski Limited"
diff --git a/libappstream-builder/asb-self-test.c b/libappstream-builder/asb-self-test.c
index cea09cd..b1fbe4d 100644
--- a/libappstream-builder/asb-self-test.c
+++ b/libappstream-builder/asb-self-test.c
@@ -427,7 +427,7 @@ asb_test_context_test_func (AsbTestContextMode mode)
 		"<releases>\n"
 		"<release version=\"2.0.2\" timestamp=\"1424116753\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.2.cab</location>\n"
-		"<checksum type=\"sha1\">ab2d0294e0d23c1718af79c6a17216c8322ae6da</checksum>\n"
+		"<checksum type=\"sha1\">522aeefdcc7f27658d73de76107fe62550c61868</checksum>\n"
 		"<description><p>This unstable release adds the following features:</p>"
 		"<ul><li>Add TakeReadingArray to enable panel latency measurements</li>"
 		"<li>Speed up the auto-scaled measurements considerably, using 256ms "
@@ -928,7 +928,7 @@ asb_test_firmware_func (void)
 		"<releases>\n"
 		"<release version=\"2.0.2\" timestamp=\"1424116753\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.2.cab</location>\n"
-		"<checksum type=\"sha1\">ab2d0294e0d23c1718af79c6a17216c8322ae6da</checksum>\n"
+		"<checksum type=\"sha1\">522aeefdcc7f27658d73de76107fe62550c61868</checksum>\n"
 		"<description><p>This unstable release adds the following features:</p>"
 		"<ul><li>Add TakeReadingArray to enable panel latency measurements</li>"
 		"<li>Speed up the auto-scaled measurements considerably, using 256ms "
@@ -936,7 +936,7 @@ asb_test_firmware_func (void)
 		"</release>\n"
 		"<release version=\"2.0.1\" timestamp=\"1424116753\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.1.cab</location>\n"
-		"<checksum type=\"sha1\">cc07f12dfea7103774decbcf48c47cc0604fbc4e</checksum>\n"
+		"<checksum type=\"sha1\">c7cc46d54d8d4ba452b677a2367214efbaa40fd5</checksum>\n"
 		"<description><p>This unstable release adds the following features:</p>"
 		"<ul><li>Use TakeReadings() to do a quick non-adaptive measurement</li>"
 		"<li>Scale XYZ measurement with a constant factor to make the CCMX more "
@@ -944,7 +944,7 @@ asb_test_firmware_func (void)
 		"</release>\n"
 		"<release version=\"2.0.0\" timestamp=\"1425168000\">\n"
 		"<location>http://www.hughski.com/downloads/colorhug2/firmware/colorhug-2.0.0.cab</location>\n"
-		"<checksum type=\"sha1\">81431b6d3c9f90c3137a8a132d4fb5e3681e6791</checksum>\n"
+		"<checksum type=\"sha1\">70de24d3ef22ad4ded97a3da0167df804c0d1a10</checksum>\n"
 		"</release>\n"
 		"</releases>\n"
 		"</component>\n"
diff --git a/libappstream-glib/as-app-inf.c b/libappstream-glib/as-app-inf.c
index 2ce16dd..80519e1 100644
--- a/libappstream-glib/as-app-inf.c
+++ b/libappstream-glib/as-app-inf.c
@@ -25,7 +25,8 @@
 #include "as-app-private.h"
 #include "as-cleanup.h"
 #include "as-inf.h"
-//#include "as-utils.h"
+
+#define AS_APP_INF_CLASS_GUID_FIRMWARE	"f2e7dd72-6468-4e36-b6f1-6488f42c1b52"
 
 /**
  * as_app_parse_inf_sanitize_guid:
@@ -40,7 +41,7 @@ as_app_parse_inf_sanitize_guid (const gchar *guid)
 
 	for (i = 0; guid[i] != '\0'; i++) {
 		if (g_ascii_isalnum (guid[i]) || guid[i] == '-')
-			g_string_append_c (id, guid[i]);
+			g_string_append_c (id, g_ascii_tolower (guid[i]));
 	}
 	return g_string_free (id, FALSE);
 }
@@ -58,6 +59,8 @@ as_app_parse_inf_file (AsApp *app,
 	guint i;
 	_cleanup_error_free_ GError *error_local = NULL;
 	_cleanup_free_ gchar *catalog_basename = NULL;
+	_cleanup_free_ gchar *class_guid = NULL;
+	_cleanup_free_ gchar *class_guid_unsafe = NULL;
 	_cleanup_free_ gchar *class = NULL;
 	_cleanup_free_ gchar *comment = NULL;
 	_cleanup_free_ gchar *filename_full = NULL;
@@ -102,22 +105,39 @@ as_app_parse_inf_file (AsApp *app,
 			     AS_APP_ERROR_INVALID_TYPE,
 			     "Driver class is '%s', not 'Firmware'", class);
 		return FALSE;
-	}
+       }
 	as_app_set_id_kind (app, AS_ID_KIND_FIRMWARE);
 
-	/* get the GUID */
-	guid = g_key_file_get_string (kf, "Version", "ClassGuid", NULL);
-	if (guid == NULL) {
+	/* get the Class GUID */
+	class_guid_unsafe = g_key_file_get_string (kf, "Version", "ClassGuid", NULL);
+	if (class_guid_unsafe == NULL) {
 		g_set_error_literal (error,
 				     AS_APP_ERROR,
 				     AS_APP_ERROR_INVALID_TYPE,
-				     "Driver ID is missing");
+				     "ClassGuid is missing");
 		return FALSE;
 	}
-
-	/* strip any curley brackets */
-	id = as_app_parse_inf_sanitize_guid (guid);
-	as_app_set_id (app, id, -1);
+	class_guid = as_app_parse_inf_sanitize_guid (class_guid_unsafe);
+	if (g_strcmp0 (class_guid, AS_APP_INF_CLASS_GUID_FIRMWARE) != 0) {
+		g_debug ("ClassGuid is '%s', not '%s', so using as an ID",
+			 class_guid, AS_APP_INF_CLASS_GUID_FIRMWARE);
+		as_app_set_id (app, class_guid, -1);
+	} else {
+		/* get the ESRT GUID */
+		guid = g_key_file_get_string (kf,
+					      "Firmware_AddReg",
+					      "HKR_FirmwareId",
+					      NULL);
+		if (guid == NULL) {
+			g_set_error_literal (error,
+					     AS_APP_ERROR,
+					     AS_APP_ERROR_INVALID_TYPE,
+					     "HKR->FirmwareId missing from [Firmware_AddReg]");
+			return FALSE;
+		}
+		id = as_app_parse_inf_sanitize_guid (guid);
+		as_app_set_id (app, id, -1);
+	}
 
 	/* get vendor */
 	vendor = g_key_file_get_string (kf, "Version", "Provider", NULL);
diff --git a/libappstream-glib/as-inf.c b/libappstream-glib/as-inf.c
index 52bda79..95e3335 100644
--- a/libappstream-glib/as-inf.c
+++ b/libappstream-glib/as-inf.c
@@ -687,6 +687,49 @@ as_inf_parse_line (AsInfHelper *helper, gchar *line, GError **error)
 		goto out;
 	}
 
+	/* convert registry entries. e.g.
+	 * "HKR,,FirmwareFilename,,firmware.bin"
+	 *   -> "HKR_FirmwareFilename"="firmware.bin"
+	 * "HKR,,FirmwareVersion,%REG_DWORD%,0x0000000"
+	 *   -> "HKR_FirmwareVersion_0x00010001"="0x0000000" */
+	if (g_strcmp0 (helper->group, "Firmware_AddReg") == 0 &&
+	    g_str_has_prefix (line, "HK")) {
+		guint i;
+		_cleanup_strv_free_ gchar **reg_split = NULL;
+		_cleanup_string_free_ GString *str = NULL;
+		str = g_string_new ("");
+		reg_split = g_strsplit (line, ",", -1);
+		for (i = 0; reg_split[i+1] != NULL; i++) {
+			g_strstrip (reg_split[i]);
+			if (reg_split[i][0] == '\0')
+				continue;
+			g_string_append_printf (str, "%s_", reg_split[i]);
+		}
+		if (str->len > 0) {
+			_cleanup_free_ gchar *key_tmp = NULL;
+
+			/* remove trailing '_' */
+			g_string_truncate (str, str->len - 1);
+
+			/* remove leading and trailing quote */
+			g_strchug (reg_split[i]);
+			if (!as_inf_strip_value (helper, reg_split[i], error)) {
+				ret = FALSE;
+				goto out;
+			}
+			if (helper->dict == NULL) {
+				helper->require_2nd_pass = TRUE;
+				helper->last_line_continuation_ignore = TRUE;
+				goto out;
+			}
+			key_tmp = as_inf_replace_variable (helper, str->str, error);
+			if (key_tmp == NULL)
+				return FALSE;
+			as_inf_set_key (helper, key_tmp, reg_split[i]);
+			goto out;
+		}
+	}
+
 	/* add fake key */
 	key = g_strdup_printf ("value%03i", helper->nokey_idx++);
 	as_inf_set_key (helper, key, line);
diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index 74ddc43..d4663fd 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -1551,6 +1551,7 @@ as_test_app_parse_file_inf_func (void)
 	g_assert_cmpstr (as_app_get_comment (app, "C"), ==,
 		"Firmware for the ColorHug Colorimeter");
 	g_assert_cmpstr (as_app_get_source_file (app), ==, filename);
+	g_assert_cmpstr (as_app_get_id (app), ==, "84f40464-9272-4ef7-9399-cd95f12da695");
 
 	/* check icon */
 	g_assert_cmpint (as_app_get_icons(app)->len, ==, 1);
@@ -3591,6 +3592,17 @@ as_test_inf_func (void)
 	g_assert_cmpstr (tmp, ==, "Firmware");
 	g_free (tmp);
 
+	/* HK */
+	tmp = g_key_file_get_string (kf, "Firmware_AddReg", "HKR_FirmwareId", NULL);
+	g_assert_cmpstr (tmp, ==, "{84f40464-9272-4ef7-9399-cd95f12da695}");
+	g_free (tmp);
+	tmp = g_key_file_get_string (kf, "Firmware_AddReg", "HKR_FirmwareVersion_0x00010001", NULL);
+	g_assert_cmpstr (tmp, ==, "0x0000000");
+	g_free (tmp);
+	tmp = g_key_file_get_string (kf, "Firmware_AddReg", "HKR_FirmwareFilename", NULL);
+	g_assert_cmpstr (tmp, ==, "firmware.bin");
+	g_free (tmp);
+
 	/* key replacement */
 	tmp = g_key_file_get_string (kf, "Firmware_CopyFiles", "value000", NULL);
 	g_assert_cmpstr (tmp, ==, "firmware.bin");