[systemd-devel] SD_BUS_VTABLE_CAPABILITY

[Lennart Poettering]

On Thu, 16.04.15 09:53, Andy Lutomirski (luto at amacapital.net) wrote:

> > It's a noop, unless people OR in SD_BUS_CREDS_AUGMENT into the flags
> > of creds they want. Doing this basically voids your warranty: it means
> > that the creds data shall be augmented with data from /proc, which are
> > good enough for logging, debugging things, but not for security
> > relevant things, such as authorizing message calls.
> 
> OK, I finally found the relevant AUGMENT check.
> 
> >
> > Our own message authorization call does not make use of this flag.
> >
> 
> mac_selinux_generic_access_check does.  It also uses sender metadata,
> which I find interesting given that you seem to strongly prefer using
> message metadata.

It certainly does. But this is not actually used for authentication
either, really, because the auth code in there only cares for the
selinux label, which is available as native connection metadata on
dbus1 already. The augmentation is used because the selinux guys want to
generate a nice log message when the permission is not granted.

The selinux code in systemd was actually contributed by the selinux
people a long time ago. It contained code to read data from /proc
manually for showing in the messages. I recently reworked this to just
use SD_BUS_CREDS_AUGMENT instead, so that reading data from /proc is
at least unified at one place. This is safe only because we know that
the selinux data is not augmented, but available anyway and safely,
and we rely on that.

You can take the selinux code in system as a hint, that getting
metadata about bus requests that one can log about is nothign we made
up, it's very real, and our selinux code added that years ago, and was
written by the selinux guys, not us...

I will grant you though that it is confusing that we use
SD_BUS_CREDS_AUGMENT here like this, and implicitly rely on that the
selinux label is not a field that is being augmented. We should make
this explicit, absolutely. I'll now add some code that will make this
assumption explicit and fails early if the selinux label happens to be
augmented. Of course in real-life this is impossible to trigger, but
it's certainly helps understanding the code.

> >> Agreed, and that's not the only vulnerability.  But code to do it
> >> certainly exists in systemd.
> >
> > "Not the only vulnerability"? I am not aware of any vulnerability, can
> > you explain?
> 
> See below and attached.
> 
> >
> >> > $ cp `which busctl` .
> >> > $ sudo setcap all=eip busctl
> >> > $ ./busctl call org.freedesktop.timedate1 /org/freedesktop/timedate1
> >> > org.freedesktop.timedate1 SetTimezone "sb" Europe/London 0
> >> >
> >>
> >> I still don't see why, even if this were bug-free, it's even remotely
> >> useful.  Keep in mind that basically no one has a capability if
> >> they're not root, and dbus users should really use *dbus* mechanisms
> >> to retain selected privileges when they drop other privileges.
> >
> > Well, some of systemd's own deamons run with an uid != 0, but some
> > caps left.
> >
> > And avahi (as a daemon I know very well, that is not part of systemd)
> > does that too, since 10 years actually... It's not that uncommon among
> > system daemons really.
> 
> And does it use those caps for the purpose of authenticating to dbus
> services?

No, it does not. Because it's unsafe to do that on dbus1. As mentioned
before, kdbus makes this safe.

> If so, perhaps you should reconsider allowing open dbus connections to
> act as capabilities (the real kind, not the Linux capability crap)
> themselves.

No, they do not use capabilities for bus authentication, since they
predate kdbus, and acquiring them on dbus1/af_unix is not safe.

> >> Sure.  Unshare your user namespace, set things up right, and systemd
> >> or any other server will see you as having all capabilities.  You've
> >> fixed that in kdbus, but you haven't (and probably can't!) fix it in
> >> the legacy code, and that legacy code is still there (!).  (Actually,
> >> that code seems to have been actively developed as recently as
> >> November 2014.)
> >
> > Hmm? I do not understand what you are referring to.
> 
> See the attached code.  If you LD_PRELOAD it and send a message that
> causes the receiver to read CapEff, then *boom*.  So either:
> 
> a) all the code to read CapEff in the systemd tree doesn't do anything, or
> b) it's wrong, or
> c) it's only used for diagnostics
> 
> As an illustration, LD_PRELOAD the attachment into "sleep 1h", then
> cat /proc/`pidof sleep`/status.  Note the rather large CapEff value :)
>  Again, this has nothing to do with kdbus, and races aren't needed.

I don't see how this is a vulnerability. Reading stuff from /proc is
unsafe. Hence we do not use it for authentication. We all know that. I
am really not sure what you are trying to prove here.

> > Can you please explain how precisely you think that sd-bus or systemd
> > or the way they use capabilities is exploitable in any way? You keep
> > claiming that, but I never have seen more than vague words about
> > this.

Again, this question is still open. Can you please explain to me how
you think that sd-bus or systemd's capability code is exploitable? You
are still very vague about this, and claim it was vulnerable, but I
really don't see that. I am still genuinely curious?

> > b) sd-bus does not use capabalities for authentication on dbus1
> >
> > c) sd-bus uses the caller's capabilities on kdbus for authenticating
> >    message calls. The kernel provides them racefreely in this case and
> >    translates them between namespaces if necessary.
> 
> The kernel will not provide that unless Linus ignores my NACK on that
> particular point or someone convinces me that (a) there's any reason
> at all to do so and (b) said reason is a damn good reason.  So far the
> rather low bar of (a) hasn't been achieved.

Well, first, please see the the other discussion on LKML, the
CAP_SYS_REBOOT example mentioned there.

It's easy to construct similar examples, for example for timedated,
where setting the system clock is subject to CAP_SYS_TIME, exactly
like the underlying system call. Using timedated instead of the system
call gives you the benefit of syncing things into RTC and some tohers,
but ultimately it's all about the system clock and should hence be
protected by the same privilege as the actual system call. Protecting
the "unsafe" raw system call with fewer privileges than the "safer"
path through timedated is certainly wrong and the other way round
to. It should really use the same privs!

The thing is that simply binding things to the UID opens up things a
lot more broad than just opening up things to one explicit cap. Sure,
caps are not particularly well designed, and some of them are really
stupidly broad, however others make a ton of sense, especially in
combination with other lock-down technologies, and CAP_SYS_TIME is
certainly one of the good ones.

> >> The ratio of complexity of capability code the kdbus folks have
> >> already written (hundreds of lines across multiple files) to its
> >> utility (very near zero AFAICT) is, in my book, not a good sign at
> >> all.
> >
> > Well, a good chunk of this complexity certainly originates in the fact
> > that we currently support multiple protocols under one API, and the
> > old dbus1 connection code even in two modes (direct and bus mode). I
> > am looking forward to the simplifications kdbus gets us, so that we
> > can focus on one mode and one protocol only.
> >
> 
> I'm talking about the complexity of using caps at all.
> 
> Honestly, I feel like I'm hearing two contradictory things here:
> 
> 1. systemd never uses caps except in the kdbus case
> 
> 2. systemd has lots of code to support it in the non-kdbus case (or at
> least the case where kdbus doesn't provide those bits of metadata),
> but it's not really used (?)
>
> I'm so confused.

it's actually really easy: we can read the caps from /proc for you, if
you acknowledge that it is dangerous. But we won't do this for
authentication purposes.

Lennart

-- 
Lennart Poettering, Red Hat