Re: [Fwd: Re: EVM: Permission denied with overlayfs]

[Mimi Zohar <zohar@linux.ibm.com>]

On Thu, 2018-12-20 at 17:56 +0200, Amir Goldstein wrote:
> On Thu, Dec 20, 2018 at 4:20 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2018-12-20 at 14:04 +0200, Amir Goldstein wrote:
> >
> > > > > 2. What does the call stack look like when failing to verify the hash
> > > > > on oo.text?
> > > >
> > > > *** Cat of same file on merged dir fails (ino != 258)
> > > > cat: overlay/merged/abc.text: Permission denied
> > > >
> > > > [  476.770869] evm: ino: 38593 258 38593 abc.text
> > > > [  476.770876] evm: ino: 38593
> > > > [  476.770883] CPU: 3 PID: 3928 Comm: cat Not tainted 4.20.0-rc2+ #1287
> > > > [  476.770887] Hardware name: LENOVO 20BTS1NJ00/20BTS1NJ00, BIOS N14ET48W (1.26 ) 06/11/2018
> > > > [  476.770890] Call Trace:
> > > > [  476.770906]  dump_stack+0x46/0x5b
> > > > [  476.770913]  hmac_add_misc+0x171/0x180
> > > > [  476.770920]  evm_calc_hmac_or_hash+0x1c9/0x280
> > > > [  476.770927]  evm_verify_hmac+0x11f/0x2b0
> > > > [  476.770933]  ? evm_protected_xattr+0x6c/0x90
> > > > [  476.770940]  ima_appraise_measurement+0x83/0x510
> > > > [  476.770948]  process_measurement+0x646/0x6f0
> > > > [  476.770955]  ? selinux_file_open+0xa8/0xc0
> > > > [  476.770961]  ? do_dentry_open+0x25c/0x340
> > > > [  476.770966]  ? open_with_fake_path+0x48/0x70
> > > > [  476.770974]  ? ovl_open_realfile+0x56/0xe0
> > > > [  476.770981]  ima_file_check+0x4a/0x60
> > >
> > > If you let this check return success even though appraisal failed,
> > > you will see that ovl_open_realfile() will end up calling ima_file_check()
> > > again with the "real" file and this check should not fail.
> > > Hence, my suggestion to mark the overlayfs sb with SB_NOIMA.
> >
> > There's all sorts of caching of results involved in both EVM and IMA.
> > Not so easy to modify.  Assuming that I properly removed the caching,
> >  I'm not seeing another call to ima_file_check.
> >
> 
> Mmm right. open_with_fake_path() doesn't call ima_file_check(). My bad.
> 
> My intuition is that all d_backing_inode() calls under security/integrity
> should be converted to d_real_inode(). After all, users of d_backing_inode()
> probably want the lower/upper inode, because that is what the helper
> claims to do. The reason things worked in v4.18 is because
> d_backing_inode(file_dentry(file)) was the real inode.
> 
> My intuition is that this statement is true for other subsystems that
> use d_backing_inode() as well. We could have changed the
> implementation of d_backing_inode() to call d_real_inode()
> to potentially fix all those subsystems with less churn, but it is hard to
> audit all caller to make sure this is correct for all of them.
> In fact, it is probably not correct for code in fs/*.c.
> 
> So the safer approach is to fix one subsystem at a time by converting
> callers to d_real_inode() and in the end get rid of d_backing_inode().
> 
> Mimi, please try if this fixes EVM over overlayfs.

Yes, changing the hmac_add_misc() call in evm_calc_hmac_or_hash()
fixes the problem.

Mimi

> 
> Miklos, do you have other thoughts about how to address this issue?
> 
> Thanks,
> Amir.
>