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

Groups > linux.kernel > #1700455

Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook

Show all headers | View raw

On Wed, Jul 19, 2017 at 9:53 PM, Andy Lutomirski <luto@kernel.org> wrote:
> On Tue, Jul 18, 2017 at 6:10 PM, Andy Lutomirski <luto@kernel.org> wrote:
>> On Tue, Jul 18, 2017 at 3:25 PM, Kees Cook <keescook@chromium.org> wrote:
>>> The commoncap implementation of the bprm_secureexec hook is the only LSM
>>> that depends on the final call to its bprm_set_creds hook (since it may
>>> be called for multiple files, it ignores bprm->called_set_creds). As a
>>> result, it cannot safely _clear_ bprm->secureexec since other LSMs may
>>> have set it.  Instead, remove the bprm_secureexec hook by introducing a
>>> new flag to bprm specific to commoncap: cap_elevated. This is similar to
>>> cap_effective, but that is used for a specific subset of elevated
>>> privileges, and exists solely to track state from bprm_set_creds to
>>> bprm_secureexec. As such, it will be removed in the next patch.
>>>
>>> Here, set the new bprm->cap_elevated flag when setuid/setgid has happened
>>> from bprm_fill_uid() or fscapabilities have been prepared. This temporarily
>>> moves the bprm_secureexec hook to a static inline. The helper will be
>>> removed in the next patch; this makes the step easier to review and bisect,
>>> since this does not introduce any changes to inputs nor outputs to the
>>> "elevated privileges" calculation.
>>>
>>> The new flag is merged with the bprm->secureexec flag in setup_new_exec()
>>> since this marks the end of any further prepare_binprm() calls.
>>
>> Reviewed-by: Andy Lutomirski <luto@kernel.org>
>>
>> with the redundant caveat that...
>>
>>> --- a/fs/exec.c
>>> +++ b/fs/exec.c
>>> @@ -1330,6 +1330,13 @@ EXPORT_SYMBOL(would_dump);
>>>
>>>  void setup_new_exec(struct linux_binprm * bprm)
>>>  {
>>> +       /*
>>> +        * Once here, prepare_binrpm() will not be called any more, so
>>> +        * the final state of setuid/setgid/fscaps can be merged into the
>>> +        * secureexec flag.
>>> +        */
>>> +       bprm->secureexec |= bprm->cap_elevated;
>>> +
>>
>> ...the weird placement of the other assignments to bprm->secureexec
>> makes this exceedingly confusing.
>
> Can you just put the bprm->secureexec |=
> security_bprm_secureexec(bprm); assignment in prepare_binprm() right
> after security_bprm_set_creds()? This would make patch 1 make sense
> and make this make sense too, I think.  Or is there some reason why it
> wouldn't work?  If the latter, I think the patch descriptions and
> comments should maybe be fixed up.

Yeah, I'll make this change for the next version. It makes things a
little less ugly in the series. In this version I was trying to focus
on eliminating the LSM hook instead of first moving it (to
setup_new_exec()) and then moving it a second time (to the
bprm_set_creds() hook).

Have you had a chance to review the later consolidation patches? So
far no one else has reviewed those. (David, any chance you have some
time too?) I'd love to get at least some Reviewed-bys for them...

-Kees

-- 
Kees Cook
Pixel Security

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[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 Kees Cook <keescook@chromium.org> - 2017-08-01 00:50 +0200
          Re: [PATCH v3 06/15] commoncap: Refactor to remove bprm_secureexec hook Andy Lutomirski <luto@kernel.org> - 2017-08-01 15:20 +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

csiph-web