Approval for fwupd classic snap

Hi,

We were looking for approval for the classic snap “fwupd”. It’s been uploaded to the store.

It is not the full fwupd daemon/ fwupdmgr client, but rather a standalone tool (fwupdtool) that will run all plugins and not interact with the web service. It’s intention is specifically for devices that are supported only in a newer fwupd for advanced users to be able to install updates.

It doesn’t conflict with the system fwupd/fwupdmgr, but it does need to access all around the filesystem for the various different plugins. For example UEFI plugin needs /boot, Synaptics MST plugin needs /dev/drm_dp_aux#, various USB plugins need to use libusb to access devices, Thunderbolt needs /sys/bus/thunderbolt, Thunderbolt WMI needs /sys/bus/wmi.

There are others and this list will continue to grow. So ideally only advanced users will install the classic snap to update newer devices.

Wouldn’t this be better named ‘fwupdtool’? There is already a ‘fwupd’ interface in snap which is intended to be used with a fwupd service (indeed, the uefi-fw-tools snap does exactly this). Naming the snap the same as the daemon and the snapd interface but not providing or using either may confuse people.

It isn’t clear to me how this snap relates to the fwupd interface.

Furthermore, you’ve enumerated a discrete number of accesses so it seems it is possible to add accesses to an existing or new snapd interface, and update snapd with new accesses as we go, so it isn’t clear why this actually needs classic.

Actually I proposed the same thing originally (fwupdtool).
We had some discussion upstream (see https://github.com/hughsie/fwupd/issues/543) and @hughsie prefers to keep it as fwupd rather than fwupdtool to avoid confusion. We may at a future time integrate the daemon and traditional fwupdmgr, but right now the priority was to have availability for the standalone tool.

For example with this snap I’d expect that people could update Logitech receivers from Ubuntu 16.04, which wasn’t previously possible.

It isn’t clear to me how this snap relates to the fwupd interface.

I think that was a poorly named interface. it should have been “fwupdate-uefi” or something more prescriptive of what’s going on.

The fwupd interface that’s in snapd doesn’t work with anything but the UEFI plugin, and even then only from a very old version of fwupd. I don’t believe it would would work with a current fwupd because at daemon startup there are more things accessed that aren’t present in the snap interface.

Let me ask this - if the snap interface did support all the things that need to be accessed by a current fwupd, can the snap be configured to only work on a new enough snapd? The goal of doing this as a classic snap was specifically to let it work in a lot of places for advanced people only.

The current implementation AFAIK is only limited to UEFI because that it what was needed at the time it was implemented. Indeed, its summary is simply “allows operating as the fwupd service”. snapd can and should update interfaces as software matures (in fact, we do this all the time). If the fwupd interface is out of date, by all means, lets get it updated and expand it to its full capabilities. Do note that snapd both undergoes SRU-style updates across distributions and also supports ‘re-exec’ on various distributions where on classic distro the snapd from the deb/rpm checks if the snapd from the installed core snap is newer then itself, and launches it instead if it is. In other words, we are free to update the interfaces and expect those changes to propagate to the world at large.

As for “if the snap interface did support all the things that need to be accessed by a current fwupd, can the snap be configured to only work on a new enough snapd”> yes, snapd supports something called “assumes” in snap.yaml which accepts a list of features. The fwupd interface could be reworked to export this flag. Another way to do that would be to introduce a new interface.

Today the fwupd interface is designed like so:

  • a snap slots fwupd and gets security policy that allows the slotting command various accesses on the system (such as the efi/uefi updates) and to listen on DBus org.freedesktop.fwupd
  • another snap may plugs fwupd which allows the plugging snap to talk to the slotting snap’s DBus service

It sounds like what needs to happen is:

  1. a new interface is created (fwupdtool-support?) that is an implicit slot (ie, provided by snapd itself). When a plugging snap ‘plugs: [ fwupdtool-support ]’, it is allowed all those enumerated paths you mentioned before
  2. the fwupd interface is (optionally for you at this time it sounds like) updated to also support working as an implicit classic slot (akin to PR 5230 for udisks2) with whatever new DBus API updates might have come up since it was first implemented. A plugging snap that ‘plugs: [ fwupd ]’ is then allowed to talk to the running fwupd over DBus (whether from a snap or the system)
  3. in the fullness of time, the uefi-fw-tools snap is updated to itself ‘plugs: [ fwupdtool-support ]’ if it needs all the new accesses provided by fwupdtool-support (we’ll leave the current uefi accesses in the ‘fwupd’ slot permissions for now so uefi-fw-tools continues to function. The fwupdtools-support interface is expected to have a superset of these older rules)

Ok, i’ve swapped things around to devmode for now and it’s publishing to edge channel in devmode. After reading through your description a few times I think I follow but could use a little direction.

Given what exists today in the fwupd interface I almost think it makes more sense to have the fwupd snap contain 3 pieces:

  1. fwupd daemon
  2. fwupdmgr command line tool
  3. fwupdtool standalone tool

Then fwupd and fwupdmgr as part of this snap would basically behave and do everything uefi-fw-tools does (and probably supercede the need for it). fwupdtool would be the optional standalone tool as it exists today in the fwupd snap.

So I would think if I took that approach then the fwupd interface can just be extended to do everything needed, fwupd (daemon) would slot fwupd (interface), fwupdmgr (client) would plug fwupd (interface) and fwupdtool would both slot and plug fwupd.

I took a stab at the paths I think would be needed. Do these all make sense to come into the fwupd interface?

diff --git a/interfaces/builtin/fwupd.go b/interfaces/builtin/fwupd.go
index ea137ea63..7d82674c7 100644
--- a/interfaces/builtin/fwupd.go
+++ b/interfaces/builtin/fwupd.go
@@ -52,11 +52,42 @@ const fwupdPermanentSlotAppArmor = `
   # For udev
   network netlink raw,
 
+  # Access usb devices
+  /sys/bus/usb/devices/, r
+
   # File accesses
   # Allow access for EFI System Resource Table in the UEFI 2.5+ specification
   /sys/firmware/efi/esrt/entries/ r,
   /sys/firmware/efi/esrt/entries/** r,
 
+  # Access SMBIOS information (fwupd core)
+  /sys/firmware/dmi/tables/ r,
+  /sys/firmware/dmi/tables/** r,
+
+  # Access DP Aux (synaptics mst plugin)
+  /dev/drm_dp_aux0 rw,
+  /dev/drm_dp_aux1 rw,
+  /dev/drm_dp_aux2 rw,
+
+  # Access Thunderbolt (Thunderbolt plugin)
+  /sys/bus/thunderbolt/ r,
+  /sys/bus/thunderbolt/** r,
+  /sys/bus/thunderbolt/devices/*/nvm_non_active*/ r,
+  /sys/bus/thunderbolt/devices/*/nvm_non_active*/** rw,
+
+  # Communicate over SMBIOS WMI (dell plugin)
+  /dev/wmi/dell-smbios rw,
+  /sys/devices/platform/dell-smbios.*/tokens/ r,
+  /sys/devices/platform/dell-smbios.*/tokens/** r,
+
+  # Communicate over SMBIOS SMI (dell plugin)
+  /sys/devices/platform/dcdbas r,
+  /sys/devices/platform/dcdbas/** rw,
+
+  # Turn on Thunderbolt (Thunderbolt force power plugin)
+  /sys/bus/wmi/drivers/intel-wmi-thunderbolt/86CCFD48-205E-4A77-9C48-2021CBEDE341/ r,
+  /sys/bus/wmi/drivers/intel-wmi-thunderbolt/86CCFD48-205E-4A77-9C48-2021CBEDE341/force_power rw,
+
   # Allow fwupd to access system information
   /sys/devices/virtual/dmi/id/product_name r,
   /sys/devices/virtual/dmi/id/sys_vendor r,

So can you please let me know what you think of this approach I propose?

@jdstrand Do you know how should the snap access /boot/efi on Ubuntu Classic if strictly confined? In an all-snap Ubuntu Core system it’s fine but in a Ubuntu Classic currently /boot/efi is not bind mounted.

The snap will need to access the /boot/efi directory to upgrade the UEFI firmware for next boot and on a Ubuntu Classic the core snap provided its own /boot/efi and it’s not the real one on the system.

Sounds fine

This is all fine except that it is strange the fwupdtool is slotting ‘fwupd’. While it does gives fwupdtool the necessary accesses to run, it means that the tool is claiming to provide fwupd’s DBus interfaces, which is wrong. Better would be to create a new fwupdtool-support implicit interface that has all the accesses fwupdtool needs (look at network_control.go for inspriation), and fwupdtool plugs it instead. Then at some future date when the fwupd daemon supports everything fwupdtool can do, it can also plugs fwupdtool-support (and we leave the subset of access that were in PermanentAppArmor in fwupd.go alone so it is mainly focusing on the DBus bits and rely on any new accesses to be in fwupdtool_support.go).

On the whole, I think the accesses are appropriate, though we might fine-tune a bit in PR review.

Thanks for looking at this!

snap-confine needs an update: https://github.com/snapcore/snapd/blob/master/cmd/snap-confine/mount-support.c#L610

Had a call with @superm1, the snap advocacy team, et al and the requirements are understood. The plan of record is to allow classic now and work towards strict as laid out here: Fwupd snap, needed interfaces.

This is now live.

1 Like

The classic snap is failing the following automated checks:


human review required due to 'deny-connection' constraint for 'slot-attributes' from base declaration declaration-snap-v2_slots_deny-connection (daemon, dbus)
confinement 'classic' not allowed with plugs/slots lint-snap-v2_confinement_classic_with_interfaces What does this mean?

Previously I believe this was being put into the unit by the plugs/slots I had put in place. But because I removed the plug to try to help the earlier lintian error I don’t think it’s with the slot alone is enough. It’s failing though with this problem when trying to run now.

The name org.freedesktop.fwupd was not provided by any .service files

What’s the proper way to get the systemd unit to register properly with a classic snap?

classic snaps are not allowed to define any plugs/slots at all … remove them from your snapcraft.yaml as long as your snap is classic … (they would be useless anyway given classic grants you full system access)

1 Like

Well that does let it through, but then how do I properly get it to register the service? The generated systemd unit is wrong. It should have these options included.

Type=dbus
BusName=org.freedesktop.fwupd

Here is the generated unit for reference (you can also see it on the beta channel now)

$ cat /etc/systemd/system/snap.fwupd.fwupd.service 
[Unit]
# Auto-generated, DO NOT EDIT
Description=Service for snap application fwupd.fwupd
Requires=snap-fwupd-368.mount
Wants=network-online.target
After=snap-fwupd-368.mount network-online.target
X-Snappy=yes

[Service]
ExecStart=/usr/bin/snap run fwupd --verbose
SyslogIdentifier=fwupd.fwupd
Restart=on-failure
WorkingDirectory=/var/snap/fwupd/368
TimeoutStopSec=30
Type=simple

[Install]
WantedBy=multi-user.target

Oh I think it’s a matter of a few more things landing in the filesystem for dbus to recognize it. I think I’ve got it now.

2 Likes

Alright I’ve got the rest of dbus errors and TLS errors sorted out now with it on my local classic build. I’ll promote this to candidate after it builds and tests OK.

How do I request a mapping from the store for fwupd.fwupdmgr -> fwupdmgr?

1 Like

Answering myself, I filed this separately here: Requesting default aliasing for fwupd

1 Like