[systemd-devel] Work on adding polkit support to systemd1
stefw at redhat.com
Fri Aug 15 09:25:28 PDT 2014
On 13.08.2014 20:27, Lennart Poettering wrote:
> On Wed, 06.08.14 13:23, Stef Walter (stefw at redhat.com) wrote:
>> I've done initial work on adding polkit support to systemd1 DBus
>> methods. You can see it here:
Thanks for the review. Worked on this a bit more.
I might drop off the face of the earth for a couple weeks. In case I do,
I thought I'd update my public branch. But if I'm around, I'll test and
prepare a patch set early next week.
Updated this branch ^^
>> The way that each callback in sd-bus has to handle verification seems a
>> bit risky to me. So I've only opened up the specific interfaces I
>> touched in the DBus policy file.
Your comments below address what I was worried about here ^^
> Regarding the fourth: please don't bind any checks against the UID,
> check for CAP_SYS_ADMIN instead, we make it equally easy.
> Why is verify_root_sync() invoked that often? I mean, for these cases
> the dbus1 policy should not grant access anyway, so we don't really need
> to check for permissions here again. I'd really prefer if on dbus1 would
> use the dbus1 policy for as much access control work as useful, and only
> do it on our own if we really need to.
> reload (and probably also reexecute) should probably hook into polkit
> too, with a new action? it sounds useful enough for people to invoke it
New action is called: org.freedesktop.systemd1.reload-daemon
> Note that on kdbus access control works differently than on dbus1: it's
> the client's responsibility to implement access control. To make this
> easy our sd-bus library actually allows encoding in the method vtable
> which calls are accessible for unpriviliged processes. That's what the
> SD_BUS_VTABLE_UNPRIVILEGED flag in the object vtables is for (grep for
> it). The flag (or its absence) has no effect on dbus1 daemons at all,
> and only matters for kdbus. To make sure your changes also work
> correctly and as intended on kdbus you need to add the flag to all
> methods that you are now opening up via polkit. Because otherwise method
> calls won't even get that far.
> Or in other words: everything that is opened up in the dbus1 policy also
> needs to be opened up with SD_BUS_VTABLE_UNPRIVILIGED in the object
Makes sense ... but out of interest, why don't you just rely on this for
dbus1 as well? It seems that maintaining these two distinct access
control methods is risky.
> Sometiems you added spurious newlines to the patches, please don't.
Removed a couple extra newlines. I hope I found them all.
> I'd prefer if the dbus1 policy would precisely list the method calls
> that are now opened up...
One other thing is that it seems like the ofs.Manager.LoadUnit() DBus
method call is wide open for anyone to call. Is this intentional? I
don't know all the implications, but wanted to highlight it.
More information about the systemd-devel