Bug 101902 - Containers (#100344): message filtering/policy
Summary: Containers (#100344): message filtering/policy
Status: ASSIGNED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: All All
: medium enhancement
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on: 101354
Blocks: 100344
  Show dependency treegraph
 
Reported: 2017-07-24 19:01 UTC by Simon McVittie
Modified: 2017-10-27 16:59 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2017-07-24 19:01:54 UTC
+++ This bug was initially created as a clone of Bug #100344 +++

Bug #101354 does not let container managers arrange for messages to be filtered. This is part of the wish-list for Bug #100344, because it would eventually let us get rid of flatpak-dbus-proxy.

Design sketch:

* New named parameter:

      Allow: a(usos)
          Array of tuples (flags: u, bus name: s, object path: o, interface: s)

          If this parameter is given, it sets the messages and operations
          that are allowed.

          An empty array allows sending method calls to the message
          bus, and receiving exactly one reply or error for each method call
          successfully sent (unless it was NO_REPLY). It also allows
          receiving method calls from connections that are not in a container,
          and sending exactly one reply or error for each method call
          received (unless it was NO_REPLY). It does not allow receiving
          broadcast signals. This is the most restrictive possible policy.

          [TODO: Does it allow receiving unicast signals from connections
          that are not in a container?]

          Each array entry is a /rule/ which allows additional operations
          according to its flags. If the bus name is the empty string,
          it matches all bus names (and hence all possible connections).
          If the interface is the empty string, it matches messages with
          any interface or no interface. If the object path is "/" and
          the OBJECT_PATH_IS_SUBTREE flag is present, it matches messages
          with any object path or no object path.

              SEE:
                  The connection may call GetNameOwner(), NameHasOwner(),
                  GetConnectionCredentials(), etc. to inspect connections
                  that are in the queue to own the given bus name.
                  The object path and interface are ignored when checking
                  for SEE access.
              CALL_METHODS:
                  The connection may send method call messages to
                  connections that are in the queue to own the given
                  bus name. Implies SEE.
              RECEIVE_BROADCASTS (or RECEIVE_SIGNALS?):
                  The connection may receive broadcasts (or all signals?)
                  from connections that are in the queue to own the given
                  bus name, if they match the given object path and
                  interface.
              TALK: alias for SEE | CALL_METHODS | RECEIVE_whatever
              BUS_NAME_IS_SUBTREE:
                  The bus name is matched in the same way as arg0namespace
                  in signal match rules, instead of as an exact match.
              OBJECT_PATH_IS_SUBTREE:
                  In addition to messages that match exactly, the object
                  path matches messages with an object path that has
                  additional components. For example, "/foo" matches messages
                  with object path "/foo" or "/foo/bar" but not "/foolish".
                  Note that this is not identical to arg0path in signal
                  match rules.
              SEND_UNIX_FDS, RECEIVE_UNIX_FDS:
                  The connection may send, receive messages that contain
                  Unix fds and match the given bus name, object path and
                  interface.
              OWN_BUS_NAME:
                  The connection may use RequestName and ReleaseName
                  to own the given bus name. Implies SEE and TALK.
                  The object path must be "/" and the interface must
                  be "".

          Not giving this parameter is equivalent to providing policies
          that allow the confined connection to send and receive every
          message, own every bus name, etc. This is the least restrictive
          possible policy.

* o.fd.Containers.GrantAccess(a(uos): rules)
      Rules are as above, but bus name is implicitly the caller's
      unique name

* o.fd.Containers.RevokeAccess(a(uos): rules)
      Rules are as for GrantAccess. If a rule was added more than
      once, each revocation removes one of the identical copies.
Comment 1 Simon McVittie 2017-07-24 20:06:32 UTC
We also need to either allow all confined apps to send broadcast signals, or have a flag that lets them.

We also need to either allow them to send unicast signals, or have a flag that lets them, or consider it to be part of CALL_METHODS (perhaps renamed to SEND_UNICAST in that case).
Comment 2 Philip Withnall 2017-08-01 11:31:27 UTC
(In reply to Simon McVittie from comment #0)
>           [TODO: Does it allow receiving unicast signals from connections
>           that are not in a container?]

Yes, that’s used by a few system service implementations, right?
Comment 3 Simon McVittie 2017-10-26 16:56:04 UTC
For feature parity with Flatpak, we need the equivalents of:

* flatpak-dbus-proxy with no additional arguments

* flatpak-dbus-proxy --filter --own=APP_ID --own=APP_ID.*
  [--own=NAME[.*]...] [--talk=NAME[.*]...]

* flatpak-dbus-proxy --filter --sloppy-names 
  --filter=org.a11y.atspi.Registry=IFACE.METHOD@PATH

flatpak-dbus-proxy also has --see=NAME[.*] but Flatpak never actually uses that, as far as I can tell. (However, the "see" policy can happen implicitly at runtime, because whenever a peer sends a message to the container, flatpak-dbus-proxy gives the container SEE access to that peer.)

flatpak-dbus-proxy also has --log but that's Bug #103470.
Comment 4 Simon McVittie 2017-10-26 17:46:19 UTC
Feature parity with flatpak-dbus-proxy:

Levels of access in Flatpak: NONE < SEE < FILTERED < TALK < OWN. Access to a unique name is always >= the level of access we have to any well-known name that it owns. Unlike dbus-daemon <allow> rules, well-known names that it is merely queueing for don't count (but that's probably just because Flatpak can't know that).

Unexpected replies are always blocked, in both directions. Expected replies are always allowed, in both directions.

Sending broadcast signals is always allowed.

Peer-to-peer messages to the dbus-daemon are always allowed. In practice, the dbus-daemon only handles Peer messages here (anything else has to have o.fd.DBus as destination).

Directed messages (method calls or unicast signals) to connections other than o.fd.DBus for which the container has TALK or OWN access are always allowed.

Directed method calls (only - no unicast signals) sent to connections for which we have FILTERED access are allowed if they match the filter.

Introspecting o.fd.DBus is always allowed. So are Hello, RemoveMatch and GetId.

Special case for AddMatch: it is allowed, if it isn't eavesdropping.

UpdateActivationEnvironment and BecomeMonitor are never allowed.

RequestName, ReleaseName and ListQueuedOwners are allowed iff we have OWN access to the name.

Special case for ListActivatableNames, ListNames, NameHasOwner, GetNameOwner, GetConnectionWhatever: names to which we don't have SEE access are invisible.

Special case for StartServiceByName: it requires TALK access (in particular FILTERED access is not enough).

Special case for NameOwnerChanged: only names to which we have >= SEE access are listed, but --sloppy-names is equivalent to having SEE access to every unique name. If a client has >= SEE access to a well-known name, and receives NameOwnerChanged for that well-known name, then that client (but not other clients in the same proxy?) is granted SEE access to the old and new owners as well.

Other dbus-daemon methods and interfaces are blocked.

Receiving a method call or a unicast signal from a peer grants the recipient (but no other client of the same proxy) SEE access to that peer. I think we can relax that to giving everything in the same container instance SEE access to that peer, because there is no security boundary within a container instance.

Receiving broadcast signals from outside the container instance checks for TALK access to the sender.

flatpak-dbus-proxy users can always TALK to their own unique name. Because OWN implies TALK, they can also TALK to anything in the same container instance, as long as communication was initiated using the well-known name. I think it would be reasonable to relax this to say that talking to other unique names in the same container instance is fine, because there is no security boundary within a container instance.
Comment 5 Simon McVittie 2017-10-26 17:53:40 UTC
(In reply to Simon McVittie from comment #3)
> * flatpak-dbus-proxy with no additional arguments

That's a call to AddServer() without having Allow in the named parameters at all. Technically, for full parity with Flatpak, it should also have privileged access (ability to escape the container or spy on other processes by talking to dbus-daemon) re-enabled (Bug #103458).

> * flatpak-dbus-proxy --filter --own=APP_ID --own=APP_ID.*
>   [--own=NAME[.*]...] [--talk=NAME[.*]...]

The equivalent of --filter can be activated by having the Allow parameter at all. We will need to divide up the "always allowed" rules from Flatpak into things that are always allowed, and things that Flatpak allows by adding rules.

The equivalent of --talk=NAME is (flags=SEE|CALL_METHODS|SEND_UNICAST_SIGNALS|RECEIVE_BROADCASTS|SEND_UNIX_FDS|RECEIVE_UNIX_FDS|OBJECT_PATH_IS_SUBTREE|ACTIVATE, bus_name=NAME, object_path=/, interface=""). (New flags needed here since Comment #0: SEND_UNICAST_SIGNALS, ACTIVATE.)

If NAME ends with ".*", then remove that and add BUS_NAME_IS_SUBTREE.

The equivalent of --own=NAME is the same as --talk, but add OWN_BUS_NAME to the flags.

> * flatpak-dbus-proxy --filter --sloppy-names 
>   --filter=org.a11y.atspi.Registry=IFACE.METHOD@PATH

The equivalent of --sloppy-names would need to be a new flag (either an Allow rule with the flag or a separate named parameter), SEE_ALL_UNIQUE_NAMES or SeeAllUniqueNames.

We need a new flag INTERFACE_IS_REALLY_METHOD, then the equivalent of that --filter rule would be (flags=SEE|CALL_METHODS|SEND_UNIX_FDS|RECEIVE_UNIX_FDS|INTERFACE_IS_REALLY_METHOD, bus_name=org.a11y.atspi.Registry, object_path=PATH, interface=IFACE.METHOD). I don't think it's worth adding a new top-level struct member for this: filtering at method granularity is hopefully really rare (interface granularity is better).

> flatpak-dbus-proxy also has --see=NAME[.*] but Flatpak never actually uses
> that, as far as I can tell. (However, the "see" policy can happen implicitly
> at runtime, because whenever a peer sends a message to the container,
> flatpak-dbus-proxy gives the container SEE access to that peer.)

This could be added as (flags=SEE,bus_name=NAME, object_path="/", interface="") (optionally adding OBJECT_PATH_IS_SUBTREE, but the object path is ignored for SEE anyway).

We could emulate this by having a data structure (hash table?) containing all the unique names (or equivalently, connections) that the container instance is allowed to SEE, and ensuring that every peer that sent a message to the container instance is added to that data structure. This is probably better than synthesizing "full-fat" Allow rules for them.
Comment 6 Simon McVittie 2017-10-26 18:12:12 UTC
(In reply to Simon McVittie from comment #4)
> Unexpected replies are always blocked, in both directions. Expected replies
> are always allowed, in both directions.

I think having the Allow named-parameter at all should implicitly allow this, then we don't need a representation of it.

> Sending broadcast signals is always allowed.

I'm a little hesitant about *always* allowing this, so I would be tempted to make it conditional. If we create a new SEND_BROADCASTS flag, then Flatpak could add (flags=SEND_BROADCASTS|OBJECT_PATH_IS_SUBTREE, bus_name="", object_path="/", interface=""). bus_name would be ignored for SEND_BROADCASTS, unless we wanted to have some condition like "if it's non-empty, the sender must own it".

> Peer-to-peer messages to the dbus-daemon are always allowed. In practice,
> the dbus-daemon only handles Peer messages here (anything else has to have
> o.fd.DBus as destination).

Replying to a Peer message is no more expensive than sending back an error, so I think we just allow this implicitly. If we want to do something to anonymize GetMachineId(), I think we should probably virtualize it (a HMAC where the real machine ID is the secret and the app ID is the message?) instead.

> Directed messages (method calls or unicast signals) to connections other
> than o.fd.DBus for which the container has TALK or OWN access are always
> allowed.

Comment #5 implements this.

> Directed method calls (only - no unicast signals) sent to connections for
> which we have FILTERED access are allowed if they match the filter.

Comment #5 implements this.

> Introspecting o.fd.DBus is always allowed. So are Hello, RemoveMatch and
> GetId.

I think these are OK to always allow.

> Special case for AddMatch: it is allowed, if it isn't eavesdropping.

dbus-daemon already treats eavesdropping as privileged, so that's straightforward.

> UpdateActivationEnvironment and BecomeMonitor are never allowed.

These are currently privileged, although see Bug #103458.

> RequestName, ReleaseName and ListQueuedOwners are allowed iff we have OWN
> access to the name.

These will need special cases in their implementations, but we can encapsulate that in the OWN_BUS_NAME flag.

> Special case for ListActivatableNames, ListNames, NameHasOwner,
> GetNameOwner, GetConnectionWhatever: names to which we don't have SEE access
> are invisible.

These will need special cases in their implementations, which we can encapsulate in the SEE flag and the data structure in Comment #5.

> Special case for StartServiceByName: it requires TALK access (in particular
> FILTERED access is not enough).

In Comment #5 I suggested a new ACTIVATE flag for this.

> Special case for NameOwnerChanged: only names to which we have >= SEE access
> are listed, but --sloppy-names is equivalent to having SEE access to every
> unique name. If a client has >= SEE access to a well-known name, and
> receives NameOwnerChanged for that well-known name, then that client (but
> not other clients in the same proxy?) is granted SEE access to the old and
> new owners as well.

We'd need a special case in the implementation. Comment #5 provides this.

> Other dbus-daemon methods and interfaces are blocked.

Because this *is* dbus-daemon, we can use METHOD_FLAG_PRIVILEGED, METHOD_FLAG_NO_CONTAINERS, or special-cases in the implementation.

> Receiving a method call or a unicast signal from a peer

I think it would be reasonable to say that method calls and unicast signals from the outside are always controlled by the sender, and container instances always allow receiving them.

> Receiving a method call or a unicast signal from a peer grants the recipient
> (but no other client of the same proxy) SEE access to that peer. I think we
> can relax that to giving everything in the same container instance SEE
> access to that peer, because there is no security boundary within a
> container instance.

The data structure suggested in Comment #5 can provide this.

> Receiving broadcast signals from outside the container instance checks for
> TALK access to the sender.

Comment #5 proposes RECEIVE_BROADCASTS.

> flatpak-dbus-proxy users can always TALK to their own unique name. Because
> OWN implies TALK, they can also TALK to anything in the same container
> instance, as long as communication was initiated using the well-known name.
> I think it would be reasonable to relax this to say that talking to other
> unique names in the same container instance is fine, because there is no
> security boundary within a container instance.

I think this can just be implicit.
Comment 7 Simon McVittie 2017-10-27 13:55:29 UTC
(In reply to Simon McVittie from comment #5)
> (In reply to Simon McVittie from comment #3)
> > * flatpak-dbus-proxy --filter --sloppy-names 
> 
> The equivalent of --sloppy-names would need to be a new flag (either an
> Allow rule with the flag or a separate named parameter),
> SEE_ALL_UNIQUE_NAMES or SeeAllUniqueNames.

It's a little less strict than flatpak-dbus-proxy, but I think it would be better to be a bit more consistent, and say that this flag also makes ListNames, NameHasOwner and GetNameOwner see all unique names. (In principle ListActivatableNames is also affected, but unique names aren't activatable.)
Comment 8 Simon McVittie 2017-10-27 15:29:04 UTC
(In reply to Simon McVittie from comment #5)
> The equivalent of --talk=NAME is
> (flags=SEE|CALL_METHODS|SEND_UNICAST_SIGNALS|RECEIVE_BROADCASTS|SEND_UNIX_FDS
> |RECEIVE_UNIX_FDS|OBJECT_PATH_IS_SUBTREE|ACTIVATE, bus_name=NAME,
> object_path=/, interface=""). (New flags needed here since Comment #0:
> SEND_UNICAST_SIGNALS, ACTIVATE.)

Alternatively, ACTIVATE could be implied by CALL_METHODS rather than existing as a separate flag, so you can StartServiceByName(foo) if there exists any method that you can call on foo. (Justification: if you can call any method on it, then you could autostart it, so there is no point in not letting you activate it explicitly.)

The reason I'm separating SEND_UNICAST_SIGNALS from sending broadcasts is that unicast signals have been used to exploit bugs in D-Bus users in the past (CVE-2013-0292 is a particularly egregious example).

The reason I'm separating SEND_UNICAST_SIGNALS from CALL_METHODS is that the object path, interface and (if present) member name have different meanings: for a signal they're facts about the sender, but for a method call they're facts about the recipient.

> We need a new flag INTERFACE_IS_REALLY_METHOD

Or INTERFACE_IS_MEMBER to generalize it to signals, although flatpak-dbus-proxy can't actually do that.

Flatpak uses com.example.Interface.Member or com.example.Interface.*, but I think I prefer to emphasize the common use-case (access at a per-interface granularity), which works well if you break up your interfaces by intended audience/level of privilege.

We could call the struct member interface_or_member and the flag SPECIFIC_MEMBER to make the naming less weird.
Comment 9 Simon McVittie 2017-10-27 16:00:11 UTC
(In reply to Simon McVittie from comment #5)
> This could be added as (flags=SEE,bus_name=NAME, object_path="/",
> interface="") (optionally adding OBJECT_PATH_IS_SUBTREE, but the object path
> is ignored for SEE anyway).

Justification for having SEE as an explicit flag: if we don't, then we can't use (RECEIVE_UNIX_FDS|OBJECT_PATH_IS_SUBTREE, "", "/", "") to mean "trusted processes can fd-pass when they call this container's methods" 

I want to lock down fd-passing by default so that we have the option of doing so: because the access control model is purely additive, we should only allow things by default if we're absolutely sure they're always fine. We have an "escape hatch" to tighten it up by adding new named parameters, but let's try not to need that. While fd-passing is extremely useful, CVE-2014-3637, CVE-2014-3635, CVE-2014-3636, CVE-2014-3532, CVE-2014-3533 are not a particularly inspiring track record.

I don't think Flatpak is designed to prevent denial of service by a contained app, in which case only CVE-2014-3635 would have been considered to be a problem; so it's probably the right trade-off for Flatpak to opt in to sending and receiving fds. But, as with the restriction on sending broadcasts, let's not rule out container technologies that might want to be more strict than Flatpak.
Comment 10 Simon McVittie 2017-10-27 16:51:05 UTC
(In reply to Philip Withnall from comment #2)
> (In reply to Simon McVittie from comment #0)
> >           [TODO: Does it allow receiving unicast signals from connections
> >           that are not in a container?]
> 
> Yes, that’s used by a few system service implementations, right?

We need to make this possible, sure; my question was whether it's always allowed, or whether it's something we opt-in to with a flag. I think I've come down on the side of always allowing it.
Comment 11 Simon McVittie 2017-10-27 16:59:15 UTC
(In reply to Simon McVittie from comment #4)
> Special case for StartServiceByName: it requires TALK access (in particular
> FILTERED access is not enough).

I think this is more strict than it needs to be. It is sufficient that there exists at least one method call that we are allowed to send to the service (because if so, we could call it and rely on auto-starting).


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.