linux-kernel - Re: [RFC][PATCH 1/2 v2] proc: Relax /proc/<tid>/timerslack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5jKPu2pwgPdGmRdTad3eLrDPYSkj8u2VT7CCk5zNSexQTw@mail.gmail.com>
Date:	Fri, 15 Jul 2016 11:56:33 -0700
From:	Kees Cook <keescook@...omium.org>
To:	John Stultz <john.stultz@...aro.org>,
	James Morris <jmorris@...ei.org>,
	Casey Schaufler <casey@...aufler-ca.com>
Cc:	Nick Kralevich <nnk@...gle.com>,
	lkml <linux-kernel@...r.kernel.org>,
	"Serge E. Hallyn" <serge@...lyn.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Oren Laadan <orenl@...lrox.com>,
	Ruchi Kandoi <kandoiruchi@...gle.com>,
	Rom Lemarchand <romlem@...roid.com>,
	Todd Kjos <tkjos@...gle.com>, Colin Cross <ccross@...roid.com>,
	Dmitry Shmidt <dimitrysh@...gle.com>,
	Elliott Hughes <enh@...gle.com>,
	Android Kernel Team <kernel-team@...roid.com>,
	LSM List <linux-security-module@...r.kernel.org>,
	SELinux <selinux@...ho.nsa.gov>
Subject: Re: [RFC][PATCH 1/2 v2] proc: Relax /proc/<tid>/timerslack_ns
 capability requirements

On Fri, Jul 15, 2016 at 11:42 AM, John Stultz <john.stultz@...aro.org> wrote:
> On Fri, Jul 15, 2016 at 10:51 AM, Nick Kralevich <nnk@...gle.com> wrote:
>> On Fri, Jul 15, 2016 at 10:24 AM, John Stultz <john.stultz@...aro.org> wrote:
>>> +       if (!capable(CAP_SYS_NICE))
>>> +               return -EPERM;
>>> +
>>
>> Since you're going the LSM route (from your second patch of this
>
> Well, you suggested it, so I sent out an RFC. I'm not married to it yet. :)
>
>
>> series), the capability check above should be moved to the LSM hook in
>> security/commoncap.c.  Only one security call to
>> security_task_settimerslack is needed, which will cover the standard
>> capabilities check as well as the SELinux check.
>
> Huh. Ok. I was looking at the implementation of nice(), which does:
>
>  if (increment < 0 && !can_nice(current, nice))
>                 return -EPERM;
> retval = security_task_setnice(current, nice);
> if (retval)
> ...
>
> Which made it seem like standard checks are done first, then finer
> grain lsm checks second.

I'm on the fence about this: it can be argued that if it's a cap check
it should live in the commoncap.c checks, but most of our cap checks
for these kinds of access controls are directly in the function, prior
the the security_* calls. I've added James and Casey who may have a
more well constructed rationale for doing this one way or the other.

> (...and now you can guess where my accidental "current" usage in the
> next patch came from :)
>
>
>>
>>>         p = get_proc_task(inode);
>>>         if (!p)
>>>                 return -ESRCH;
>>>
>>
>> Per your patch #2, you'd call security_task_settimerslack here. This
>> would call into the capability LSM hook you added in
>> security/commoncap.c
>
> Though I was hoping to keep the CAP_SYS_PTRACE -> CAP_SYS_NICE change
> first, then add the LSM hooks, as it makes the needed ABI change more
> obvious. I worry swapping it around with the LSM hook being added
> first makes it significantly less obvious, as (at least for me) the
> security_task_* functions get indirect and difficult to follow quickly
> ("wait, why are we checking SETSCHED for nice?").
>
> A side curiosity: why does "git grep PROCESS__SETSCHED" miss the
> definition? Is the av_permissions.h file somehow caught by .gitignore?
>
> thanks
> -john

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security