From: Amir Goldstein <amir73il@gmail.com>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: Goldwyn Rodrigues <rgoldwyn@suse.com>,
Miklos Szeredi <miklos@szeredi.hu>,
Christian Brauner <brauner@kernel.org>,
Jeff Layton <jlayton@kernel.org>, Jan Kara <jack@suse.cz>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: Fwd: [PATCH] ima: fix wrong dereferences of file->f_path
Date: Mon, 18 Sep 2023 14:56:01 +0300 [thread overview]
Message-ID: <CAOQ4uxhoasXiBst4Y+7DhkOkvgVZpcRpt2g1EVygRx=R--kv6Q@mail.gmail.com> (raw)
In-Reply-To: <ddd831c35bdcd46c5912b0f34486a36785f17622.camel@linux.ibm.com>
On Mon, Sep 18, 2023 at 2:00 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Fri, 2023-09-15 at 12:57 +0300, Amir Goldstein wrote:
>
> > > Assuming IMA is configured, just add "ima_policy=tcb" to the command
> > > line. This will measure all files executed, mmap'ed, kernel modules,
> > > firmware, and all files opened by root. Normally the builtin policy is
> > > replaced with a finer grained one.
> > >
> > > Below are a few commands, but Ken Goldman is writing documentation -
> > > https://ima-doc.readthedocs.io/en/latest/
> > >
> > > 1. Display the IMA measurement list:
> > > # cat /sys/kernel/security/ima/ascii_runtime_measurements
> > > # cat /sys/kernel/security/ima/binary_runtime_measurements
> > >
> > > 2. Display the IMA policy (or append to the policy)
> > > # cat /sys/kernel/security/ima/policy
> > >
> > > 3. Display number of measurements
> > > # cat /sys/kernel/security/ima/runtime_measurements_count
> > >
> >
> > Nice.
> > This seems to work fine and nothing pops up when running
> > fstests unionmount tests of overlayfs over xfs.
> >
> > What strikes me as strange is that there are measurements
> > of files in xfs and in overlayfs, but no measurements of files in tmpfs.
> > I suppose that is because no one can tamper with the storage
> > of tmpfs, but following the same logic, nobody can tamper with
> > the storage of overlayfs files without tampering with storage of
> > underlying fs (e.g. xfs), so measuring overlayfs files should not
> > bring any extra security to the system.
> >
> > Especially, since if files are signed they are signed in the real
> > storage (e.g. xfs) and not in overlayfs.
> >
> > So in theory, we should never ever measure files in the
> > "virtual" overlayfs and only measure them in the real fs.
> > The only problem is the the IMA hooks when executing,
> > mmaping, reading files from overlayfs, don't work on the real fs.
> >
> > fsnotify also was not working correctly in that respect, because
> > fs operations on overlayfs did not always trigger fsnotify events
> > on the underlying real fs.
> >
> > This was fixed in 6.5 by commit bc2473c90fca ("ovl: enable fsnotify
> > events on underlying real files") and the file_real_path() infrastructure
> > was added to enable this.
> >
> > This is why I say, that in most likelihood, IMA hook should always use
> > file_real_path() and file_dentry() to perform the measurements
> > and record the path of the real fs when overlayfs is performing the
> > actual read/mmap on the real fs and IMA hooks should ideally
> > do nothing at all (as in tmpfs) when the operation is performed
> > on the "virtual" overlayfs object.
>
> tmpfs is excluded from the builtin policy, since there is no way of
> storing the file signature in the initramfs (CPIO). There have been a
> number of attempts at extending the initramfs CPIO format, but none
> have been upstreamed.
>
> Agreed, IMA should always use the real file for both the lower and the
> upper overlayfs.
>
I took a quick look at some IMA security hooks and I think it's not going
to be trivial to fix IMA over overlayfs.
Simply adding a bunch of file_real_path() is not going to solve all cases.
I still think that my patch is correct, but in order to fix the syzbot crash
and other issues, a developer will need to run all the IMA test cases
over overlayfs and examine every case more closely.
If it is acceptable I would recommend to opt-out of IMA
measure/appraise of overlayfs files for the default policy, but that means
that underlying real files will not be measure/appraise as well.
This way we at least shut up syzbot, because we know that this
configuration is broken.
Anyway, syzbot has just confirmed that the regressing commit is
"IMA: use vfs_getattr_nosec to get the i_version"
Thanks,
Amir.
prev parent reply other threads:[~2023-09-18 11:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-13 7:37 [PATCH] ima: fix wrong dereferences of file->f_path Amir Goldstein
2023-09-13 12:09 ` Amir Goldstein
2023-09-14 15:26 ` Mimi Zohar
[not found] ` <CAOQ4uxg2_d2eFfSy45JCCLE41qCPZtLFytnZ5x5C1uXdCMUA=Q@mail.gmail.com>
[not found] ` <4919dcc1066d6952190dc224004e1f6bcba5e9df.camel@linux.ibm.com>
[not found] ` <CAOQ4uxiKgYO5Z25DFG=GQj3GeGZ8unSPExM-jn1HL_U8qncrtA@mail.gmail.com>
[not found] ` <428533f7393ab4a9f5c243b3a61ff65d27ee80be.camel@linux.ibm.com>
2023-09-15 9:57 ` Fwd: " Amir Goldstein
2023-09-15 11:33 ` Mimi Zohar
2023-09-15 13:22 ` Amir Goldstein
2023-09-18 10:36 ` Mimi Zohar
2023-09-18 11:56 ` Amir Goldstein [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAOQ4uxhoasXiBst4Y+7DhkOkvgVZpcRpt2g1EVygRx=R--kv6Q@mail.gmail.com' \
--to=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=jlayton@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-unionfs@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=rgoldwyn@suse.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).