From 1bde4fdd29180ea5b55c4151e86e2a93d6ef412e Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Mon, 20 Apr 2020 11:43:01 +0100 Subject: [PATCH] Fix the DeviceID set by GetDetails The returned ID is the result of the SHA1 hash of the actual device ID. This does not match anything found by the client, and so the install fails. The symbol is exported as I think the device ID is an important identifier and used in various fwupd tools. When backported to the stable branch the verification should just be a static function in src/fu-engine.c rather than a new symbol. --- libfwupd/fwupd-common.c | 28 ++++++++++++++++++++++++++++ libfwupd/fwupd-common.h | 1 + libfwupd/fwupd-self-test.c | 14 ++++++++++++++ libfwupd/fwupd.map | 6 ++++++ libfwupdplugin/fu-device.c | 15 ++++++++++----- 5 files changed, 59 insertions(+), 5 deletions(-) diff --git a/libfwupd/fwupd-common.c b/libfwupd/fwupd-common.c index c4eac8eec..7dd42962f 100644 --- a/libfwupd/fwupd-common.c +++ b/libfwupd/fwupd-common.c @@ -817,6 +817,34 @@ fwupd_guid_hash_data (const guint8 *data, gsize datasz, FwupdGuidFlags flags) return fwupd_guid_to_string ((const fwupd_guid_t *) &uu_new, flags); } +/** + * fwupd_device_id_is_valid: + * @device_id: string to check, e.g. `d3fae86d95e5d56626129d00e332c4b8dac95442` + * + * Checks the string is a valid non-partial device ID. It is important to note + * that the wildcard ID of `*` is not considered a valid ID in this function and + * the client must check for this manually if this should be allowed. + * + * Returns: %TRUE if @guid was a fwupd device ID, %FALSE otherwise + * + * Since: 1.4.1 + **/ +gboolean +fwupd_device_id_is_valid (const gchar *device_id) +{ + if (device_id == NULL) + return FALSE; + if (strlen (device_id) != 40) + return FALSE; + for (guint i = 0; device_id[i] != '\0'; i++) { + gchar tmp = device_id[i]; + /* isalnum isn't case specific */ + if ((tmp < 'a' || tmp > 'f') && (tmp < '0' || tmp > '9')) + return FALSE; + } + return TRUE; +} + /** * fwupd_guid_is_valid: * @guid: string to check, e.g. `00112233-4455-6677-8899-aabbccddeeff` diff --git a/libfwupd/fwupd-common.h b/libfwupd/fwupd-common.h index f8d5234f0..e4409ac74 100644 --- a/libfwupd/fwupd-common.h +++ b/libfwupd/fwupd-common.h @@ -48,6 +48,7 @@ gchar *fwupd_build_machine_id (const gchar *salt, GHashTable *fwupd_get_os_release (GError **error); gchar *fwupd_build_history_report_json (GPtrArray *devices, GError **error); +gboolean fwupd_device_id_is_valid (const gchar *device_id); #ifndef __GI_SCANNER__ gchar *fwupd_guid_to_string (const fwupd_guid_t *guid, FwupdGuidFlags flags); diff --git a/libfwupd/fwupd-self-test.c b/libfwupd/fwupd-self-test.c index a1e895f2a..679360b02 100644 --- a/libfwupd/fwupd-self-test.c +++ b/libfwupd/fwupd-self-test.c @@ -584,6 +584,19 @@ fwupd_common_machine_hash_func (void) g_assert_cmpstr (mhash2, !=, mhash1); } +static void +fwupd_common_device_id_func (void) +{ + g_assert_false (fwupd_device_id_is_valid (NULL)); + g_assert_false (fwupd_device_id_is_valid ("")); + g_assert_false (fwupd_device_id_is_valid ("1ff60ab2-3905-06a1-b476-0371f00c9e9b")); + g_assert_false (fwupd_device_id_is_valid ("aaaaaad3fae86d95e5d56626129d00e332c4b8dac95442")); + g_assert_false (fwupd_device_id_is_valid ("x3fae86d95e5d56626129d00e332c4b8dac95442")); + g_assert_false (fwupd_device_id_is_valid ("D3FAE86D95E5D56626129D00E332C4B8DAC95442")); + g_assert_false (fwupd_device_id_is_valid (FWUPD_DEVICE_ID_ANY)); + g_assert_true (fwupd_device_id_is_valid ("d3fae86d95e5d56626129d00e332c4b8dac95442")); +} + static void fwupd_common_guid_func (void) { @@ -648,6 +661,7 @@ main (int argc, char **argv) /* tests go here */ g_test_add_func ("/fwupd/enums", fwupd_enums_func); g_test_add_func ("/fwupd/common{machine-hash}", fwupd_common_machine_hash_func); + g_test_add_func ("/fwupd/common{device-id}", fwupd_common_device_id_func); g_test_add_func ("/fwupd/common{guid}", fwupd_common_guid_func); g_test_add_func ("/fwupd/release", fwupd_release_func); g_test_add_func ("/fwupd/device", fwupd_device_func); diff --git a/libfwupd/fwupd.map b/libfwupd/fwupd.map index a076504d3..11e99740b 100644 --- a/libfwupd/fwupd.map +++ b/libfwupd/fwupd.map @@ -439,3 +439,9 @@ LIBFWUPD_1.4.0 { fwupd_remote_load_signature; local: *; } LIBFWUPD_1.3.7; + +LIBFWUPD_1.4.1 { + global: + fwupd_device_id_is_valid; + local: *; +} LIBFWUPD_1.4.0; diff --git a/libfwupdplugin/fu-device.c b/libfwupdplugin/fu-device.c index 5deecf5e5..887d7d52b 100644 --- a/libfwupdplugin/fu-device.c +++ b/libfwupdplugin/fu-device.c @@ -1588,9 +1588,9 @@ fu_device_set_name (FuDevice *self, const gchar *value) * device, so that any similar device plugged into a different slot will * have a different @id string. * - * The @id will be converted to a SHA1 hash before the device is added to the - * daemon, and plugins should not assume that the ID that is set here is the - * same as what is returned by fu_device_get_id(). + * The @id will be converted to a SHA1 hash if required before the device is + * added to the daemon, and plugins should not assume that the ID that is set + * here is the same as what is returned by fu_device_get_id(). * * Since: 0.7.1 **/ @@ -1603,8 +1603,13 @@ fu_device_set_id (FuDevice *self, const gchar *id) g_return_if_fail (FU_IS_DEVICE (self)); g_return_if_fail (id != NULL); - id_hash = g_compute_checksum_for_string (G_CHECKSUM_SHA1, id, -1); - g_debug ("using %s for %s", id_hash, id); + /* allow sane device-id to be set directly */ + if (fwupd_device_id_is_valid (id)) { + id_hash = g_strdup (id); + } else { + id_hash = g_compute_checksum_for_string (G_CHECKSUM_SHA1, id, -1); + g_debug ("using %s for %s", id_hash, id); + } fwupd_device_set_id (FWUPD_DEVICE (self), id_hash); priv->device_id_valid = TRUE;