Re: EVM: Permission denied with overlayfs

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

On Wed, 2018-12-19 at 18:38 +0200, Amir Goldstein wrote:
> On Wed, Dec 19, 2018 at 5:39 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2018-12-18 at 18:00 -0500, Mimi Zohar wrote:
> > > Hi Ignaz,
> > >
> > > On Tue, 2018-12-18 at 20:49 +0100, Ignaz Forster wrote:
> > > > Hi,
> > > >
> > > > as a follow up to my attempts to use overlayfs on an IMA protected
> > > > system[1] I've now tried to also enable EVM. From what I understand this
> > > > should - at least in theory - be possible: EVM will call
> > > > d_backing_inode(dentry), which I thought would get the inode of the
> > > > underlying file system[2], and use that for HMAC verification.
> > > >
> > > > In practice simply trying to access an existing file will fail with
> > > > "Permission denied" already. In the corresponding audit log I can see
> > > > the file access (failed with "invalid-HMAC"), but with an inode number
> > > > unknown to me - stat returns a completely different number for the file
> > > > in the lower and target dir.
> > > >
> > > > For testing purposes I added a new hashing algorithm to
> > > > evm_ima_xattr_type which will not add the file system specific
> > > > attributes (inode number, generation, file system uuid) to the hash -
> > > > just like EVM_XATTR_PORTABLE_DIGSIG, but with the hashes generated by
> > > > the kernel. Files created with this signature can be read correctly,
> > > > though writing the files will still fail.
> > > >
> > > > Unfortunately I'm out of ideas what is happening here. If anybody wants
> > > > to have a look at this: Any help would be appreciated.
> > > >
> > > > Kind Regards,
> > > > Ignaz
> > > >
> > > > [1] https://www.spinics.net/lists/linux-integrity/msg03593.html
> > > > [2] https://www.kernel.org/doc/htmldocs/filesystems/API-d-backing-inode.html
> > > >
> > >
> > > After creating a file on the overlay, I wasn't able to access it from
> > > the overlay, but was able to access it from "upper".  Both "stat" and
> > > "getfattr -m ^security" returned exactly the same things for both
> > > pathnames.  However, the ino in the audit log was different.
> > >
> > > After modifying evm_calc_hmac_or_hash(), replacing d_backing_inode()
> > > with d_real_inode(), the hmac properly calculated for both the overlay
> > > and the upper pathnames.
> > >
> > > Something must have changed in d_backing_inode().
> >
> > Confirmed, in linux-4.18.y d_backing_inode returns the real i_ino, but
> > newer kernels do not.  This is a problem for EVM as the i_ino is
> > included in the HMAC calculation.
> >
> 
> Hi Mimi,
> 
> v4.19 has a big change that removes many VFS hacks in favor of
> overlayfs stacked file operations.
> 
> The major implication for VFS code is that file_inode(file) is now the overlayfs
> inode and not the real inode. Therefore, file_dentry(file) is also the overlayfs
> dentry and not the real dentry.
> 
> I am not sure how that change impacts EVM ?
> FWIW, d_backing_inode(dentry) was never anything more than d_inode(dentry).
> 
> Can you please try to describe in more details for someone who has no clue what
> EVM does how exactly the v4.19 change is manifested in the EVM use case.

IMA calculates and stores a file hash/signature on the file data
(security.ima).  EVM calculates and stores an HMAC/signature on the
file metadata (security.evm).  Some data needs to be included in the
HMAC/signature that binds the file metadata with the file data.  That
data is the inode's ino, generation, uid, gid, mode and the uuid.

> 
> AFAIKT, evm_calc_hmac_or_hash() would get the overlayfs dentry both in
> v4.18 and v4.19 and therefore d_backing_inode(dentry) should be the
> overlayfs inode in both kernels (?).
> 
> The value of overlayfs inode i_ino can be identical to i_ino of the real inode
> under some conditions, but far from always and the value of overlayfs inode
> i_generation is almost guaranteed to not match that of the real inode.
> 
> Ignaz, can you add some more debug prints to shed some light on what
> exactly has changed, between the two kernels?
> If the calculated hashes do not match in two different execution paths,
> please provide two sample stack traces that see different i_ino, so we can
> examine them.

Assuming you've created and overlay mounted the lower, upper, work,
and merged directories, accessing files only in the merged directory
fails.

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 4f9126ebfbf4..d0ffa08d4b23 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -193,6 +193,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 				 uint8_t type, struct evm_digest *data)
 {
 	struct inode *inode = d_backing_inode(dentry);
+	struct inode *inode1 = d_real_inode(dentry);
 	struct xattr_list *xattr;
 	struct shash_desc *desc;
 	size_t xattr_size = 0;
@@ -241,6 +242,9 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		if (is_ima)
 			ima_present = true;
 	}
+	if (inode != inode1)
+		pr_info("ino: %lu %lu %lu %s\n", inode->i_ino, inode1->i_ino,
+				dentry->d_inode->i_ino, dentry->d_name.name);
 	hmac_add_misc(desc, inode, type, data->digest);
 
 	/* Portable EVM signatures must include an IMA hash */
-- 

Mimi