linux-kernel - Re: [PATCH 2/2] exec: Compute file based creds only once

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 29 May 2020 22:18:15 -0700
From:   Kees Cook <keescook@...omium.org>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     linux-kernel@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>, Jann Horn <jannh@...gle.com>,
        Greg Ungerer <gerg@...ux-m68k.org>,
        Rob Landley <rob@...dley.net>,
        Bernd Edlinger <bernd.edlinger@...mail.de>,
        linux-fsdevel@...r.kernel.org, Al Viro <viro@...IV.linux.org.uk>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        linux-security-module@...r.kernel.org,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Andy Lutomirski <luto@...capital.net>
Subject: Re: [PATCH 2/2] exec: Compute file based creds only once

On Fri, May 29, 2020 at 10:28:41PM -0500, Eric W. Biederman wrote:
> The range-diff winds up being:
> 1:  c9258ef4879b ! 1:  a7868323c263 exec: Add a per bprm->file version of per_clear
>     @@ Commit message
>      
>          History Tree: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>          Fixes: 1bb0fa189c6a ("[PATCH] NX: clean up legacy binary support")
>     +    Reviewed-by: Kees Cook <keescook@...omium.org>
>          Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
>      
>       ## fs/exec.c ##
>     @@ include/linux/lsm_hooks.h
>        *	transitions between security domains).
>        *	The hook must set @bprm->active_secureexec to 1 if AT_SECURE should be set to
>        *	request libc enable secure mode.
>     -+ *	The hook must set @bprm->pf_per_clear to the personality flags that
>     -+ *	should be cleared from current->personality.
>     ++ *	The hook must add to @bprm->pf_per_clear any personality flags that
>     ++ * 	should be cleared from current->personality.
>        *	@bprm contains the linux_binprm structure.
>        *	Return 0 if the hook is successful and permission is granted.
>        * @bprm_check_security:
> 2:  e6f20c69b96e ! 2:  56305aa9b6fa exec: Compute file based creds only once
>     @@ Commit message
>          secureity attribute and derive capabilities from the fact the
>          user had uid 0 has been added.
>      
>     +    Reviewed-by: Kees Cook <keescook@...omium.org>
>          Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
>      
>       ## fs/binfmt_misc.c ##
>     @@ include/linux/lsm_hooks.h
>      + *	between security domains).
>      + *	The hook must set @bprm->secureexec to 1 if AT_SECURE should be set to
>        *	request libc enable secure mode.
>     -- *	The hook must set @bprm->pf_per_clear to the personality flags that
>     -+ *	The hook must set @bprm->per_clear to the personality flags that
>     -  *	should be cleared from current->personality.
>     +- *	The hook must add to @bprm->pf_per_clear any personality flags that
>     ++ *	The hook must add to @bprm->per_clear any personality flags that
>     +  * 	should be cleared from current->personality.
>        *	@bprm contains the linux_binprm structure.
>        *	Return 0 if the hook is successful and permission is granted.

Awesome; thanks!

> > The cap_ambient_invariant_ok() test is needlessly repeated: it doesn't
> > examine securebits, and nonroot_raised_pE appears to have no
> > side-effects.
> >
> > One of those can be dropped, yes?
> 
> That is what it looks like to me.

Okay, cool. I was worried I was missing something in the mess of tiny
helper calls. :)

> I hope that when the dust clears the function can become a
> straightforward implementation of the capability equations.
> We will see.

Yeah, this looks better and better every day! I'm glad you're able to
dig through all of this.

-- 
Kees Cook