On Fri, Apr 11, 2025 at 5:07 AM Roberto Sassu
<roberto.sa...@huaweicloud.com> wrote:
> On Thu, 2025-04-10 at 17:24 +0000, ser...@kernel.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().I'm still trying to make sure I understand everything here, so I've got a few questions: * How important is it for IMA to vet the intermediate binaries? Those binaries don't actually do anything with the program/scripts, right? * Based on the comment block at the top of begin_new_exec(), I'm assuming that using the security_bprm_creds_from_file() hook would be a problem due to challenges in returning an error code? There might also be an issue for any LSMs that run *before* capabilities, but I think that would only be Lockdown in the default case so likely not a big problem. * This patch has been out for almost five years and presumably offers a performance bump when doing an exec; I'm skeptical that Eric, Linus, or anyone outside of security/ would be interested in doing a revert to better support the AT_EXECVE_CHECK for a LSM. Yes, I might be wrong, but for a moment let's assume a revert is not an option, what would you propose to solve this? If you can't think of a general solution, can you think of an IMA specific solution? -- paul-moore.com
