| Message ID | 20240221-idmap-fscap-refactor-v2-14-3039364623bd@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | fs: use type-safe uid representation for filesystem capabilities | expand |
On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote: > Support the new fscaps security hooks by converting the vfs_caps to raw > xattr data and then handling them the same as other xattrs. Hi Seth I started looking at this patch set. The first question I have is if you are also going to update libcap (and also tar, I guess), since both deal with the raw xattr. From IMA/EVM perspective (Mimi will add on that), I guess it is important that files with a signature/HMAC continue to be accessible after applying this patch set. Looking at the code, it seems the case (if I understood correctly, vfs_getxattr_alloc() is still allowed). To be sure that everything works, it would be really nice if you could also extend our test suite: https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/portable_signatures.test and https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/evm_hmac.test The first test we would need to extend is check_cp_preserve_xattrs, which basically does a cp -a. We would need to set fscaps in the origin, copy to the destination, and see if the latter is accessible. I would also extend: check_tar_extract_xattrs_different_owner check_tar_extract_xattrs_same_owner check_metadata_change check_evm_revalidate check_evm_portable_sig_ima_appraisal check_evm_portable_sig_ima_measurement_list It should not be too complicated. The purpose would be to exercise your code below. Regarding the second test, we would need to extend just check_evm_hmac. Just realized, before extending the tests, it would be necessary to modify also evmctl.c, to retrieve fscaps through the new interfaces, and to let users provide custom fscaps the HMAC or portable signature is calculated on. You can run the tests locally (even with UML linux), or make a PR in Github for both linux and ima-evm-utils, and me and Mimi will help to run them. For Github, for now please use: https://github.com/linux-integrity/linux https://github.com/mimizohar/ima-evm-utils/ Thanks Roberto > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org> > --- > include/linux/evm.h | 39 +++++++++++++++++++++++++ > security/integrity/evm/evm_main.c | 60 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 99 insertions(+) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 36ec884320d9..aeb9ff52ad22 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -57,6 +57,20 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry, > { > return evm_inode_post_setxattr(dentry, acl_name, NULL, 0); > } > +extern int evm_inode_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, int flags); > +static inline int evm_inode_remove_fscaps(struct dentry *dentry) > +{ > + return evm_inode_set_fscaps(&nop_mnt_idmap, dentry, NULL, XATTR_REPLACE); > +} > +extern void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, int flags); > +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry) > +{ > + return evm_inode_post_set_fscaps(&nop_mnt_idmap, dentry, NULL, 0); > +} > > int evm_inode_init_security(struct inode *inode, struct inode *dir, > const struct qstr *qstr, struct xattr *xattrs, > @@ -164,6 +178,31 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry, > return; > } > > +static inline int evm_inode_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, int flags) > +{ > + return 0; > +} > + > +static inline int evm_inode_remove_fscaps(struct dentry *dentry) > +{ > + return 0; > +} > + > +static inline void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, > + int flags) > +{ > + return; > +} > + > +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry) > +{ > + return; > +} > + > static inline int evm_inode_init_security(struct inode *inode, struct inode *dir, > const struct qstr *qstr, > struct xattr *xattrs, > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index cc7956d7878b..ecf4634a921a 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -805,6 +805,66 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) > evm_update_evmxattr(dentry, xattr_name, NULL, 0); > } > > +int evm_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, > + const struct vfs_caps *caps, int flags) > +{ > + struct inode *inode = d_inode(dentry); > + struct vfs_ns_cap_data nscaps; > + const void *xattr_data = NULL; > + int size = 0; > + > + /* Policy permits modification of the protected xattrs even though > + * there's no HMAC key loaded > + */ > + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) > + return 0; > + > + if (caps) { > + size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps, > + sizeof(nscaps)); > + if (size < 0) > + return size; > + xattr_data = &nscaps; > + } > + > + return evm_protect_xattr(idmap, dentry, XATTR_NAME_CAPS, xattr_data, size); > +} > + > +void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, > + const struct vfs_caps *caps, int flags) > +{ > + struct inode *inode = d_inode(dentry); > + struct vfs_ns_cap_data nscaps; > + const void *xattr_data = NULL; > + int size = 0; > + > + if (!evm_revalidate_status(XATTR_NAME_CAPS)) > + return; > + > + evm_reset_status(dentry->d_inode); > + > + if (!(evm_initialized & EVM_INIT_HMAC)) > + return; > + > + if (is_unsupported_fs(dentry)) > + return; > + > + if (caps) { > + size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps, > + sizeof(nscaps)); > + /* > + * The fscaps here should have been converted to an xattr by > + * evm_inode_set_fscaps() already, so a failure to convert > + * here is a bug. > + */ > + if (WARN_ON_ONCE(size < 0)) > + return; > + xattr_data = &nscaps; > + } > + > + evm_update_evmxattr(dentry, XATTR_NAME_CAPS, xattr_data, size); > +} > + > static int evm_attr_change(struct mnt_idmap *idmap, > struct dentry *dentry, struct iattr *attr) > { >
On Fri, Mar 01, 2024 at 10:19:13AM +0100, Roberto Sassu wrote: > On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote: > > Support the new fscaps security hooks by converting the vfs_caps to raw > > xattr data and then handling them the same as other xattrs. > > Hi Seth > > I started looking at this patch set. > > The first question I have is if you are also going to update libcap > (and also tar, I guess), since both deal with the raw xattr. > > From IMA/EVM perspective (Mimi will add on that), I guess it is > important that files with a signature/HMAC continue to be accessible > after applying this patch set. > > Looking at the code, it seems the case (if I understood correctly, > vfs_getxattr_alloc() is still allowed). > > To be sure that everything works, it would be really nice if you could > also extend our test suite: > > https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/portable_signatures.test > > and > > https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/evm_hmac.test > > > The first test we would need to extend is check_cp_preserve_xattrs, > which basically does a cp -a. We would need to set fscaps in the > origin, copy to the destination, and see if the latter is accessible. > > I would also extend: > > check_tar_extract_xattrs_different_owner > check_tar_extract_xattrs_same_owner > check_metadata_change > check_evm_revalidate > check_evm_portable_sig_ima_appraisal > check_evm_portable_sig_ima_measurement_list > > It should not be too complicated. The purpose would be to exercise your > code below. > > > Regarding the second test, we would need to extend just check_evm_hmac. > > > Just realized, before extending the tests, it would be necessary to > modify also evmctl.c, to retrieve fscaps through the new interfaces, > and to let users provide custom fscaps the HMAC or portable signature > is calculated on. While request for tests are obviously fine they should be added by the respective experts for IMA/EVM in this case. I don't think it's appropriate to expect Seth to do that especially because you seem to imply that you currently don't have any tests for fscaps at all. We're always happy to test things and if that'd be adding new IMA/EVM specific features than it would be something to discuss but really we're refactoring so the fact that you don't have tests we can run is not the fault of this patchset and IMA/EVM is just a small portion of it.
On Fri, 2024-03-01 at 13:54 +0100, Christian Brauner wrote: > On Fri, Mar 01, 2024 at 10:19:13AM +0100, Roberto Sassu wrote: > > On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote: > > > Support the new fscaps security hooks by converting the vfs_caps to raw > > > xattr data and then handling them the same as other xattrs. > > > > Hi Seth > > > > I started looking at this patch set. > > > > The first question I have is if you are also going to update libcap > > (and also tar, I guess), since both deal with the raw xattr. > > > > From IMA/EVM perspective (Mimi will add on that), I guess it is > > important that files with a signature/HMAC continue to be accessible > > after applying this patch set. > > > > Looking at the code, it seems the case (if I understood correctly, > > vfs_getxattr_alloc() is still allowed). > > > > To be sure that everything works, it would be really nice if you could > > also extend our test suite: > > > > https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/portable_signatures.test > > > > and > > > > https://github.com/mimizohar/ima-evm-utils/blob/next-testing/tests/evm_hmac.test > > > > > > The first test we would need to extend is check_cp_preserve_xattrs, > > which basically does a cp -a. We would need to set fscaps in the > > origin, copy to the destination, and see if the latter is accessible. > > > > I would also extend: > > > > check_tar_extract_xattrs_different_owner > > check_tar_extract_xattrs_same_owner > > check_metadata_change > > check_evm_revalidate > > check_evm_portable_sig_ima_appraisal > > check_evm_portable_sig_ima_measurement_list > > > > It should not be too complicated. The purpose would be to exercise your > > code below. > > > > > > Regarding the second test, we would need to extend just check_evm_hmac. > > > > > > Just realized, before extending the tests, it would be necessary to > > modify also evmctl.c, to retrieve fscaps through the new interfaces, > > and to let users provide custom fscaps the HMAC or portable signature > > is calculated on. > > While request for tests are obviously fine they should be added by the > respective experts for IMA/EVM in this case. I don't think it's > appropriate to expect Seth to do that especially because you seem to > imply that you currently don't have any tests for fscaps at all. We're > always happy to test things and if that'd be adding new IMA/EVM specific > features than it would be something to discuss but really we're > refactoring so the fact that you don't have tests we can run is not the > fault of this patchset and IMA/EVM is just a small portion of it. Hi Christian I have seen this policy of adding tests in other subsystems (eBPF), which in my opinion makes sense, since you want anyway to check that you didn't break existing code. And yes, I agree that we should have better tests, and a better workflow (we are working on improving it). In this particular case, I was not asking to write a test from scratch, that should not be difficult per se, but adding additional commands. If I got it correctly, even if current tests for fscaps would have existed, they would not work anyway, since they would have been based on getting/setting the raw xattrs (as far as I know, at least for tar). Happy to try adding the tests, would appreciate your help to review if the tests are done in the correct way. Thanks Roberto
> I have seen this policy of adding tests in other subsystems (eBPF), It makes sense if the drive of the patchset would be IMA/EVM features not refactoring of existing code. > Happy to try adding the tests, would appreciate your help to review if Cool, happy to help review them.
On Fri, Mar 01, 2024 at 10:19:13AM +0100, Roberto Sassu wrote: > On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote: > > Support the new fscaps security hooks by converting the vfs_caps to raw > > xattr data and then handling them the same as other xattrs. > > Hi Seth > > I started looking at this patch set. > > The first question I have is if you are also going to update libcap > (and also tar, I guess), since both deal with the raw xattr. There are no changes needed for userspace; it will still deal with raw xattrs. As I mentioned in the cover letter, capabilities tests from libcap2, libcap-ng, ltp, and xfstests all pass against this sereies. That's with no modifications to userspace. > From IMA/EVM perspective (Mimi will add on that), I guess it is > important that files with a signature/HMAC continue to be accessible > after applying this patch set. > > Looking at the code, it seems the case (if I understood correctly, > vfs_getxattr_alloc() is still allowed). So this is something that would change based on Christian's request to stop using the xattr handlers entirely for fscaps as was done for acls. I see how this would impact EVM, but we should be able to deal with it. I am a little curious now about this code in evm_calc_hmac_or_hash(): size = vfs_getxattr_alloc(&nop_mnt_idmap, dentry, xattr->name, &xattr_value, xattr_size, GFP_NOFS); if (size == -ENOMEM) { error = -ENOMEM; goto out; } if (size < 0) continue; user_space_size = vfs_getxattr(&nop_mnt_idmap, dentry, xattr->name, NULL, 0); if (user_space_size != size) pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n", dentry->d_name.name, xattr->name, size, user_space_size); Because with the current fscaps code you actually could end up getting different sizes from these two interfaces, as vfs_getxattr_alloc() reads the xattr directly from disk but vfs_getxattr() goes through cap_inode_getsecurity(), which may do conversion between v2 and v3 formats which are different sizes. Thanks, Seth
On Fri, 2024-03-01 at 08:39 -0600, Seth Forshee (DigitalOcean) wrote: > On Fri, Mar 01, 2024 at 10:19:13AM +0100, Roberto Sassu wrote: > > On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote: > > > Support the new fscaps security hooks by converting the vfs_caps to raw > > > xattr data and then handling them the same as other xattrs. > > > > Hi Seth > > > > I started looking at this patch set. > > > > The first question I have is if you are also going to update libcap > > (and also tar, I guess), since both deal with the raw xattr. > > There are no changes needed for userspace; it will still deal with raw > xattrs. As I mentioned in the cover letter, capabilities tests from > libcap2, libcap-ng, ltp, and xfstests all pass against this sereies. > That's with no modifications to userspace. Yes, figured it out after applying the patch set. Then yes, IMA/EVM tests should work too. > > From IMA/EVM perspective (Mimi will add on that), I guess it is > > important that files with a signature/HMAC continue to be accessible > > after applying this patch set. > > > > Looking at the code, it seems the case (if I understood correctly, > > vfs_getxattr_alloc() is still allowed). > > So this is something that would change based on Christian's request to > stop using the xattr handlers entirely for fscaps as was done for acls. > I see how this would impact EVM, but we should be able to deal with it. > > I am a little curious now about this code in evm_calc_hmac_or_hash(): > > size = vfs_getxattr_alloc(&nop_mnt_idmap, dentry, xattr->name, > &xattr_value, xattr_size, GFP_NOFS); > if (size == -ENOMEM) { > error = -ENOMEM; > goto out; > } > if (size < 0) > continue; > > user_space_size = vfs_getxattr(&nop_mnt_idmap, dentry, > xattr->name, NULL, 0); > if (user_space_size != size) > pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n", > dentry->d_name.name, xattr->name, size, > user_space_size); > > Because with the current fscaps code you actually could end up getting > different sizes from these two interfaces, as vfs_getxattr_alloc() reads > the xattr directly from disk but vfs_getxattr() goes through > cap_inode_getsecurity(), which may do conversion between v2 and v3 > formats which are different sizes. Yes, that was another source of confusion. It happened that security.selinux in the disk was without '\0', and the one from vfs_getxattr() had it (of course the HMAC wouldn't match). So, basically, you set something in user space and you get something different. Example: # setfattr -n security.selinux -v "unconfined_u:object_r:admin_home_t:s0" test-file SELinux active: # getfattr -m - -d -e hex test-file security.selinux=0x756e636f6e66696e65645f753a6f626a6563745f723a61646d696e5f686f6d655f743a733000 Smack active: # getfattr -m - -d -e hex test-file security.selinux=0x756e636f6e66696e65645f753a6f626a6563745f723a61646d696e5f686f6d655f743a7330 evmctl (will) allow to provide a hex xattr value for fscaps. That should be the one to be used (and vfs_getxattr_alloc() does that). However, I guess if the conversion happens, evmctl cannot correctly verify anymore the file, unless the same string is specified for verification (otherwise it reads the xattr through vfs_getxattr(), which would be different). Roberto
On Wed, 2024-02-21 at 15:24 -0600, Seth Forshee (DigitalOcean) wrote: > Support the new fscaps security hooks by converting the vfs_caps to raw > xattr data and then handling them the same as other xattrs. I realized that you need to register hooks for IMA too. This should be the content to add in ima_appraise.c: int ima_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, const struct vfs_caps *caps, int flags) { if (evm_revalidate_status(XATTR_NAME_CAPS)) ima_reset_appraise_flags(d_backing_inode(dentry), false); return 0; } int ima_inode_remove_fscaps(struct mnt_idmap *idmap, struct dentry *dentry) { return ima_inode_set_fscaps(idmap, dentry, NULL, XATTR_REPLACE); } Thanks Roberto > Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org> > --- > include/linux/evm.h | 39 +++++++++++++++++++++++++ > security/integrity/evm/evm_main.c | 60 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 99 insertions(+) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 36ec884320d9..aeb9ff52ad22 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -57,6 +57,20 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry, > { > return evm_inode_post_setxattr(dentry, acl_name, NULL, 0); > } > +extern int evm_inode_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, int flags); > +static inline int evm_inode_remove_fscaps(struct dentry *dentry) > +{ > + return evm_inode_set_fscaps(&nop_mnt_idmap, dentry, NULL, XATTR_REPLACE); > +} > +extern void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, int flags); > +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry) > +{ > + return evm_inode_post_set_fscaps(&nop_mnt_idmap, dentry, NULL, 0); > +} > > int evm_inode_init_security(struct inode *inode, struct inode *dir, > const struct qstr *qstr, struct xattr *xattrs, > @@ -164,6 +178,31 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry, > return; > } > > +static inline int evm_inode_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, int flags) > +{ > + return 0; > +} > + > +static inline int evm_inode_remove_fscaps(struct dentry *dentry) > +{ > + return 0; > +} > + > +static inline void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, > + struct dentry *dentry, > + const struct vfs_caps *caps, > + int flags) > +{ > + return; > +} > + > +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry) > +{ > + return; > +} > + > static inline int evm_inode_init_security(struct inode *inode, struct inode *dir, > const struct qstr *qstr, > struct xattr *xattrs, > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index cc7956d7878b..ecf4634a921a 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -805,6 +805,66 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) > evm_update_evmxattr(dentry, xattr_name, NULL, 0); > } > > +int evm_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, > + const struct vfs_caps *caps, int flags) > +{ > + struct inode *inode = d_inode(dentry); > + struct vfs_ns_cap_data nscaps; > + const void *xattr_data = NULL; > + int size = 0; > + > + /* Policy permits modification of the protected xattrs even though > + * there's no HMAC key loaded > + */ > + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) > + return 0; > + > + if (caps) { > + size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps, > + sizeof(nscaps)); > + if (size < 0) > + return size; > + xattr_data = &nscaps; > + } > + > + return evm_protect_xattr(idmap, dentry, XATTR_NAME_CAPS, xattr_data, size); > +} > + > +void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, > + const struct vfs_caps *caps, int flags) > +{ > + struct inode *inode = d_inode(dentry); > + struct vfs_ns_cap_data nscaps; > + const void *xattr_data = NULL; > + int size = 0; > + > + if (!evm_revalidate_status(XATTR_NAME_CAPS)) > + return; > + > + evm_reset_status(dentry->d_inode); > + > + if (!(evm_initialized & EVM_INIT_HMAC)) > + return; > + > + if (is_unsupported_fs(dentry)) > + return; > + > + if (caps) { > + size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps, > + sizeof(nscaps)); > + /* > + * The fscaps here should have been converted to an xattr by > + * evm_inode_set_fscaps() already, so a failure to convert > + * here is a bug. > + */ > + if (WARN_ON_ONCE(size < 0)) > + return; > + xattr_data = &nscaps; > + } > + > + evm_update_evmxattr(dentry, XATTR_NAME_CAPS, xattr_data, size); > +} > + > static int evm_attr_change(struct mnt_idmap *idmap, > struct dentry *dentry, struct iattr *attr) > { >
diff --git a/include/linux/evm.h b/include/linux/evm.h index 36ec884320d9..aeb9ff52ad22 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -57,6 +57,20 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry, { return evm_inode_post_setxattr(dentry, acl_name, NULL, 0); } +extern int evm_inode_set_fscaps(struct mnt_idmap *idmap, + struct dentry *dentry, + const struct vfs_caps *caps, int flags); +static inline int evm_inode_remove_fscaps(struct dentry *dentry) +{ + return evm_inode_set_fscaps(&nop_mnt_idmap, dentry, NULL, XATTR_REPLACE); +} +extern void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, + struct dentry *dentry, + const struct vfs_caps *caps, int flags); +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry) +{ + return evm_inode_post_set_fscaps(&nop_mnt_idmap, dentry, NULL, 0); +} int evm_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, struct xattr *xattrs, @@ -164,6 +178,31 @@ static inline void evm_inode_post_set_acl(struct dentry *dentry, return; } +static inline int evm_inode_set_fscaps(struct mnt_idmap *idmap, + struct dentry *dentry, + const struct vfs_caps *caps, int flags) +{ + return 0; +} + +static inline int evm_inode_remove_fscaps(struct dentry *dentry) +{ + return 0; +} + +static inline void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, + struct dentry *dentry, + const struct vfs_caps *caps, + int flags) +{ + return; +} + +static inline void evm_inode_post_remove_fscaps(struct dentry *dentry) +{ + return; +} + static inline int evm_inode_init_security(struct inode *inode, struct inode *dir, const struct qstr *qstr, struct xattr *xattrs, diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index cc7956d7878b..ecf4634a921a 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -805,6 +805,66 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) evm_update_evmxattr(dentry, xattr_name, NULL, 0); } +int evm_inode_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, + const struct vfs_caps *caps, int flags) +{ + struct inode *inode = d_inode(dentry); + struct vfs_ns_cap_data nscaps; + const void *xattr_data = NULL; + int size = 0; + + /* Policy permits modification of the protected xattrs even though + * there's no HMAC key loaded + */ + if (evm_initialized & EVM_ALLOW_METADATA_WRITES) + return 0; + + if (caps) { + size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps, + sizeof(nscaps)); + if (size < 0) + return size; + xattr_data = &nscaps; + } + + return evm_protect_xattr(idmap, dentry, XATTR_NAME_CAPS, xattr_data, size); +} + +void evm_inode_post_set_fscaps(struct mnt_idmap *idmap, struct dentry *dentry, + const struct vfs_caps *caps, int flags) +{ + struct inode *inode = d_inode(dentry); + struct vfs_ns_cap_data nscaps; + const void *xattr_data = NULL; + int size = 0; + + if (!evm_revalidate_status(XATTR_NAME_CAPS)) + return; + + evm_reset_status(dentry->d_inode); + + if (!(evm_initialized & EVM_INIT_HMAC)) + return; + + if (is_unsupported_fs(dentry)) + return; + + if (caps) { + size = vfs_caps_to_xattr(idmap, i_user_ns(inode), caps, &nscaps, + sizeof(nscaps)); + /* + * The fscaps here should have been converted to an xattr by + * evm_inode_set_fscaps() already, so a failure to convert + * here is a bug. + */ + if (WARN_ON_ONCE(size < 0)) + return; + xattr_data = &nscaps; + } + + evm_update_evmxattr(dentry, XATTR_NAME_CAPS, xattr_data, size); +} + static int evm_attr_change(struct mnt_idmap *idmap, struct dentry *dentry, struct iattr *attr) {
Support the new fscaps security hooks by converting the vfs_caps to raw xattr data and then handling them the same as other xattrs. Signed-off-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org> --- include/linux/evm.h | 39 +++++++++++++++++++++++++ security/integrity/evm/evm_main.c | 60 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+)