Message-ID:
 <GV2PPF74270EBEE90CDCD964F69E806EF58E4D9A@GV2PPF74270EBEE.EURP195.PROD.OUTLOOK.COM>
Date: Wed, 3 Dec 2025 14:16:29 +0100
From: Bernd Edlinger <bernd.edlinger@...mail.de>
To: "Eric W. Biederman" <ebiederm@...ssion.com>,
 Roberto Sassu <roberto.sassu@...weicloud.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
 Alexey Dobriyan <adobriyan@...il.com>, Oleg Nesterov <oleg@...hat.com>,
 Kees Cook <kees@...nel.org>, Andy Lutomirski <luto@...capital.net>,
 Will Drewry <wad@...omium.org>, Christian Brauner <brauner@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>, Michal Hocko <mhocko@...e.com>,
 Serge Hallyn <serge@...lyn.com>, James Morris
 <jamorris@...ux.microsoft.com>, Randy Dunlap <rdunlap@...radead.org>,
 Suren Baghdasaryan <surenb@...gle.com>, Yafang Shao <laoar.shao@...il.com>,
 Helge Deller <deller@....de>, Adrian Reber <areber@...hat.com>,
 Thomas Gleixner <tglx@...utronix.de>, Jens Axboe <axboe@...nel.dk>,
 Alexei Starovoitov <ast@...nel.org>,
 "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
 linux-security-module@...r.kernel.org, tiozhang <tiozhang@...iglobal.com>,
 Luis Chamberlain <mcgrof@...nel.org>,
 "Paulo Alcantara (SUSE)" <pc@...guebit.com>,
 Sergey Senozhatsky <senozhatsky@...omium.org>,
 Frederic Weisbecker <frederic@...nel.org>, YueHaibing
 <yuehaibing@...wei.com>, Paul Moore <paul@...l-moore.com>,
 Aleksa Sarai <cyphar@...har.com>, Stefan Roesch <shr@...kernel.io>,
 Chao Yu <chao@...nel.org>, xu xin <xu.xin16@....com.cn>,
 Jeff Layton <jlayton@...nel.org>, Jan Kara <jack@...e.cz>,
 David Hildenbrand <david@...hat.com>, Dave Chinner <dchinner@...hat.com>,
 Shuah Khan <shuah@...nel.org>, Elena Reshetova <elena.reshetova@...el.com>,
 David Windsor <dwindsor@...il.com>, Mateusz Guzik <mjguzik@...il.com>,
 Ard Biesheuvel <ardb@...nel.org>,
 "Joel Fernandes (Google)" <joel@...lfernandes.org>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Hans Liljestrand <ishkamiel@...il.com>,
 Penglei Jiang <superman.xpt@...il.com>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Adrian Ratiu <adrian.ratiu@...labora.com>, Ingo Molnar <mingo@...nel.org>,
 "Peter Zijlstra (Intel)" <peterz@...radead.org>,
 Cyrill Gorcunov <gorcunov@...il.com>, Eric Dumazet <edumazet@...gle.com>,
 zohar@...ux.ibm.com, linux-integrity@...r.kernel.org,
 Ryan Lee <ryan.lee@...onical.com>, apparmor <apparmor@...ts.ubuntu.com>
Subject: Re: Are setuid shell scripts safe? (Implied by
 security_bprm_creds_for_exec)

On 12/1/25 19:53, Eric W. Biederman wrote:
> Roberto Sassu <roberto.sassu@...weicloud.com> writes:
> 
>> On Mon, 2025-12-01 at 10:06 -0600, Eric W. Biederman wrote:
>>> Roberto Sassu <roberto.sassu@...weicloud.com> 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/20251008113503.2433343-1-roberto.sassu@huaweicloud.com/
>>>
>>> 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?
>>
>> We intentionally changed the behavior of CREDS_CHECK to be invoked only
>> for the final file. We are monitoring for bug reports, if we receive
>> complains from people that the patch breaks their expectation we will
>> revisit the issue.
>>
>> Any LSM implementing bprm_check_security looking for brpm->cred would
>> be affected by recalculating the DAC credentials for the final binary.
>>
>>> 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.
>>
>> Uhm, I can be wrong, but most LSMs calculate their state change in
>> bprm_creds_for_exec (git grep bprm_creds_for_exec|grep LSM_HOOK_INIT).
>>
>>> If the patch you posted for review works that helps sort that mess out.
>>
>> Well, it works because we changed the expectation :)
> 
> I just haven't seen that code land in Linus's tree yet so I am a bit
> cautious in adopting that.  It is definitely needed as the behavior
> of IMA as v6.18 simply does not work in general.
> 
>>>> 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.
>>
>> If I'm interpreting this behavior correctly (please any LSM maintainer
>> could comment on it), the intent is just to transition to a different
>> security context where a different set of rules could apply (since we
>> are executing a script).
>>
>> Imagine if for every script, the security transition is based on the
>> interpreter, it would be hard to differentiate between scripts and
>> associate to the respective processes different security labels.
>>
>>> 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.
>>
>> Definitely I lack a lot of context...
> 
> From the usenet comp.unix.faq that was probably updated in 1994:
>     http://www.faqs.org/faqs/unix-faq/faq/part4/section-7.html
> 
> I have been trying to remember enough details by looking it up, but the
> short version is that one of the big problems is there is a race between
> the kernel doing it's thing and the shell opening the shell script.
> 
> Clever people have been able to take advantage of that race and insert
> arbitrary code in that window for the shell to execute.  All you have to
> do is google for how to find a reproducer if the one in the link above
> is not enough.
> 
>>> 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.
>>
>> Yes, it would be really nice to fix it!
> 
> After 30 years I really don't expect that is even a reasonable request.
> 
> I think we are solidly into "Don't do that then", and the LSM security
> hooks are definitely doing that.
> 
> There is the partial solution of passing /dev/fd instead of passing the
> name of the script.  I suspect that would break things.  I don't
> remember why that was never adopted.
> 
> I think even with the TOCTOU race fixed there were other serious issues.
> 
> I really think it behooves any security module people who want to use
> the shell script as the basis of their security decisions to research
> all of the old well known issues and describe how they don't apply.
> 
> All I have energy for is to point out it is broken as is and to start
> moving code down into bprm_creds_from_file to avoid the race.
> 
> Right now as far as I can tell anything based upon the script itself
> is worthless junk so changing that would not be breaking anything that
> wasn't already broken.
> 
>>> 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.
>>
>> A simple example for SELinux. Suppose that the parent process has type
>> initrc_t, then the SELinux policy configures the following transitions
>> based on the label of the first file executed (sesearch -T -s initrc_t
>> -c process):
>>
>> type_transition initrc_t NetworkManager_dispatcher_exec_t:process NetworkManager_dispatcher_t;
>> type_transition initrc_t NetworkManager_exec_t:process NetworkManager_t;
>> type_transition initrc_t NetworkManager_initrc_exec_t:process initrc_t;
>> type_transition initrc_t NetworkManager_priv_helper_exec_t:process NetworkManager_priv_helper_t;
>> type_transition initrc_t abrt_dump_oops_exec_t:process abrt_dump_oops_t;
>> type_transition initrc_t abrt_exec_t:process abrt_t;
>> [...]
>>
>> (there are 747 rules in my system).
>>
>> If the transition would be based on the interpreter label, it would be
>> hard to express with rules.
> 
> Which is a problem for the people making the rules engine.  Because
> 30 years of experience with this problem says basing anything on the
> script is already broken.
> 
> I understand the frustration, but it requires a new way of launching
> shell scripts to even begin to make it secure.
> 
>> If the transition does not occur for any reason the parent process
>> policy would still apply, but maybe it would not have the necessary
>> permissions for the execution of the script.
> 
> Yep.
> 
>>> 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?
>>
>> Take this with a looot of salt, if there is a TOCTOU race, the script
>> will be executed with a security context that does not belong to it.
>> But the transition already happened. Not sure if it is safe.
> 
> Historically it hasn't been safe.
> 
>> I also don't know how the TOCTOU race could be solved, but I also would
>> like it to be fixed. I'm available to comment on any proposal!
> 
> I am hoping someone who helped put these security hooks where they are
> will speak up, and tell me what I am missing.
> 
> All I have the energy for right now is to point out security policies
> based upon shell scripts appear to be security policies that only
> protect you from well behaved programs.
> 

Hmm, yes, that looks like an issue.

I would have expected the security engine to look at bprm->filenanme
especially in the case, when bprm->interp != bprm->filename,
and check that it is not a sym-link with write-access for the
current user and of course also that the bprm->file is not a regular file
which is writable by the current user, if that is the case I would have expected
the secuity engine to enforce non-new-privs on a SUID executable somehow.


Bernd.

Powered by blists - more mailing lists