Re: [PATCH] exec: Make suid_dumpable apply to SUID/SGID binaries irrespective of invoking users

[Waiman Long]

On 12/21/21 12:35, Eric W. Biederman wrote:
> Adding a couple of other people who have expressed opinions on how
> to mitigate this issue in the kernel.
>
> Waiman Long <longman@xxxxxxxxxx> writes:
>
> > On 12/21/21 10:55, Eric W. Biederman wrote:
> > > Waiman Long <longman@xxxxxxxxxx> writes:
> > >
> > > > The begin_new_exec() function checks for SUID or SGID binaries by
> > > > comparing effective uid and gid against real uid and gid and using
> > > > the suid_dumpable sysctl parameter setting only if either one of them
> > > > differs.
> > > >
> > > > In the special case that the uid and/or gid of the SUID/SGID binaries
> > > > matches the id's of the user invoking it, the suid_dumpable is not
> > > > used and SUID_DUMP_USER will be used instead. The documentation for the
> > > > suid_dumpable sysctl parameter does not include that exception and so
> > > > this will be an undocumented behavior.
> > > >
> > > > Eliminate this undocumented behavior by adding a flag in the linux_binprm
> > > > structure to designate a SUID/SGID binary and use it for determining
> > > > if the suid_dumpable setting should be applied or not.
> > > I see that you are making the code match the documentation.
> > > What harm/problems does this mismatch cause in practice?
> > > What is the motivation for this change?
> > >
> > > I am trying to see the motivation but all I can see is that
> > > in the case where suid and sgid do nothing in practice the code
> > > does not change dumpable.  The point of dumpable is to refuse to
> > > core dump when it is not safe.  In this case since nothing happened
> > > in practice it is safe.
> > >
> > > So how does this matter in practice.  If there isn't a good
> > > motivation my feel is that it is the documentation that needs to be
> > > updated rather than the code.
> > >
> > > There are a lot of warts to the suid/sgid handling during exec.  This
> > > just doesn't look like one of them
> > This patch is a minor mitigation in response to the security
> > vulnerability as posted in
> > https://www.openwall.com/lists/oss-security/2021/10/20/2 (aka
> > CVE-2021-3864). In particular, the Su PoC (tested on CentOS 7) showing
> > that the su invokes /usr/sbin/unix_chkpwd which is also a SUID
> > binary. The initial su invocation won't generate a core dump because
> > the real uid and euid differs, but the second unix_chkpwd invocation
> > will. This patch eliminates this hole by making sure that all SUID
> > binaries follow suid_dumpable setting.
> All that is required to take advantage of this vulnerability is
> for an suid program to exec something that will coredump.  That
> exec resets the dumpability.
>
> While the example exploit is execing a suid program it is not required
> that the exec'd program be suid.
>
> This makes your proposed change is not a particularly effective mitigation.

Yes, I am aware of that. That is why I said it is just a minor 
mitigation. This patch was inspired after investigating this problem, 
but I do think it is good to make the code consistent with the 
documentation. Of course, we can go either way. I prefer my approach to 
use a flag to indicate a suid binary instead of just comparing ruid and 
euid.


> The best idea I have seen to mitigate this from the kernel side is:
>
> 1) set RLIMIT_CORE to 0 during an suid exec
> 2) update do_coredump to honor an rlimit of 0 for pipes
>
> Anecdotally this should not effect the common systems that pipe
> coredumps into programs as those programs are reported to honor
> RLIMIT_CORE of 0.  This needs to be verified.
>
> If those programs do honor RLIMIT_CORE of 0 we won't have any user
> visible changes if they never see coredumps from a program with a
> RLIMIT_CORE of 0.
>
>
> I have been meaning to audit userspace and see if the common coredump
> catchers truly honor an RLIMIT_CORE of 0.  Unfortunately I have not
> found time to do that yet.

Default RLIMIT_CORE to 0 will likely mitigate this vulnerability. 
However, there are still some userspace impacts as existing behavior 
will be modified. For instance, we may need to modify su to restore a 
proper value for RLIMIT_CORE after successful authentication.

Cheers,
Longman