On Thu, 2023-12-14 at 17:09 +0200, Amir Goldstein wrote:

On Thu, Dec 14, 2023 at 3:43 PM Roberto Sassu
[off-list ref] wrote:

quoted

On Tue, 2023-12-12 at 10:27 -0500, Mimi Zohar wrote:

quoted

On Tue, 2023-12-12 at 14:13 +0100, Roberto Sassu wrote:

quoted

On 12.12.23 11:44, Amir Goldstein wrote:

quoted

On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu
[off-list ref] wrote:

quoted

On 11.12.23 19:01, Christian Brauner wrote:

quoted

quoted

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.

"Changes to the underlying filesystems while part of a mounted overlay
filesystem are not allowed. If the underlying filesystem is changed, the
behavior of the overlay is undefined, though it will not result in a
crash or deadlock."

https://docs.kernel.org/filesystems/overlayfs.html#changes-to-underlying-filesystems

So I don't know why this would be a problem.

+ Eric Snowberg

Ok, that would reduce the surface of attack. However, when looking at:

       ovl: Always reevaluate the file signature for IMA

       Commit db1d1e8b9867 ("IMA: use vfs_getattr_nosec to get the
i_version")
       partially closed an IMA integrity issue when directly modifying a file
       on the lower filesystem.  If the overlay file is first opened by a
user
       and later the lower backing file is modified by root, but the extended
       attribute is NOT updated, the signature validation succeeds with
the old
       original signature.

Ok, so if the behavior of overlayfs is undefined if the lower backing
file is modified by root, do we need to reevaluate? Or instead would be
better to forbid the write from IMA (legitimate, I think, since the
behavior is documented)? I just saw that we have d_real_inode(), we can
use it to determine if the write should be denied.

There may be several possible legitimate actions in this case, but the
overall concept IMO should be the same as I said about EVM -
overlayfs does not need an IMA signature of its own, because it
can use the IMA signature of the underlying file.

Whether overlayfs reads a file from lower fs or upper fs, it does not
matter, the only thing that matters is that the underlying file content
is attested when needed.

The only incident that requires special attention is copy-up.
This is what the security hooks security_inode_copy_up() and
security_inode_copy_up_xattr() are for.

When a file starts in state "lower" and has security.ima,evm xattrs
then before a user changes the file, it is copied up to upper fs
and suppose that security.ima,evm xattrs are copied as is?

For IMA copying up security.ima is fine.  Other than EVM portable
signatures, security.evm contains filesystem specific metadata.
Copying security.evm up only works if the metadata is the same on both
filesystems.  Currently the i_generation and i_sb->s_uuid are
different.

quoted

quoted

When later the overlayfs file content is read from the upper copy
the security.ima signature should be enough to attest that file content
was not tampered with between going from "lower" to "upper".

security.evm may need to be fixed on copy up, but that should be
easy to do with the security_inode_copy_up_xattr() hook. No?

Writing security.evm requires the existing security.evm to be valid.
After each security xattr in the protected list is modified,
security.evm HMAC needs to be updated.  Perhaps calculating and writing
security.evm could be triggered by security_inode_copy_up_xattr().
Just copying a non-portable EVM signature wouldn't work, or for that
matter copying an EVM HMAC with different filesystem metadata.

There is another problem, when delayed copy is used. The content comes
from one source, metadata from another.

I initially created test-file-lower on the lower directory
(overlayfs/data), before mounting overlayfs. After mount on
overlayfs/mnt:

# getfattr -m - -e hex -d overlayfs/mnt/test-file-lower
# file: overlayfs/mnt/test-file-lower
security.evm=0x02c86ec91a4c0cf024537fd24347b780b90973402e
security.ima=0x0404f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2
security.selinux=0x73797374656d5f753a6f626a6563745f723a756e6c6162656c65645f743a733000

# chcon -t unconfined_t overlayfs/mnt/test-file-lower

After this, IMA creates an empty file in the upper directory
(overlayfs/root/data), and writes security.ima at file close.
Unfortunately, this is what is presented from overlayfs, which is not
in sync with the content.

# getfattr -m - -e hex -d overlayfs/mnt/test-file-lower
# file: overlayfs/mnt/test-file-lower
security.evm=0x021d71e7df78c36745e3b651ce29cb9f47dc301248
security.ima=0x04048855508aade16ec573d21e6a485dfd0a7624085c1a14b5ecdd6485de0c6839a4
security.selinux=0x73797374656d5f753a6f626a6563745f723a756e636f6e66696e65645f743a733000

# sha256sum overlayfs/mnt/test-file-lower
f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2  overlayfs/mnt/test-file-lower

# sha256sum overlayfs/root/data/test-file-lower
8855508aade16ec573d21e6a485dfd0a7624085c1a14b5ecdd6485de0c6839a4  overlayfs/root/data/test-file-lower (upperdir)

We would need to use the lower security.ima until the copy is made, but
at the same time we need to keep the upper valid (with all xattrs) so
that IMA can update the next time overlayfs requests that.

Yap.

As Seth wrote, overlayfs is a combination of upper and lower.
The information that IMA needs should be accessible from either lower
or upper, but sometimes we will need to make the right choice.

The case of security.ima is similar to that of st_blocks -
it is a data-related metadata, so it needs to be taken from the lowerdata inode
(not even the lower inode). See example of getting STATX_BLOCKS
in ovl_getattr().

I would accept a patch that special cases security.ima in ovl_xattr_get()
and gets it from ovl_i_path_lowerdata(), which would need to be
factored out of ovl_path_lowerdata().

I would also accept filtering out security.{ima,evm} from

But I would only accept it if I know that IMA is not trying to write the
security.ima xattr when closing an overlayfs file, only when closing the
real underlying upper file.

I don't see how that would be possible.  As far as I'm aware, the
correlation is between the overlay and the underlying lower/uppper
file, not the other way around.  How could a close on the underlying
file trigger IMA on an overlay file?

I would also expect IMA to filter out security.{ima,evm} xattrs in
security_inode_copy_up_xattr() (i.e. return 1).
and most importantly, a documentation of the model of IMA/EVM
and overlayfs.

Ok.

Mimi