[systemd-devel] SD_BUS_VTABLE_CAPABILITY

[Andy Lutomirski]

On Thu, Apr 16, 2015 at 10:30 AM, Lennart Poettering
<lennart at poettering.net> wrote:
> 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, ly, 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?
>

I assumed that the sd-bus CapEff code made authentication decisions.
If it really doesn't, then it's probably not exploitable as such.

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

Sorry, but this is a logical fallacy.  Just because actions do similar
things doesn't mean they should be protected by the same security
mechanism.

It's kind of like how, on traditional POSIX, root can do everything
and no one else can do anything interesting.  That doesn't mean that
root ought to be able to ask a userspace daemon to do X and therefore
the daemon should grow special fancy hard-wired code to allow that,
even when the usual authentication mechanism for X is completely
unrelated to being root.

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

No, it's not certainly wrong.  It's maybe unexpected.  On the other
hand, as I not-very-educated dbus user, I'd expect the actual dbus
policy in the policy files to be authoritative.

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

You really like rebooting and setting time.  What about, say,
associating to a wireless network?  How about changing the hostname?
Are you going to allow CAP_DAC_OVERRIDE to change the hostname even if
normal policy wouldn't allow it?

--Andy