All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jann Horn <jann@thejh.net>
To: "Roberts, William C" <william.c.roberts@intel.com>
Cc: Kees Cook <keescook@chromium.org>,
	"kernel-hardening@lists.openwall.com"
	<kernel-hardening@lists.openwall.com>
Subject: Re: [kernel-hardening] RE: [PATCH] printk: introduce kptr_restrict level 3
Date: Fri, 7 Oct 2016 17:15:41 +0200	[thread overview]
Message-ID: <20161007151541.GC14666@pc.thejh.net> (raw)
In-Reply-To: <476DC76E7D1DF2438D32BFADF679FC561CD14FCA@ORSMSX103.amr.corp.intel.com>

[-- Attachment #1: Type: text/plain, Size: 7503 bytes --]

On Fri, Oct 07, 2016 at 03:05:43PM +0000, Roberts, William C wrote:
> 
> 
> > -----Original Message-----
> > From: Jann Horn [mailto:jann@thejh.net]
> > Sent: Friday, October 7, 2016 10:30 AM
> > To: Roberts, William C <william.c.roberts@intel.com>
> > Cc: Kees Cook <keescook@chromium.org>; kernel-
> > hardening@lists.openwall.com
> > Subject: Re: [kernel-hardening] RE: [PATCH] printk: introduce kptr_restrict level 3
> > 
> > On Fri, Oct 07, 2016 at 02:19:43PM +0000, Roberts, William C wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: keescook@google.com [mailto:keescook@google.com] On Behalf Of
> > > > Kees Cook
> > > > Sent: Thursday, October 6, 2016 5:05 PM
> > > > To: Roberts, William C <william.c.roberts@intel.com>
> > > > Cc: kernel-hardening@lists.openwall.com
> > > > Subject: Re: [PATCH] printk: introduce kptr_restrict level 3
> > > >
> > > > On Thu, Oct 6, 2016 at 8:18 AM, Roberts, William C
> > > > <william.c.roberts@intel.com>
> > > > wrote:
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: keescook@google.com [mailto:keescook@google.com] On Behalf
> > > > >> Of Kees Cook
> > > > >> Sent: Wednesday, October 5, 2016 3:34 PM
> > > > >> To: Roberts, William C <william.c.roberts@intel.com>
> > > > >> Cc: kernel-hardening@lists.openwall.com; Jonathan Corbet
> > > > >> <corbet@lwn.net>; linux-doc@vger.kernel.org; LKML
> > > > >> <linux-kernel@vger.kernel.org>; Nick Desaulniers
> > > > >> <ndesaulniers@google.com>; Dave Weinstein <olorin@google.com>
> > > > >> Subject: Re: [PATCH] printk: introduce kptr_restrict level 3
> > > > >>
> > > > >> On Wed, Oct 5, 2016 at 11:04 AM,  <william.c.roberts@intel.com> wrote:
> > > > >> > From: William Roberts <william.c.roberts@intel.com>
> > > > >> >
> > > > >> > Some out-of-tree modules do not use %pK and just use %p, as
> > > > >> > it's the common C paradigm for printing pointers. Because of
> > > > >> > this, kptr_restrict has no affect on the output and thus, no
> > > > >> > way to contain the kernel address leak.
> > > > >>
> > > > >> Solving this is certainly a good idea -- I'm all for finding a solid solution.
> > > > >>
> > > > >> > Introduce kptr_restrict level 3 that causes the kernel to treat
> > > > >> > %p as if it was %pK and thus always prints zeros.
> > > > >>
> > > > >> I'm worried that this could break kernel internals where %p is
> > > > >> being used and not exposed to userspace. Maybe those situations don't
> > exist...
> > > > >>
> > > > >> Regardless, I would rather do what Grsecurity has done in this
> > > > >> area, and whitelist known-safe values instead. For example, they
> > > > >> have %pP for approved pointers, and %pX for approved
> > > > >> dereference_function_descriptor() output. Everything else is
> > > > >> censored if it is a value in kernel memory and destined for a
> > > > >> user-space memory
> > > > >> buffer:
> > > > >>
> > > > >>         if ((unsigned long)ptr > TASK_SIZE && *fmt != 'P' && *fmt
> > > > >> != 'X' && *fmt != 'K' && is_usercopy_object(buf)) {
> > > > >>                 printk(KERN_ALERT "grsec: kernel infoleak detected!
> > > > >> Please report this log to spender@grsecurity.net.\n");
> > > > >>                 dump_stack();
> > > > >>                 ptr = NULL;
> > > > >>         }
> > > > >>
> > > > >> The "is_usercopy_object()" test is something we can add, which is
> > > > >> testing for a new SLAB flag that is used to mark slab caches as
> > > > >> either used by user-space or not, which is done also through whitelisting.
> > > > >> (For more details on this, see:
> > > > >> http://www.openwall.com/lists/kernel-hardening/2016/06/08/10)
> > > > >>
> > > > >> Would you have time/interest to add the slab flags and
> > is_usercopy_object()?
> > > > >> The hardened usercopy part of the slab whitelisting can be
> > > > >> separate, since it likely needs a different usercopy interface to
> > > > >> sanely integrate with
> > > > upstream.
> > > > >
> > > > > A couple of questions off hand:
> > > > > 1. What about bss statics? I am assuming that when the loader
> > > > > loads up a
> > > > module
> > > > >      That it's dynamically allocating the .bss section or some equivalent. I
> > would
> > > > >      Also assume the method you describe would catch that, is that correct?
> > > > >
> > > > > 2. What about stack variables?
> > > >
> > > > It looks like what Grsecurity is doing is saying "if the address is
> > > > outside of user- space" (" > TASK_SIZE") and it's not whitelisted
> > > > ('P',
> > > > 'X') and it's going to land in a user-space buffer ("is_usercopy_object()",
> > censor it.
> > > > ("K" is already censored -- they're just optimizing to avoid
> > > > re-checking it
> > > > needlessly.)
> > > >
> > > > So, in this case, all kernel memory, bss and stack included, would
> > > > be outside the user-space address range. (I am curious, however, how
> > > > to apply this to an architecture like s390 which has overlapping
> > > > address ranges... probably the TASK_SIZE test needs to use some
> > > > other "is in kernel memory" check that compiles down to TASK_SIZE on
> > > > non-s390, and DTRT on s390, etc.)
> > > >
> > >
> > > Before I go off and attempt this, I just have another dumb question to ask:
> > >
> > > If the printk copies it into the kernel ring buffer, at some point,
> > > someone comes And asks for a copy into a userspace buffer either via dmesg or
> > proc/kmsg interfaces.
> > 
> > IMO that's fine - I don't think pointers in the kernel ring buffer should be
> > restricted.
> > Instead, access to dmesg / proc/kmsg should be restricted appropriately.
> > 
> > I guess it depends on what the goal here is. Do we really want to stop root from
> > ever seeing a kernel pointer (in which case OOPS messages wouldn't really work
> > anymore)? My view is that restricting these interfaces so far that only root can
> > access them and it's unlikely that root accidentally does so is sufficient.
> 
> I'm running Ubuntu 14.04, and perhaps they just got it wrong, but I can do dmesg as
> an unprivileged user. I usually only work on Android, and its restricted to root. I'm
> not sure how other distro's set this up. Ideally, on non-debug systems, I'd like to
> see %p's filtered no matter what, I'd think opt in (whitelist) is ok. At least than,
> you did it intentionally. I verified that %p filtering gets us coredumps and kernel
> oops's have addresses.

On a system with panic_on_oops=0 and dmesg_restrict=0, if you want to leak where the
kernel and data are to be able to exploit a memory corruption properly, you can
probably often just trigger the bug, get the addresses from the resulting oops, set up
the addresses in your exploit properly and retry the attack.


> Ideally on user images debug interfaces are closed, but I would like to make rogue %p's
> without opt in go away.


> I did a little research, and /proc/sys/kernel/dmesg_restrict seems to enable or disable this
> ability.
> 
> After setting that to 1, it appears I got the restriction I wanted:
> $ dmesg
> dmesg: klogctl failed: Operation not permitted

Yup, exactly.


> One of my friends who does hypervisor development and is familiar with a few systems
> told me that Apple products just kill %p on production builds from logs.
> 
> I'm kind of torn between the various approaches discussed on this thread.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-10-07 15:15 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-05 18:04 [kernel-hardening] [PATCH] printk: introduce kptr_restrict level 3 william.c.roberts
2016-10-05 18:04 ` william.c.roberts
2016-10-05 19:34 ` [kernel-hardening] " Kees Cook
2016-10-05 19:34   ` Kees Cook
2016-10-06 13:17   ` [kernel-hardening] " Roberts, William C
2016-10-06 13:17     ` Roberts, William C
2016-10-06 15:18   ` [kernel-hardening] " Roberts, William C
2016-10-06 21:04     ` [kernel-hardening] " Kees Cook
2016-10-07 14:19       ` [kernel-hardening] " Roberts, William C
2016-10-07 14:29         ` Jann Horn
2016-10-07 15:05           ` Roberts, William C
2016-10-07 15:15             ` Jann Horn [this message]
2016-10-07 19:12           ` Kees Cook
2016-10-11 18:11             ` Roberts, William C
2016-10-05 20:52 ` [kernel-hardening] " Rasmus Villemoes
2016-10-05 20:52   ` Rasmus Villemoes
2016-10-06 13:23   ` [kernel-hardening] " Roberts, William C
2016-10-06 13:23     ` Roberts, William C
2016-10-06 13:31 ` [kernel-hardening] " Christoph Hellwig
2016-10-06 13:31   ` Christoph Hellwig
2016-10-06 13:47   ` [kernel-hardening] " Roberts, William C
2016-10-06 13:47     ` Roberts, William C
2016-10-06 13:56     ` [kernel-hardening] " Christoph Hellwig
2016-10-06 13:56       ` Christoph Hellwig
2016-10-06 14:59       ` [kernel-hardening] " Roberts, William C
2016-10-06 14:59         ` Roberts, William C
2016-10-06 21:00       ` [kernel-hardening] " Kees Cook
2016-10-06 21:00         ` Kees Cook
2016-10-06 21:19         ` [kernel-hardening] " Joe Perches
2016-10-06 21:19           ` Joe Perches
2016-10-06 21:25           ` [kernel-hardening] " Kees Cook
2016-10-06 21:25             ` Kees Cook
2016-10-07 14:21         ` [kernel-hardening] " Roberts, William C
2016-10-07 14:21           ` Roberts, William C
2016-10-06 14:05     ` [kernel-hardening] " Jann Horn
2016-10-06 14:46       ` Jann Horn
2016-10-07 11:52       ` Jann Horn

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=20161007151541.GC14666@pc.thejh.net \
    --to=jann@thejh.net \
    --cc=keescook@chromium.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=william.c.roberts@intel.com \
    /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.