Re: [kernel-hardening] [PATCH 2/2] security,perf: Allow further restriction of perf_event_open

[Arnaldo Carvalho de Melo]

Em Fri, Jun 17, 2016 at 12:16:47PM -0400, Daniel Micay escreveu:
> On Fri, 2016-06-17 at 08:54 +0200, Peter Zijlstra wrote:
> > This Changelog is completely devoid of information. _WHY_ are you
> > doing this?
 
> Attack surface reduction. It's possible to use seccomp-bpf for some
> limited cases, but it's not flexible enough. There are lots of
> information leaks and local privilege escalation vulnerabilities via
> perf events, yet on most Linux installs it's not ever being used. So
> turning it off by default on those installs is an easy win. The holes
> are reduced to root -> kernel (and that's not a meaningful boundary in
> mainline right now - although as is the case here, Debian has a bunch of
> securelevel patches for that).

Is ptrace also disabled on such systems, or any of the other more recent
syscalls? The same arguments could probably be used to disable those:
reduce attack surface, possibly the new ones have bugs as they are
relatively new and it takes a long time for new syscalls to be more
generally used, if we go on disabling them in such a way, they will
probably never get used :-\

Wouldn't the recent bump in perf_event_paranoid to 2 enough? I.e. only
allow profiling of user tasks?

Or is there something more specific that we should disable/constrain to
reduce such surface contact without using such a big hammer?

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] [PATCH 2/2] security,perf: Allow further restriction of perf_event_open

[Daniel Micay]

On Fri, 2016-06-17 at 08:54 +0200, Peter Zijlstra wrote:
> On Thu, Jun 16, 2016 at 03:27:55PM -0700, Kees Cook wrote:
> > Hi guys,
> > 
> > This patch wasn't originally CCed to you (I'm fixing that now).
> > Would
> > you consider taking this into the perf tree? 
> 
> No.
> 
> > It's been in active use
> > in both Debian and Android for a while now.
> 
> Very nice of you all to finally inform us I suppose :/

It was in Debian a lot longer than Android, although the Android feature
came from a downstream variant where it was done much earlier:

https://android-review.googlesource.com/#/c/233736/

> > > > > 
> > > > > access to performance events by users without CAP_SYS_ADMIN.
> > > > > Add a Kconfig symbol CONFIG_SECURITY_PERF_EVENTS_RESTRICT that
> > > > > makes this value the default.
> > > > > 
> > > > > This is based on a similar feature in grsecurity
> > > > > (CONFIG_GRKERNSEC_PERF_HARDEN).  This version doesn't include
> > > > > making
> > > > > the variable read-only.  It also allows enabling further
> > > > > restriction
> > > > > at run-time regardless of whether the default is changed.
> 
> This Changelog is completely devoid of information. _WHY_ are you
> doing
> this?

Attack surface reduction. It's possible to use seccomp-bpf for some
limited cases, but it's not flexible enough. There are lots of
information leaks and local privilege escalation vulnerabilities via
perf events, yet on most Linux installs it's not ever being used. So
turning it off by default on those installs is an easy win. The holes
are reduced to root -> kernel (and that's not a meaningful boundary in
mainline right now - although as is the case here, Debian has a bunch of
securelevel patches for that).

signature.asc
Description: This is a digitally signed message part

Re: [kernel-hardening] [PATCH 2/2] security,perf: Allow further restriction of perf_event_open

[Peter Zijlstra]

On Thu, Jun 16, 2016 at 03:27:55PM -0700, Kees Cook wrote:
> Hi guys,
> 
> This patch wasn't originally CCed to you (I'm fixing that now). Would
> you consider taking this into the perf tree? 

No.

> It's been in active use
> in both Debian and Android for a while now.

Very nice of you all to finally inform us I suppose :/

> >>> When kernel.perf_event_open is set to 3 (or greater), disallow all
> >>> access to performance events by users without CAP_SYS_ADMIN.
> >>> Add a Kconfig symbol CONFIG_SECURITY_PERF_EVENTS_RESTRICT that
> >>> makes this value the default.
> >>>
> >>> This is based on a similar feature in grsecurity
> >>> (CONFIG_GRKERNSEC_PERF_HARDEN).  This version doesn't include making
> >>> the variable read-only.  It also allows enabling further restriction
> >>> at run-time regardless of whether the default is changed.

This Changelog is completely devoid of information. _WHY_ are you doing
this?

Also, hate the CONFIG.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kernel-hardening] [PATCH 2/2] security,perf: Allow further restriction of perf_event_open

[Kees Cook]

Hi guys,

This patch wasn't originally CCed to you (I'm fixing that now). Would
you consider taking this into the perf tree? It's been in active use
in both Debian and Android for a while now.

(If need be, I can resend it.)

Thanks!

-Kees

On Sat, Jun 4, 2016 at 1:49 PM, Jeffrey Vander Stoep  wrote:
> Acked-by: Jeff Vander Stoep 
>
> In addition to Debian, this patch has been merged into AOSP and is a
> requirement for Android:
> https://android-review.googlesource.com/#/q/topic:CONFIG_SECURITY_PERF_EVENTS_RESTRICT
>
>
> On Wed, Apr 13, 2016 at 9:12 AM, Kees Cook  wrote:
>> On Mon, Jan 11, 2016 at 7:23 AM, Ben Hutchings 
>> wrote:
>>> When kernel.perf_event_open is set to 3 (or greater), disallow all
>>> access to performance events by users without CAP_SYS_ADMIN.
>>> Add a Kconfig symbol CONFIG_SECURITY_PERF_EVENTS_RESTRICT that
>>> makes this value the default.
>>>
>>> This is based on a similar feature in grsecurity
>>> (CONFIG_GRKERNSEC_PERF_HARDEN).  This version doesn't include making
>>> the variable read-only.  It also allows enabling further restriction
>>> at run-time regardless of whether the default is changed.
>>>
>>> Signed-off-by: Ben Hutchings 
>>
>> Whoops, I entirely missed this email! Just found it now.
>>
>> Ben, can you resend this with Perf maintainers in CC? This seems
>> sensible enough to me.
>>
>> -Kees
>>
>>> ---
>>> I made a similar change to Debian's kernel packages in August,
>>> including the more restrictive default, and no-one has complained yet.
>>>
>>> Ben.
>>>
>>>  Documentation/sysctl/kernel.txt | 4 +++-
>>>  include/linux/perf_event.h  | 5 +
>>>  kernel/events/core.c| 8 
>>>  security/Kconfig| 9 +
>>>  4 files changed, 25 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/sysctl/kernel.txt
>>> b/Documentation/sysctl/kernel.txt
>>> index 88a2c8e..76e2ca8 100644
>>> --- a/Documentation/sysctl/kernel.txt
>>> +++ b/Documentation/sysctl/kernel.txt
>>> @@ -629,12 +629,14 @@ allowed to execute.
>>>  perf_event_paranoid:
>>>
>>>  Controls use of the performance events system by unprivileged
>>> -users (without CAP_SYS_ADMIN).  The default value is 1.
>>> +users (without CAP_SYS_ADMIN).  The default value is 3 if
>>> +CONFIG_SECURITY_PERF_EVENTS_RESTRICT is set, or 1 otherwise.
>>>
>>>   -1: Allow use of (almost) all events by all users
>>>  >=0: Disallow raw tracepoint access by users without CAP_IOC_LOCK
>>>  >=1: Disallow CPU event access by users without CAP_SYS_ADMIN
>>>  >=2: Disallow kernel profiling by users without CAP_SYS_ADMIN
>>> +>=3: Disallow all event access by users without CAP_SYS_ADMIN
>>>
>>>  ==
>>>
>>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>>> index f9828a4..aa72940 100644
>>> --- a/include/linux/perf_event.h
>>> +++ b/include/linux/perf_event.h
>>> @@ -989,6 +989,11 @@ extern int perf_cpu_time_max_percent_handler(struct
>>> ctl_table *table, int write,
>>> loff_t *ppos);
>>>
>>>
>>> +static inline bool perf_paranoid_any(void)
>>> +{
>>> +   return sysctl_perf_event_paranoid > 2;
>>> +}
>>> +
>>>  static inline bool perf_paranoid_tracepoint_raw(void)
>>>  {
>>> return sysctl_perf_event_paranoid > -1;
>>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>>> index cfc227c..85bc810 100644
>>> --- a/kernel/events/core.c
>>> +++ b/kernel/events/core.c
>>> @@ -175,8 +175,13 @@ static struct srcu_struct pmus_srcu;
>>>   *   0 - disallow raw tracepoint access for unpriv
>>>   *   1 - disallow cpu events for unpriv
>>>   *   2 - disallow kernel profiling for unpriv
>>> + *   3 - disallow all unpriv perf event use
>>>   */
>>> +#ifdef CONFIG_SECURITY_PERF_EVENTS_RESTRICT
>>> +int sysctl_perf_event_paranoid __read_mostly = 3;
>>> +#else
>>>  int sysctl_perf_event_paranoid __read_mostly = 1;
>>> +#endif
>>>
>>>  /* Minimum for 512 kiB + 1 user control page */
>>>  int sysctl_perf_event_mlock __read_mostly = 512 + (PAGE_SIZE / 1024); /*
>>> 'free' kiB per user */
>>> @@ -8265,6 +8270,9 @@ SYSCALL_DEFINE5(perf_event_open,
>>> if (flags & ~PERF_FLAG_ALL)
>>> return -EINVAL;
>>>
>>> +   if (perf_paranoid_any() && !capable(CAP_SYS_ADMIN))
>>> +   return -EACCES;
>>> +
>>> err = perf_copy_attr(attr_uptr, );
>>> if (err)
>>> return err;
>>> diff --git a/security/Kconfig b/security/Kconfig
>>> index e452378..30a2603 100644
>>> --- a/security/Kconfig
>>> +++ b/security/Kconfig
>>> @@ -18,6 +18,15 @@ config SECURITY_DMESG_RESTRICT
>>>
>>>   If you are unsure how to answer this question, answer N.
>>>
>>> +config SECURITY_PERF_EVENTS_RESTRICT
>>> +   bool "Restrict unprivileged use of performance events"
>>> +   depends on PERF_EVENTS
>>> +   help
>>> + If you say Y here, the