[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bbc39aec812383f836ad51bc91b013fa8de8a410.camel@huaweicloud.com>
Date: Fri, 11 Apr 2025 11:07:31 +0200
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: sergeh@...nel.org
Cc: Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner
<brauner@...nel.org>, Kees Cook <kees@...nel.org>, Paul Moore
<paul@...l-moore.com>, James Morris <jmorris@...ei.org>, "Serge E. Hallyn"
<serge@...lyn.com>, "Eric W. Biederman" <ebiederm@...ssion.com>,
linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org,
linux-integrity@...r.kernel.org, zohar@...ux.ibm.com
Subject: Re: Credentials not fully initialized before bprm_check LSM hook
On Thu, 2025-04-10 at 17:24 +0000, sergeh@...nel.org wrote:
> On Thu, Apr 10, 2025 at 01:47:07PM +0200, Roberto Sassu wrote:
> > Hi everyone
> >
> > recently I discovered a problem in the implementation of our IMA
> > bprm_check hook, in particular when the policy is matched against the
> > bprm credentials (to be committed later during execve().
> >
> > Before commit 56305aa9b6fab ("exec: Compute file based creds only
> > once"), bprm_fill_uid() was called in prepare_binprm() and filled the
> > euid/egid before calling security_bprm_check(), which in turns calls
> > IMA.
> >
> > After that commit, bprm_fill_uid() was moved to begin_new_exec(), which
> > is when the last interpreter is found.
> >
> > The consequence is that IMA still sees the not yet ready credentials
> > and an IMA rule like:
> >
> > measure func=CREDS_CHECK euid=0
>
> "IMA still sees" at which point exactly?
IMA sees the credentials in bprm->cred prepared with
prepare_bprm_creds(), where the euid/egid are taken from the current
process.
> Do I understand right that the problem is that ima's version of
> security_bprm_creds_for_exec() needs to run after
> bprm_creds_from_file()?
IMA's version of security_bprm_check(). security_bprm_creds_for_exec()
is for checking scripts executed by the interpreters with execveat()
and the AT_EXECVE_CHECK flag.
Uhm, it would not be technically a problem to move the IMA hook later,
but it would miss the intermediate binary search steps, which are
visible with security_bprm_check().
> Given that Eric's commit message said that no bprm handlers use
> the uid, it seems it should be safe to just move that?
Well, we just found one :)
Thanks
Roberto
> > will not be matched for sudo-like applications.
> >
> > It does work however with SELinux, because it computes the transition
> > before IMA in the bprm_creds_for_exec hook.
> >
> > Since IMA needs to be involved for each execution in the chain of
> > interpreters, we cannot move to the bprm_creds_from_file hook.
> >
> > How do we solve this problem? The commit mentioned that it is an
> > optimization, so probably would not be too hard to partially revert it
> > (and keeping what is good).
> >
> > Thanks
> >
> > Roberto
> >
Powered by blists - more mailing lists