On Wed, Nov 1, 2023 at 10:54 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > On Wed, Nov 01, 2023 at 08:40:06PM -0400, Paul Moore wrote: > > On Mon, Oct 23, 2023 at 11:52 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > > On Oct 4, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > > > > fsverity represents a mechanism to support both integrity and > > > > authenticity protection of a file, supporting both signed and unsigned > > > > digests. > > > > > > > > An LSM which controls access to a resource based on authenticity and > > > > integrity of said resource, can then use this data to make an informed > > > > decision on the authorization (provided by the LSM's policy) of said > > > > claim. > > > > > > > > This effectively allows the extension of a policy enforcement layer in > > > > LSM for fsverity, allowing for more granular control of how a > > > > particular authenticity claim can be used. For example, "all (built-in) > > > > signed fsverity files should be allowed to execute, but only these > > > > hashes are allowed to be loaded as kernel modules". > > > > > > > > This enforcement must be done in kernel space, as a userspace only > > > > solution would fail a simple litmus test: Download a self-contained > > > > malicious binary that never touches the userspace stack. This > > > > binary would still be able to execute. > > > > > > > > Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx> > > > > Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> > > > > --- > > > > v1-v6: > > > > + Not present > > > > > > > > v7: > > > > Introduced > > > > > > > > v8: > > > > + Split fs/verity/ changes and security/ changes into separate patches > > > > + Change signature of fsverity_create_info to accept non-const inode > > > > + Change signature of fsverity_verify_signature to accept non-const inode > > > > + Don't cast-away const from inode. > > > > + Digest functionality dropped in favor of: > > > > ("fs-verity: define a function to return the integrity protected > > > > file digest") > > > > + Reworded commit description and title to match changes. > > > > + Fix a bug wherein no LSM implements the particular fsverity @name > > > > (or LSM is disabled), and returns -EOPNOTSUPP, causing errors. > > > > > > > > v9: > > > > + No changes > > > > > > > > v10: > > > > + Rename the signature blob key > > > > + Cleanup redundant code > > > > + Make the hook call depends on CONFIG_FS_VERITY_BUILTIN_SIGNATURES > > > > > > > > v11: > > > > + No changes > > > > --- > > > > fs/verity/fsverity_private.h | 2 +- > > > > fs/verity/open.c | 26 +++++++++++++++++++++++++- > > > > include/linux/fsverity.h | 2 ++ > > > > 3 files changed, 28 insertions(+), 2 deletions(-) > > > > > > We need an ACK from some VFS folks on this. > > > > Eric and/or Ted, can we get either an ACK or some feedback on this patch? > > > > For reference, the full patchset can be found on lore at the link below: > > > > https://lore.kernel.org/linux-security-module/1696457386-3010-1-git-send-email-wufan@xxxxxxxxxxxxxxxxxxx/ > > Well, technically I already gave some (minor) feedback on this exact patch, and > it's not yet been addressed: > https://lore.kernel.org/linux-security-module/20231005022707.GA1688@quark.localdomain/ Hopefully Fan can comment on that, unless I'm forgetting some implementation details it seems like a reasonable request. > Of course, it would also be nice if the commit message mentioned what the patch > actually does. While I think the commit description does provide a reasonable summary of IPE as it relates to fsverify, I agree that the specifics of the changes presented in the patch are lacking. Fan, could you update the commit description to add a paragraph explaining what IPE would do in the security_inode_setsecurity() hook called from within fsverity_inode_setsecurity()? > At a higher level, I've said before, I'm not super happy about the use of > fsverity builtin signatures growing. (For some of the reasons why, see the > guidance in the fsverity documentation at > https://docs.kernel.org/filesystems/fsverity.html#built-in-signature-verification) > That being said, if the people who are doing the broader review of IPE believe > this is how its fsverity integration should work, I can live with that ... Fan can surely elaborate on this more, but my understanding is that IPE can help provide the missing authorization piece. > ... I don't > intend to block the IPE patchset if enough people want it to be merged. I've > really been hoping to see engagement with the people involved in IMA, as IPE > basically duplicates/replaces IMA. But I haven't seen that, so maybe things > need to move on without them. We are getting a bit beyond the integration of IPE and fsverity so I don't want to spend a lot of time here, but IPE and IMA work quite a bit differently as they serve different purposes. IPE provides a file authorization mechanism that grants access based on the specified policy and the file's underlying backing store; IPE does not measure files like IMA to provide additional integrity checking, it relies on the storage medium's integrity mechanisms. I have no doubt Fan could provide a much better summary if needed, and of course there are the documentation patches in the patchset too. -- paul-moore.com
![]() |