| Message ID | 20231208172308.2876481-1-roberto.sassu@huaweicloud.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [RFC] overlayfs: Redirect xattr ops on security.evm to security.evm_overlayfs | expand |
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. 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. 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. Thanks, Amir.
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. > 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. > > 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. I want us to go the other way. Make the overlayfs layer completely irrelevant for EVM and IMA. See a related discussion here: Subject: Re: [PATCH 09/16] fs: add vfs_set_fscaps() https://lore.kernel.org/r/ZXHZ8uNEg1IK5WMW@do-x1extreme Amir, if you have some time I'd appreciate a comment on that.
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
On Mon, Dec 11, 2023 at 03:56:06PM +0100, Roberto Sassu wrote: > 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. I don't know all of the inner workings of overlayfs in detail, but is it not true that whatever metadata an overlayfs mount presents for a given inode is stored in the lower and/or upper filesystem inodes? If the metadata for those inodes is verified with EVM, why is it also necessary to verify the metadata at the overlayfs level? If some overlayfs metadata is currently omitted from the checks on the lower/upper inodes, is there any reason EVM couldn't start including that its checksums? Granted that there could be some backwards compatibility issues, but maybe inclusion of the overlayfs metadata could be opt-in. Thanks, Seth
On Mon, 2023-12-11 at 09:36 -0600, Seth Forshee wrote: > On Mon, Dec 11, 2023 at 03:56:06PM +0100, Roberto Sassu wrote: > > 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. > > I don't know all of the inner workings of overlayfs in detail, but is it > not true that whatever metadata an overlayfs mount presents for a given > inode is stored in the lower and/or upper filesystem inodes? If the > metadata for those inodes is verified with EVM, why is it also necessary > to verify the metadata at the overlayfs level? If some overlayfs > metadata is currently omitted from the checks on the lower/upper inodes, > is there any reason EVM couldn't start including that its checksums? Currently, the metadata where there is a misalignment are: i_generation, s_uuid, (i_ino?). Maybe there is more? If metadata are aligned, there is no need to store two separate HMACs. Thanks Roberto > Granted that there could be some backwards compatibility issues, but > maybe inclusion of the overlayfs metadata could be opt-in. > > Thanks, > Seth
On Mon, Dec 11, 2023 at 04:41:46PM +0100, Roberto Sassu wrote: > On Mon, 2023-12-11 at 09:36 -0600, Seth Forshee wrote: > > On Mon, Dec 11, 2023 at 03:56:06PM +0100, Roberto Sassu wrote: > > > 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. > > > > I don't know all of the inner workings of overlayfs in detail, but is it > > not true that whatever metadata an overlayfs mount presents for a given > > inode is stored in the lower and/or upper filesystem inodes? If the > > metadata for those inodes is verified with EVM, why is it also necessary > > to verify the metadata at the overlayfs level? If some overlayfs > > metadata is currently omitted from the checks on the lower/upper inodes, > > is there any reason EVM couldn't start including that its checksums? > > Currently, the metadata where there is a misalignment are: > i_generation, s_uuid, (i_ino?). Maybe there is more? > > If metadata are aligned, there is no need to store two separate HMACs. I can only think of three possible sources for the metadata overlayfs presents: 1. It comes directly from the underlying filesystems 2. overlayfs synthesizes if from the underlying filesystem data 3. It's purely generated at runtime Are there others? 1 and 2 should be covered by EVM on the underlying filesystems. If 3 is happening then it seems like hashing that data is just confirming that overlayfs consistently generates the same values for that data, and verifying code behavior doesn't seem in-scope for EVM.
> 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. > 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 So I think phrasing it this way is confusiong. All that overlayfs does is to forward that setxattr request to the upper layer. So really the overlayfs layer here is irrelevant? > since EVM does not allow the VFS to directly update the HMAC). Callchains and details, please. I don't understand what you mean. > > 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 won't support any of this going in unless there's a comprehensive description of where this is all supposed to go and there's a comprehensive and coherent story of what EVM and IMA want to achieve for overlayfs or stacking filesystems in general. The past months we've seen a bunch of ductape to taper over this pretty basic question and there's no end in sight apparently. Really, we need a comprehensive solution for both IMA and EVM it seems. And before that is solved we'll not be merging anything of this sort and won't make any impactful uapi changes such as exposing a new security.* xattr.
On Mon, Dec 11, 2023 at 7:15 PM Seth Forshee <sforshee@kernel.org> wrote: > > On Mon, Dec 11, 2023 at 04:41:46PM +0100, Roberto Sassu wrote: > > On Mon, 2023-12-11 at 09:36 -0600, Seth Forshee wrote: > > > On Mon, Dec 11, 2023 at 03:56:06PM +0100, Roberto Sassu wrote: > > > > 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. > > > > > > I don't know all of the inner workings of overlayfs in detail, but is it > > > not true that whatever metadata an overlayfs mount presents for a given > > > inode is stored in the lower and/or upper filesystem inodes? If the > > > metadata for those inodes is verified with EVM, why is it also necessary > > > to verify the metadata at the overlayfs level? If some overlayfs > > > metadata is currently omitted from the checks on the lower/upper inodes, > > > is there any reason EVM couldn't start including that its checksums? > > > > Currently, the metadata where there is a misalignment are: > > i_generation, s_uuid, (i_ino?). Maybe there is more? > > > > If metadata are aligned, there is no need to store two separate HMACs. > > I can only think of three possible sources for the metadata overlayfs > presents: > > 1. It comes directly from the underlying filesystems > 2. overlayfs synthesizes if from the underlying filesystem data > 3. It's purely generated at runtime > > Are there others? 3.b. purely generated and persisted in overlay private xattr but IIRC only s_uuid fits in that category > > 1 and 2 should be covered by EVM on the underlying filesystems. If 3 is > happening then it seems like hashing that data is just confirming that > overlayfs consistently generates the same values for that data, and > verifying code behavior doesn't seem in-scope for EVM. I agree. I don't think that IMA/EVM has a reason to assets overlayfs specific metadata. Thanks, Amir.
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. 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. 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.
On 11.12.23 19:01, Christian Brauner wrote: >> 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. >> 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 > > So I think phrasing it this way is confusiong. All that overlayfs does > is to forward that setxattr request to the upper layer. So really the > overlayfs layer here is irrelevant? I guess you meant (s/?/./). The problem is calling vfs_setxattr()/vfs_removexattr() which makes EVM verify those calls. If overlayfs should be irrelevant, then it should use calls that don't cause EVM to be invoked. Otherwise, from EVM perspective it is equal to do an xattr operation from user space. >> since EVM does not allow the VFS to directly update the HMAC). > > Callchains and details, please. I don't understand what you mean. Originally I wanted to analyze the stacktraces together, then for sake of brevity and (I thought) clarity, I decided to not include them. So, let's go through them. #0 evm_update_evmxattr (dentry=dentry@entry=0x6b6b6ba0, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302o\322", '\314' <repeats 36 times>, "@x:e", xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_crypto.c:358 #1 0x00000000604f011a in evm_inode_post_setxattr (dentry=dentry@entry=0x6b6b6ba0, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742, xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_main.c:753 #2 0x00000000604a36ff in security_inode_post_setxattr (dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39cf0 "security.ima", value=value@entry=0x653a7742, size=size@entry=34, flags=flags@entry=0) at security/security.c:2286 #3 0x0000000060232570 in __vfs_setxattr_noperm (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39cf0 "security.ima", value=value@entry=0x653a7742, size=size@entry=34, flags=flags@entry=0) at fs/xattr.c:239 #4 0x0000000060232778 in __vfs_setxattr_locked (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39cf0 "security.ima", value=0x653a7742, size=size@entry=34, flags=flags@entry=0, delegated_inode=0xe1003b18) at fs/xattr.c:296 #5 0x0000000060232829 in vfs_setxattr (idmap=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39cf0 "security.ima", value=<optimized out>, value@entry=0x653a7742, size=size@entry=34, flags=flags@entry=0) at fs/xattr.c:322 #6 0x00000000603a4440 in ovl_do_setxattr (ofs=0x65366008, ofs=0x65366008, flags=0, size=34, value=0x653a7742, name=0x60b39cf0 "security.ima", dentry=0x6b6b6ba0) at ./include/linux/mount.h:80 #7 ovl_xattr_set (dentry=0x6b6b7dd8, inode=0x6367cbb8, name=0x60b39cf0 "security.ima", value=0x653a7742, size=size@entry=34, flags=flags@entry=0) at fs/overlayfs/xattrs.c:69 #8 0x00000000603a4698 in ovl_other_xattr_set (handler=<optimized out>, idmap=<optimized out>, dentry=<optimized out>, inode=<optimized out>, name=<optimized out>, value=<optimized out>, size=34, flags=0) at fs/overlayfs/xattrs.c:233 #9 0x0000000060231bd6 in __vfs_setxattr (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b7dd8, inode=inode@entry=0x6367cbb8, name=<optimized out>, name@entry=0x60b39cf0 "security.ima", value=value@entry=0x653a7742, size=size@entry=34, flags=0) at fs/xattr.c:201 #10 0x000000006023246e in __vfs_setxattr_noperm (idmap=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39cf0 "security.ima", value=0x653a7742, size=34, flags=flags@entry=0) at fs/xattr.c:235 #11 0x00000000604ed684 in ima_fix_xattr (dentry=dentry@entry=0x6b6b7dd8, iint=iint@entry=0x651d2008) at security/integrity/ima/ima_appraise.c:101 #12 0x00000000604ee4c8 in ima_update_xattr (iint=iint@entry=0x651d2008, file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_appraise.c:624 #13 0x00000000604e32cc in ima_check_last_writer (iint=0x651d2008, inode=inode@entry=0x6367cbb8, file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_main.c:180 #14 0x00000000604e461c in ima_file_free (file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_main.c:204 #15 0x00000000601f3cd8 in __fput (file=0x6a3c76c0) at fs/file_table.c:388 #16 0x00000000601f40c4 in ____fput (work=<optimized out>) at fs/file_table.c:422 #17 0x000000006007d87c in task_work_run () at kernel/task_work.c:180 #18 0x000000006002f294 in resume_user_mode_work (regs=<optimized out>) at ./include/linux/resume_user_mode.h:49 #19 interrupt_end () at arch/um/kernel/process.c:108 #20 0x0000000060049bf8 in userspace (regs=0x632b1360, aux_fp_regs=0xe1000020) at arch/um/os-Linux/skas/process.c:499 #21 0x000000006002ee94 in new_thread_handler () at arch/um/kernel/process.c:136 Here IMA is updating security.ima at file close after a write (frame #12). The overlayfs dentry is 0x6b6b7dd8, the upper dentry is 0x6b6b6ba0. Issuing a setxattr operation to overlayfs causes the latter to call vfs_setxattr() (frame #5). From EVM perspective, this is just a setxattr operation on the upper filesystem, so it processes it normally (we are already past evm_inode_setxattr() which succeeded, now we are in the post method after the operation). What evm_update_evmxattr() does is the following: rc = __vfs_setxattr_noperm(&nop_mnt_idmap, dentry, XATTR_NAME_EVM, &data.hdr.xattr.data[1], SHA1_DIGEST_SIZE + 1, 0); This is still done on the upper dentry: #0 __vfs_setxattr_noperm (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39c34 "security.evm", value=value@entry=0xe1003923, size=size@entry=21, flags=flags@entry=0) at fs/xattr.c:226 #1 0x00000000604f0e06 in evm_update_evmxattr (dentry=dentry@entry=0x6b6b6ba0, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302o\322", '\314' <repeats 36 times>, "@x:e", xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_crypto.c:378 Everything good until this point. EVM just set the HMAC on the upper dentry. Let's see the next call to evm_update_evmxattr(): #0 evm_update_evmxattr (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302o\322", '\314' <repeats 36 times>, "@x:e", xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_crypto.c:358 #1 0x00000000604f011a in evm_inode_post_setxattr (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742, xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_main.c:753 #2 0x00000000604a36ff in security_inode_post_setxattr (dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39cf0 "security.ima", value=value@entry=0x653a7742, size=size@entry=34, flags=flags@entry=0) at security/security.c:2286 #3 0x0000000060232570 in __vfs_setxattr_noperm (idmap=<optimized out>, dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39cf0 "security.ima", value=0x653a7742, size=34, flags=flags@entry=0) at fs/xattr.c:239 #4 0x00000000604ed684 in ima_fix_xattr (dentry=dentry@entry=0x6b6b7dd8, iint=iint@entry=0x651d2008) at security/integrity/ima/ima_appraise.c:101 #5 0x00000000604ee4c8 in ima_update_xattr (iint=iint@entry=0x651d2008, file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_appraise.c:624 #6 0x00000000604e32cc in ima_check_last_writer (iint=0x651d2008, inode=inode@entry=0x6367cbb8, file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_main.c:180 #7 0x00000000604e461c in ima_file_free (file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_main.c:204 #8 0x00000000601f3cd8 in __fput (file=0x6a3c76c0) at fs/file_table.c:388 #9 0x00000000601f40c4 in ____fput (work=<optimized out>) at fs/file_table.c:422 #10 0x000000006007d87c in task_work_run () at kernel/task_work.c:180 #11 0x000000006002f294 in resume_user_mode_work (regs=<optimized out>) at ./include/linux/resume_user_mode.h:49 #12 interrupt_end () at arch/um/kernel/process.c:108 #13 0x0000000060049bf8 in userspace (regs=0x632b1360, aux_fp_regs=0xe1000020) at arch/um/os-Linux/skas/process.c:499 #14 0x000000006002ee94 in new_thread_handler () at arch/um/kernel/process.c:136 #15 0x0000000000000000 in ?? () This is the relationship between the two stacktraces: int __vfs_setxattr_noperm(struct mnt_idmap *idmap, struct dentry *dentry, const char *name, const void *value, size_t size, int flags) { struct inode *inode = dentry->d_inode; int error = -EAGAIN; int issec = !strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN); if (issec) inode->i_flags &= ~S_NOSEC; if (inode->i_opflags & IOP_XATTR) { error = __vfs_setxattr(idmap, dentry, inode, name, value, ====> first stacktrace (frame #9) size, flags); if (!error) { fsnotify_xattr(dentry); security_inode_post_setxattr(dentry, name, value, ====> second stacktrace (frame #2) size, flags); } Now we are back in the post method after the setxattr call issued to overlayfs. EVM treats it no differently from the first setxattr operation on the upper filesystem, and again tries to set security.evm, this time on overlayfs: Breakpoint 2, __vfs_setxattr_noperm (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39c34 "security.evm", value=value@entry=0xe1003b63, size=size@entry=21, flags=flags@entry=0) at fs/xattr.c:226 226 { If we were in the upper filesystem, there would not have been any problem, __vfs_setxattr() does not call evm_inode_setxattr() which fails for security.evm and an HMAC as value. However, we are in the overlayfs, so we pass the setxattr operation through: #0 evm_inode_setxattr (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, xattr_name=xattr_name@entry=0x60b39c34 "security.evm", xattr_value=xattr_value@entry=0xe1003b63, xattr_value_len=xattr_value_len@entry=21) at security/integrity/evm/evm_main.c:571 #1 0x00000000604a344f in security_inode_setxattr (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39c34 "security.evm", value=value@entry=0xe1003b63, size=size@entry=21, flags=flags@entry=0) at security/security.c:2191 #2 0x00000000602326e8 in __vfs_setxattr_locked (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39c34 "security.evm", value=0xe1003b63, size=size@entry=21, flags=flags@entry=0, delegated_inode=0xe1003988) at fs/xattr.c:287 #3 0x0000000060232829 in vfs_setxattr (idmap=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b6ba0, name=name@entry=0x60b39c34 "security.evm", value=<optimized out>, value@entry=0xe1003b63, size=size@entry=21, flags=flags@entry=0) at fs/xattr.c:322 #4 0x00000000603a4440 in ovl_do_setxattr (ofs=0x65366008, ofs=0x65366008, flags=0, size=21, value=0xe1003b63, name=0x60b39c34 "security.evm", dentry=0x6b6b6ba0) at ./include/linux/mount.h:80 #5 ovl_xattr_set (dentry=0x6b6b7dd8, inode=0x6367cbb8, name=0x60b39c34 "security.evm", value=0xe1003b63, size=size@entry=21, flags=flags@entry=0) at fs/overlayfs/xattrs.c:69 #6 0x00000000603a4698 in ovl_other_xattr_set (handler=<optimized out>, idmap=<optimized out>, dentry=<optimized out>, inode=<optimized out>, name=<optimized out>, value=<optimized out>, size=21, flags=0) at fs/overlayfs/xattrs.c:233 #7 0x0000000060231bd6 in __vfs_setxattr (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b7dd8, inode=inode@entry=0x6367cbb8, name=<optimized out>, name@entry=0x60b39c34 "security.evm", value=value@entry=0xe1003b63, size=size@entry=21, flags=0) at fs/xattr.c:201 #8 0x000000006023246e in __vfs_setxattr_noperm (idmap=idmap@entry=0x60ea0250 <nop_mnt_idmap>, dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39c34 "security.evm", value=value@entry=0xe1003b63, size=size@entry=21, flags=flags@entry=0) at fs/xattr.c:235 #9 0x00000000604f0e06 in evm_update_evmxattr (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302o\322", '\314' <repeats 36 times>, "@x:e", xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_crypto.c:378 #10 0x00000000604f011a in evm_inode_post_setxattr (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7742, xattr_value_len=xattr_value_len@entry=34) at security/integrity/evm/evm_main.c:753 #11 0x00000000604a36ff in security_inode_post_setxattr (dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39cf0 "security.ima", value=value@entry=0x653a7742, size=size@entry=34, flags=flags@entry=0) at security/security.c:2286 #12 0x0000000060232570 in __vfs_setxattr_noperm (idmap=<optimized out>, dentry=dentry@entry=0x6b6b7dd8, name=name@entry=0x60b39cf0 "security.ima", value=0x653a7742, size=34, flags=flags@entry=0) at fs/xattr.c:239 #13 0x00000000604ed684 in ima_fix_xattr (dentry=dentry@entry=0x6b6b7dd8, iint=iint@entry=0x651d2008) at security/integrity/ima/ima_appraise.c:101 #14 0x00000000604ee4c8 in ima_update_xattr (iint=iint@entry=0x651d2008, file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_appraise.c:624 #15 0x00000000604e32cc in ima_check_last_writer (iint=0x651d2008, inode=inode@entry=0x6367cbb8, file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_main.c:180 #16 0x00000000604e461c in ima_file_free (file=file@entry=0x6a3c76c0) at security/integrity/ima/ima_main.c:204 #17 0x00000000601f3cd8 in __fput (file=0x6a3c76c0) at fs/file_table.c:388 #18 0x00000000601f40c4 in ____fput (work=<optimized out>) at fs/file_table.c:422 #19 0x000000006007d87c in task_work_run () at kernel/task_work.c:180 #20 0x000000006002f294 in resume_user_mode_work (regs=<optimized out>) at ./include/linux/resume_user_mode.h:49 #21 interrupt_end () at arch/um/kernel/process.c:108 #22 0x0000000060049bf8 in userspace (regs=0x632b1360, aux_fp_regs=0xe1000020) at arch/um/os-Linux/skas/process.c:499 #23 0x000000006002ee94 in new_thread_handler () at arch/um/kernel/process.c:136 #24 0x0000000000000000 in ?? () This is the stacktrace before failing. It should be clear now what is happening. vfs_setxattr() (frame #3) calls security_inode_setxattr() (frame #1), which calls evm_inode_setxattr() (frame #0). Again, the final result is that the written file has a security.evm set by the upper filesystem with the HMAC calculated on the upper inode metadata (setting security.evm from overlayfs failed). So, next time we try to read the file and EVM attempts to verify inode metadata (frame #5), we have: Breakpoint 5, vfs_getxattr (idmap=0x60ea0250 <nop_mnt_idmap>, dentry=0x6b6b6ba0, name=name@entry=0x60b39c34 "security.evm", value=value@entry=0x0, size=size@entry=0) at fs/xattr.c:431 431 { (gdb) bt #0 vfs_getxattr (idmap=0x60ea0250 <nop_mnt_idmap>, dentry=0x6b6b6ba0, name=name@entry=0x60b39c34 "security.evm", value=value@entry=0x0, size=size@entry=0) at fs/xattr.c:431 #1 0x00000000603a479a in ovl_xattr_get (size=0, value=0x0, name=0x60b39c34 "security.evm", inode=<optimized out>, dentry=<optimized out>) at ./include/linux/mount.h:80 #2 ovl_other_xattr_get (handler=<optimized out>, dentry=<optimized out>, inode=<optimized out>, name=0x60b39c34 "security.evm", buffer=0x0, size=0) at fs/overlayfs/xattrs.c:224 #3 0x00000000602329f3 in vfs_getxattr_alloc (idmap=<optimized out>, dentry=dentry@entry=0x6b6b7dd8, name=<optimized out>, name@entry=0x60b39c34 "security.evm", xattr_value=xattr_value@entry=0xe1003858, xattr_size=xattr_size@entry=0, flags=flags@entry=3136) at fs/xattr.c:394 #4 0x00000000604ef2d0 in evm_verify_hmac (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7840 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302", <incomplete sequence \322>, xattr_value_len=xattr_value_len@entry=34, iint=iint@entry=0x651d2008) at security/integrity/evm/evm_main.c:187 #5 0x00000000604ef77a in evm_verifyxattr (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7840, xattr_value_len=xattr_value_len@entry=34, iint=iint@entry=0x651d2008) at security/integrity/evm/evm_main.c:416 #6 0x00000000604ee3f5 in ima_appraise_measurement (func=func@entry=FILE_CHECK, iint=iint@entry=0x651d2008, file=file@entry=0x6a3c6cc0, filename=<optimized out>, xattr_value=0x653a7840, xattr_len=xattr_len@entry=34, modsig=0x0) at security/integrity/ima/ima_appraise.c:522 #7 0x00000000604e410e in process_measurement (file=file@entry=0x6a3c6cc0, cred=<optimized out>, secid=secid@entry=1, buf=buf@entry=0x0, size=size@entry=0, mask=mask@entry=10, func=<optimized out>) at security/integrity/ima/ima_main.c:374 #8 0x00000000604e458d in ima_file_check (file=file@entry=0x6a3c6cc0, mask=<optimized out>) at security/integrity/ima/ima_main.c:557 #9 0x00000000602050ba in do_open (nd=nd@entry=0xe1003c90, file=file@entry=0x6a3c6cc0, op=op@entry=0xe1003dd0) at fs/namei.c:3624 #10 0x000000006020b0e5 in path_openat (nd=nd@entry=0xe1003c90, op=op@entry=0xe1003dd0, flags=flags@entry=65) at fs/namei.c:3779 #11 0x000000006020be8a in do_filp_open (dfd=dfd@entry=-100, pathname=pathname@entry=0x63321100, op=op@entry=0xe1003dd0) at fs/namei.c:3809 #12 0x00000000601eea16 in do_sys_openat2 (dfd=-100, filename=<optimized out>, how=how@entry=0xe1003e20) at fs/open.c:1440 #13 0x00000000601eed8e in do_sys_open (mode=<optimized out>, flags=<optimized out>, filename=<optimized out>, dfd=<optimized out>) at fs/open.c:1455 #14 __do_sys_openat (mode=<optimized out>, flags=<optimized out>, filename=<optimized out>, dfd=<optimized out>) at fs/open.c:1471 #15 __se_sys_openat (dfd=<optimized out>, filename=<optimized out>, flags=<optimized out>, mode=<optimized out>) at fs/open.c:1466 #16 0x0000000060032da0 in handle_syscall (r=r@entry=0x632b1360) at arch/um/kernel/skas/syscall.c:45 #17 0x00000000600494a9 in handle_trap (pid=pid@entry=2974892, regs=regs@entry=0x632b1360, local_using_sysemu=local_using_sysemu@entry=2) at arch/um/os-Linux/skas/process.c:222 #18 0x0000000060049bdf in userspace (regs=0x632b1360, aux_fp_regs=0xe1000020) at arch/um/os-Linux/skas/process.c:477 #19 0x000000006002ee94 in new_thread_handler () at arch/um/kernel/process.c:136 #20 0x0000000000000000 in ?? () Overlayfs is getting security.evm from the upper filesystem (frame #2), but that HMAC was calculated with the upper inode metadata, so when EVM tries to calculate the HMAC on the overlayfs inode metadata, it fails: #0 evm_calc_hmac_or_hash (dentry=dentry@entry=0x6b6b7dd8, req_xattr_name=req_xattr_name@entry=0x60b39cf0 "security.ima", req_xattr_value=req_xattr_value@entry=0x653a7840 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302", <incomplete sequence \322>, req_xattr_value_len=req_xattr_value_len@entry=34, type=type@entry=2 '\002', data=data@entry=0xe1003860) at security/integrity/evm/evm_crypto.c:225 #1 0x00000000604f0cad in evm_calc_hmac (dentry=dentry@entry=0x6b6b7dd8, req_xattr_name=req_xattr_name@entry=0x60b39cf0 "security.ima", req_xattr_value=req_xattr_value@entry=0x653a7840 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302", <incomplete sequence \322>, req_xattr_value_len=req_xattr_value_len@entry=34, data=data@entry=0xe1003860) at security/integrity/evm/evm_crypto.c:310 #2 0x00000000604ef40e in evm_verify_hmac (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7840 "\004\004\362\312\033\266\307\351\a\320m\257\344h~W\237\316v\263~N\223\267`P\"\332R\346\314\302", <incomplete sequence \322>, xattr_value_len=xattr_value_len@entry=34, iint=iint@entry=0x651d2008) at security/integrity/evm/evm_main.c:214 #3 0x00000000604ef77a in evm_verifyxattr (dentry=dentry@entry=0x6b6b7dd8, xattr_name=xattr_name@entry=0x60b39cf0 "security.ima", xattr_value=xattr_value@entry=0x653a7840, xattr_value_len=xattr_value_len@entry=34, iint=iint@entry=0x651d2008) at security/integrity/evm/evm_main.c:416 #4 0x00000000604ee3f5 in ima_appraise_measurement (func=func@entry=FILE_CHECK, iint=iint@entry=0x651d2008, file=file@entry=0x6a3c6cc0, filename=<optimized out>, xattr_value=0x653a7840, xattr_len=xattr_len@entry=34, modsig=0x0) at security/integrity/ima/ima_appraise.c:522 #5 0x00000000604e410e in process_measurement (file=file@entry=0x6a3c6cc0, cred=<optimized out>, secid=secid@entry=1, buf=buf@entry=0x0, size=size@entry=0, mask=mask@entry=10, func=<optimized out>) at security/integrity/ima/ima_main.c:374 #6 0x00000000604e458d in ima_file_check (file=file@entry=0x6a3c6cc0, mask=<optimized out>) at security/integrity/ima/ima_main.c:557 #7 0x00000000602050ba in do_open (nd=nd@entry=0xe1003c90, file=file@entry=0x6a3c6cc0, op=op@entry=0xe1003dd0) at fs/namei.c:3624 #8 0x000000006020b0e5 in path_openat (nd=nd@entry=0xe1003c90, op=op@entry=0xe1003dd0, flags=flags@entry=65) at fs/namei.c:3779 #9 0x000000006020be8a in do_filp_open (dfd=dfd@entry=-100, pathname=pathname@entry=0x63321100, op=op@entry=0xe1003dd0) at fs/namei.c:3809 #10 0x00000000601eea16 in do_sys_openat2 (dfd=-100, filename=<optimized out>, how=how@entry=0xe1003e20) at fs/open.c:1440 #11 0x00000000601eed8e in do_sys_open (mode=<optimized out>, flags=<optimized out>, filename=<optimized out>, dfd=<optimized out>) at fs/open.c:1455 #12 __do_sys_openat (mode=<optimized out>, flags=<optimized out>, filename=<optimized out>, dfd=<optimized out>) at fs/open.c:1471 #13 __se_sys_openat (dfd=<optimized out>, filename=<optimized out>, flags=<optimized out>, mode=<optimized out>) at fs/open.c:1466 #14 0x0000000060032da0 in handle_syscall (r=r@entry=0x632b1360) at arch/um/kernel/skas/syscall.c:45 #15 0x00000000600494a9 in handle_trap (pid=pid@entry=2974892, regs=regs@entry=0x632b1360, local_using_sysemu=local_using_sysemu@entry=2) at arch/um/os-Linux/skas/process.c:222 #16 0x0000000060049bdf in userspace (regs=0x632b1360, aux_fp_regs=0xe1000020) at arch/um/os-Linux/skas/process.c:477 #17 0x000000006002ee94 in new_thread_handler () at arch/um/kernel/process.c:136 #18 0x0000000000000000 in ?? () 275 iint->evm_status = evm_status; (gdb) p evm_status $4 = INTEGRITY_FAIL >> >> 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 won't support any of this going in unless there's a comprehensive > description of where this is all supposed to go and there's a > comprehensive and coherent story of what EVM and IMA want to achieve for > overlayfs or stacking filesystems in general. The past months we've seen > a bunch of ductape to taper over this pretty basic question and there's > no end in sight apparently. > > Really, we need a comprehensive solution for both IMA and EVM it seems. > And before that is solved we'll not be merging anything of this sort and > won't make any impactful uapi changes such as exposing a new security.* > xattr. Fair enough, I'm going to answer to Amir about if we need EVM for overlayfs... Roberto
On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > > On 11.12.23 19:01, Christian Brauner wrote: > >> 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? 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? Thanks, Amir.
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.
On 12.12.23 11:44, Amir Goldstein wrote: > On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu > <roberto.sassu@huaweicloud.com> wrote: >> >> On 11.12.23 19:01, Christian Brauner wrote: >>>> 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? > > 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? It is not yet clear to me. EVM will be seeing the creation of a new file, and for new files setting xattrs is already allowed. Maybe the security_inode_copy_up*() would be useful for IMA/EVM to authorize writes by overlayfs, which would be otherwise denied to the others (according to my solution). Still, would like to hear Mimi's opinion. Thanks Roberto
On Tue, 2023-12-12 at 14:13 +0100, Roberto Sassu wrote: > On 12.12.23 11:44, Amir Goldstein wrote: > > On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu > > <roberto.sassu@huaweicloud.com> wrote: > >> > >> On 11.12.23 19:01, Christian Brauner wrote: > >>>> 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. > > 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. > It is not yet clear to me. EVM will be seeing the creation of a new > file, and for new files setting xattrs is already allowed. > > Maybe the security_inode_copy_up*() would be useful for IMA/EVM to > authorize writes by overlayfs, which would be otherwise denied to the > others (according to my solution). > > Still, would like to hear Mimi's opinion. Thanks Roberto for all your work and analysis. I'm still looking at security_inode_copy_up_xattr(). Mimi
On 12.12.23 11:44, Amir Goldstein wrote: > On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu > <roberto.sassu@huaweicloud.com> wrote: >> >> On 11.12.23 19:01, Christian Brauner wrote: >>>> 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. Just some thoughts... Ok, so we attest the lower/upper file. What about the path the application specified to access that file (just an example)? Not that it particularly matters (we are not protecting it yet), but we are not recording in the IMA measurement list what the application requested/sees. I don't have a good example for inode metadata, but we already started recording them too. Also, I'm thinking about overlayfs-own xattrs. Shouldn't they be protected? If they change during an offline attack, it would change how information are presented by overlayfs (I don't know much, for now). Roberto > 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? > > 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? > > Thanks, > Amir.
On Tue, 2023-12-12 at 10:27 -0500, Mimi Zohar wrote: > On Tue, 2023-12-12 at 14:13 +0100, Roberto Sassu wrote: > > On 12.12.23 11:44, Amir Goldstein wrote: > > > On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu > > > <roberto.sassu@huaweicloud.com> wrote: > > > > > > > > On 11.12.23 19:01, Christian Brauner wrote: > > > > > > 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. > > > > 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. Roberto > > It is not yet clear to me. EVM will be seeing the creation of a new > > file, and for new files setting xattrs is already allowed. > > > > Maybe the security_inode_copy_up*() would be useful for IMA/EVM to > > authorize writes by overlayfs, which would be otherwise denied to the > > others (according to my solution). > > > > Still, would like to hear Mimi's opinion. > > Thanks Roberto for all your work and analysis. I'm still looking at > security_inode_copy_up_xattr(). > > Mimi >
On Thu, Dec 14, 2023 at 3:43 PM Roberto Sassu <roberto.sassu@huaweicloud.com> wrote: > > On Tue, 2023-12-12 at 10:27 -0500, Mimi Zohar wrote: > > On Tue, 2023-12-12 at 14:13 +0100, Roberto Sassu wrote: > > > On 12.12.23 11:44, Amir Goldstein wrote: > > > > On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu > > > > <roberto.sassu@huaweicloud.com> wrote: > > > > > > > > > > On 11.12.23 19:01, Christian Brauner wrote: > > > > > > > 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. > > > > > > 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 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. Thanks, Amir.
On Thu, 2023-12-14 at 17:09 +0200, Amir Goldstein wrote: > On Thu, Dec 14, 2023 at 3:43 PM Roberto Sassu > <roberto.sassu@huaweicloud.com> wrote: > > > > On Tue, 2023-12-12 at 10:27 -0500, Mimi Zohar wrote: > > > On Tue, 2023-12-12 at 14:13 +0100, Roberto Sassu wrote: > > > > On 12.12.23 11:44, Amir Goldstein wrote: > > > > > On Tue, Dec 12, 2023 at 12:25 PM Roberto Sassu > > > > > <roberto.sassu@huaweicloud.com> wrote: > > > > > > > > > > > > On 11.12.23 19:01, Christian Brauner wrote: > > > > > > > > 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. > > > > > > > > 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
> > > 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? > Well, you are right. it cannot. What I meant is that close of overlayfs file should NOT open and read the overlayfs file and recalculate security.ima to store in overlayfs inode because close of overlayfs file will follow a close of the upper file that should recalculate and store security.ima in the upper inode. It is possible that a close of an overlayfs file will update the security state of the overlayfs inode by copying the security state from the upper inode. But then again, I could be misunderstanding the IMA workflows and it could be more complicated than I try to present it. This is the reason that I requested the documentation of how IMA+overlayfs is *expected* to work. Thanks, Amir.
On Thu, 2023-12-14 at 20:06 +0200, Amir Goldstein wrote: > > > > 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? > > > > Well, you are right. it cannot. > > What I meant is that close of overlayfs file should NOT open and read > the overlayfs file and recalculate security.ima to store in overlayfs inode > because close of overlayfs file will follow a close of the upper file that > should recalculate and store security.ima in the upper inode. > > It is possible that a close of an overlayfs file will update the security > state of the overlayfs inode by copying the security state from the > upper inode. Thank you for the explanation. Basically IMA should differentiate between file close on the underlying upper/lower file and the overlay file. Since IMA doesn't define inode_copy_up_xattr, security.ima will be copied up. Re-calculating security.ima on the overlay is unnecessary. > But then again, I could be misunderstanding the IMA workflows > and it could be more complicated than I try to present it. > This is the reason that I requested the documentation of how > IMA+overlayfs is *expected* to work. Ok Mimi
diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c index 383978e4663c..1141d2fa01db 100644 --- a/fs/overlayfs/xattrs.c +++ b/fs/overlayfs/xattrs.c @@ -65,6 +65,9 @@ static int ovl_xattr_set(struct dentry *dentry, struct inode *inode, const char goto out; old_cred = ovl_override_creds(dentry->d_sb); + if (!strcmp(name, XATTR_NAME_EVM)) + name = XATTR_NAME_EVM_OVERLAYFS; + if (value) { err = ovl_do_setxattr(ofs, realdentry, name, value, size, flags); @@ -88,6 +91,9 @@ static int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char const struct cred *old_cred; struct path realpath; + if (!strcmp(name, XATTR_NAME_EVM)) + name = XATTR_NAME_EVM_OVERLAYFS; + ovl_i_path_real(inode, &realpath); old_cred = ovl_override_creds(dentry->d_sb); res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size); @@ -101,6 +107,9 @@ static bool ovl_can_list(struct super_block *sb, const char *s) if (ovl_is_private_xattr(sb, s)) return false; + if (!strcmp(s, XATTR_NAME_EVM_OVERLAYFS)) + return false; + /* List all non-trusted xattrs */ if (strncmp(s, XATTR_TRUSTED_PREFIX, XATTR_TRUSTED_PREFIX_LEN) != 0) return true; diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h index 9463db2dfa9d..93930300f69e 100644 --- a/include/uapi/linux/xattr.h +++ b/include/uapi/linux/xattr.h @@ -51,6 +51,10 @@ #define XATTR_EVM_SUFFIX "evm" #define XATTR_NAME_EVM XATTR_SECURITY_PREFIX XATTR_EVM_SUFFIX +#define XATTR_EVM_OVERLAYFS_SUFFIX "evm_overlayfs" +#define XATTR_NAME_EVM_OVERLAYFS \ + XATTR_SECURITY_PREFIX XATTR_EVM_OVERLAYFS_SUFFIX + #define XATTR_IMA_SUFFIX "ima" #define XATTR_NAME_IMA XATTR_SECURITY_PREFIX XATTR_IMA_SUFFIX