busd icon indicating copy to clipboard operation
busd copied to clipboard

Implement essential policy

Open zeenix opened this issue 1 year ago • 7 comments

busd will have to implement a policy. However, I don't think we need to go as far as existing implementations. Most of the policies are not super useful. More specifically, we'll want service-level policy but not method-level. Admins can specify which names can be owned by which users and who can make calls to them. In the D-Bus configuration XML language, this would mean enable configurations like:

    <allow send_destination="org.gnome.DisplayManager"/>

and not supporting send_interface or send_member:

    <allow send_destination="org.gnome.DisplayManager"/>
      send_interface="org.gnome.DisplayManager.Manager"/>
      send_member="GetRemoteHostname"/>

We should not error out on encountering unsupported configuration nodes though, but rather just warn about them.

Prerequisites: #78.

zeenix avatar Apr 29 '24 14:04 zeenix

and not supporting send_interface or send_member:

Just to clarify, by "not supporting", I mean we don't restrict based on interface and member etc. IOW, if someone is allowed access to a service, they're allowed complete access to that service.

zeenix avatar Oct 07 '24 16:10 zeenix

I don't know if this is a good idea security wise. It's entirely possible for a service to allow access to an interface, but now allow generalized access to all methods (but only those that are protected by Polkit)

I'm not saying I have an API like this in mind, I'm just saying that an API like this is feasible. Hell, IIRC at one point I implemented an API like this in homed. Maybe logind actually relies on this, with some methods only callable by the PAM plugin (that runs as root)?

Anyways, my point is this sounds like a setup for a very juicy privilege escalation vulnerability.

AdrianVovk avatar Oct 19 '24 04:10 AdrianVovk

I don't know if this is a good idea security wise. It's entirely possible for a service to allow access to an interface, but now allow generalized access to all methods (but only those that are protected by Polkit)

Hmm.. perhaps we should still allow interface and path ACL so if services do a good job of splitting their API in different paths and interfaces, they can achieve this. :thinking:

Also, if really needed, it be much easier to add support for fine-grained policies w/o breakage than to remove supported configuration later. So we should start simple in any case.

zeenix avatar Oct 21 '24 00:10 zeenix

Over in #159 , I implemented ...interface but left ...member as ignored with a warning

jokeyrhyme avatar Nov 17 '24 04:11 jokeyrhyme

Okay, I noticed an edge case when I was adding more tests

<deny send_destination="A" send_member"B" /> would block all of A if we just ignored the send_member part, so I've updated #159 to completely drop such deny rules

The equivalent allow rule is still kept, just without the send_member part

jokeyrhyme avatar Nov 18 '24 07:11 jokeyrhyme

Okay, I noticed an edge case when I was adding more tests

<deny send_destination="A" send_member"B" /> would block all of A if we just ignored the send_member part

Hmm.. true.

<deny send_destination="A" send_member"B" /> would block all of A if we just ignored the send_member part, so I've updated #159 to completely drop such deny rules

I think we should just bite the bullet and handle all the attributes. I know that goes against what I've previous been saying but obviously I didn't think this through. :facepalm:

zeenix avatar Nov 18 '24 11:11 zeenix

We still ignore send|receive_requested_reply and eavesdrop the way dbus-broker does, at least

jokeyrhyme avatar Nov 18 '24 19:11 jokeyrhyme