On 18.08.2014 18:22, Lennart Poettering wrote: > On Fri, 15.08.14 19:28, Stef Walter (st...@redhat.com) wrote: > >> >> On 15.08.2014 18:56, Lennart Poettering wrote: >>> On Fri, 15.08.14 18:25, Stef Walter (st...@redhat.com) wrote: >>> >>>> >>>> On 13.08.2014 20:27, Lennart Poettering wrote: >>>>> On Wed, 06.08.14 13:23, Stef Walter (st...@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. >>>> >>>>>> https://github.com/stefwalter/systemd/commits/polkit-systemd1 >>> >>> Hmm, yuck. There's a security issue here... Reading the capabilities >>> from the sender on dbus1 is racy, since we have to read it from >>> /proc/$PID/stat and don't get it sent along with the message, like we do >>> on kdbus. A rogue client could send a message, quickly invoke some suid >>> binary, and we'd consider the client trusted. >>> >>> Now for the low-level implementation of the vtable bit we are actually >>> smart, and check by UID on dbus1, and by cap on kdbus, in order to avoid >>> the vulnerability. >>> >>> Hmm, now I wonder how to best handle this for cases like this, we >>> probably need some generic way how clients can make this decision in an >>> always safe way... >>> >>> I need to think more about this... >> >> By the way, there's some similar problematic code in the modified >> KillUnit() method implementation ... changed from specifying the >> CAP_KILL in the vtable, and now it does a manual check. >> >>> Patch set looks great otherwise. I'll come up with something for the >>> security issue, then adapt your patch, and merge it. >> >> I haven't tested the updated branch at all :) So it may go boom... > > I have now pushed this, after reworking this on top some major changes > to bus_verify_polkit(), which avoids having to pass the original > callbacks through to the function that ultimately does the verification. > > While merging I also made another change, you are probably not going to > like: I turned of the interactivity for the polkit checks. Interactivity > needs to be optional, and it currently is for all out polkit-enabled bus > methods. And we should do the same for the PID 1 offered methods.
Ugh. > Now, of course, we should open this up for inetractive (after all, > that's what polkit is good for), but we probably need a new set of > methods for that, which take the original arguments but also take a > boolean argument to enable ineractivity. Hence, we probably should have > StartUnit2() in addition to StartUnit(). That seems ugly. I think we should either: * Have a method which we can invoke to make a client opt into interactive polkit prompting for any invoked method. * Version all the org.freedesktop.systemd1.Manager to org.freedesktop.systemd1.Manager2 or something like that and support both interfaces. Cheers, Stef _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
