[systemd-devel] SD_BUS_VTABLE_CAPABILITY

[Andy Lutomirski]

On Thu, Apr 16, 2015 at 8:59 AM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Thu, 16.04.15 07:52, Andy Lutomirski (luto at amacapital.net) wrote:
>
>> I'm looking at sd_bus_query_sender_privilege, which does:
>>
>> r = sd_bus_query_sender_creds(call,
>> SD_BUS_CREDS_UID|SD_BUS_CREDS_EUID|SD_BUS_CREDS_EFFECTIVE_CAPS,
>> &creds);
>>
>> That, in turn, does:
>>
>>         if (!c || !(c->mask & SD_BUS_CREDS_PID)) {
>>                 /* We couldn't read anything from the call, let's try
>>                  * to get it from the sender or peer */
>>
>>                 if (call->sender)
>>                         return sd_bus_get_name_creds(call->bus,
>> call->sender, mask, creds);
>>                 else
>>                         return sd_bus_get_owner_creds(call->bus, mask, creds);
>>         }
>>
>>         return bus_creds_extend_by_pid(c, mask, creds);
>>
>> The sd_bus_get_name_creds call seems questionable to me -- isn't that
>> trying to augment missing information from the time of call with
>> metadata from the sender's connection?  (I really think this dichotomy
>> needs to be removed, *especially* since it looks like code already
>> exists to try to use both metadata sources.  This seems like it's just
>> asking for security screw-ups.)
>
> This information is missing only for dbus1 clients connecting via the
> dbus1 compat proxy. In that case we need to fall back to connection
> credentials, because that's how authentication works on dbus1.
>
>> The sd_bus_get_owner_creds call looks totally bogus.  I don't know
>> when it would be called, but I hope the answer is "provably never in
>> response to a message from an unprivileged user", in which case I'd
>> wonder why it's there.
>
> Messages with empty senders are only used for dbus1 *direct*
> connections. This is a special setup dbus1 supports where
> communication is not done via the bus daemon, but via a direct AF_UNIX
> connection between two peers. It's very seldom used, and mostly a hack
> to avoid the performance penalty that the dbus-daemon means. These
> direct connections are entirely redundant in a kdbus world, and you
> better shouldn't think about them.
>
> Ultimately, this code exists in sd-bus for one reason only: in a
> non-kdbus world systemd must be accessible even if dbus-daemon is not
> up. Since dbus-daemon is only started relatively late during boot
> there's a window in early boot where we need to be accessible by some
> other means: for that we allow privileged clients to connect to us
> directly. We then still speak the dbus protocol, and offer similar
> APIs, but we do not use the bus topology. In kdbus all of that goes
> away, since kdbus is available all the time, since earliest time of
> boot.
>
> For such direct dbus1 connections it's the creds of the peer of the
> dbus connection that we need to authenticate, that is exposed as
> "owner creds".
>
> Or in other words, the if check above you can read like this:
>
> if (we_get_a_full_kdbus_message)
>         use_the_attached_creds();
> else {
>         if (we_get_a_compat_dbus1_message_via_kdbus_or_native_dbus1)
>                 use_the_senders_creds()
>         else if (we_get_a_message_via_dbus_direct_connection())
>                 use_the_peer_creds();
> }
>
> To make this easier to understand I'll add some comments to this code:
>
> http://cgit.freedesktop.org/systemd/systemd/commit/?id=2d0c1561340efff3265fe89b05eae4ee8f4037a7
>
>> The bus_creds_extend_by_pid is what I'm asking about.  When is it
>> used?
>
> 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.

> Also see the comment about this in sd-bus.h:
>
> http://cgit.freedesktop.org/systemd/systemd/tree/src/systemd/sd-bus.h?id=2d0c1561340efff3265fe89b05eae4ee8f4037a7#n89
>
> Or check the man page for busctl that explains this too:
>
> http://www.freedesktop.org/software/systemd/man/busctl.html#--augment-creds=BOOL
>
>> In any event, under certain (strewn-across-multiple-files with
>> duplicated code in various places) conditions, all three paths above
>> seem to land in bus_creds_add_more.
>>
>> It looks like some of this is gated on whether the sending PID is
>> known (IMO this is odd -- it should be gated by whether you think it's
>> a good idea, not whether a piece of information is known), but maybe
>> mac_selinux_generic_access_check can circumvent that.
>
> Well, the augmentation code needs the PID to read the data from /proc,
> that's what it does after all...
>
>> So why exactly are capabilities only used with kdbus?  I don't see it,
>> and I do see code that tries to extract them from /proc.
>
> The augmentation code is only used if people opt-in to it, by setting
> SD_BUS_CREDS_AUGMENT when querying the creds. And if you do that, you
> are not supposed to use them for authentication, and our own code
> certainly does not, and the documentation (though terse) actually
> highlights that...
>
>> > /proc/$PID/status. But this opens this up for a vulnerability: if a
>> > client quickly issues a method call, and then exec()s a suid binary,
>> > it could happen that the service side would read the capabilities from
>> > that SUID process, and not the original process, and the exploit was
>> > successful. We cannot allow that, hence no capabilities on dbus1,
>> > instead we open up things to a much, much broader uid == 0. kdbus
>> > allows us to be much stricter there, by allowing us to check only one
>> > specific capability.
>>
>> 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?

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

>> >> In the interest of full disclosure, I'm asking because I think that
>> >> one of two things is true:
>> >>
>> >> 1. The SD_BUS_VTABLE_CAPABILITY code is useless and should therefore be deleted.
>> >
>> > This is not the case.
>> >
>> >> 2. The SD_BUS_VTABLE_CAPABILITY code is exploitably buggy and should
>> >> therefore be deleted.
>> >
>> > I have heard this claim from you many times, but so far I haven't been
>> > able to figure out what you have in mind. Could you either give an
>> > example of an exploit or at least the general scheme you have in mind?
>>
>> 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.

> 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.
>
> Please, either provide a concrete example, or at least explain in
> detail how you think this is expliotable, I am genuinely interested.
>
> Let me summarize this again:
>
> a) sd-bus contains code for augmenting creds from /proc. You have to
>    enable it explicitly though, and you should not use it for
>    authentication, and we don't use it for that.

You do in at least one place, I think.

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

Keep in mind that achieving (b) means finding a use case that wouldn't
benefit from the kind of sandboxing that involves user namespaces and
would therefore prevent the mechanism from working entirely.  But
again, I still haven't seen a clear description of a *single* use case
that benefits *at all* from the caps check.  But you're not known for
writing code for no reason, so I suspect you have something in mind,
and I'm curious what it is.

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

--Andy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: unshare_preload.c
Type: text/x-csrc
Size: 1225 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20150416/9bea4178/attachment.c>