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
