snapd

snapd copied to clipboard

Here are 4 commits fixing udisks2 to work with newer version on udisks2 snap. And also on non UC distributions. And using polkit when available.

• allow installation of daemon on classic
• allow users to connect to daemon
• add missing AppArmor permissions
• add attribute to load udev rules

Also a commit fixing polkit. udisksd does send calls to methods with the name of the sender, but directly with the address of polkit. So filtering the peer label does not work and does not let udiskd talk to polkit.

Tested to work with https://github.com/valentindavid/snap-udisks2

it's still not clear to me: why are you renaming the `name="org.freedesktop.PolicyKit1" match on the D-Bus rules?

Here is the commit message for it:

udisks2 which uses the C polkit library, does not send calls to the peer name but to the peer address directly. So AppArmor can not properly filter based on peer name.

There is two ways to address in d-bus, unique connection name or well-known name. See https://dbus.freedesktop.org/doc/dbus-specification.html#message-protocol-names-bus

Forcing a name in the apparmor rule forces to use a well-known name. Udisks2 does not use that.

Forcing a name in the apparmor rule forces to use a well-known name. Udisks2 does not use that.

I see. If the udisks2 client library uses the unique name, that seems like a bug to me, but anyway. I also hoped that our AppArmor D-Bus mediation was a bit smarter and would understand that the unique name corresponds to the allowed well-known name, but this is a question for security (ping @alexmurray :-) ).

Anyway, I'm fine with the change :-)

From what I can see the udisks2 daemon uses libpolkit (https://github.com/storaged-project/udisks/blob/master/src/udisksdaemon.c#L373), and this in turn does specify the name "org.freedesktop.PolicyKit1" https://gitlab.freedesktop.org/polkit/polkit/-/blob/master/src/polkit/polkitauthority.c#L320 - so I do not think this change to remove this name from the polkit interface is correct. Please reinstate this.

@alexmurray,

In gdbusproxy.c, g_dbus_proxy_call_internalseems to use get_destination_for_call which uses proxy->priv->name_owner which should be the should be the result of org.freedesktop.DBus.GetNameOwner which according to the specifications should be the unique name. Unless it is using dbus activation to start the service and the name has no owner yet. In this case it would use the well-known name.

I can confirm testing it again that the method call to polkit from udisks2 in the dbus-monitor looks like:

method call time=1663833433.068752 sender=:1.106 -> destination=:1.9 serial=14 path=/org/freedesktop/PolicyKit1/Authority; interface=org.freedesktop.PolicyKit1.Authority; member=CheckAuthorization

Note the destination is :1.9.

It seems to me that anything using glib may not use the well-known name for method calls.

That said, I am not sure how apparmor for dbus is supposed to work. If it does not keep track of the mapping of well-known name to unique names, then we should not use names in apparmor profiles. But maybe it is supposed to keep track.

Since only one application can register objects at a given path on dbus, the removal of the name part for PolicyKit is not likely to be problematic - however if as is suggested the behaviour has changed in glib meaning that the dbus AppArmor integration is not able to easily use the name of a peer then perhaps that needs some extra support in the dbus AppArmor integration patches. @jrjohansen do you know what might be going on here?

Either way, since this change appears to be required for this to function properly, I don't think it should be blocked waiting on dbus-apparmor - as such, LGTM.