Linux-Integrity Archive mirror
 help / color / mirror / Atom feed
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
To: Christian Brauner <brauner@kernel.org>,
	Amir Goldstein <amir73il@gmail.com>,
	 Seth Forshee <sforshee@kernel.org>
Cc: miklos@szeredi.hu, linux-unionfs@vger.kernel.org,
	 linux-kernel@vger.kernel.org, zohar@linux.ibm.com,
	paul@paul-moore.com,  stefanb@linux.ibm.com, jlayton@kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: Re: [RFC][PATCH] overlayfs: Redirect xattr ops on security.evm to security.evm_overlayfs
Date: Mon, 11 Dec 2023 15:56:06 +0100	[thread overview]
Message-ID: <c95b24f27021052209ec6911d2b7e7b20e410f43.camel@huaweicloud.com> (raw)
In-Reply-To: <20231208-tauziehen-zerfetzt-026e7ee800a0@brauner>

On Fri, 2023-12-08 at 23:01 +0100, Christian Brauner wrote:
> On Fri, Dec 08, 2023 at 11:55:19PM +0200, Amir Goldstein wrote:
> > On Fri, Dec 8, 2023 at 7:25 PM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > 
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > 
> > > EVM updates the HMAC in security.evm whenever there is a setxattr or
> > > removexattr operation on one of its protected xattrs (e.g. security.ima).
> > > 
> > > Unfortunately, since overlayfs redirects those xattrs operations on the
> > > lower filesystem, the EVM HMAC cannot be calculated reliably, since lower
> > > inode attributes on which the HMAC is calculated are different from upper
> > > inode attributes (for example i_generation and s_uuid).
> > > 
> > > Although maybe it is possible to align such attributes between the lower
> > > and the upper inode, another idea is to map security.evm to another name
> > > (security.evm_overlayfs)
> > 
> > If we were to accept this solution, this will need to be trusted.overlay.evm
> > to properly support private overlay xattr escaping.
> > 
> > > during an xattr operation, so that it does not
> > > collide with security.evm set by the lower filesystem.
> > 
> > You are using wrong terminology and it is very confusing to me.
> 
> Same.

Argh, sorry...

> > see the overlay mount command has lowerdir= and upperdir=.
> > Seems that you are using lower filesystem to refer to the upper fs
> > and upper filesystem to refer to overlayfs.
> > 
> > > 
> > > Whenever overlayfs wants to set security.evm, it is actually setting
> > > security.evm_overlayfs calculated with the upper inode attributes. The
> > > lower filesystem continues to update security.evm.
> > > 
> > 
> > I understand why that works, but I am having a hard time swallowing
> > the solution, mainly because I feel that there are other issues on the
> > intersection of overlayfs and IMA and I don't feel confident that this
> > addresses them all.

This solution is specifically for the collisions on HMACs, nothing
else. Does not interfere/solve any other problem.

> > If you want to try to convince me, please try to write a complete
> > model of how IMA/EVM works with overlayfs, using the section
> > "Permission model" in Documentation/filesystems/overlayfs.rst
> > as a reference.

Ok, I will try.

I explain first how EVM works in general, and then why EVM does not
work with overlayfs.

EVM gets called before there is a set/removexattr operation, and after,
if that operation is successful. Before the set/removexattr operation
EVM calculates the HMAC on current inode metadata (i_ino, i_generation,
i_uid, i_gid, i_mode, POSIX ACLs, protected xattrs). Finally, it
compares the calculated HMAC with the one in security.evm.

If the verification and the set/removexattr operation are successful,
EVM calculates again the HMAC (in the post hooks) based on the updated
inode metadata, and sets security.evm with the new HMAC.

The problem is the combination of: overlayfs inodes have different
metadata than the lower/upper inodes; overlayfs calls the VFS to
set/remove xattrs.

The first problem basically means the HMAC on lower/upper inodes and
overlayfs ones is different.

The second problem is that one security.evm is not enough. We need two,
to store the two different HMACs. And we need both at the same time,
since when overlayfs is mounted the lower/upper directories can be
still accessible.

In the example I described, IMA tries to update security.ima, but this
causes EVM to attempt updating security.evm twice (once after the upper
filesystem performed the setxattr requested by overlayfs, another after
overlayfs performed the setxattr requested by IMA; the latter fails
since EVM does not allow the VFS to directly update the HMAC).

Remapping security.evm to security.evm_overlayfs (now
trusted.overlay.evm) allows us to store both HMACs separately and to
know which one to use.

I just realized that the new xattr name should be public, because EVM
rejects HMAC updates, so we should reject HMAC updates based on the new
xattr name too.

> I want us to go the other way. Make the overlayfs layer completely
> irrelevant for EVM and IMA. See a related discussion here:

Not sure it is possible, as long as overlayfs uses VFS xattr calls.

> Subject: Re: [PATCH 09/16] fs: add vfs_set_fscaps()
> https://lore.kernel.org/r/ZXHZ8uNEg1IK5WMW@do-x1extreme

I will also read this patch, in case I missed something.

Thanks

Roberto


  reply	other threads:[~2023-12-11 14:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-08 17:23 [RFC][PATCH] overlayfs: Redirect xattr ops on security.evm to security.evm_overlayfs Roberto Sassu
2023-12-08 21:55 ` Amir Goldstein
2023-12-08 22:01   ` Christian Brauner
2023-12-11 14:56     ` Roberto Sassu [this message]
2023-12-11 15:36       ` Seth Forshee
2023-12-11 15:41         ` Roberto Sassu
2023-12-11 17:15           ` Seth Forshee
2023-12-11 18:24             ` Amir Goldstein
2023-12-11 18:01       ` Christian Brauner
2023-12-12 10:24         ` Roberto Sassu
2023-12-12 10:44           ` Amir Goldstein
2023-12-12 13:13             ` Roberto Sassu
2023-12-12 15:27               ` Mimi Zohar
2023-12-14 13:42                 ` Roberto Sassu
2023-12-14 15:09                   ` Amir Goldstein
2023-12-14 16:09                     ` Mimi Zohar
2023-12-14 18:06                       ` Amir Goldstein
2023-12-14 19:36                         ` Mimi Zohar
2023-12-12 16:20             ` Roberto Sassu
2023-12-11 18:31       ` Amir Goldstein
2023-12-12 12:41         ` Roberto Sassu

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=c95b24f27021052209ec6911d2b7e7b20e410f43.camel@huaweicloud.com \
    --to=roberto.sassu@huaweicloud.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=paul@paul-moore.com \
    --cc=roberto.sassu@huawei.com \
    --cc=sforshee@kernel.org \
    --cc=stefanb@linux.ibm.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).