RFE: dbus AppArmor mediation matching by message type

Bug #1692582 reported by Simon McVittie on 2017-05-22
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
apparmor (Ubuntu)
Undecided
Unassigned

Bug Description

Suppose you're writing an AppArmor profile for a D-Bus service like Tracker. The service might get compromised (perhaps it's network-facing) so you don't want it to be able to act as a client of privileged processes like systemd --user. However, imagine you do want arbitrary third-party "apps" to be allowed to use the service if they have appropriate rules in their own AppArmor profiles.

One reasonably natural way to encode this without tightly coupling the service and its clients would be:

    profile /usr/bin/my-service {
      ...
      dbus send bus=session type=signal,
      dbus receive bus=session type=method_call,
    }

    profile /usr/bin/my-client-app {
      ...
      dbus (send,receive) bus=session peer=(label=/usr/bin/my-service),
    }

However, the AppArmor integration in src:dbus and the rule parser in src:apparmor don't allow this: they match fine-grained information like the method name and object path, but have no concept of message types. This seems backwards: I only expect the object path to be useful in very rare/niche cases, but the message type is "larger" and more important than anything else from the message payload.

Simon McVittie (smcv) wrote :

If I'm reading the AppArmor user-space source code correctly, if backwards compatibility wasn't a concern then this could be achieved by adding an additional user-defined field to vec in dbus_rule::gen_policy_re(Profile&) and passing the new number of fields to add_rule_vec(), then adding that same field to the queries built by dbus-daemon in bus/apparmor.c build_message_query().

Unfortunately, again if I'm reading correctly, the query works by building a long string with embedded \0 bytes, then matching it against a DFA representing a single long regular expression that also has embedded \0 bytes - if true, this would mean the number of fields can't usefully be varied.

If extensibility is desired, I think the ideal thing might be if extra fields in the query were ignored (always match) and extra fields in the rule were compared as though the query had an empty string at that point in the vector, but I don't know how feasible that would be.

John Johansen (jjohansen) wrote :

The message type certain could be added. However it is not the only way this separation can be achieved.

The label in particular should be able to be used without tying it to a specific service. Admittedly this is somewhat limited atm.

1. the label name on a service does not have to match its executable name so an executable could be labeled with a more generic profile. This however will not work in cases where an executable maybe servicing multiple service end points that want different labels, and would be require #2.

2. while conceptually apparmor supports having none application (domain) labels on objects, the support for enabling a service to provide a different label while creating sockets has not landed yet, so until it does, apparmor policy currently is tying the service confinement and policy tighter than it should.

John Johansen (jjohansen) wrote :

There are actually a couple of ways to add it, and still keep userspace compatibility. Kernel side we are actually often checking partial matches, and due is a permission but AA_CONTINUE to indicate that if permissions aren't satisfied to continue the match.

This could be emulated in userspace a couple different ways, either by the dbus pulling the policy it needs and doing the match entirely in userspace (currently this would be the entire policy but the way things are setup this could be split out so dbus could request on the dbus relevant bits), ideally all this would just be a couple of calls into libapparmor. Or the code could use the current interface and round trip the kernel a couple of times (not ideal), though this approach becomes less and less serviceable long term).

A third approach would be to tack the type field onto the larger match string and do it as a single match.

Another approach would be to improve the kernel interfaces and update the dbus code to the improved query/matching.

All of these of course rely on the policy being able to carry some information to dbus to allow it to know if it should use the newer policy, do the extra match, what ever approach is chosen. AppArmor already has support for a key/value pair storage (its even in the upstream kernel) in policy that could be leveraged to do this.

So there are lots of options available, its a matter of choosing a design and doing the work.

Simon McVittie (smcv) wrote :

> 1. the label name on a service does not have to match its executable name so an executable could be labeled with a more generic profile

Sure, but I'm not sure how this helps me to achieve what I'm aiming for, which is:

                                         privileged
    anything -------> my service ---X - > processes
                                         (systemd?)

where method calls go in the direction of the arrow, broadcast signals go in the opposite direction, and the X indicates being blocked.

If all I want is to allow my-client-app to be a client of my-service, I can already do:

    profile /usr/bin/my-service {
      ...
      dbus (send,receive) bus=session,
    }

    profile /usr/bin/my-client-app {
      ...
      dbus (send,receive) bus=session peer=(label=/usr/bin/my-service),
    }

but then my-service is also allowed to send method calls to every other session service, including systemd --user (which in practice is unconfined) - I've enabled the left hand side of the diagram above, but not blocked the right hand side.

I could also do something like

    profile foobar-service { # implemented by my-service
      ...
      dbus (send,receive) bus=session peer=(label=*_foobar-client),
    }

    profile app-123_foobar-client {
      ...
      dbus (send,receive) bus=session peer=(label=foobar-service),
    }

to restrict the profiles to being about the left hand side of my diagram, but that doesn't seem like it really scales when a particular app might be a client of lots of services.

Without distinguishing between messages by type or payload, it's also difficult to let unconfined processes use my service as clients (the platform I'm working on uses this a lot, for regression tests), without also letting my service (if it gets compromised) use unconfined services like systemd as a client. So, getting the left part but blocking the right part in this special case:

    unconfined unconfined
    utility or -------> my service ---X - > service
    automated test (systemd)

When I say "is a client of" I'm referring to the common pattern where n clients all call methods on a service and receive signals from that service, like the way GNOME Music and GNOME Documents are clients of Tracker, and Empathy and Polari are clients of Telepathy Mission Control.

John Johansen (jjohansen) wrote :

I think performance, and flexibility wise, the best solution would be to move mediation entirely to userspace.

Use the key/value store to provide flexibility on what match ordering to use, userspace policy caching so we don't have to round trip the kernel except when the policy is invalidated by a policy reload, etc.

This would be the most flexible and performant solution and if done right.

John Johansen (jjohansen) wrote :

@Simmon,

You are right, that will require extending what is supported in the mediation, beyond even landing support for #2. It will take a bit of work, but we can definitely do it. My preferred solution is more work than the quickest/easiest solution, as it requires landing a few things that haven't landed yet or are in the process of landing, but overall I would rather go with the best long term solution.

What time frame are you looking for to land fixes for this. Earliest possible for the userspace mediation would be the next apparmor release in the fall. The dbus code changes could be done off of development trees, and as long as care is taken to maintain backwards compatibility, along with a few build conditionals, it could land any time that is suitable for dbus.

A going with a solution that uses the current interfaces and doesn't have any build deps could potentially be added to an apparmor point release, and land a bit sooner.

Simon McVittie (smcv) wrote :

> What time frame are you looking for to land fixes for this

I don't have a specific timeline for this, I just wanted to raise it as a missing feature before I forgot about it.

I think the project that I wanted this for might be able to work around the missing feature with rules like

    dbus (send,receive) bus=session path=/com/example/MyService/**,
    dbus receive bus=session peer=(label=unconfined),

so that unconfined developer/test processes can call Ping() and Introspect(), all processes can call the service's methods and receive its signals, but the service can't call methods on (the interesting object paths of) more-privileged processes like systemd.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers