On Mon, Dec 1, 2025 at 11:34 AM Eric W. Biederman <[email protected]> wrote: > > Roberto Sassu <[email protected]> writes: > > > + Mimi, linux-integrity (would be nice if we are in CC when linux- > > security-module is in CC). > > > > Apologies for not answering earlier, it seems I don't receive the > > emails from the linux-security-module mailing list (thanks Serge for > > letting me know!). > > > > I see two main effects of this patch. First, the bprm_check_security > > hook implementations will not see bprm->cred populated. That was a > > problem before we made this patch: > > > > https://patchew.org/linux/[email protected]/ > > Thanks, that is definitely needed. > > Does calling process_measurement(CREDS_CHECK) on only the final file > pass review? Do you know of any cases where that will break things? > > As it stands I don't think it should be assumed that any LSM has > computed it's final creds until bprm_creds_from_file. Not just the > uid and gid. > > If the patch you posted for review works that helps sort that mess out. > > > to work around the problem of not calculating the final DAC credentials > > early enough (well, we actually had to change our CREDS_CHECK hook > > behavior). > > > > The second, I could not check. If I remember well, unlike the > > capability LSM, SELinux/Apparmor/SMACK calculate the final credentials > > based on the first file being executed (thus the script, not the > > interpreter). Is this patch keeping the same behavior despite preparing > > the credentials when the final binary is found? > > The patch I posted was. > > My brain is still reeling from the realization that our security modules > have the implicit assumption that it is safe to calculate their security > information from shell scripts. > > In the first half of the 90's I remember there was lots of effort to try > and make setuid shell scripts and setuid perl scripts work, and the > final conclusion was it was a lost cause. > > Now I look at security_bprm_creds_for_exec and security_bprm_check which > both have the implicit assumption that it is indeed safe to compute the > credentials from a shell script. > > When passing a file descriptor to execat we have > BINPRM_FLAGS_PATH_INACCESSIBLE and use /dev/fd/NNN as the filename > which reduces some of the races. > > However when just plain executing a shell script we pass the filename of > the shell script as a command line argument, and expect the shell to > open the filename again. This has been a time of check to time of use > race for decades, and one of the reasons we don't have setuid shell > scripts. > > Yet the IMA implementation (without the above mentioned patch) assumes > the final creds will be calculated before security_bprm_check is called, > and security_bprm_creds_for_exec busily calculate the final creds. > > For some of the security modules I believe anyone can set any label they > want on a file and they remain secure (At which point I don't understand > the point of having labels on files). I don't believe that is the case > for selinux, or in general. > > So just to remove the TOCTOU race the security_bprm_creds_for_exec > and security_bprm_check hooks need to be removed, after moving their > code into something like security_bprm_creds_from_file. > > Or am I missing something and even with the TOCTOU race are setuid shell > scripts somehow safe now?
setuid shell scripts are not safe. But SELinux (and likely AppArmor and others) have long relied on the ability to transition on shell scripts to _shed_ permissions. That's a matter of writing your policy sensibly. Changing it would break existing userspace and policies.
