[PATCH v2 14/25] evm: add support for fscaps security hooks

[Christian Brauner]

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.