fwupd / fwupd

A system daemon to allow session software to update firmware
GNU Lesser General Public License v2.1
2.93k stars 438 forks source link

lvfs-testing: no signature method in results #2223

Closed joakim-tjernlund closed 4 years ago

joakim-tjernlund commented 4 years ago

Describe the bug fwupdmgr refresh Fetching signature https://cdn.fwupd.org/downloads/firmware-testing.xml.gz.jcat Downloading… [] Fetching metadata https://cdn.fwupd.org/downloads/firmware-00432-testing.xml.gz Downloading… [] Failed to update metadata for lvfs-testing: no signature method in results

fwupd version information fwupdmgr --version client version: 1.4.4 compile-time dependency versions gusb: 0.3.4 efivar: 37 daemon version: 1.4.4

emerge sys-apps/fwupd

Additional questions

hughsie commented 4 years ago

Operating system and version: Gentoo

Did you compile libjcat without GPG and GNUTLS? What libjcat version are you using?

joakim-tjernlund commented 4 years ago

libjcat 0.1.3

It is built with both gpg an gnutls

joakim-tjernlund commented 4 years ago

Program diff found: YES (/usr/bin/diff)
Found pkg-config: /usr/bin/x86_64-pc-linux-gnu-pkg-config (0.29.2)
Run-time dependency gio-2.0 found: YES 2.62.6
Run-time dependency gio-unix-2.0 found: YES 2.62.6
Run-time dependency json-glib-1.0 found: YES 1.4.4
Run-time dependency gnutls found: YES 3.6.14
Library gpgme found: YES
Library gpg-error found: YES
Configuring config.h using configuration                                                            
Configuring jcat-version.h using configuration
joakim-tjernlund commented 4 years ago

This might be a clue:


fwupdtool refresh
Loading…                 [-                                      ]ERROR:esys:src/tss2-esys/esys_context.c:69:Esys_Initialize() Initialize default tcti. ErrorCode (0x000a000a) 
Loading…                 [***************************************]
11:48:02:0244 GLib-GObject         g_object_get_is_valid_property: object class 'JcatGpgEngine' has no property named 'verify-kind'
11:48:02:0244 GLib-GObject         g_object_get_is_valid_property: object class 'JcatPkcs7Engine' has no property named 'verify-kind'
11:48:02:0244 GLib-GObject         g_object_get_is_valid_property: object class 'JcatSha1Engine' has no property named 'verify-kind'
11:48:02:0244 GLib-GObject         g_object_get_is_valid_property: object class 'JcatSha256Engine' has no property named 'verify-kind'
hughsie commented 4 years ago

11:48:02:0244 GLib-GObject g_object_get_is_valid_property: object class 'JcatGpgEngine' has no property named 'verify-kind'

I think you're using a fwupd built against the old version with a new version of jcat.

joakim-tjernlund commented 4 years ago

11:48:02:0244 GLib-GObject g_object_get_is_valid_property: object class 'JcatGpgEngine' has no property named 'verify-kind'

I think you're using a fwupd built against the old version with a new version of jcat.

Yes, that was it :) Though I figured libjcat was backwards compatible ?

joakim-tjernlund commented 4 years ago

fwuptool still complains though(fwupdmgr refresh is OK):


fwuptool refresh
-bash: fwuptool: command not found
se-jocke3-lx files # fwupdtool refresh
Loading…                 [-                                      ]ERROR:esys:src/tss2-esys/esys_context.c:69:Esys_Initialize() Initialize default tcti. ErrorCode (0x000a000a) 
Loading…                 [***************************************]
12:28:58:0983 GLib                 g_path_get_basename: assertion 'file_name != NULL' failed
Child process exited with code 1
hughsie commented 4 years ago

Though I figured libjcat was backwards compatible ?

It's supposed to be. I had a thinko in the last release. I think we can work around it in fwupd.

joakim-tjernlund commented 4 years ago

fwuptool still complains though(fwupdmgr refresh is OK):

fwuptool refresh
-bash: fwuptool: command not found
se-jocke3-lx files # fwupdtool refresh
Loading…                 [-                                      ]ERROR:esys:src/tss2-esys/esys_context.c:69:Esys_Initialize() Initialize default tcti. ErrorCode (0x000a000a) 
Loading…                 [***************************************]
12:28:58:0983 GLib                 g_path_get_basename: assertion 'file_name != NULL' failed
Child process exited with code 1

Some deatils:


2:44:27:0435 FuEngine             ignoring 0.1.37 as older than 0.1.39
12:44:27:0435 XbSilo               ignoring for OR statement: indexed string '112b3acf-d42a-53cc-91c8-458c3b58f359' was unfound
12:44:27:0435 XbSilo               ignoring for OR statement: indexed string 'e2c96871-8be3-530e-a796-5652cee64eeb' was unfound
12:44:27:0435 XbSilo               ignoring for OR statement: indexed string '3545c1f3-4d81-59f7-b9d9-d25d5811e4be' was unfound
12:44:27:0435 XbSilo               ignoring for OR statement: indexed string 'fa8ac445-2c73-5bd7-99ab-6cfd94d1e40c' was unfound
12:44:27:0435 XbSilo               ignoring for OR statement: indexed string 'e2c96871-8be3-530e-a796-5652cee64eeb' was unfound
12:44:27:0435 XbSilo               ignoring for OR statement: indexed string '6f3f32bd-ac98-523e-9128-ef67941fb614' was unfound
12:44:27:0435 FuCommon             creating path /var/cache/fwupd/motd.d
12:44:27:0451 FuCommon             reading /root/.cache/fwupdmgr/firmware.xml.gz with 403227 bytes
12:44:27:0451 GLib                 g_path_get_basename: assertion 'file_name != NULL' failed
12:44:27:0451 FuCommon             running 'wget'
12:44:27:0451 GLib                 posix_spawn avoided (fd close requested) (child_setup specified) 
Child process exited with code 1
superm1 commented 4 years ago

Regarding the assertion, can you check what XDG_CACHE_HOME is for your user that you're running it as? https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-get-user-cache-dir? According to the spec it should be $HOME/.cache if not set. https://specifications.freedesktop.org/basedir-spec/latest/ar01s03.html I think this is where assertion is coming from: https://github.com/fwupd/fwupd/blob/master/src/fu-util-common.c#L263

superm1 commented 4 years ago

If I was going to guess what's happening is that your XDG_CACHE_HOME isn't set and so it's spawning wget and wget -O with nothing passed doesn't work.

joakim-tjernlund commented 4 years ago

XDG_CACHE_HOME

Right, it wasn't set:


XDG_CONFIG_DIRS=/etc/xdg
XDG_DATA_HOME=/root/.local/share
XDG_CONFIG_HOME=/root/.config
XDG_SESSION_TYPE=tty
XDG_SESSION_CLASS=user
XDG_SESSION_ID=3
XDG_RUNTIME_DIR=/run/user/0
XDG_DATA_DIRS=/usr/local/share:/usr/share
``
but setting it:
  export XDG_CACHE_HOME=/root/.cache
does not help, same problem
superm1 commented 4 years ago

Would you be able to walk through this with a debugger and see if my hypothesis looks correct in your scenario?

joakim-tjernlund commented 4 years ago

Would you be able to walk through this with a debugger and see if my hypothesis looks correct in your scenario?

Will this do?


(gdb) bt full
#0  fu_util_get_user_cache_path (fn=0x0) at ../fwupd-1.4.4/src/fu-util-common.c:252
        root = 0x555555734030 "/root/.cache"
        basename = 0x7fffffffd510 "\200\325\377\377\377\177"
        cachedir_legacy = 0x7ffff7f7fe1f <fwupd_remote_get_metadata_uri_sig+40> "\205\300u!H\215\025$P"
#1  0x000055555556b36a in fu_util_refresh_remote (priv=0x5555555dc840, remote=0x5555556031b0 [FwupdRemote], error=0x7fffffffd6a8) at ../fwupd-1.4.4/src/fu-tool.c:1865
        fn_raw = 0x555555720b40 "/root/.cache/fwupdmgr/firmware.xml.gz"
        fn_sig = 0x0
        bytes_raw = 0x55555564fb00
        bytes_sig = 0x0
#2  0x000055555556b54c in fu_util_refresh (priv=0x5555555dc840, values=0x5555555c24d0, error=0x7fffffffd6a8) at ../fwupd-1.4.4/src/fu-tool.c:1902
        remote = 0x5555556031b0 [FwupdRemote]
        i = 1
        remotes = 0x5555555dce20
#3  0x000055555558cfe7 in fu_util_cmd_array_run (array=0x5555555dca00, priv=0x5555555dc840, command=0x7fffffffde2a "refresh", values=0x7fffffffda88, error=0x7fffffffd6a8)
    at ../fwupd-1.4.4/src/fu-util-common.c:534
        item = 0x5555555e1400
        i = 21
        values_copy = 0x5555555c24d0
#4  0x000055555556c56a in main (argc=2, argv=0x7fffffffda78) at ../fwupd-1.4.4/src/fu-tool.c:2278
        allow_older = 0
        allow_reinstall = 0
        force = 0
        ret = 1
        version = 0
        interactive = 1
        plugin_glob = 0x0
        priv = 0x5555555dc840
        error = 0x0
        cmd_array = 0x5555555dca00
        cmd_descriptions = 0x5555555e3c50 "  activate [DEVICE-ID|GUID]         Activate pending devices\n  attach DEVICE-ID|GUID", ' ' <repeats 13 times>, "Attach to firmware mode\n  build-firmware FILE-IN FILE-OUT [SCRIPT] [OUTPUT]\n", ' ' <repeats 27 times>...
        filter = 0x0
        options = 
            {{long_name = 0x55555559076d "version", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x7fffffffd688, description = 0x555555590778 "Show client and daemon versions", arg_description = 0x0}, {long_name = 0x555555590798 "allow-reinstall", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x7fffffffd680, description = 0x5555555907a8 "Allow reinstalling existing firmware versions", arg_description = 0x0}, {long_name = 0x5555555907d6 "allow-older", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x7fffffffd67c, description = 0x5555555907e8 "Allow downgrading firmware versions", arg_description = 0x0}, {long_name = 0x55555559080c "force", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x7fffffffd684, description = 0x555555590812 "Override plugin warning", arg_description = 0x0}, {long_name = 0x55555559082a "no-reboot-check", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc868, descript--Type <RET> for more, q to quit, c to continue without paging--
ion = 0x555555590840 "Do not check for reboot after update", arg_description = 0x0}, {long_name = 0x555555590865 "no-safety-check", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc86c, description = 0x555555590878 "Do not perform device safety checks", arg_description = 0x0}, {long_name = 0x55555559089c "show-all-devices", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc880, description = 0x5555555908b0 "Show devices that are not updatable", arg_description = 0x0}, {long_name = 0x5555555908d4 "plugin-whitelist", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_STRING_ARRAY, arg_data = 0x7fffffffd698, description = 0x5555555908e8 "Manually whitelist specific plugins", arg_description = 0x0}, {long_name = 0x55555559090c "prepare", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc870, description = 0x555555590918 "Run the plugin composite prepare routine when using install-blob", arg_description = 0x0}, {long_name = 0x555555590959 "cleanup", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc874, description = 0x555555590968 "Run the plugin composite cleanup routine when using install-blob", arg_description = 0x0}, {long_name = 0x5555555909a9 "enable-json-state", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc878, description = 0x5555555909c0 "Save device state into a JSON file between executions", arg_description = 0x0}, {long_name = 0x5555555909f6 "disable-ssl-strict", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x5555555dc884, description = 0x555555590a10 "Ignore SSL strict checks when downloading files", arg_description = 0x0}, {long_name = 0x555555590a40 "filter", short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_STRING, arg_data = 0x7fffffffd6c0, description = 0x555555590a48 "Filter with a set of device flags using a ~ prefix to exclude, e.g. 'internal,~needs-reboot'", arg_description = 0x0}, {long_name = 0x0, short_name = 0 '\000', flags = 0, arg = G_OPTION_ARG_NONE, arg_data = 0x0, description = 0x0, arg_description = 0x0}}
(gdb) 
joakim-tjernlund commented 4 years ago

Some more data, I think this is causing it:


977 fwupd_remote_get_metadata_uri_sig (FwupdRemote *self)
978 {
979     FwupdRemotePrivate *priv = GET_PRIVATE (self);
980     g_return_val_if_fail (FWUPD_IS_REMOTE (self), NULL);
981     return priv->metadata_uri_sig;
982 }
983 
984 /**
985  * fwupd_remote_get_firmware_base_uri:
(gdb) print self
$8 = 0x5555556031b0 [FwupdRemote]
(gdb) print *self
$9 = {parent_instance = {g_type_instance = {g_class = 0x5555556010e0 [g_type: FwupdRemote]}, ref_count = 1, qdata = 0x0}}
(gdb) print *priv
$10 = {parent_instance = {g_type_instance = {g_class = <error reading variable: Cannot access memory at address 0x0>}, ref_count = 0, qdata = 0x0}, 
  kind = FWUPD_REMOTE_KIND_DIRECTORY, keyring_kind = FWUPD_KEYRING_KIND_NONE, id = 0x555555719680 "vendor-directory", firmware_base_uri = 0x0, report_uri = 0x0, 
  metadata_uri = 0x0, metadata_uri_sig = 0x0, username = 0x0, password = 0x0, title = 0x5555557337f0 "Vendor (Automatic)", agreement = 0x0, checksum = 0x0, 
  filename_cache = 0x5555555f8b00 "/usr/share/fwupd/remotes.d/vendor/firmware", filename_cache_sig = 0x0, 
  filename_source = 0x5555555f9220 "/etc/fwupd/remotes.d/vendor-directory.conf", enabled = 0, approval_required = 0, priority = 0, mtime = 1593101393, order_after = 0x0, 
  order_before = 0x0, remotes_dir = 0x55555564eff0 "/var/lib/fwupd/remotes.d", automatic_reports = 0}
joakim-tjernlund commented 4 years ago

/etc/fwupd/remotes.d :
 cat vendor.conf 
[fwupd Remote]
# this remote provides metadata shipped by the OS vendor and can be found in
# /usr/share/fwupd/remotes.d/vendor and firmware in /usr/share/fwupd/remotes.d/vendor/firmware
Enabled=false
Title=Vendor
Keyring=none
MetadataURI=file:///usr/share/fwupd/remotes.d/vendor/vendor.xml.gz
ApprovalRequired=false
jocke@gentoo-jocke remotes.d $ cat vendor-directory.conf 
[fwupd Remote]
# this remote provides dynamically generated metadata shipped by the OS vendor and can
# be found in /usr/share/fwupd/remotes.d/vendor/firmware
Enabled=false
Title=Vendor (Automatic)
Keyring=none
MetadataURI=file:///usr/share/fwupd/remotes.d/vendor/firmware
ApprovalRequired=false

ls /usr/share/fwupd/remotes.d/vendor/firmware
README.md
ls /usr/share/fwupd/remotes.d/vendor/vendor.xml.gz
ls: cannot access '/usr/share/fwupd/remotes.d/vendor/vendor.xml.gz': No such file or directory
joakim-tjernlund commented 4 years ago

if move away vendor.conf and vendor-directory.conf, the error goes away

joakim-tjernlund commented 4 years ago

This is annoying though:


ERROR:esys:src/tss2-esys/esys_context.c:69:Esys_Initialize() Initialize default tcti. ErrorCode (0x000a000a)
joakim-tjernlund commented 4 years ago

This seems to do the trick:


diff -u ./src/fu-tool.c.org ./src/fu-tool.c
--- ./src/fu-tool.c.org 2020-06-25 19:01:32.385329785 +0200
+++ ./src/fu-tool.c 2020-06-25 19:10:40.581544494 +0200
@@ -1845,6 +1845,7 @@
 static gboolean
 fu_util_refresh_remote (FuUtilPrivate *priv, FwupdRemote *remote, GError **error)
 {
+   const gchar *meta_uri = NULL;
    g_autofree gchar *fn_raw = NULL;
    g_autofree gchar *fn_sig = NULL;
    g_autoptr(GBytes) bytes_raw = NULL;
@@ -1862,9 +1863,12 @@
        return FALSE;

    /* signature */
-   fn_sig = fu_util_get_user_cache_path (fwupd_remote_get_metadata_uri_sig (remote));
-   if (!fu_util_download_out_of_process (fwupd_remote_get_metadata_uri_sig (remote),
-                         fn_sig, error))
+   meta_uri = fwupd_remote_get_metadata_uri_sig (remote);
+   if (!meta_uri)
+       return FALSE;
+
+   fn_sig = fu_util_get_user_cache_path (meta_uri);
+   if (!fu_util_download_out_of_process (meta_uri, fn_sig, error))
        return FALSE;
    bytes_sig = fu_common_get_contents_bytes (fn_sig, error);
    if (bytes_sig == NULL)
joakim-tjernlund commented 4 years ago

This is annoying though:

ERROR:esys:src/tss2-esys/esys_context.c:69:Esys_Initialize() Initialize default tcti. ErrorCode (0x000a000a)

On Gentoo, UEFI pulls in tpm2-tss. Is that really needed? Gentoo has a special USE=tpm which also pulls in tpm2-tss

superm1 commented 4 years ago

Thanks for digging into it. I think that approach works, but we should just be checking the remote type and only refreshing download type remotes.

superm1 commented 4 years ago

Hmm, we do have a check for FWUPD_REMOTE_KIND_DOWNLOAD in fu_util_refresh already..

superm1 commented 4 years ago

And also another check that the remote is enabled too..

joakim-tjernlund commented 4 years ago

Thanks for digging into it. I think that approach works, but we should just be checking the remote type and only refreshing download type remotes.

Sure, but ... These are Remotes ? And they are disabled too. There should probably be some more on top but this will keep an invalid uri away, regardless how you got there.

superm1 commented 4 years ago

That's true. I think we should do your fix, would you mind doing a proper PR for it? We also need to get to the bottom of this problem. It seems like it might be related to the init path for the remote not setting it up properly.

hughsie commented 4 years ago
  • if (!meta_uri)
  • return FALSE;

Needs to be == NULL and also set a GError.

joakim-tjernlund commented 4 years ago

That's true. I think we should do your fix, would you mind doing a proper PR for it?

If I could get away without, it will take anothe 30 mins to get setup to do that. Please?

We also need to get to the bottom of this problem. It seems like it might be related to the init path for the remote not setting it up properly.

joakim-tjernlund commented 4 years ago
  • if (!meta_uri)
  • return FALSE;

Needs to be == NULL and also set a GError.

Why? other cases just return FALSE in that same function and uses !xxx

joakim-tjernlund commented 4 years ago

--- ./src/fu-tool.c.org 2020-06-25 19:01:32.385329785 +0200
+++ ./src/fu-tool.c 2020-06-25 19:35:39.844426390 +0200
@@ -1845,6 +1845,7 @@
 static gboolean
 fu_util_refresh_remote (FuUtilPrivate *priv, FwupdRemote *remote, GError **error)
 {
+   const gchar *meta_uri = NULL;
    g_autofree gchar *fn_raw = NULL;
    g_autofree gchar *fn_sig = NULL;
    g_autoptr(GBytes) bytes_raw = NULL;
@@ -1862,9 +1863,17 @@
        return FALSE;

    /* signature */
-   fn_sig = fu_util_get_user_cache_path (fwupd_remote_get_metadata_uri_sig (remote));
-   if (!fu_util_download_out_of_process (fwupd_remote_get_metadata_uri_sig (remote),
-                         fn_sig, error))
+   meta_uri = fwupd_remote_get_metadata_uri_sig (remote);
+   if (meta_uri == NULL) {
+       g_set_error_literal (error,
+                    FWUPD_ERROR,
+                    FWUPD_ERROR_NOTHING_TO_DO,
+                    "no meta URI available");
+       return FALSE;
+   }
+
+   fn_sig = fu_util_get_user_cache_path (meta_uri);
+   if (!fu_util_download_out_of_process (meta_uri, fn_sig, error))
        return FALSE;
    bytes_sig = fu_common_get_contents_bytes (fn_sig, error);
    if (bytes_sig == NULL)
joakim-tjernlund commented 4 years ago

FYI, the scanning stop after the first error so vendor-directory.conf is never check

hughsie commented 4 years ago

fwupdtool refresh 11:48:02:0244 GLib-GObject g_object_get_is_valid_property: object class 'JcatGpgEngine' has no property named 'verify-kind'

@joakim-tjernlund can you do a pretty complex thing for me please?

  1. build and install libjcat 0.1.2
  2. build and install fwupd against that
  3. build and install the wip/hughsie/GObjectABI branch of libjcat
  4. run fwupd again without rebuilding it

Thanks. If that works I'll commit the fix to libjcat to restore the old ABI.

joakim-tjernlund commented 4 years ago

fwupdtool refresh 11:48:02:0244 GLib-GObject g_object_get_is_valid_property: object class 'JcatGpgEngine' has no property named 'verify-kind'

@joakim-tjernlund can you do a pretty complex thing for me please?

1. build and install libjcat **0.1.2**

2. build and install fwupd against that

3. build and install the wip/hughsie/GObjectABI branch of libjcat

4. run fwupd again **without** rebuilding it

Thanks. If that works I'll commit the fix to libjcat to restore the old ABI.

Did that and fwupdtool refresh worked fine in step 4. :)

hughsie commented 4 years ago

Did that and fwupdtool refresh worked fine in step 4. :)

Much appreciated. If Mario approves of the workaround I'll commit to libjcat master.

superm1 commented 4 years ago

@hughsie can you backport the fwupd one to 1_4_X for @joakim-tjernlund? Although both of those fixes are landed in master, I'm worried there is still something funky and unexplainable though. Why was that refresh code running in the first place? If the remotes are disabled and marked as !download that code path isn't supposed to be running. I couldn't reproduce it, and I really don't understand.

joakim-tjernlund commented 4 years ago

@hughsie can you backport the fwupd one to 1_4_X for @joakim-tjernlund? Although both of those fixes are landed in master, I'm worried there is still something funky and unexplainable though. Why was that refresh code running in the first place? If the remotes are disabled and marked as !download that code path isn't supposed to be running. I couldn't reproduce it, and I really don't understand.

I am waiting for Lenovo Dock support so I will go to 1.5 as soon it is out(which I hope is soon).

hughsie commented 4 years ago

Lenovo Dock support so I will go to 1.5

We're aiming on getting all the docks supported in 1.4.x too. Most of it is there already.