Messages in this thread | | | Date | Tue, 21 Dec 2021 13:01:29 -0500 | Subject | Re: [PATCH] exec: Make suid_dumpable apply to SUID/SGID binaries irrespective of invoking users | From | 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@redhat.com> writes: > >> On 12/21/21 10:55, Eric W. Biederman wrote: >>> Waiman Long <longman@redhat.com> 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
| |