On 5/29/2024 8:06 PM, Eric Biggers wrote:
On Wed, May 29, 2024 at 09:46:57PM -0400, Paul Moore wrote:
On Fri, May 24, 2024 at 4:46 PM Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:
This patch enhances fsverity's capabilities to support both integrity and
authenticity protection by introducing the exposure of built-in
signatures through a new LSM hook. This functionality allows LSMs,
e.g. IPE, to enforce policies based on the authenticity and integrity of
files, specifically focusing on built-in fsverity signatures. It enables
a policy enforcement layer within LSMs for fsverity, offering granular
control over the usage of authenticity claims. For instance, a policy
could be established to permit the execution of all files with verified
built-in fsverity signatures while restricting kernel module loading
from specified fsverity files via fsverity digests.
The introduction of a security_inode_setintegrity() hook call within
fsverity's workflow ensures that the verified built-in signature of a file
is exposed to LSMs. This enables LSMs to recognize and label fsverity files
that contain a verified built-in fsverity signature. This hook is invoked
subsequent to the fsverity_verify_signature() process, guaranteeing the
signature's verification against fsverity's keyring. This mechanism is
crucial for maintaining system security, as it operates in kernel space,
effectively thwarting attempts by malicious binaries to bypass user space
stack interactions.
The second to last commit in this patch set will add a link to the IPE
documentation in fsverity.rst.
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
v12:
+ Add constification to the hook call
v13:
+ No changes
v14:
+ Add doc/comment to built-in signature verification
v15:
+ Add more docs related to IPE
+ Switch the hook call to security_inode_setintegrity()
v16:
+ Explicitly mention "fsverity builtin signatures" in the commit
message
+ Amend documentation in fsverity.rst
+ Fix format issue
+ Change enum name
v17:
+ Fix various documentation issues
+ Use new enum name LSM_INT_FSVERITY_BUILTINSIG_VALID
v18:
+ Fix typos
+ Move the inode_setintegrity hook call into fsverity_verify_signature()
v19:
+ Cleanup code w.r.t inode_setintegrity hook refactoring
---
Documentation/filesystems/fsverity.rst | 23 +++++++++++++++++++++--
fs/verity/signature.c | 18 +++++++++++++++++-
include/linux/security.h | 1 +
3 files changed, 39 insertions(+), 3 deletions(-)
Eric, can you give this patch in particular a look to make sure you
are okay with everything? I believe Fan has addressed all of your
previous comments and it would be nice to have your Ack/Review tag if
you are okay with the current revision.
Sorry, I've just gotten a bit tired of finding so many basic issues in this
patchset even after years of revisions.
This patch in particular is finally looking better. There are a couple issues
that I still see. (BTW, you're welcome to review it too to help find these
things, given that you seem to have an interest in getting this landed...):
+ err = security_inode_setintegrity(inode,
+ LSM_INT_FSVERITY_BUILTINSIG_VALID,
+ signature,
+ le32_to_cpu(sig_size));
This is doing le32_to_cpu() on a variable of type size_t, which will do the
wrong thing on big endian systems and will generate a 'sparse' warning.
Sorry for the mistake. As sig_size is already converted in open.c, there
is indeed no need to call this function again. I will remove this
unnecessary conversion.
Also, the commit message still incorrectly claims that this patch allows
"restricting kernel module loading from specified fsverity files via fsverity
digests". As I said before (sigh...), this is not correct as that can be done
without this patch.
- Eric
As for the commit message, my intention was to provide an example of a
policy that with the patch IPE can enforce, not to claim that this
specific restriction requires the patch. However, I will remove it as it
seems to be causing confusion.
-Fan
