Re: Trusted kernel patchset for Secure Boot lockdown

[Matthew Garrett]

On Thu, 2014-03-13 at 23:21 +0000, One Thousand Gnomes wrote:
> On Thu, 13 Mar 2014 21:30:48 +0000
> Matthew Garrett <matthew.garrett@xxxxxxxxxx> wrote:
> 
> > On Thu, 2014-03-13 at 21:24 +0000, One Thousand Gnomes wrote:
> > 
> > > If I have CAP_SYS_RAWIO I can make arbitary ring 0 calls from userspace,
> > > trivially and in a fashion well known and documented.
> > 
> > How?
> 
> You want a list... there are load of places all over the kernel that have
> assumptions that RAWIO = safe from the boringly mundane like MSR access
> to the more obscure driver "this is RAWIO trust the user" cases of which
> there are plenty.

Have you actually looked at these patches? I've looked at every case of
RAWIO in the kernel. For cases that are hardware specific and tied to
fairly old hardware, I've ignored them. For cases which provide an
obvious mechanism for exploitation, I've added an additional check. For
cases where I can't see a reasonable mechanism for executing arbitrary
code in the kernel, I've done nothing.

If you have specific examples of processes with CAP_SYS_RAWIO being able
to execute arbitrary code in the kernel even with this patchset applied,
please, give them.

>You can even avoid the userspace issues with a small amount of
>checking. If you don't want to touch capability sets then make the
>default behaviour for capable(x) in fact be
>
>        capable(x & ~secure_forbidden)
>
>for a measured kernel and add a 
>
>        capable_always()
>
>for the cases you want to not break.

We could do that, but now the behaviour of the patchset is far less
obvious. capable(CAP_SYS_RAWIO) now means something different to every
other use of capable(), and we still need get_trusted_kernel() calls for
cases where the checks have nothing to do with processes and so
capabilities can't be used. It still involves auditing every use of
CAP_SYS_RAWIO. In fact, in some cases we need to *add* CAP_SYS_RAWIO
checks - which, again, breaks userspace.

> As for mem= and exactmap, it has nothing to do with /dev/mem and
> everything to do with giving the kernel a memory map where some of the
> space it thinks is RAM is in fact devices, rom, space etc. If the kernel
> is given a false memory map it will misbehave. Exploitably - well given
> the kind of things people have achieved in the past - quite possibly.

Sure. That's a worthwhile thing to fix, and it's something that dropping
CAP_SYS_RAWIO would do nothing to help you with.

> If you are not prepared to do the job right, then I don't think it
> belongs upstream. Let's do it right, and if we have to tweak a few bits
> of userspace to make them work in measured mode (but without breaking
> anything in normal modes) then it's worth doing the job properly.

We can do this without unnecessarily breaking any userspace. We just
can't do it by fiddling with capabilities.

> I don't think we need to break any userspace for "normal" mode to do
> this. Userspace in measured mode is going to change anyway. It already
> has just for things like module signing.

This has been discussed at length. Nobody who's actually spent time
working on the problem wants to use capabilities. CAP_SYS_RAWIO is not
semantically identical to the trusted kernel bit. Trying to make them
semantically identical will break existing userspace.

> (As an aside you may also then want to think about whether you allow
> measured userspace elements that secure_forbidden is considered to be 0
> for so you can sign userspace apps that are allowed to do RAWIO)

I'd be amazed if any of the applications that need RAWIO have had any
kind of meaningful security audit, with the possible exception of X (and
then we'd need to add support for signed X modules and sign all the
DDXes and seriously just no). I've no objection to someone doing that
work (and Vivek did a pile of it when looking at implementing kexec via
signed userspace), but I don't see any real use cases - pretty much
everyone using bits of RAWIO that are gated in the trusted kernel case
should be using a real kernel interface instead.

-- 
Matthew Garrett <matthew.garrett@xxxxxxxxxx>