Bug 103457 - Containers (#100344): consider adding an option to let other uids in
Summary: Containers (#100344): consider adding an option to let other uids in
Status: NEW
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: git master
Hardware: All All
: low enhancement
Assignee: Simon McVittie
QA Contact: D-Bus Maintainers
URL:
Whiteboard:
Keywords:
Depends on: 101354 104610
Blocks: 100344
  Show dependency treegraph
 
Reported: 2017-10-25 17:16 UTC by Simon McVittie
Modified: 2018-01-12 19:58 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[1/2] containers test: Don't assert that GetConnectionCredentials has Type, Name (1.52 KB, patch)
2017-12-19 20:05 UTC, Simon McVittie
Details | Splinter Review
[2/2] GetConnectionCredentials: Don't include Containers1 Type, Name (1.28 KB, patch)
2017-12-19 20:06 UTC, Simon McVittie
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2017-10-25 17:16:34 UTC
::: bus/containers.c
@@ +318,5 @@
> +   *
> +   * TODO: For the system bus we might want a way to opt-in to allowing
> +   * other uids, in which case we would refrain from overwriting the
> +   * BusContext's unix_user_function; but that isn't part of the
> +   * lowest-common-denominator implementation. */
Comment 1 Simon McVittie 2017-12-12 17:56:23 UTC
See Bug #100344, comments starting at 15, for some more thoughts on whether/when this might be desirable.
Comment 2 Simon McVittie 2017-12-19 19:55:38 UTC
On Bug #100344, David Herrmann had some more ideas about this (see its comments 19 to 21).

At the moment we require connecting users to have the same uid as the creator of the container socket, so the difference is only theoretical anyway, and we don't *have* to solve this right now.

(In reply to David Herrmann on https://bugs.freedesktop.org/show_bug.cgi?id=100344#c21)
> (In reply to Simon McVittie on https://bugs.freedesktop.org/show_bug.cgi?id=100344#c20)
> > In that case Geoclue would be asking Alice "Allow Weather to see your
> > location? [y/n]", and the only security issue is one that she has inflicted
> > on herself, by delegating a socket to Bob that is pre-authenticated to act
> > as Alice (the fd acts like a capability - in the capability-based security
> > sense, not the CAP_SYS_ADMIN sense).
> 
> Right. I would expect the origin of the listener socket to determine the
> capabilities of anyone talking through it.

I'm not so sure that that follows. If you give someone else access to a connected IPC connection, it seems reasonable that they can do IPC as though they were the user who connected. If you give someone else access to a listening socket, and they connect to it themselves, that doesn't really feel the same to me - particularly since dbus-daemon already *has* a listening socket, and if you connect to that one, you don't get to pretend to be messagebus!

> This would mean this entire container infrastructure is just a fancy way to
> multiplex a single socket to the daemon. So semantically speaking, we could
> make this container infrastructure mirror xdg-proxy: Whenever someone
> connects through such a listener, they get the credentials of the
> listener-owner enforced, rather than capturing their credentials at
> connect-time.

That's an interesting way to think about it, but I would tend to think that the flatpak-dbus-proxy works like this because nothing else was implementable in a proxy, rather than because that's what its authors ideally wanted.
Comment 3 Simon McVittie 2017-12-19 19:59:16 UTC
> If we went this route, we would probably have to omit the type and uid from
> GetConnectionCredentials(), because in this world-view they are no use (and
> potentially actively harmful) when not used in conjunction with the container
> instance initiator's identity, and putting the container instance initiator's
> identity in GetConnectionCredentials() seems easy to confuse with the actual
> connection's identity.

In the interests of kicking this can further down the road, I'm very tempted to remove o.fd.D.Containers1.Type and o.fd.D.Containers1.Name from GetConnectionCredentials(), leaving only o.fd.D.Containers1.Instance (the object path); and if there's demand for them, we can (solve the issue of who to believe, then) put them back in.
Comment 4 Simon McVittie 2017-12-19 20:05:38 UTC
Created attachment 136300 [details] [review]
[1/2] containers test: Don't assert that GetConnectionCredentials has Type, Name

It isn't clear whether we want these in GetConnectionCredentials(),
because if we later open up different-uid access to the container
sockets, it raises questions of how strongly we can trust that
information.
Comment 5 Simon McVittie 2017-12-19 20:06:11 UTC
Created attachment 136301 [details] [review]
[2/2] GetConnectionCredentials: Don't include Containers1 Type, Name

It isn't clear whether we want these in GetConnectionCredentials(),
because if we later open up different-uid access to the container
sockets, it raises questions of how strongly we can trust that
information.
Comment 6 Simon McVittie 2017-12-19 20:08:13 UTC
The other thing I should do here in the short term is to add the container instance's initiator's credentials to GetInstanceInfo() and and GetConnectionInstance() (which will be an API break, but that's OK, this is still very much at the experimental stage and has never been in a release). Then anyone calling one of those functions will at least have all the tools to make their own decisions.
Comment 7 Simon McVittie 2017-12-19 20:20:07 UTC
(In reply to David Herrmann in https://bugs.freedesktop.org/show_bug.cgi?id=100344#c19)
> At the same time, if Bob decides to
> access random resources, he must be aware that they might be maliciously put
> there.

It's a bit more complicated than this if the reason you're sandboxing apps is that you do not 100% trust them. The situation I was concerned about in https://bugs.freedesktop.org/show_bug.cgi?id=100344#c15 was: Bob has two apps, let's say GoodApp and MalwareApp. The malware wants to get sensitive data from a system bus service; Bob does not want this. Another user Alice, conspiring with the author of the malware, can create a system bus listening socket that claims to be GoodApp. If Bob's instance of MalwareApp is able to connect to that socket and send a request to the service, then the service will believe that the request to see the sensitive data came from GoodApp (admittedly running as Alice, so it's detectable, but still seems weird).

For the specific case of Geoclue and location (which is what I said on #100344), it doesn't really matter, because the sensitive data (geolocation) is the same for both Alice and Bob anyway; so if Alice is willing to leak that information to the malware author, Bob has already lost. But perhaps there are other system services that process data on behalf of specific users, so Alice would not be able to do the equivalent except by tricking Bob?

Again, until we let users connect to sockets they didn't create, this is purely theoretical (and my only concern right now is to avoid committing to an API that, later, will turn out to have been a mistake).
Comment 8 Simon McVittie 2018-01-12 19:58:00 UTC
(In reply to Simon McVittie from comment #6)
> The other thing I should do here in the short term is to add the container
> instance's initiator's credentials to GetInstanceInfo() and and
> GetConnectionInstance() (which will be an API break, but that's OK, this is
> still very much at the experimental stage and has never been in a release).
> Then anyone calling one of those functions will at least have all the tools
> to make their own decisions.

Kicked out to Bug #104610.
Comment 9 Simon McVittie 2018-01-12 19:58:17 UTC
Comment on attachment 136300 [details] [review]
[1/2] containers test: Don't assert that GetConnectionCredentials has Type, Name

Obsoleted by a commit on Bug #104610
Comment 10 Simon McVittie 2018-01-12 19:58:29 UTC
Comment on attachment 136301 [details] [review]
[2/2] GetConnectionCredentials: Don't include Containers1 Type, Name

Obsoleted by a commit on Bug #104610


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.