From b6f79556f1c1d8710cd061e4b36c074afdba7f52 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Sat, 11 Nov 2017 07:58:17 +0000 Subject: [PATCH] Use a SHA1 hash for the internal DeviceID It's actually less scary to see a SHA1 hash than it is to see a path like /sys/devices/pci0000:00/0000:00:1d.0/usb1/1-1/1-1.2. It's also way easier to copy and paste into the various fwupdmgr command that require a device ID and also means we can match a partial prefix much like git allows. If we also move to a model where plugins can be changed during different stages of the update (e.g. during detach) then the device might change connection type and then the sysfs path not only becomes difficult to paste, but incorrect. Session software doesn't care about the format of the device ID (it is supposed to be an implementation detail) and so there's no API or ABI break here. A few plugins also needed to be ported, but nothing too worrying. --- plugins/altos/fu-altos-device.c | 7 +- plugins/altos/fu-plugin-altos.c | 5 +- plugins/dfu/fu-plugin-dfu.c | 25 +++---- plugins/ebitdo/fu-ebitdo-device.c | 10 ++- plugins/ebitdo/fu-plugin-ebitdo.c | 10 +-- plugins/nitrokey/fu-nitrokey-device.c | 2 +- plugins/raspberrypi/fu-self-test.c | 3 +- plugins/thunderbolt/fu-plugin-thunderbolt.c | 6 +- plugins/thunderbolt/fu-self-test.c | 6 +- plugins/unifying/fu-plugin-unifying.c | 6 +- src/fu-device.c | 66 ++++++++++++++++++ src/fu-device.h | 6 +- src/fu-engine.c | 58 ++++++++++++---- src/fu-main.c | 47 +++++++++++++ src/fu-pending.c | 3 +- src/fu-self-test.c | 75 ++++++++++++++++++--- 16 files changed, 269 insertions(+), 66 deletions(-) diff --git a/plugins/altos/fu-altos-device.c b/plugins/altos/fu-altos-device.c index 0bd18dbee..6afe18fea 100644 --- a/plugins/altos/fu-altos-device.c +++ b/plugins/altos/fu-altos-device.c @@ -770,8 +770,11 @@ fu_altos_device_init_real (FuAltosDevice *device) g_autofree gchar *vendor_id = NULL; /* allowed, but requires manual bootloader step */ - fu_device_add_flag (FU_DEVICE (device), - FWUPD_DEVICE_FLAG_UPDATABLE); + fu_device_add_flag (FU_DEVICE (device), FWUPD_DEVICE_FLAG_UPDATABLE); + + /* set USB platform ID */ + fu_device_set_platform_id (FU_DEVICE (device), + g_usb_device_get_platform_id (priv->usb_device)); /* set default vendor */ fu_device_set_vendor (FU_DEVICE (device), "altusmetrum.org"); diff --git a/plugins/altos/fu-plugin-altos.c b/plugins/altos/fu-plugin-altos.c index 8dfc961cd..773e0343c 100644 --- a/plugins/altos/fu-plugin-altos.c +++ b/plugins/altos/fu-plugin-altos.c @@ -55,15 +55,12 @@ fu_plugin_altos_device_added (FuPlugin *plugin, return FALSE; } - /* create the device */ - platform_id = g_usb_device_get_platform_id (usb_device); - fu_device_set_id (FU_DEVICE (dev), platform_id); - /* get device properties */ if (!fu_altos_device_probe (dev, error)) return FALSE; /* only the bootloader can do the update */ + platform_id = g_usb_device_get_platform_id (usb_device); runtime_id = g_strdup_printf ("%s-runtime", platform_id); if (fu_altos_device_get_kind (dev) == FU_ALTOS_DEVICE_KIND_BOOTLOADER) { FuDevice *dev_runtime; diff --git a/plugins/dfu/fu-plugin-dfu.c b/plugins/dfu/fu-plugin-dfu.c index 2df9f6e00..4a987235e 100644 --- a/plugins/dfu/fu-plugin-dfu.c +++ b/plugins/dfu/fu-plugin-dfu.c @@ -163,7 +163,7 @@ fu_plugin_dfu_device_added_cb (DfuContext *ctx, /* create new device */ dev = fu_device_new (); - fu_device_set_id (dev, platform_id); + fu_device_set_platform_id (dev, platform_id); if (!fu_plugin_dfu_device_update (plugin, dev, device, &error)) { g_debug ("ignoring device: %s", error->message); return; @@ -245,7 +245,7 @@ fu_plugin_update_detach (FuPlugin *plugin, FuDevice *dev, GError **error) /* get device */ device = dfu_context_get_device_by_platform_id (data->context, - fu_device_get_id (dev), + fu_device_get_platform_id (dev), error); if (device == NULL) return FALSE; @@ -290,7 +290,7 @@ fu_plugin_update_attach (FuPlugin *plugin, FuDevice *dev, GError **error) /* get device */ device = dfu_context_get_device_by_platform_id (data->context, - fu_device_get_id (dev), + fu_device_get_platform_id (dev), error); if (device == NULL) return FALSE; @@ -342,7 +342,7 @@ fu_plugin_update (FuPlugin *plugin, /* get device */ device = dfu_context_get_device_by_platform_id (data->context, - fu_device_get_id (dev), + fu_device_get_platform_id (dev), error); if (device == NULL) return FALSE; @@ -390,7 +390,6 @@ fu_plugin_verify (FuPlugin *plugin, FuPluginData *data = fu_plugin_get_data (plugin); GBytes *blob_fw; DfuDevice *device; - const gchar *platform_id; g_autoptr(DfuFirmware) dfu_firmware = NULL; g_autoptr(FuDeviceLocker) locker = NULL; g_autoptr(GError) error_local = NULL; @@ -400,18 +399,11 @@ fu_plugin_verify (FuPlugin *plugin, 0 }; /* get device */ - platform_id = fu_device_get_id (dev); device = dfu_context_get_device_by_platform_id (data->context, - platform_id, - &error_local); - if (device == NULL) { - g_set_error (error, - FWUPD_ERROR, - FWUPD_ERROR_INTERNAL, - "cannot find device %s: %s", - platform_id, error_local->message); + fu_device_get_platform_id (dev), + error); + if (device == NULL) return FALSE; - } /* open it */ locker = fu_device_locker_new_full (device, @@ -423,7 +415,8 @@ fu_plugin_verify (FuPlugin *plugin, FWUPD_ERROR, FWUPD_ERROR_INTERNAL, "failed to open DFU device %s: %s", - platform_id, error_local->message); + fu_device_get_id (dev), + error_local->message); return FALSE; } g_signal_connect (device, "state-changed", diff --git a/plugins/ebitdo/fu-ebitdo-device.c b/plugins/ebitdo/fu-ebitdo-device.c index 44994062e..8d9d35756 100644 --- a/plugins/ebitdo/fu-ebitdo-device.c +++ b/plugins/ebitdo/fu-ebitdo-device.c @@ -651,8 +651,11 @@ fu_ebitdo_device_init_real (FuEbitdoDevice *device) g_autofree gchar *vendor_id = NULL; /* allowed, but requires manual bootloader step */ - fu_device_add_flag (FU_DEVICE (device), - FWUPD_DEVICE_FLAG_UPDATABLE); + fu_device_add_flag (FU_DEVICE (device), FWUPD_DEVICE_FLAG_UPDATABLE); + + /* set USB platform ID */ + fu_device_set_platform_id (FU_DEVICE (device), + g_usb_device_get_platform_id (priv->usb_device)); /* set name and vendor */ name = g_strdup_printf ("%s Gamepad", @@ -666,6 +669,9 @@ fu_ebitdo_device_init_real (FuEbitdoDevice *device) vendor_id = g_strdup_printf ("USB:0x%04X", g_usb_device_get_vid (priv->usb_device)); fu_device_set_vendor_id (FU_DEVICE (device), vendor_id); + /* add a hardcoded icon name */ + fu_device_add_icon (FU_DEVICE (device), "input-gaming"); + /* add USB\VID_0000&PID_0000 */ devid1 = g_strdup_printf ("USB\\VID_%04X&PID_%04X", g_usb_device_get_vid (priv->usb_device), diff --git a/plugins/ebitdo/fu-plugin-ebitdo.c b/plugins/ebitdo/fu-plugin-ebitdo.c index 9fb655394..04fa9acf9 100644 --- a/plugins/ebitdo/fu-plugin-ebitdo.c +++ b/plugins/ebitdo/fu-plugin-ebitdo.c @@ -48,7 +48,6 @@ fu_plugin_ebitdo_device_added (FuPlugin *plugin, g_assert (ptask != NULL); /* create the device */ - platform_id = g_usb_device_get_platform_id (usb_device); dev = fu_ebitdo_device_new (usb_device); if (dev == NULL) { g_set_error_literal (error, @@ -57,10 +56,6 @@ fu_plugin_ebitdo_device_added (FuPlugin *plugin, "invalid 8Bitdo device type detected"); return FALSE; } - fu_device_set_id (dev, platform_id); - - /* add a hardcoded icon name */ - fu_device_add_icon (dev, "input-gaming"); /* open the device */ locker = fu_device_locker_new_full (dev, @@ -72,6 +67,7 @@ fu_plugin_ebitdo_device_added (FuPlugin *plugin, ebitdo_kind = fu_ebitdo_device_get_kind (dev); /* only the bootloader can do the update */ + platform_id = g_usb_device_get_platform_id (usb_device); runtime_id = g_strdup_printf ("%s-runtime", platform_id); if (ebitdo_kind == FU_EBITDO_DEVICE_KIND_BOOTLOADER) { FuEbitdoDevice *dev_runtime; @@ -114,15 +110,13 @@ fu_plugin_update (FuPlugin *plugin, { GUsbContext *usb_ctx = fu_plugin_get_usb_context (plugin); FuEbitdoDevice *ebitdo_dev = FU_EBITDO_DEVICE (dev); - const gchar *platform_id; g_autoptr(FuDeviceLocker) locker = NULL; g_autoptr(GUsbDevice) usb_device = NULL; g_autoptr(GUsbDevice) usb_device2 = NULL; /* get version */ - platform_id = fu_device_get_id (dev); usb_device = g_usb_context_find_by_platform_id (usb_ctx, - platform_id, + fu_device_get_platform_id (dev), error); if (usb_device == NULL) return FALSE; diff --git a/plugins/nitrokey/fu-nitrokey-device.c b/plugins/nitrokey/fu-nitrokey-device.c index 4ab96841d..c34bbb8db 100644 --- a/plugins/nitrokey/fu-nitrokey-device.c +++ b/plugins/nitrokey/fu-nitrokey-device.c @@ -304,7 +304,7 @@ fu_nitrokey_device_open (FuNitrokeyDevice *device, GError **error) * handled by multiple plugins this won't be required... */ platform_id = g_usb_device_get_platform_id (priv->usb_device); platform_id_fixed = g_strdup_printf ("%s_workaround", platform_id); - fu_device_set_id (FU_DEVICE (device), platform_id_fixed); + fu_device_set_platform_id (FU_DEVICE (device), platform_id_fixed); fu_device_set_name (FU_DEVICE (device), "Nitrokey Storage"); fu_device_set_vendor (FU_DEVICE (device), "Nitrokey"); fu_device_set_summary (FU_DEVICE (device), "A secure memory stick"); diff --git a/plugins/raspberrypi/fu-self-test.c b/plugins/raspberrypi/fu-self-test.c index b60f5d3a7..f4549105c 100644 --- a/plugins/raspberrypi/fu-self-test.c +++ b/plugins/raspberrypi/fu-self-test.c @@ -87,7 +87,8 @@ fu_plugin_raspberrypi_func (void) /* check we did the right thing */ g_assert_cmpint (cnt, ==, 0); g_assert (device != NULL); - g_assert_cmpstr (fu_device_get_id (device), ==, "raspberry-pi"); + g_assert_cmpstr (fu_device_get_id (device), ==, + "94f01ac16574856944fb7e7583db423360430f97"); g_assert_cmpstr (fu_device_get_guid_default (device), ==, "91dd7368-8640-5d72-a217-a505c034dd0b"); g_assert_cmpstr (fu_device_get_version (device), ==, diff --git a/plugins/thunderbolt/fu-plugin-thunderbolt.c b/plugins/thunderbolt/fu-plugin-thunderbolt.c index 948d9811b..d1b6ec81b 100644 --- a/plugins/thunderbolt/fu-plugin-thunderbolt.c +++ b/plugins/thunderbolt/fu-plugin-thunderbolt.c @@ -219,7 +219,7 @@ fu_plugin_thunderbolt_add (FuPlugin *plugin, GUdevDevice *device) fu_device_add_flag (dev, FWUPD_DEVICE_FLAG_UPDATABLE); } - fu_device_set_id (dev, uuid); + fu_device_set_platform_id (dev, uuid); fu_device_set_metadata (dev, "sysfs-path", devpath); name = g_udev_device_get_sysfs_attr (device, "device_name"); @@ -589,7 +589,7 @@ on_wait_for_device_removed (FuPlugin *plugin, return; fu_plugin_cache_remove (plugin, id); - uuid = fu_device_get_id (dev); + uuid = fu_device_get_platform_id (dev); g_hash_table_insert (up_data->changes, (gpointer) uuid, g_object_ref (dev)); @@ -647,7 +647,7 @@ fu_plugin_thunderbolt_wait_for_device (FuPlugin *plugin, g_autoptr(GHashTable) changes = NULL; up_data.mainloop = mainloop = g_main_loop_new (NULL, FALSE); - up_data.target_uuid = fu_device_get_id (dev); + up_data.target_uuid = fu_device_get_platform_id (dev); /* this will limit the maximum amount of time we wait for * the device (i.e. 'dev') to re-appear. */ diff --git a/plugins/thunderbolt/fu-self-test.c b/plugins/thunderbolt/fu-self-test.c index 423f960c0..e2ca1d57e 100644 --- a/plugins/thunderbolt/fu-self-test.c +++ b/plugins/thunderbolt/fu-self-test.c @@ -470,7 +470,7 @@ sync_device_added (FuPlugin *plugin, FuDevice *device, gpointer user_data) { SyncContext *ctx = (SyncContext *) user_data; MockTree *tree = ctx->tree; - const char *uuid = fu_device_get_id (device); + const gchar *uuid = fu_device_get_platform_id (device); MockTree *target; target = (MockTree *) mock_tree_find_uuid (tree, uuid); @@ -491,7 +491,7 @@ sync_device_removed (FuPlugin *plugin, FuDevice *device, gpointer user_data) { SyncContext *ctx = (SyncContext *) user_data; MockTree *tree = ctx->tree; - const char *uuid = fu_device_get_id (device); + const gchar *uuid = fu_device_get_platform_id (device); MockTree *target; target = (MockTree *) mock_tree_find_uuid (tree, uuid); @@ -551,7 +551,7 @@ mock_tree_plugin_device_added (FuPlugin *plugin, FuDevice *device, gpointer user { AttachContext *ctx = (AttachContext *) user_data; MockTree *tree = ctx->tree; - const char *uuid = fu_device_get_id (device); + const gchar *uuid = fu_device_get_platform_id (device); MockTree *target; target = (MockTree *) mock_tree_find_uuid (tree, uuid); diff --git a/plugins/unifying/fu-plugin-unifying.c b/plugins/unifying/fu-plugin-unifying.c index 5c6486870..afba023d6 100644 --- a/plugins/unifying/fu-plugin-unifying.c +++ b/plugins/unifying/fu-plugin-unifying.c @@ -61,7 +61,7 @@ fu_plugin_unifying_device_added (FuPlugin *plugin, dev = fu_device_new (); if (lu_device_has_flag (device, LU_DEVICE_FLAG_CAN_FLASH)) fu_device_add_flag (dev, FWUPD_DEVICE_FLAG_UPDATABLE); - fu_device_set_id (dev, lu_device_get_platform_id (device)); + fu_device_set_platform_id (dev, lu_device_get_platform_id (device)); fu_device_set_name (dev, lu_device_get_product (device)); if (lu_device_get_kind (device) == LU_DEVICE_KIND_PERIPHERAL) { const gchar *tmp; @@ -123,7 +123,9 @@ fu_plugin_unifying_get_device (FuPlugin *plugin, GError **error) { FuPluginData *data = fu_plugin_get_data (plugin); - return lu_context_find_by_platform_id (data->ctx, fu_device_get_id (dev), error); + return lu_context_find_by_platform_id (data->ctx, + fu_device_get_platform_id (dev), + error); } static gboolean diff --git a/src/fu-device.c b/src/fu-device.c index 777b875df..798bff54f 100644 --- a/src/fu-device.c +++ b/src/fu-device.c @@ -313,6 +313,72 @@ fu_device_set_name (FuDevice *device, const gchar *value) fwupd_device_set_name (FWUPD_DEVICE (device), new->str); } +/** + * fu_device_set_id: + * @device: A #FuDevice + * @id: a string, e.g. `tbt-port1` + * + * Sets the ID on the device. The ID should represent the *connection* of the + * 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(). + * + * Since: 0.7.1 + **/ +void +fu_device_set_id (FuDevice *device, const gchar *id) +{ + g_autofree gchar *id_hash = NULL; + g_return_if_fail (FU_IS_DEVICE (device)); + 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); + fwupd_device_set_id (FWUPD_DEVICE (device), id_hash); +} + +/** + * fu_device_set_platform_id: + * @device: A #FuDevice + * @platform_id: a platform string, e.g. `/sys/devices/usb1/1-1/1-1.2` + * + * Sets the Platform ID on the device. If unset, the ID will automatically + * be set using a hash of the @platform_id value. + * + * Since: 1.0.2 + **/ +void +fu_device_set_platform_id (FuDevice *device, const gchar *platform_id) +{ + g_return_if_fail (FU_IS_DEVICE (device)); + g_return_if_fail (platform_id != NULL); + + /* automatically use this */ + if (fu_device_get_id (device) == NULL) + fu_device_set_id (device, platform_id); + fu_device_set_metadata (device, "platform-id", platform_id); +} + +/** + * fu_device_get_platform_id: + * @device: A #FuDevice + * + * Gets the Platform ID set for the device, which represents the connection + * string used to compare devices. + * + * Returns: a string value, or %NULL if never set. + * + * Since: 1.0.2 + **/ +const gchar * +fu_device_get_platform_id (FuDevice *device) +{ + g_return_val_if_fail (FU_IS_DEVICE (device), NULL); + return fu_device_get_metadata (device, "platform-id"); +} + static void fwupd_pad_kv_str (GString *str, const gchar *key, const gchar *value) { diff --git a/src/fu-device.h b/src/fu-device.h index 98a86e629..8512bc618 100644 --- a/src/fu-device.h +++ b/src/fu-device.h @@ -47,7 +47,6 @@ FuDevice *fu_device_new (void); #define fu_device_set_description(d,v) fwupd_device_set_description(FWUPD_DEVICE(d),v) #define fu_device_set_flags(d,v) fwupd_device_set_flags(FWUPD_DEVICE(d),v) #define fu_device_has_guid(d,v) fwupd_device_has_guid(FWUPD_DEVICE(d),v) -#define fu_device_set_id(d,v) fwupd_device_set_id(FWUPD_DEVICE(d),v) #define fu_device_set_modified(d,v) fwupd_device_set_modified(FWUPD_DEVICE(d),v) #define fu_device_set_plugin(d,v) fwupd_device_set_plugin(FWUPD_DEVICE(d),v) #define fu_device_set_summary(d,v) fwupd_device_set_summary(FWUPD_DEVICE(d),v) @@ -99,6 +98,11 @@ void fu_device_set_metadata_boolean (FuDevice *device, void fu_device_set_metadata_integer (FuDevice *device, const gchar *key, guint value); +void fu_device_set_id (FuDevice *device, + const gchar *id); +const gchar *fu_device_get_platform_id (FuDevice *device); +void fu_device_set_platform_id (FuDevice *device, + const gchar *platform_id); void fu_device_set_name (FuDevice *device, const gchar *value); diff --git a/src/fu-engine.c b/src/fu-engine.c index 8ee4828c9..b4ab95b23 100644 --- a/src/fu-engine.c +++ b/src/fu-engine.c @@ -224,19 +224,53 @@ fu_engine_item_free (FuDeviceItem *item) static FuDeviceItem * fu_engine_get_item_by_id (FuEngine *self, const gchar *device_id, GError **error) { - for (guint i = 0; i < self->devices->len; i++) { - FuDeviceItem *item = g_ptr_array_index (self->devices, i); - if (g_strcmp0 (fu_device_get_id (item->device), device_id) == 0) - return item; - if (g_strcmp0 (fu_device_get_equivalent_id (item->device), device_id) == 0) - return item; + FuDeviceItem *item = NULL; + gboolean multiple_matches = FALSE; + gsize device_id_len; + + g_return_val_if_fail (device_id != NULL, NULL); + + /* support abbreviated hashes */ + device_id_len = strlen (device_id); + for (gsize i = 0; i < self->devices->len; i++) { + FuDeviceItem *item_tmp = g_ptr_array_index (self->devices, i); + const gchar *ids[] = { + fu_device_get_id (item_tmp->device), + fu_device_get_equivalent_id (item_tmp->device), + NULL }; + for (guint j = 0; ids[j] != NULL; j++) { + if (ids[j] == NULL) + continue; + if (strncmp (ids[j], device_id, device_id_len) == 0) { + if (item != NULL) + multiple_matches = TRUE; + item = item_tmp; + } + } } - g_set_error (error, - FWUPD_ERROR, - FWUPD_ERROR_INVALID_FILE, - "Device %s was not found", - device_id); - return NULL; + + /* nothing at all matched */ + if (item == NULL) { + g_set_error (error, + FWUPD_ERROR, + FWUPD_ERROR_NOT_FOUND, + "device ID %s was not found", + device_id); + return NULL; + } + + /* multiple things matched */ + if (multiple_matches) { + g_set_error (error, + FWUPD_ERROR, + FWUPD_ERROR_NOT_SUPPORTED, + "device ID %s was not unique", + device_id); + return NULL; + } + + /* something found */ + return item; } static FuDeviceItem * diff --git a/src/fu-main.c b/src/fu-main.c index 110288c63..268b31ef4 100644 --- a/src/fu-main.c +++ b/src/fu-main.c @@ -412,6 +412,21 @@ fu_main_authorize_install_cb (GObject *source, GAsyncResult *res, gpointer user_ g_dbus_method_invocation_return_value (helper->invocation, NULL); } +static gboolean +fu_main_device_id_valid (const gchar *device_id, GError **error) +{ + if (g_strcmp0 (device_id, FWUPD_DEVICE_ID_ANY) == 0) + return TRUE; + if (device_id != NULL && strlen (device_id) >= 4) + return TRUE; + g_set_error (error, + FWUPD_ERROR, + FWUPD_ERROR_INTERNAL, + "invalid device ID: %s", + device_id); + return FALSE; +} + static void fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, const gchar *object_path, const gchar *interface_name, @@ -439,6 +454,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, g_autoptr(GPtrArray) releases = NULL; g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } releases = fu_engine_get_releases (priv->engine, device_id, &error); if (releases == NULL) { g_dbus_method_invocation_return_gerror (invocation, error); @@ -453,6 +472,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, g_autoptr(GPtrArray) releases = NULL; g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } releases = fu_engine_get_downgrades (priv->engine, device_id, &error); if (releases == NULL) { g_dbus_method_invocation_return_gerror (invocation, error); @@ -467,6 +490,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, g_autoptr(GPtrArray) releases = NULL; g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } releases = fu_engine_get_upgrades (priv->engine, device_id, &error); if (releases == NULL) { g_dbus_method_invocation_return_gerror (invocation, error); @@ -504,6 +531,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, g_autoptr(FwupdDevice) result = NULL; g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } result = fu_engine_get_results (priv->engine, device_id, &error); if (result == NULL) { g_dbus_method_invocation_return_gerror (invocation, error); @@ -563,6 +594,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } /* authenticate */ fu_main_set_status (priv, FWUPD_STATUS_WAITING_FOR_AUTH); @@ -619,6 +654,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, /* check the id exists */ g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } /* create helper object */ helper = g_new0 (FuMainAuthHelper, 1); @@ -642,6 +681,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, const gchar *device_id = NULL; g_variant_get (parameters, "(&s)", &device_id); g_debug ("Called %s(%s)", method_name, device_id); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } if (!fu_engine_verify (priv->engine, device_id, &error)) { g_dbus_method_invocation_return_gerror (invocation, error); return; @@ -665,6 +708,10 @@ fu_main_daemon_method_call (GDBusConnection *connection, const gchar *sender, /* check the id exists */ g_variant_get (parameters, "(&sha{sv})", &device_id, &fd_handle, &iter); g_debug ("Called %s(%s,%i)", method_name, device_id, fd_handle); + if (!fu_main_device_id_valid (device_id, &error)) { + g_dbus_method_invocation_return_gerror (invocation, error); + return; + } /* create helper object */ helper = g_new0 (FuMainAuthHelper, 1); diff --git a/src/fu-pending.c b/src/fu-pending.c index 63c486954..9b5eaa309 100644 --- a/src/fu-pending.c +++ b/src/fu-pending.c @@ -268,7 +268,8 @@ fu_pending_device_sqlite_cb (void *data, g_debug ("FuPending: got sql result %s", argv[0]); for (gint i = 0; i < argc; i++) { if (g_strcmp0 (col_name[i], "device_id") == 0) { - fu_device_set_id (device, argv[i]); + /* we don't want to hash-the-hash */ + fwupd_device_set_id (FWUPD_DEVICE (device), argv[i]); continue; } if (g_strcmp0 (col_name[i], "filename") == 0) { diff --git a/src/fu-self-test.c b/src/fu-self-test.c index 09fae5996..05432ceff 100644 --- a/src/fu-self-test.c +++ b/src/fu-self-test.c @@ -48,10 +48,62 @@ #include "fu-keyring-pkcs7.h" #endif +static void +fu_engine_partial_hash_func (void) +{ + gboolean ret; + g_autoptr(FuDevice) device1 = fu_device_new (); + g_autoptr(FuDevice) device2 = fu_device_new (); + g_autoptr(FuEngine) engine = fu_engine_new (); + g_autoptr(FuPlugin) plugin = fu_plugin_new (); + g_autoptr(GError) error = NULL; + g_autoptr(GError) error_none = NULL; + g_autoptr(GError) error_both = NULL; + + /* add two dummy devices */ + fu_device_set_id (device1, "device1"); + fu_device_add_guid (device1, "12345678-1234-1234-1234-123456789012"); + fu_engine_add_device (engine, plugin, device1); + fu_device_set_id (device2, "device21"); + fu_device_set_equivalent_id (device2, "b92f5b7560b84ca005a79f5a15de3c003ce494cf"); + fu_device_add_guid (device2, "12345678-1234-1234-1234-123456789012"); + fu_engine_add_device (engine, plugin, device2); + + /* match nothing */ + ret = fu_engine_unlock (engine, "deadbeef", &error_none); + g_assert_error (error_none, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND); + g_assert (!ret); + + /* match both */ + ret = fu_engine_unlock (engine, "9", &error_both); + g_assert_error (error_both, FWUPD_ERROR, FWUPD_ERROR_NOT_SUPPORTED); + g_assert (!ret); + + /* match one exactly */ + fu_device_add_flag (device1, FWUPD_DEVICE_FLAG_LOCKED); + fu_device_add_flag (device2, FWUPD_DEVICE_FLAG_LOCKED); + ret = fu_engine_unlock (engine, "934b4162a6daa0b033d649c8d464529cec41d3de", &error); + g_assert_no_error (error); + g_assert (ret); + + /* match one partially */ + fu_device_add_flag (device1, FWUPD_DEVICE_FLAG_LOCKED); + fu_device_add_flag (device2, FWUPD_DEVICE_FLAG_LOCKED); + ret = fu_engine_unlock (engine, "934b", &error); + g_assert_no_error (error); + g_assert (ret); + + /* match equivalent ID */ + fu_device_add_flag (device1, FWUPD_DEVICE_FLAG_LOCKED); + fu_device_add_flag (device2, FWUPD_DEVICE_FLAG_LOCKED); + ret = fu_engine_unlock (engine, "b92f", &error); + g_assert_no_error (error); + g_assert (ret); +} + static void fu_engine_require_hwid_func (void) { - const gchar *device_id = "test_device"; gboolean ret; g_autofree gchar *filename = NULL; g_autoptr(AsStore) store = NULL; @@ -83,7 +135,8 @@ fu_engine_require_hwid_func (void) fu_engine_add_device (engine, plugin, device); /* install it */ - ret = fu_engine_install (engine, device_id, store, blob_cab, FWUPD_INSTALL_FLAG_NONE, &error); + ret = fu_engine_install (engine, fu_device_get_id (device), + store, blob_cab, FWUPD_INSTALL_FLAG_NONE, &error); g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_INVALID_FILE); g_assert (error != NULL); g_assert_cmpstr (error->message, ==, @@ -424,13 +477,13 @@ fu_plugin_delay_func (void) fu_device_set_id (device, "testdev"); fu_plugin_device_add (plugin, device); g_assert (device_tmp != NULL); - g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "testdev"); + g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "b7eccd0059d6d7dc2ef76c35d6de0048cc8c029d"); g_clear_object (&device_tmp); /* remove device */ fu_plugin_device_remove (plugin, device); g_assert (device_tmp != NULL); - g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "testdev"); + g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "b7eccd0059d6d7dc2ef76c35d6de0048cc8c029d"); g_clear_object (&device_tmp); /* add it with a small delay */ @@ -438,7 +491,7 @@ fu_plugin_delay_func (void) g_assert (device_tmp == NULL); fu_test_loop_run_with_timeout (1000); g_assert (device_tmp != NULL); - g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "testdev"); + g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "b7eccd0059d6d7dc2ef76c35d6de0048cc8c029d"); g_clear_object (&device_tmp); /* add it again, twice quickly */ @@ -447,7 +500,7 @@ fu_plugin_delay_func (void) g_assert (device_tmp == NULL); fu_test_loop_run_with_timeout (1000); g_assert (device_tmp != NULL); - g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "testdev"); + g_assert_cmpstr (fu_device_get_id (device_tmp), ==, "b7eccd0059d6d7dc2ef76c35d6de0048cc8c029d"); g_clear_object (&device_tmp); } @@ -539,7 +592,7 @@ fu_plugin_module_func (void) /* check we did the right thing */ g_assert_cmpint (cnt, ==, 0); g_assert (device != NULL); - g_assert_cmpstr (fu_device_get_id (device), ==, "FakeDevice"); + g_assert_cmpstr (fu_device_get_id (device), ==, "08d460be0f1f9f128413f816022a6439e0078018"); g_assert_cmpstr (fu_device_get_version_lowest (device), ==, "1.2.0"); g_assert_cmpstr (fu_device_get_version (device), ==, "1.2.3"); g_assert_cmpstr (fu_device_get_version_bootloader (device), ==, "0.1.2"); @@ -656,6 +709,7 @@ fu_pending_func (void) /* add some extra data */ device = fu_device_new (); + /* the SHA1SUM of this is 2ba16d10df45823dd4494ff10a0bfccfef512c9d */ fu_device_set_id (device, "self-test"); ret = fu_pending_set_state (pending, device, FWUPD_UPDATE_STATE_PENDING, &error); g_assert_no_error (error); @@ -666,10 +720,10 @@ fu_pending_func (void) g_object_unref (device); /* get device */ - device = fu_pending_get_device (pending, "self-test", &error); + device = fu_pending_get_device (pending, "2ba16d10df45823dd4494ff10a0bfccfef512c9d", &error); g_assert_no_error (error); g_assert (device != NULL); - g_assert_cmpstr (fu_device_get_id (device), ==, "self-test"); + g_assert_cmpstr (fu_device_get_id (device), ==, "2ba16d10df45823dd4494ff10a0bfccfef512c9d"); g_assert_cmpstr (fu_device_get_name (device), ==, "ColorHug"); g_assert_cmpstr (fu_device_get_version (device), ==, "3.0.1"); g_assert_cmpint (fu_device_get_update_state (device), ==, FWUPD_UPDATE_STATE_PENDING); @@ -693,7 +747,7 @@ fu_pending_func (void) g_object_unref (device); /* get device that does not exist */ - device = fu_pending_get_device (pending, "self-test", &error); + device = fu_pending_get_device (pending, "2ba16d10df45823dd4494ff10a0bfccfef512c9d", &error); g_assert_error (error, FWUPD_ERROR, FWUPD_ERROR_NOT_FOUND); g_assert (device == NULL); g_clear_error (&error); @@ -1006,6 +1060,7 @@ main (int argc, char **argv) g_test_add_func ("/fwupd/device{metadata}", fu_device_metadata_func); g_test_add_func ("/fwupd/engine", fu_engine_func); g_test_add_func ("/fwupd/engine{require-hwid}", fu_engine_require_hwid_func); + g_test_add_func ("/fwupd/engine{partial-hash}", fu_engine_partial_hash_func); g_test_add_func ("/fwupd/hwids", fu_hwids_func); g_test_add_func ("/fwupd/smbios", fu_smbios_func); g_test_add_func ("/fwupd/smbios3", fu_smbios3_func);