Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1690768 > unrolled thread

[PATCH v3 00/15] exec: Use sane stack rlimit under secureexec

Started byKees Cook <keescook@chromium.org>
First post2017-07-19 00:30 +0200
Last post2017-07-19 07:30 +0200
Articles 20 on this page of 42 — 8 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
    [PATCH v3 05/15] smack: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
      Re: [PATCH v3 05/15] smack: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-26 06:00 +0200
        Re: [PATCH v3 05/15] smack: Refactor to remove bprm_secureexec hook Casey Schaufler <casey@schaufler-ca.com> - 2017-07-26 20:00 +0200
    [PATCH v3 12/15] smack: Remove redundant pdeath_signal clearing Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
    [PATCH v3 14/15] exec: Use sane stack rlimit under secureexec Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
      Re: [PATCH v3 14/15] exec: Use sane stack rlimit under secureexec James Morris <jmorris@namei.org> - 2017-07-19 11:50 +0200
    [PATCH v3 11/15] exec: Use secureexec for clearing pdeath_signal Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
    [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
      Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Andy Lutomirski <luto@kernel.org> - 2017-07-19 03:20 +0200
        Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 06:50 +0200
        Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Andy Lutomirski <luto@kernel.org> - 2017-07-20 07:00 +0200
      Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec  hook James Morris <jmorris@namei.org> - 2017-07-19 11:30 +0200
    [PATCH v3 01/15] binfmt: Introduce secureexec flag Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
      Re: [PATCH v3 01/15] binfmt: Introduce secureexec flag John Johansen <john.johansen@canonical.com> - 2017-07-19 02:10 +0200
      Re: [PATCH v3 01/15] binfmt: Introduce secureexec flag Andy Lutomirski <luto@kernel.org> - 2017-07-19 03:10 +0200
    [PATCH v3 15/15] exec: Consolidate pdeath_signal clearing Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
    [PATCH v3 13/15] exec: Consolidate dumpability logic Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
    [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
      Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds Andy Lutomirski <luto@kernel.org> - 2017-07-19 04:00 +0200
      Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into  bprm_set_creds James Morris <jmorris@namei.org> - 2017-07-19 11:30 +0200
    [PATCH v3 10/15] exec: Use secureexec for setting dumpability Kees Cook <keescook@chromium.org> - 2017-07-19 00:30 +0200
      Re: [PATCH v3 10/15] exec: Use secureexec for setting dumpability Kees Cook <keescook@chromium.org> - 2017-07-26 06:00 +0200
    [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-19 00:40 +0200
      Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 02:10 +0200
        Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 02:20 +0200
          Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-20 03:40 +0200
            Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 15:50 +0200
              Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-20 19:10 +0200
                Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-20 22:50 +0200
                  Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-21 17:50 +0200
                    Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Kees Cook <keescook@chromium.org> - 2017-07-21 19:40 +0200
                      Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook Paul Moore <paul@paul-moore.com> - 2017-07-21 21:20 +0200
    [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Kees Cook <keescook@chromium.org> - 2017-07-19 00:40 +0200
      Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to  called_set_creds John Johansen <john.johansen@canonical.com> - 2017-07-19 02:10 +0200
      Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Andy Lutomirski <luto@kernel.org> - 2017-07-19 03:10 +0200
        Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Kees Cook <keescook@chromium.org> - 2017-07-19 06:50 +0200
      Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to  called_set_creds James Morris <jmorris@namei.org> - 2017-07-19 11:30 +0200
      Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds Paul Moore <paul@paul-moore.com> - 2017-07-20 02:00 +0200
    Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-19 01:10 +0200
    Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec "Serge E. Hallyn" <serge@hallyn.com> - 2017-07-19 05:30 +0200
      Re: [PATCH v3 00/15] exec: Use sane stack rlimit under secureexec Kees Cook <keescook@chromium.org> - 2017-07-19 07:30 +0200

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →


#1691187 — Re: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds

FromJames Morris <jmorris@namei.org>
Date2017-07-19 11:30 +0200
SubjectRe: [PATCH v3 07/15] commoncap: Move cap_elevated calculation into bprm_set_creds
Message-ID<u4TsU-6zb-81@gated-at.bofh.it>
In reply to#1690780
On Tue, 18 Jul 2017, Kees Cook wrote:

> Instead of a separate function, open-code the cap_elevated test, which
> lets us entirely remove bprm->cap_effective (to use the local "effective"
> variable instead), and more accurately examine euid/egid changes via the
> existing local "is_setid".
> 
> Cc: Serge Hallyn <serge@hallyn.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>


Reviewed-by: James Morris <james.l.morris@oracle.com>


-- 
James Morris
<jmorris@namei.org>

[toc] | [prev] | [next] | [standalone]


#1690782 — [PATCH v3 10/15] exec: Use secureexec for setting dumpability

FromKees Cook <keescook@chromium.org>
Date2017-07-19 00:30 +0200
Subject[PATCH v3 10/15] exec: Use secureexec for setting dumpability
Message-ID<u4Jab-8f4-41@gated-at.bofh.it>
In reply to#1690768
The examination of "current" to decide dumpability is wrong. This was a
check of and euid/uid (or egid/gid) mismatch in the existing process,
not the newly created one. This appears to stretch back into even the
"history.git" tree. Luckily, dumpability is later set in commit_creds().
In earlier kernel versions before creds existed, similar checks also
existed late in the exec flow, covering up the mistake as far back as I
could find.

Note that because the commit_creds() check examines differences of euid,
uid, egid, gid, and capabilities between the old and new creds, it would
look like the setup_new_exec() dumpability test could be entirely removed.
However, the secureexec test may cover a different set of tests (specific
to the LSMs) than what commit_creds() checks for. So, fix this test to
use secureexec (the removed euid tests are redundant to the commoncap
secureexec checks now).

Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/exec.c b/fs/exec.c
index f9480d3e0b82..5241c8f25f5d 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1353,7 +1353,7 @@ void setup_new_exec(struct linux_binprm * bprm)
 
 	current->sas_ss_sp = current->sas_ss_size = 0;
 
-	if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
+	if (!bprm->secureexec)
 		set_dumpable(current->mm, SUID_DUMP_USER);
 	else
 		set_dumpable(current->mm, suid_dumpable);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1696763 — Re: [PATCH v3 10/15] exec: Use secureexec for setting dumpability

FromKees Cook <keescook@chromium.org>
Date2017-07-26 06:00 +0200
SubjectRe: [PATCH v3 10/15] exec: Use secureexec for setting dumpability
Message-ID<u7lEm-3wP-7@gated-at.bofh.it>
In reply to#1690782
On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote:
> The examination of "current" to decide dumpability is wrong. This was a
> check of and euid/uid (or egid/gid) mismatch in the existing process,
> not the newly created one. This appears to stretch back into even the
> "history.git" tree. Luckily, dumpability is later set in commit_creds().
> In earlier kernel versions before creds existed, similar checks also
> existed late in the exec flow, covering up the mistake as far back as I
> could find.
>
> Note that because the commit_creds() check examines differences of euid,
> uid, egid, gid, and capabilities between the old and new creds, it would
> look like the setup_new_exec() dumpability test could be entirely removed.
> However, the secureexec test may cover a different set of tests (specific
> to the LSMs) than what commit_creds() checks for. So, fix this test to
> use secureexec (the removed euid tests are redundant to the commoncap
> secureexec checks now).
>
> Cc: David Howells <dhowells@redhat.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>

David (or anyone else), how does this (and the following undiscussed
patches) look? I only have a few unreviewed patches in this series,
and I'd like to get some more eyes on it.

Thanks!

-Kees

> ---
>  fs/exec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index f9480d3e0b82..5241c8f25f5d 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1353,7 +1353,7 @@ void setup_new_exec(struct linux_binprm * bprm)
>
>         current->sas_ss_sp = current->sas_ss_size = 0;
>
> -       if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
> +       if (!bprm->secureexec)
>                 set_dumpable(current->mm, SUID_DUMP_USER);
>         else
>                 set_dumpable(current->mm, suid_dumpable);
> --
> 2.7.4
>



-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1690783 — [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromKees Cook <keescook@chromium.org>
Date2017-07-19 00:40 +0200
Subject[PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u4JjP-8ii-3@gated-at.bofh.it>
In reply to#1690768
The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
hook since it's dealing with the same information, and all of the details
are finalized during the first call to the bprm_set_creds hook via
prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
via bprm->called_set_creds).

Here, the test can just happen at the end of the bprm_set_creds hook,
and the bprm_secureexec hook can be dropped.

Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 security/selinux/hooks.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 0f1450a06b02..18038f73a2f7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
 
 		/* Clear any possibly unsafe personality bits on exec: */
 		bprm->per_clear |= PER_CLEAR_ON_SETID;
-	}
-
-	return 0;
-}
-
-static int selinux_bprm_secureexec(struct linux_binprm *bprm)
-{
-	const struct task_security_struct *tsec = current_security();
-	u32 sid, osid;
-	int atsecure = 0;
-
-	sid = tsec->sid;
-	osid = tsec->osid;
 
-	if (osid != sid) {
 		/* Enable secure mode for SIDs transitions unless
 		   the noatsecure permission is granted between
 		   the two SIDs, i.e. ahp returns 0. */
-		atsecure = avc_has_perm(osid, sid,
-					SECCLASS_PROCESS,
-					PROCESS__NOATSECURE, NULL);
+		rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
+				  SECCLASS_PROCESS, PROCESS__NOATSECURE,
+				  NULL);
+		bprm->secureexec |= !!rc;
 	}
 
-	return !!atsecure;
+	return 0;
 }
 
 static int match_file(const void *p, struct file *file, unsigned fd)
@@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds),
 	LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds),
 	LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds),
-	LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec),
 
 	LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
 	LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1692296 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromPaul Moore <paul@paul-moore.com>
Date2017-07-20 02:10 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u57ct-7FH-7@gated-at.bofh.it>
In reply to#1690783
On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
> hook since it's dealing with the same information, and all of the details
> are finalized during the first call to the bprm_set_creds hook via
> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
> via bprm->called_set_creds).
>
> Here, the test can just happen at the end of the bprm_set_creds hook,
> and the bprm_secureexec hook can be dropped.
>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  security/selinux/hooks.c | 24 +++++-------------------
>  1 file changed, 5 insertions(+), 19 deletions(-)

This seems reasonable in the context of the other changes.

Stephen just posted an AT_SECURE test for the selinux-testsuite on the
SELinux mailing list, it would be nice to ensure that this patchset
doesn't run afoul of that.

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 0f1450a06b02..18038f73a2f7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>
>                 /* Clear any possibly unsafe personality bits on exec: */
>                 bprm->per_clear |= PER_CLEAR_ON_SETID;
> -       }
> -
> -       return 0;
> -}
> -
> -static int selinux_bprm_secureexec(struct linux_binprm *bprm)
> -{
> -       const struct task_security_struct *tsec = current_security();
> -       u32 sid, osid;
> -       int atsecure = 0;
> -
> -       sid = tsec->sid;
> -       osid = tsec->osid;
>
> -       if (osid != sid) {
>                 /* Enable secure mode for SIDs transitions unless
>                    the noatsecure permission is granted between
>                    the two SIDs, i.e. ahp returns 0. */
> -               atsecure = avc_has_perm(osid, sid,
> -                                       SECCLASS_PROCESS,
> -                                       PROCESS__NOATSECURE, NULL);
> +               rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
> +                                 SECCLASS_PROCESS, PROCESS__NOATSECURE,
> +                                 NULL);
> +               bprm->secureexec |= !!rc;
>         }
>
> -       return !!atsecure;
> +       return 0;
>  }
>
>  static int match_file(const void *p, struct file *file, unsigned fd)
> @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>         LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds),
>         LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds),
>         LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds),
> -       LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec),
>
>         LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
>         LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
> --
> 2.7.4

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1692299 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromPaul Moore <paul@paul-moore.com>
Date2017-07-20 02:20 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u57ma-7KQ-1@gated-at.bofh.it>
In reply to#1692296
On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
>> hook since it's dealing with the same information, and all of the details
>> are finalized during the first call to the bprm_set_creds hook via
>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
>> via bprm->called_set_creds).
>>
>> Here, the test can just happen at the end of the bprm_set_creds hook,
>> and the bprm_secureexec hook can be dropped.
>>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Cc: Stephen Smalley <sds@tycho.nsa.gov>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  security/selinux/hooks.c | 24 +++++-------------------
>>  1 file changed, 5 insertions(+), 19 deletions(-)
>
> This seems reasonable in the context of the other changes.
>
> Stephen just posted an AT_SECURE test for the selinux-testsuite on the
> SELinux mailing list, it would be nice to ensure that this patchset
> doesn't run afoul of that.

Quick follow-up: I just merged Stephen's test into the test suite:

* https://github.com/SELinuxProject/selinux-testsuite

> Acked-by: Paul Moore <paul@paul-moore.com>
>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 0f1450a06b02..18038f73a2f7 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -2413,30 +2413,17 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>>
>>                 /* Clear any possibly unsafe personality bits on exec: */
>>                 bprm->per_clear |= PER_CLEAR_ON_SETID;
>> -       }
>> -
>> -       return 0;
>> -}
>> -
>> -static int selinux_bprm_secureexec(struct linux_binprm *bprm)
>> -{
>> -       const struct task_security_struct *tsec = current_security();
>> -       u32 sid, osid;
>> -       int atsecure = 0;
>> -
>> -       sid = tsec->sid;
>> -       osid = tsec->osid;
>>
>> -       if (osid != sid) {
>>                 /* Enable secure mode for SIDs transitions unless
>>                    the noatsecure permission is granted between
>>                    the two SIDs, i.e. ahp returns 0. */
>> -               atsecure = avc_has_perm(osid, sid,
>> -                                       SECCLASS_PROCESS,
>> -                                       PROCESS__NOATSECURE, NULL);
>> +               rc = avc_has_perm(old_tsec->sid, new_tsec->sid,
>> +                                 SECCLASS_PROCESS, PROCESS__NOATSECURE,
>> +                                 NULL);
>> +               bprm->secureexec |= !!rc;
>>         }
>>
>> -       return !!atsecure;
>> +       return 0;
>>  }
>>
>>  static int match_file(const void *p, struct file *file, unsigned fd)
>> @@ -6151,7 +6138,6 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
>>         LSM_HOOK_INIT(bprm_set_creds, selinux_bprm_set_creds),
>>         LSM_HOOK_INIT(bprm_committing_creds, selinux_bprm_committing_creds),
>>         LSM_HOOK_INIT(bprm_committed_creds, selinux_bprm_committed_creds),
>> -       LSM_HOOK_INIT(bprm_secureexec, selinux_bprm_secureexec),
>>
>>         LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
>>         LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),
>> --
>> 2.7.4
>
> --
> paul moore
> www.paul-moore.com



-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1692334 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromKees Cook <keescook@chromium.org>
Date2017-07-20 03:40 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u58BA-51-1@gated-at.bofh.it>
In reply to#1692299
On Wed, Jul 19, 2017 at 5:19 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
>>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
>>> hook since it's dealing with the same information, and all of the details
>>> are finalized during the first call to the bprm_set_creds hook via
>>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
>>> via bprm->called_set_creds).
>>>
>>> Here, the test can just happen at the end of the bprm_set_creds hook,
>>> and the bprm_secureexec hook can be dropped.
>>>
>>> Cc: Paul Moore <paul@paul-moore.com>
>>> Cc: Stephen Smalley <sds@tycho.nsa.gov>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>>  security/selinux/hooks.c | 24 +++++-------------------
>>>  1 file changed, 5 insertions(+), 19 deletions(-)
>>
>> This seems reasonable in the context of the other changes.
>>
>> Stephen just posted an AT_SECURE test for the selinux-testsuite on the
>> SELinux mailing list, it would be nice to ensure that this patchset
>> doesn't run afoul of that.
>
> Quick follow-up: I just merged Stephen's test into the test suite:
>
> * https://github.com/SELinuxProject/selinux-testsuite

Is there a quick how-to on just running the AT_SECURE test?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1692948 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromPaul Moore <paul@paul-moore.com>
Date2017-07-20 15:50 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u5k02-8cN-27@gated-at.bofh.it>
In reply to#1692334
On Wed, Jul 19, 2017 at 9:37 PM, Kees Cook <keescook@chromium.org> wrote:
> On Wed, Jul 19, 2017 at 5:19 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Wed, Jul 19, 2017 at 8:03 PM, Paul Moore <paul@paul-moore.com> wrote:
>>> On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> The SELinux bprm_secureexec hook can be merged with the bprm_set_creds
>>>> hook since it's dealing with the same information, and all of the details
>>>> are finalized during the first call to the bprm_set_creds hook via
>>>> prepare_binprm() (subsequent calls due to binfmt_script, etc, are ignored
>>>> via bprm->called_set_creds).
>>>>
>>>> Here, the test can just happen at the end of the bprm_set_creds hook,
>>>> and the bprm_secureexec hook can be dropped.
>>>>
>>>> Cc: Paul Moore <paul@paul-moore.com>
>>>> Cc: Stephen Smalley <sds@tycho.nsa.gov>
>>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>>> ---
>>>>  security/selinux/hooks.c | 24 +++++-------------------
>>>>  1 file changed, 5 insertions(+), 19 deletions(-)
>>>
>>> This seems reasonable in the context of the other changes.
>>>
>>> Stephen just posted an AT_SECURE test for the selinux-testsuite on the
>>> SELinux mailing list, it would be nice to ensure that this patchset
>>> doesn't run afoul of that.
>>
>> Quick follow-up: I just merged Stephen's test into the test suite:
>>
>> * https://github.com/SELinuxProject/selinux-testsuite
>
> Is there a quick how-to on just running the AT_SECURE test?

You'll need a functional SELinux system to start, I run it against
Fedora Rawhide regularly* with various development kernels, but recent
stable Fedora releases should work too.  Occasionally I hear of people
running it on Debian, but I haven't had a Debian SELinux system in
some time so I can't say for certain everything is 100% working there.
Once you've gotten a working system in enforcing mode, read the README
file in the test suite to install the necessary dependencies (look in
the "Userland and Base Policy" section), then build the tests/policy
(you should be able to skip this step, as the make dependencies will
handle it, but it is nice to do it separately to make sure you have
the build dependencies sorted):

  # make

... load the test policy

  # make -C policy load

... run the tests:

  # cd tests/atsecure
  # ./test

... optionally uninstall the test policy:

  # make -C policy unload

In some ways it is easier to just run the entire test suite:

  # make
  # make test

Alternatively, if you've got a fairly recent git repo with all the
patches merged I can build a test kernel and give it a shot for you,
although fair warning it may take a day or two for me to get to it.

* It is worth noting that the current 4.13-rcX releases have two bugs
that affect the selinux-testsuite.  The worst is a kernel panic due to
a bug in overlayfs' xattr code, there is a patch available to fix it,
but as of yesterday it hadn't yet hit Linus tree (I can dig it up if
you need it).  The second issue related to IPsec and getting peer
label information over UDP connections, I haven't had a chance to sort
that out yet, but at least it isn't a kernel panic.

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1693099 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromKees Cook <keescook@chromium.org>
Date2017-07-20 19:10 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u5n7A-1TV-7@gated-at.bofh.it>
In reply to#1692948
On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote:
> Alternatively, if you've got a fairly recent git repo with all the
> patches merged I can build a test kernel and give it a shot for you,
> although fair warning it may take a day or two for me to get to it.

Hurm, I think this will take quite a bit of time for me to set up. :P
If you have a chance, I'd appreciate it if you could test the series.
It's currently based on v4.12:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook

If it doesn't work out or takes too much time I can work on setting up
the test environment next week (travelling at the moment).

Thanks for the details!

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1693226 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromPaul Moore <paul@paul-moore.com>
Date2017-07-20 22:50 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u5qyu-3Q5-9@gated-at.bofh.it>
In reply to#1693099
On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote:
> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote:
>> Alternatively, if you've got a fairly recent git repo with all the
>> patches merged I can build a test kernel and give it a shot for you,
>> although fair warning it may take a day or two for me to get to it.
>
> Hurm, I think this will take quite a bit of time for me to set up. :P
> If you have a chance, I'd appreciate it if you could test the series.
> It's currently based on v4.12:
> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook
>
> If it doesn't work out or takes too much time I can work on setting up
> the test environment next week (travelling at the moment).

Building a kernel now, in case anyone on Fedora wants to play with it,
you can find it here (when it finishes):

* https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1693807 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromPaul Moore <paul@paul-moore.com>
Date2017-07-21 17:50 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u5IlH-6Ek-15@gated-at.bofh.it>
In reply to#1693226
On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote:
>>> Alternatively, if you've got a fairly recent git repo with all the
>>> patches merged I can build a test kernel and give it a shot for you,
>>> although fair warning it may take a day or two for me to get to it.
>>
>> Hurm, I think this will take quite a bit of time for me to set up. :P
>> If you have a chance, I'd appreciate it if you could test the series.
>> It's currently based on v4.12:
>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook
>>
>> If it doesn't work out or takes too much time I can work on setting up
>> the test environment next week (travelling at the moment).
>
> Building a kernel now, in case anyone on Fedora wants to play with it,
> you can find it here (when it finishes):
>
> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947

Quick follow up, the kernel above passes the selinux-testsuite atsecure test.

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1693902 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromKees Cook <keescook@chromium.org>
Date2017-07-21 19:40 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u5K49-7JS-3@gated-at.bofh.it>
In reply to#1693807
On Fri, Jul 21, 2017 at 8:40 AM, Paul Moore <paul@paul-moore.com> wrote:
> On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote:
>>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote:
>>>> Alternatively, if you've got a fairly recent git repo with all the
>>>> patches merged I can build a test kernel and give it a shot for you,
>>>> although fair warning it may take a day or two for me to get to it.
>>>
>>> Hurm, I think this will take quite a bit of time for me to set up. :P
>>> If you have a chance, I'd appreciate it if you could test the series.
>>> It's currently based on v4.12:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook
>>>
>>> If it doesn't work out or takes too much time I can work on setting up
>>> the test environment next week (travelling at the moment).
>>
>> Building a kernel now, in case anyone on Fedora wants to play with it,
>> you can find it here (when it finishes):
>>
>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947
>
> Quick follow up, the kernel above passes the selinux-testsuite atsecure test.

Awesome, thanks for taking the time to test it. :) Can I add your Tested-by?

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1693932 — Re: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook

FromPaul Moore <paul@paul-moore.com>
Date2017-07-21 21:20 +0200
SubjectRe: [PATCH v3 04/15] selinux: Refactor to remove bprm_secureexec hook
Message-ID<u5LCV-lm-9@gated-at.bofh.it>
In reply to#1693902
On Fri, Jul 21, 2017 at 1:37 PM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Jul 21, 2017 at 8:40 AM, Paul Moore <paul@paul-moore.com> wrote:
>> On Thu, Jul 20, 2017 at 4:42 PM, Paul Moore <paul@paul-moore.com> wrote:
>>> On Thu, Jul 20, 2017 at 1:06 PM, Kees Cook <keescook@chromium.org> wrote:
>>>> On Thu, Jul 20, 2017 at 6:42 AM, Paul Moore <paul@paul-moore.com> wrote:
>>>>> Alternatively, if you've got a fairly recent git repo with all the
>>>>> patches merged I can build a test kernel and give it a shot for you,
>>>>> although fair warning it may take a day or two for me to get to it.
>>>>
>>>> Hurm, I think this will take quite a bit of time for me to set up. :P
>>>> If you have a chance, I'd appreciate it if you could test the series.
>>>> It's currently based on v4.12:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=kspp/setuid-rlimits/secureexec-no-hook
>>>>
>>>> If it doesn't work out or takes too much time I can work on setting up
>>>> the test environment next week (travelling at the moment).
>>>
>>> Building a kernel now, in case anyone on Fedora wants to play with it,
>>> you can find it here (when it finishes):
>>>
>>> * https://copr.fedorainfracloud.org/coprs/pcmoore/kernel-testing/build/581947
>>
>> Quick follow up, the kernel above passes the selinux-testsuite atsecure test.
>
> Awesome, thanks for taking the time to test it. :) Can I add your Tested-by?

Sorry, I should have included that, here ya go:

Tested-by: Paul Moore <paul@paul-moore.com>

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1690787 — [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds

FromKees Cook <keescook@chromium.org>
Date2017-07-19 00:40 +0200
Subject[PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds
Message-ID<u4JjQ-8ii-19@gated-at.bofh.it>
In reply to#1690768
The cred_prepared bprm flag has a misleading name. It has nothing to do
with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
been called. Rename this flag and improve its comment.

Cc: David Howells <dhowells@redhat.com>
Cc: John Johansen <john.johansen@canonical.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <james.l.morris@oracle.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/binfmt_flat.c           | 2 +-
 fs/exec.c                  | 2 +-
 include/linux/binfmts.h    | 8 ++++++--
 security/apparmor/domain.c | 2 +-
 security/selinux/hooks.c   | 2 +-
 security/smack/smack_lsm.c | 2 +-
 security/tomoyo/tomoyo.c   | 2 +-
 7 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 2edcefc0a294..a722530cc468 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
 	 * as we're past the point of no return and are dealing with shared
 	 * libraries.
 	 */
-	bprm.cred_prepared = 1;
+	bprm.called_set_creds = 1;
 
 	res = prepare_binprm(&bprm);
 
diff --git a/fs/exec.c b/fs/exec.c
index 904199086490..925c85a45d97 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1547,7 +1547,7 @@ int prepare_binprm(struct linux_binprm *bprm)
 	retval = security_bprm_set_creds(bprm);
 	if (retval)
 		return retval;
-	bprm->cred_prepared = 1;
+	bprm->called_set_creds = 1;
 
 	memset(bprm->buf, 0, BINPRM_BUF_SIZE);
 	return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 9508b5f83c7e..36be5a67517a 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -25,8 +25,12 @@ struct linux_binprm {
 	struct mm_struct *mm;
 	unsigned long p; /* current top of mem */
 	unsigned int
-		cred_prepared:1,/* true if creds already prepared (multiple
-				 * preps happen for interpreters) */
+		/*
+		 * True after the bprm_set_creds hook has been called once
+		 * (multiple calls can be made via prepare_binprm() for
+		 * binfmt_script/misc).
+		 */
+		called_set_creds:1,
 		cap_effective:1,/* true if has elevated effective capabilities,
 				 * false if not; except for init which inherits
 				 * its parent's caps anyway */
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 001e133a3c8c..878407e023e3 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -350,7 +350,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	const char *name = NULL, *info = NULL;
 	int error = 0;
 
-	if (bprm->cred_prepared)
+	if (bprm->called_set_creds)
 		return 0;
 
 	ctx = cred_ctx(bprm->cred);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 819fd6858b49..0f1450a06b02 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2327,7 +2327,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
 
 	/* SELinux context only depends on initial program or script and not
 	 * the script interpreter */
-	if (bprm->cred_prepared)
+	if (bprm->called_set_creds)
 		return 0;
 
 	old_tsec = current_security();
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 658f5d8c7e76..7d4b2e221124 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -917,7 +917,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
 	struct superblock_smack *sbsp;
 	int rc;
 
-	if (bprm->cred_prepared)
+	if (bprm->called_set_creds)
 		return 0;
 
 	isp = inode->i_security;
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index 130b4fa4f65f..d25b705360e0 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -76,7 +76,7 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
 	 * Do only if this function is called for the first time of an execve
 	 * operation.
 	 */
-	if (bprm->cred_prepared)
+	if (bprm->called_set_creds)
 		return 0;
 #ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
 	/*
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]


#1690837 — Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds

FromJohn Johansen <john.johansen@canonical.com>
Date2017-07-19 02:10 +0200
SubjectRe: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds
Message-ID<u4KIV-PC-9@gated-at.bofh.it>
In reply to#1690787
On 07/18/2017 03:25 PM, Kees Cook wrote:
> The cred_prepared bprm flag has a misleading name. It has nothing to do
> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
> been called. Rename this flag and improve its comment.
> 
> Cc: David Howells <dhowells@redhat.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>

looks good

Acked-by: John Johansen <john.johansen@canonical.com>

> ---
>  fs/binfmt_flat.c           | 2 +-
>  fs/exec.c                  | 2 +-
>  include/linux/binfmts.h    | 8 ++++++--
>  security/apparmor/domain.c | 2 +-
>  security/selinux/hooks.c   | 2 +-
>  security/smack/smack_lsm.c | 2 +-
>  security/tomoyo/tomoyo.c   | 2 +-
>  7 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 2edcefc0a294..a722530cc468 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
>  	 * as we're past the point of no return and are dealing with shared
>  	 * libraries.
>  	 */
> -	bprm.cred_prepared = 1;
> +	bprm.called_set_creds = 1;
>  
>  	res = prepare_binprm(&bprm);
>  
> diff --git a/fs/exec.c b/fs/exec.c
> index 904199086490..925c85a45d97 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1547,7 +1547,7 @@ int prepare_binprm(struct linux_binprm *bprm)
>  	retval = security_bprm_set_creds(bprm);
>  	if (retval)
>  		return retval;
> -	bprm->cred_prepared = 1;
> +	bprm->called_set_creds = 1;
>  
>  	memset(bprm->buf, 0, BINPRM_BUF_SIZE);
>  	return kernel_read(bprm->file, 0, bprm->buf, BINPRM_BUF_SIZE);
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index 9508b5f83c7e..36be5a67517a 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -25,8 +25,12 @@ struct linux_binprm {
>  	struct mm_struct *mm;
>  	unsigned long p; /* current top of mem */
>  	unsigned int
> -		cred_prepared:1,/* true if creds already prepared (multiple
> -				 * preps happen for interpreters) */
> +		/*
> +		 * True after the bprm_set_creds hook has been called once
> +		 * (multiple calls can be made via prepare_binprm() for
> +		 * binfmt_script/misc).
> +		 */
> +		called_set_creds:1,
>  		cap_effective:1,/* true if has elevated effective capabilities,
>  				 * false if not; except for init which inherits
>  				 * its parent's caps anyway */
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 001e133a3c8c..878407e023e3 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -350,7 +350,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>  	const char *name = NULL, *info = NULL;
>  	int error = 0;
>  
> -	if (bprm->cred_prepared)
> +	if (bprm->called_set_creds)
>  		return 0;
>  
>  	ctx = cred_ctx(bprm->cred);
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 819fd6858b49..0f1450a06b02 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2327,7 +2327,7 @@ static int selinux_bprm_set_creds(struct linux_binprm *bprm)
>  
>  	/* SELinux context only depends on initial program or script and not
>  	 * the script interpreter */
> -	if (bprm->cred_prepared)
> +	if (bprm->called_set_creds)
>  		return 0;
>  
>  	old_tsec = current_security();
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 658f5d8c7e76..7d4b2e221124 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -917,7 +917,7 @@ static int smack_bprm_set_creds(struct linux_binprm *bprm)
>  	struct superblock_smack *sbsp;
>  	int rc;
>  
> -	if (bprm->cred_prepared)
> +	if (bprm->called_set_creds)
>  		return 0;
>  
>  	isp = inode->i_security;
> diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
> index 130b4fa4f65f..d25b705360e0 100644
> --- a/security/tomoyo/tomoyo.c
> +++ b/security/tomoyo/tomoyo.c
> @@ -76,7 +76,7 @@ static int tomoyo_bprm_set_creds(struct linux_binprm *bprm)
>  	 * Do only if this function is called for the first time of an execve
>  	 * operation.
>  	 */
> -	if (bprm->cred_prepared)
> +	if (bprm->called_set_creds)
>  		return 0;
>  #ifndef CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
>  	/*
> 

[toc] | [prev] | [next] | [standalone]


#1690863 — Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds

FromAndy Lutomirski <luto@kernel.org>
Date2017-07-19 03:10 +0200
SubjectRe: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds
Message-ID<u4LEZ-1qm-9@gated-at.bofh.it>
In reply to#1690787
On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote:
> The cred_prepared bprm flag has a misleading name. It has nothing to do
> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
> been called. Rename this flag and improve its comment.
>
> Cc: David Howells <dhowells@redhat.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  fs/binfmt_flat.c           | 2 +-
>  fs/exec.c                  | 2 +-
>  include/linux/binfmts.h    | 8 ++++++--
>  security/apparmor/domain.c | 2 +-
>  security/selinux/hooks.c   | 2 +-
>  security/smack/smack_lsm.c | 2 +-
>  security/tomoyo/tomoyo.c   | 2 +-
>  7 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 2edcefc0a294..a722530cc468 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
>          * as we're past the point of no return and are dealing with shared
>          * libraries.
>          */
> -       bprm.cred_prepared = 1;
> +       bprm.called_set_creds = 1;

WTF is this?  It's not, strictly speaking, a bug in this patch, but
it's nonsensical.  Is it fixed (presuably deleted) later?

Otherwise looks good.

[toc] | [prev] | [next] | [standalone]


#1690965 — Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds

FromKees Cook <keescook@chromium.org>
Date2017-07-19 06:50 +0200
SubjectRe: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds
Message-ID<u4P5T-3vt-7@gated-at.bofh.it>
In reply to#1690863
On Tue, Jul 18, 2017 at 6:06 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote:
>> The cred_prepared bprm flag has a misleading name. It has nothing to do
>> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
>> been called. Rename this flag and improve its comment.
>>
>> Cc: David Howells <dhowells@redhat.com>
>> Cc: John Johansen <john.johansen@canonical.com>
>> Cc: Paul Moore <paul@paul-moore.com>
>> Cc: Stephen Smalley <sds@tycho.nsa.gov>
>> Cc: Casey Schaufler <casey@schaufler-ca.com>
>> Cc: James Morris <james.l.morris@oracle.com>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  fs/binfmt_flat.c           | 2 +-
>>  fs/exec.c                  | 2 +-
>>  include/linux/binfmts.h    | 8 ++++++--
>>  security/apparmor/domain.c | 2 +-
>>  security/selinux/hooks.c   | 2 +-
>>  security/smack/smack_lsm.c | 2 +-
>>  security/tomoyo/tomoyo.c   | 2 +-
>>  7 files changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
>> index 2edcefc0a294..a722530cc468 100644
>> --- a/fs/binfmt_flat.c
>> +++ b/fs/binfmt_flat.c
>> @@ -885,7 +885,7 @@ static int load_flat_shared_library(int id, struct lib_info *libs)
>>          * as we're past the point of no return and are dealing with shared
>>          * libraries.
>>          */
>> -       bprm.cred_prepared = 1;
>> +       bprm.called_set_creds = 1;
>
> WTF is this?  It's not, strictly speaking, a bug in this patch, but
> it's nonsensical.  Is it fixed (presuably deleted) later?

binfmt_flat looks crazy, but I haven't seen any distros that enable it.

> Otherwise looks good.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1691169 — Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds

FromJames Morris <jmorris@namei.org>
Date2017-07-19 11:30 +0200
SubjectRe: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds
Message-ID<u4TsS-6zb-35@gated-at.bofh.it>
In reply to#1690787
On Tue, 18 Jul 2017, Kees Cook wrote:

> The cred_prepared bprm flag has a misleading name. It has nothing to do
> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
> been called. Rename this flag and improve its comment.
> 
> Cc: David Howells <dhowells@redhat.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>


Acked-by: James Morris <james.l.morris@oracle.com>

-- 

James Morris
<jmorris@namei.org>

[toc] | [prev] | [next] | [standalone]


#1692292 — Re: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds

FromPaul Moore <paul@paul-moore.com>
Date2017-07-20 02:00 +0200
SubjectRe: [PATCH v3 02/15] exec: Rename bprm->cred_prepared to called_set_creds
Message-ID<u572O-7mO-9@gated-at.bofh.it>
In reply to#1690787
On Tue, Jul 18, 2017 at 6:25 PM, Kees Cook <keescook@chromium.org> wrote:
> The cred_prepared bprm flag has a misleading name. It has nothing to do
> with the bprm_prepare_cred hook, and actually tracks if bprm_set_creds has
> been called. Rename this flag and improve its comment.
>
> Cc: David Howells <dhowells@redhat.com>
> Cc: John Johansen <john.johansen@canonical.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Cc: James Morris <james.l.morris@oracle.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  fs/binfmt_flat.c           | 2 +-
>  fs/exec.c                  | 2 +-
>  include/linux/binfmts.h    | 8 ++++++--
>  security/apparmor/domain.c | 2 +-
>  security/selinux/hooks.c   | 2 +-
>  security/smack/smack_lsm.c | 2 +-
>  security/tomoyo/tomoyo.c   | 2 +-
>  7 files changed, 12 insertions(+), 8 deletions(-)

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul moore
www.paul-moore.com

[toc] | [prev] | [next] | [standalone]


#1690802

FromLinus Torvalds <torvalds@linux-foundation.org>
Date2017-07-19 01:10 +0200
Message-ID<u4JMS-fj-13@gated-at.bofh.it>
In reply to#1690768

On Tue, 18 Jul 2017, Kees Cook wrote:
>
> This series has grown... :P

Hmm. It may be bigger, but I like it a lot better. Each step now looks 
fairly obvious and is well documented. 

I don't love the timing of it, but I think I'd be willing to just pull 
this in before rc2 as a "we need to do this sooner or later anyway and 
probably mark much of it for stable" kind of thing.

But it would be really good to get people to look at the series, 
particularly Andy and the LSM folks..

              Linus

[toc] | [prev] | [next] | [standalone]


Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel


csiph-web