Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement essential policy #79

Open
zeenix opened this issue Apr 29, 2024 · 7 comments
Open

Implement essential policy #79

zeenix opened this issue Apr 29, 2024 · 7 comments
Labels
feature parity Feature parity with existing bus impls

Comments

@zeenix
Copy link
Collaborator

zeenix commented Apr 29, 2024

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
Copy link
Collaborator Author

zeenix commented Oct 7, 2024

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.

@AdrianVovk
Copy link

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.

@zeenix
Copy link
Collaborator Author

zeenix commented Oct 21, 2024

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. 🤔

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.

@jokeyrhyme
Copy link
Contributor

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

@jokeyrhyme
Copy link
Contributor

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

@zeenix
Copy link
Collaborator Author

zeenix commented Nov 18, 2024

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. 🤦

@jokeyrhyme
Copy link
Contributor

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

jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 19, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 19, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 23, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 27, 2024
zeenix pushed a commit to jokeyrhyme/busd that referenced this issue Nov 27, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Nov 30, 2024
jokeyrhyme added a commit to jokeyrhyme/busd that referenced this issue Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature parity Feature parity with existing bus impls
Projects
None yet
Development

No branches or pull requests

3 participants