All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Garrett <matthew.garrett@nebula.com>
To: "gnomes@lxorguk.ukuu.org.uk" <gnomes@lxorguk.ukuu.org.uk>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"hpa@zytor.com" <hpa@zytor.com>,
	"jwboyer@fedoraproject.org" <jwboyer@fedoraproject.org>,
	"linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Subject: Re: Trusted kernel patchset for Secure Boot lockdown
Date: Fri, 14 Mar 2014 01:57:30 +0000	[thread overview]
Message-ID: <1394762250.6416.24.camel@x230.lan> (raw)
In-Reply-To: <20140313232140.03bdaac3@alan.etchedpixels.co.uk>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4264 bytes --]

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@nebula.com> 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@nebula.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

WARNING: multiple messages have this Message-ID (diff)
From: Matthew Garrett <matthew.garrett-05XSO3Yj/JvQT0dZR+AlfA@public.gmane.org>
To: "gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org"
	<gnomes-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>
Cc: "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org"
	<jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>,
	"keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org"
	<keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>,
	"linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org"
	<akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>,
	"hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org"
	<hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>,
	"jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org"
	<jwboyer-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy@public.gmane.org>,
	"linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org"
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
Subject: Re: Trusted kernel patchset for Secure Boot lockdown
Date: Fri, 14 Mar 2014 01:57:30 +0000	[thread overview]
Message-ID: <1394762250.6416.24.camel@x230.lan> (raw)
In-Reply-To: <20140313232140.03bdaac3-mUKnrFFms3BCCTY1wZZT65JpZx93mCW/@public.gmane.org>

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@nebula.com> 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@nebula.com>

  reply	other threads:[~2014-03-14  1:57 UTC|newest]

Thread overview: 128+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-26 20:11 Trusted kernel patchset for Secure Boot lockdown Matthew Garrett
2014-02-26 20:11 ` [PATCH 01/12] Add support for indicating that the booted kernel is externally trusted Matthew Garrett
2014-02-27 19:02   ` Kees Cook
2014-02-27 19:02     ` Kees Cook
2014-03-31 14:49   ` Pavel Machek
2014-02-26 20:11 ` [PATCH 02/12] Enforce module signatures when trusted kernel is enabled Matthew Garrett
2014-02-26 20:11   ` Matthew Garrett
2014-02-26 20:11 ` [PATCH 03/12] PCI: Lock down BAR access when trusted_kernel is true Matthew Garrett
2014-02-26 20:11 ` [PATCH 04/12] x86: Lock down IO port " Matthew Garrett
2014-02-26 20:11 ` [PATCH 05/12] Restrict /dev/mem and /dev/kmem " Matthew Garrett
2014-02-26 20:11 ` [PATCH 06/12] acpi: Limit access to custom_method if " Matthew Garrett
2014-02-26 20:11 ` [PATCH 07/12] acpi: Ignore acpi_rsdp kernel parameter when " Matthew Garrett
2014-02-26 20:11 ` [PATCH 08/12] kexec: Disable at runtime if " Matthew Garrett
2014-02-26 20:11 ` [PATCH 09/12] uswsusp: Disable when " Matthew Garrett
2014-02-26 20:11   ` Matthew Garrett
2014-03-31 14:49   ` Pavel Machek
2014-02-26 20:11 ` [PATCH 10/12] x86: Restrict MSR access " Matthew Garrett
2014-02-26 20:11 ` [PATCH 11/12] asus-wmi: Restrict debugfs interface " Matthew Garrett
2014-02-26 20:11 ` [PATCH 12/12] Add option to automatically set trusted_kernel when in Secure Boot mode Matthew Garrett
2014-02-26 20:11   ` Matthew Garrett
2014-02-26 22:41   ` One Thousand Gnomes
2014-02-26 22:41     ` One Thousand Gnomes
2014-02-26 22:47     ` H. Peter Anvin
2014-02-26 22:48     ` Matthew Garrett
2014-02-26 22:48       ` Matthew Garrett
2014-02-27 18:48       ` Kees Cook
2014-02-27 18:48         ` Kees Cook
2014-02-26 21:11 ` Trusted kernel patchset for Secure Boot lockdown Kees Cook
2014-02-26 22:21   ` One Thousand Gnomes
2014-02-26 22:21     ` One Thousand Gnomes
2014-02-27  9:54     ` Alon Ziv
2014-03-19 17:42     ` Florian Weimer
2014-03-19 17:42       ` Florian Weimer
2014-02-27 18:04 ` Josh Boyer
2014-02-27 18:04   ` Josh Boyer
2014-02-27 19:07   ` Greg KH
2014-02-27 19:11     ` Josh Boyer
2014-02-27 19:11       ` Josh Boyer
2014-02-28 12:50       ` Josh Boyer
2014-02-28  3:03   ` James Morris
2014-02-28  4:52     ` Matthew Garrett
2014-02-28  4:52       ` Matthew Garrett
2014-03-13  5:01     ` Matthew Garrett
2014-03-13  5:01       ` Matthew Garrett
2014-03-13  6:22       ` Kees Cook
2014-03-13  6:22         ` Kees Cook
2014-03-13  9:33         ` James Morris
2014-03-13  9:33           ` James Morris
2014-03-13 10:12           ` One Thousand Gnomes
2014-03-13 10:12             ` One Thousand Gnomes
2014-03-13 15:54             ` H. Peter Anvin
2014-03-13 15:54               ` H. Peter Anvin
2014-03-13 15:59           ` Matthew Garrett
2014-03-13 15:59             ` Matthew Garrett
2014-03-13 21:24             ` One Thousand Gnomes
2014-03-13 21:24               ` One Thousand Gnomes
2014-03-13 21:28               ` H. Peter Anvin
2014-03-13 21:28                 ` H. Peter Anvin
2014-03-13 21:32                 ` Matthew Garrett
2014-03-13 21:32                   ` Matthew Garrett
2014-03-13 21:30               ` Matthew Garrett
2014-03-13 21:30                 ` Matthew Garrett
2014-03-13 23:21                 ` One Thousand Gnomes
2014-03-13 23:21                   ` One Thousand Gnomes
2014-03-14  1:57                   ` Matthew Garrett [this message]
2014-03-14  1:57                     ` Matthew Garrett
2014-03-14 12:22                     ` One Thousand Gnomes
2014-03-14 12:22                       ` One Thousand Gnomes
2014-03-14 12:51                       ` Matthew Garrett
2014-03-14 12:51                         ` Matthew Garrett
2014-03-14 15:23                         ` Kees Cook
2014-03-14 15:23                           ` Kees Cook
2014-03-14 15:46                           ` Matthew Garrett
2014-03-14 15:46                             ` Matthew Garrett
2014-03-14 15:54                             ` Kees Cook
2014-03-14 15:54                               ` Kees Cook
2014-03-14 15:58                               ` Matthew Garrett
2014-03-14 15:58                                 ` Matthew Garrett
2014-03-14 16:28                           ` One Thousand Gnomes
2014-03-14 16:28                             ` One Thousand Gnomes
2014-03-14 17:06                         ` One Thousand Gnomes
2014-03-14 17:06                           ` One Thousand Gnomes
2014-03-14 18:11                           ` Matthew Garrett
2014-03-14 18:11                             ` Matthew Garrett
2014-03-14 19:24                             ` Matthew Garrett
2014-03-14 19:24                               ` Matthew Garrett
2014-03-14 20:37                               ` David Lang
2014-03-14 20:37                                 ` David Lang
2014-03-14 20:43                                 ` Matthew Garrett
2014-03-14 20:43                                   ` Matthew Garrett
2014-03-14 21:58                               ` One Thousand Gnomes
2014-03-14 21:58                                 ` One Thousand Gnomes
2014-03-14 22:04                                 ` Matthew Garrett
2014-03-14 22:04                                   ` Matthew Garrett
2014-03-14 21:48                             ` One Thousand Gnomes
2014-03-14 21:48                               ` One Thousand Gnomes
2014-03-14 21:56                               ` Matthew Garrett
2014-03-14 21:56                                 ` Matthew Garrett
2014-03-14 22:08                                 ` One Thousand Gnomes
2014-03-14 22:08                                   ` One Thousand Gnomes
2014-03-14 22:15                                   ` Matthew Garrett
2014-03-14 22:15                                     ` Matthew Garrett
2014-03-14 22:31                                     ` One Thousand Gnomes
2014-03-14 22:31                                       ` One Thousand Gnomes
2014-03-14 22:52                                       ` Matthew Garrett
2014-03-14 22:52                                         ` Matthew Garrett
2014-03-19 19:50                                       ` Kees Cook
2014-03-19 19:50                                         ` Kees Cook
2014-03-14 23:18                                   ` Theodore Ts'o
2014-03-14 23:18                                     ` Theodore Ts'o
2014-03-15  0:15                                     ` One Thousand Gnomes
2014-03-15  0:15                                       ` One Thousand Gnomes
2014-03-19 17:49                                     ` Florian Weimer
2014-03-19 17:49                                       ` Florian Weimer
2014-03-19 20:16                                     ` Kees Cook
2014-03-19 20:16                                       ` Kees Cook
2014-03-20 14:47                                       ` One Thousand Gnomes
2014-03-20 14:47                                         ` One Thousand Gnomes
2014-03-20 14:55                                       ` tytso
2014-03-20 14:55                                         ` tytso
2014-03-20 17:12                                         ` Matthew Garrett
2014-03-20 17:12                                           ` Matthew Garrett
2014-03-20 18:13                                           ` One Thousand Gnomes
2014-03-20 18:13                                             ` One Thousand Gnomes
2014-03-13 21:26             ` One Thousand Gnomes
2014-03-13 21:26               ` One Thousand Gnomes
2014-03-13 21:31               ` Matthew Garrett
2014-03-13 21:31                 ` Matthew Garrett

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1394762250.6416.24.camel@x230.lan \
    --to=matthew.garrett@nebula.com \
    --cc=akpm@linux-foundation.org \
    --cc=gnomes@lxorguk.ukuu.org.uk \
    --cc=gregkh@linuxfoundation.org \
    --cc=hpa@zytor.com \
    --cc=jmorris@namei.org \
    --cc=jwboyer@fedoraproject.org \
    --cc=keescook@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.