Message-ID: <CAEjxPJ61OHDxmc2fgBp=hq27OoEhkO+Wwbb+rYAf2F9fM7gdLg@mail.gmail.com>
Date: Thu, 4 Dec 2025 10:43:20 -0500
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Roberto Sassu <roberto.sassu@...weicloud.com>, 
	Bernd Edlinger <bernd.edlinger@...mail.de>, 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 Mon, Dec 1, 2025 at 11:34 AM Eric W. Biederman <ebiederm@...ssion.com> 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?
>
> 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.

Powered by blists - more mailing lists