Re: [RFC][PATCH] overlayfs: Redirect xattr ops on security.evm to security.evm_overlayfs

Roberto Sassu
2023-12-12T12:41:43Z

On 11.12.23 19:31, Amir Goldstein wrote:
> On Mon, Dec 11, 2023 at 4:56 PM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
>>
>> 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.
>>
>
> I understand both of those things.
>
> What I don't understand is WHY EVM needs to work on overlayfs?
> What is the use case?
> What is the threat model?
>
> The purpose of IMA/EVM as far as I understand it is to detect and
> protect against tampering with data/metadata offline. Right?
>
> As Seth correctly wrote, overlayfs is just the composition of existing
> underlying layers.
>
> Noone can tamper with overlayfs without tampering with the underlying
> layers.

Makes sense.

> The correct solution to your problem, and I have tried to say this many
> times, in to completely opt-out of IMA/EVM for overlayfs.
>
> EVM should not store those versions of HMAC for overlayfs and for
> the underlying layers, it should ONLY store a single version for the
> underlying layer.

If we avoid the checks in IMA and EVM for overlayfs, we need the
guarantee that everything passes through overlayfs down, and that there
is no external interference to the lower and upper filesystems (the part
that is used by overlayfs).

Maybe I'm missing something, I looked at this issue only now, and Mimi
knows it much better than me.

Roberto

> Because write() in overlayfs always follows by write() to upper layer
> and setxattr() in overlayfs always follows by setxattr() to upper layer
> IMO write() and setxattr() on overlayfs should by ignored by IMA/EVM
> and only write()/setxattr() on underlying fs should be acted by IMA/EVM
> which AFAIK, happens anyway.
>
> Please let me know if I am missing something,
>
> Thanks,
> Amir.