Message-ID: <87v7iqtcev.fsf_-_@email.froward.int.ebiederm.org>
Date: Mon, 01 Dec 2025 10:06:00 -0600
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Roberto Sassu <roberto.sassu@...weicloud.com>
Cc: 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: Are setuid shell scripts safe? (Implied by
 security_bprm_creds_for_exec)

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?

Eric

Powered by blists - more mailing lists