| Message ID | 20240415142436.2545003-1-roberto.sassu@huaweicloud.com |
|---|---|
| Headers | show |
| Series | security: digest_cache LSM | expand |
On 4/16/2024 4:33 PM, Jarkko Sakkinen wrote: > On Tue Apr 16, 2024 at 10:09 AM EEST, Roberto Sassu wrote: >> On Mon, 2024-04-15 at 22:31 +0300, Jarkko Sakkinen wrote: >>> On Mon Apr 15, 2024 at 5:24 PM EEST, Roberto Sassu wrote: >>>> From: Roberto Sassu <roberto.sassu@huawei.com> >>>> >>>> Introduce the digest_cache LSM, to collect digests from various sources >>>> (called digest lists), and to store them in kernel memory, in a set of hash >>>> tables forming a digest cache. Extracted digests can be used as reference >>>> values for integrity verification of file data or metadata. >>>> >>>> A digest cache has three types of references: in the inode security blob of >>>> the digest list the digest cache was created from (dig_owner field); in the >>>> security blob of the inodes for which the digest cache is requested >>>> (dig_user field); a reference returned by digest_cache_get(). >>>> >>>> References are released with digest_cache_put(), in the first two cases >>>> when inodes are evicted from memory, in the last case when that function is >>>> explicitly called. Obtaining a digest cache reference means that the digest >>>> cache remains valid and cannot be freed until releasing it and until the >>>> total number of references (stored in the digest cache) becomes zero. >>>> >>>> When digest_cache_get() is called on an inode to compare its digest with >>>> a reference value, the digest_cache LSM knows which digest cache to get >>>> from the new security.digest_list xattr added to that inode, which contains >>>> the file name of the desired digest list digests will be extracted from. >>>> >>>> All digest lists are expected to be in the same directory, defined in the >>>> kernel config, and modifiable (with a later patch) at run-time through >>>> securityfs. When the digest_cache LSM reads the security.digest_list xattr, >>>> it uses its value as last path component, appended to the default path >>>> (unless the default path is a file). If an inode does not have that xattr, >>>> the default path is considered as the final destination. >>>> >>>> The default path can be either a file or a directory. If it is a file, the >>>> digest_cache LSM always uses the same digest cache from that file to verify >>>> all inodes (the xattr, if present, is ignored). If it is a directory, and >>>> the inode to verify does not have the xattr, a subsequent patch will make >>>> it possible to iterate and lookup on the digest caches created from each >>>> directory entry. >>>> >>>> Digest caches are created on demand, only when digest_cache_get() is >>>> called. The first time a digest cache is requested, the digest_cache LSM >>>> creates it and sets its reference in the dig_owner and dig_user fields of >>>> the respective inode security blobs. On the next requests, the previously >>>> set reference is returned, after incrementing the reference count. >>>> >>>> Since there might be multiple digest_cache_get() calls for the same inode, >>>> or for different inodes pointing to the same digest list, dig_owner_mutex >>>> and dig_user_mutex have been introduced to protect the check and assignment >>>> of the digest cache reference in the inode security blob. >>>> >>>> Contenders that didn't get the lock also have to wait until the digest >>>> cache is fully instantiated (when the bit INIT_IN_PROGRESS is cleared). >>>> Dig_owner_mutex cannot be used for waiting on the instantiation to avoid >>>> lock inversion with the inode lock for directories. >>>> >>>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> >>>> --- >>>> MAINTAINERS | 6 + >>>> include/linux/digest_cache.h | 32 ++ >>>> include/uapi/linux/lsm.h | 1 + >>>> include/uapi/linux/xattr.h | 3 + >>>> security/Kconfig | 11 +- >>>> security/Makefile | 1 + >>>> security/digest_cache/Kconfig | 16 + >>>> security/digest_cache/Makefile | 7 + >>>> security/digest_cache/internal.h | 86 ++++ >>>> security/digest_cache/main.c | 404 ++++++++++++++++++ >>>> security/security.c | 3 +- >>>> .../selftests/lsm/lsm_list_modules_test.c | 3 + >>>> 12 files changed, 567 insertions(+), 6 deletions(-) >>>> create mode 100644 include/linux/digest_cache.h >>>> create mode 100644 security/digest_cache/Kconfig >>>> create mode 100644 security/digest_cache/Makefile >>>> create mode 100644 security/digest_cache/internal.h >>>> create mode 100644 security/digest_cache/main.c >>>> >>>> diff --git a/MAINTAINERS b/MAINTAINERS >>>> index b1ca23ab8732..72801a88449c 100644 >>>> --- a/MAINTAINERS >>>> +++ b/MAINTAINERS >>>> @@ -6193,6 +6193,12 @@ L: linux-gpio@vger.kernel.org >>>> S: Maintained >>>> F: drivers/gpio/gpio-gpio-mm.c >>>> >>>> +DIGEST_CACHE LSM >>>> +M: Roberto Sassu <roberto.sassu@huawei.com> >>>> +L: linux-security-module@vger.kernel.org >>>> +S: Maintained >>>> +F: security/digest_cache/ >>>> + >>>> DIGITEQ AUTOMOTIVE MGB4 V4L2 DRIVER >>>> M: Martin Tuma <martin.tuma@digiteqautomotive.com> >>>> L: linux-media@vger.kernel.org >>> >>> Nit: afaik, MAINTAINER updates should be split. >> >> Ok, didn't know... > > Yeah, it makes sense when you think it as a contract (or similar) > that you commit maintaining the full set of changes. > >> >>>> diff --git a/include/linux/digest_cache.h b/include/linux/digest_cache.h >>>> new file mode 100644 >>>> index 000000000000..e79f94a60b0f >>>> --- /dev/null >>>> +++ b/include/linux/digest_cache.h >>>> @@ -0,0 +1,32 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* >>>> + * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH >>>> + * >>>> + * Author: Roberto Sassu <roberto.sassu@huawei.com> >>> >>> Nit: Git has an author field in commit granularity so adding author >>> fields to files is sort of old world. >> >> Still like it if you don't mind... >> >> What happens when you get the source code from non-git? > > Not strongly opposing this this but over time there will be > multiple contributors, which maps to multiple authors. Any > company trusting just what says in the header is probably > missing a legal department. > >> >>>> + * >>>> + * Public API of the digest_cache LSM. >>>> + */ >>>> + >>>> +#ifndef _LINUX_DIGEST_CACHE_H >>>> +#define _LINUX_DIGEST_CACHE_H >>>> + >>>> +#include <linux/fs.h> >>>> + >>>> +struct digest_cache; >>> >>> Is this declaration necessary? >> >> I thought so. Users of the digest_cache LSM do: >> >> struct digest_cache *digest_cache; >> >> >> digest_cache = digest_cache_get(dentry); >> >> >> and so on. Those users don't know the internal layout of the >> digest_cache structure, but still pass it to the various functions. > > According to my test this also fully works: > > #include <stdio.h> > > struct digest_cache *digest_cache_get(void) > { > return NULL; > } > > int main(void) > { > struct digest_cache *foo; > > foo = digest_cache_get(); > > return 0; > } Yes, it builds. Will remove the unnecessary line. Thanks Roberto >>> I don't think you need forward declaration here as this does compile: >>> >>> #include <stdio.h> >>> >>> struct digest_cache *digest_cache_get(void) >>> { >>> return NULL; >>> } >>> >>> int main(void) >>> { >>> return (long)digest_cache_get(); >>> } >>> >>> >>>> + >>>> +#ifdef CONFIG_SECURITY_DIGEST_CACHE >>>> +struct digest_cache *digest_cache_get(struct dentry *dentry); >>>> +void digest_cache_put(struct digest_cache *digest_cache); >>>> + >>>> +#else >>>> +static inline struct digest_cache *digest_cache_get(struct dentry *dentry) >>>> +{ >>>> + return NULL; >>>> +} >>>> + >>>> +static inline void digest_cache_put(struct digest_cache *digest_cache) >>>> +{ >>>> +} >>>> + >>>> +#endif /* CONFIG_SECURITY_DIGEST_CACHE */ >>>> +#endif /* _LINUX_DIGEST_CACHE_H */ >>>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h >>>> index 33d8c9f4aa6b..832b3aea5c26 100644 >>>> --- a/include/uapi/linux/lsm.h >>>> +++ b/include/uapi/linux/lsm.h >>>> @@ -64,6 +64,7 @@ struct lsm_ctx { >>>> #define LSM_ID_LANDLOCK 110 >>>> #define LSM_ID_IMA 111 >>>> #define LSM_ID_EVM 112 >>>> +#define LSM_ID_DIGEST_CACHE 113 >>>> >>>> /* >>>> * LSM_ATTR_XXX definitions identify different LSM attributes >>>> diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h >>>> index 9463db2dfa9d..8a58cf4bce65 100644 >>>> --- a/include/uapi/linux/xattr.h >>>> +++ b/include/uapi/linux/xattr.h >>>> @@ -54,6 +54,9 @@ >>>> #define XATTR_IMA_SUFFIX "ima" >>>> #define XATTR_NAME_IMA XATTR_SECURITY_PREFIX XATTR_IMA_SUFFIX >>>> >>>> +#define XATTR_DIGEST_LIST_SUFFIX "digest_list" >>>> +#define XATTR_NAME_DIGEST_LIST XATTR_SECURITY_PREFIX XATTR_DIGEST_LIST_SUFFIX >>>> + >>>> #define XATTR_SELINUX_SUFFIX "selinux" >>>> #define XATTR_NAME_SELINUX XATTR_SECURITY_PREFIX XATTR_SELINUX_SUFFIX >>>> >>>> diff --git a/security/Kconfig b/security/Kconfig >>>> index 52c9af08ad35..99f99cbd94cc 100644 >>>> --- a/security/Kconfig >>>> +++ b/security/Kconfig >>>> @@ -194,6 +194,7 @@ source "security/yama/Kconfig" >>>> source "security/safesetid/Kconfig" >>>> source "security/lockdown/Kconfig" >>>> source "security/landlock/Kconfig" >>>> +source "security/digest_cache/Kconfig" >>>> >>>> source "security/integrity/Kconfig" >>>> >>>> @@ -233,11 +234,11 @@ endchoice >>>> >>>> config LSM >>>> string "Ordered list of enabled LSMs" >>>> - default "landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK >>>> - default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR >>>> - default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO >>>> - default "landlock,lockdown,yama,loadpin,safesetid,bpf" if DEFAULT_SECURITY_DAC >>>> - default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf" >>>> + default "digest_cache,landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK >>>> + default "digest_cache,landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR >>>> + default "digest_cache,landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO >>>> + default "digest_cache,landlock,lockdown,yama,loadpin,safesetid,bpf" if DEFAULT_SECURITY_DAC >>>> + default "digest_cache,landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf" >>>> help >>>> A comma-separated list of LSMs, in initialization order. >>>> Any LSMs left off this list, except for those with order >>>> diff --git a/security/Makefile b/security/Makefile >>>> index 59f238490665..e9b43e7b715a 100644 >>>> --- a/security/Makefile >>>> +++ b/security/Makefile >>>> @@ -25,6 +25,7 @@ obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/ >>>> obj-$(CONFIG_CGROUPS) += device_cgroup.o >>>> obj-$(CONFIG_BPF_LSM) += bpf/ >>>> obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/ >>>> +obj-$(CONFIG_SECURITY_DIGEST_CACHE) += digest_cache/ >>>> >>>> # Object integrity file lists >>>> obj-$(CONFIG_INTEGRITY) += integrity/ >>>> diff --git a/security/digest_cache/Kconfig b/security/digest_cache/Kconfig >>>> new file mode 100644 >>>> index 000000000000..e53fbf0779d6 >>>> --- /dev/null >>>> +++ b/security/digest_cache/Kconfig >>>> @@ -0,0 +1,16 @@ >>>> +# SPDX-License-Identifier: GPL-2.0 >>>> +config SECURITY_DIGEST_CACHE >>>> + bool "Digest_cache LSM" >>>> + default n >>>> + help >>>> + This option enables an LSM maintaining a cache of digests >>>> + (e.g. of file data or metadata). >>>> + >>>> + This LSM can support other kernel components in making access >>>> + control decisions. >>>> + >>>> +config DIGEST_LIST_DEFAULT_PATH >>>> + string >>>> + default "/etc/digest_lists" >>>> + help >>>> + Default directory where digest_cache LSM expects to find digest lists. >>>> diff --git a/security/digest_cache/Makefile b/security/digest_cache/Makefile >>>> new file mode 100644 >>>> index 000000000000..48848c41253e >>>> --- /dev/null >>>> +++ b/security/digest_cache/Makefile >>>> @@ -0,0 +1,7 @@ >>>> +# SPDX-License-Identifier: GPL-2.0 >>>> +# >>>> +# Makefile for building the digest_cache LSM. >>>> + >>>> +obj-$(CONFIG_SECURITY_DIGEST_CACHE) += digest_cache.o >>>> + >>>> +digest_cache-y := main.o >>>> diff --git a/security/digest_cache/internal.h b/security/digest_cache/internal.h >>>> new file mode 100644 >>>> index 000000000000..5f04844af3a5 >>>> --- /dev/null >>>> +++ b/security/digest_cache/internal.h >>>> @@ -0,0 +1,86 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 */ >>>> +/* >>>> + * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH >>>> + * >>>> + * Author: Roberto Sassu <roberto.sassu@huawei.com> >>> >>> ditto >>> >>>> + * >>>> + * Internal header of the digest_cache LSM. >>>> + */ >>>> + >>>> +#ifndef _DIGEST_CACHE_INTERNAL_H >>>> +#define _DIGEST_CACHE_INTERNAL_H >>>> + >>>> +#include <linux/lsm_hooks.h> >>>> +#include <linux/digest_cache.h> >>>> + >>>> +/* Digest cache bits in flags. */ >>>> +#define INIT_IN_PROGRESS 0 /* Digest cache being initialized. */ >>>> + >>>> +/** >>>> + * struct digest_cache - Digest cache >>>> + * @ref_count: Number of references to the digest cache >>>> + * @path_str: Path of the digest list the digest cache was created from >>>> + * @flags: Control flags >>>> + * >>>> + * This structure represents a cache of digests extracted from a digest list. >>>> + */ >>>> +struct digest_cache { >>>> + atomic_t ref_count; >>>> + char *path_str; >>>> + unsigned long flags; >>>> +}; >>>> + >>>> +/** >>>> + * struct digest_cache_security - Digest cache pointers in inode security blob >>>> + * @dig_owner: Digest cache created from this inode >>>> + * @dig_owner_mutex: Protects @dig_owner >>>> + * @dig_user: Digest cache requested for this inode >>>> + * @dig_user_mutex: Protects @dig_user >>>> + * >>>> + * This structure contains references to digest caches, protected by their >>>> + * respective mutex. >>>> + */ >>>> +struct digest_cache_security { >>>> + struct digest_cache *dig_owner; >>>> + struct mutex dig_owner_mutex; >>>> + struct digest_cache *dig_user; >>>> + struct mutex dig_user_mutex; >>>> +}; >>>> + >>>> +extern struct lsm_blob_sizes digest_cache_blob_sizes; >>>> +extern char *default_path_str; >>>> + >>>> +static inline struct digest_cache_security * >>>> +digest_cache_get_security(const struct inode *inode) >>>> +{ >>>> + if (unlikely(!inode->i_security)) >>>> + return NULL; >>>> + >>>> + return inode->i_security + digest_cache_blob_sizes.lbs_inode; >>>> +} >>>> + >>>> +static inline struct digest_cache * >>>> +digest_cache_ref(struct digest_cache *digest_cache) >>>> +{ >>>> + atomic_inc(&digest_cache->ref_count); >>>> + pr_debug("Ref (+) digest cache %s (ref count: %d)\n", >>>> + digest_cache->path_str, atomic_read(&digest_cache->ref_count)); >>>> + return digest_cache; >>>> +} >>>> + >>>> +static inline struct digest_cache * >>>> +digest_cache_unref(struct digest_cache *digest_cache) >>>> +{ >>>> + bool ref_is_zero = atomic_dec_and_test(&digest_cache->ref_count); >>>> + >>>> + pr_debug("Ref (-) digest cache %s (ref count: %d)\n", >>>> + digest_cache->path_str, atomic_read(&digest_cache->ref_count)); >>>> + return (ref_is_zero) ? digest_cache : NULL; >>>> +} >>>> + >>>> +/* main.c */ >>>> +struct digest_cache *digest_cache_create(struct dentry *dentry, >>>> + struct path *digest_list_path, >>>> + char *path_str, char *filename); >>>> + >>>> +#endif /* _DIGEST_CACHE_INTERNAL_H */ >>>> diff --git a/security/digest_cache/main.c b/security/digest_cache/main.c >>>> new file mode 100644 >>>> index 000000000000..14dba8915e99 >>>> --- /dev/null >>>> +++ b/security/digest_cache/main.c >>>> @@ -0,0 +1,404 @@ >>>> +// SPDX-License-Identifier: GPL-2.0 >>>> +/* >>>> + * Copyright (C) 2023-2024 Huawei Technologies Duesseldorf GmbH >>>> + * >>>> + * Author: Roberto Sassu <roberto.sassu@huawei.com> >>>> + * >>>> + * Implement the main code of the digest_cache LSM. >>>> + */ >>>> + >>>> +#define pr_fmt(fmt) "DIGEST CACHE: "fmt >>>> +#include <linux/namei.h> >>>> +#include <linux/xattr.h> >>>> + >>>> +#include "internal.h" >>>> + >>>> +static int digest_cache_enabled __ro_after_init = 1; >>>> +static struct kmem_cache *digest_cache_cache __read_mostly; >>>> + >>>> +char *default_path_str = CONFIG_DIGEST_LIST_DEFAULT_PATH; >>>> + >>>> +/** >>>> + * digest_cache_alloc_init - Allocate and initialize a new digest cache >>>> + * @path_str: Path string of the digest list >>>> + * @filename: Digest list file name (can be an empty string) >>>> + * >>>> + * This function allocates and initializes a new digest cache. >>>> + * >>>> + * Return: A digest_cache structure on success, NULL on error. >>>> + */ >>>> +static struct digest_cache *digest_cache_alloc_init(char *path_str, >>>> + char *filename) >>>> +{ >>>> + struct digest_cache *digest_cache; >>>> + >>>> + digest_cache = kmem_cache_alloc(digest_cache_cache, GFP_KERNEL); >>>> + if (!digest_cache) >>>> + return digest_cache; >>>> + >>>> + digest_cache->path_str = kasprintf(GFP_KERNEL, "%s%s%s", path_str, >>>> + filename[0] ? "/" : "", filename); >>>> + if (!digest_cache->path_str) { >>>> + kmem_cache_free(digest_cache_cache, digest_cache); >>>> + return NULL; >>>> + } >>>> + >>>> + atomic_set(&digest_cache->ref_count, 1); >>>> + digest_cache->flags = 0UL; >>>> + >>>> + pr_debug("New digest cache %s (ref count: %d)\n", >>>> + digest_cache->path_str, atomic_read(&digest_cache->ref_count)); >>> >>> Nit: kretprobe can be used to grab the same information easil and >>> do e.g. statistics and stuff like that. Traces for return values >>> are not very useful. >> >> I like to do dynamic debug, and I can control what part I want to >> debug. All these messages allowed me to debug efficiently when there >> was any issue. Would be a pity to remove them. > > OK, in this case I missed the fact that the function is static so you can > ignore it! > >> >> Thanks >> >> Roberto >> >>>> + >>>> + return digest_cache; >>>> +} >>>> + >>>> +/** >>>> + * digest_cache_free - Free all memory occupied by the digest cache >>>> + * @digest_cache: Digest cache >>>> + * >>>> + * This function frees the memory occupied by the digest cache. >>>> + */ >>>> +static void digest_cache_free(struct digest_cache *digest_cache) >>>> +{ >>>> + pr_debug("Freed digest cache %s\n", digest_cache->path_str); >>> >>> ditto for the above trace >>> >>>> + kfree(digest_cache->path_str); >>>> + kmem_cache_free(digest_cache_cache, digest_cache); >>>> +} >>>> + >>>> +/** >>>> + * digest_cache_create - Create a digest cache >>>> + * @dentry: Dentry of the inode for which the digest cache will be used >>>> + * @digest_list_path: Path structure of the digest list >>>> + * @path_str: Path string of the digest list >>>> + * @filename: Digest list file name (can be an empty string) >>>> + * >>>> + * This function first locates, from the passed path, the digest list inode >>>> + * from which the digest cache will be created or retrieved (if it already >>>> + * exists). >>>> + * >>>> + * If dig_owner is NULL in the inode security blob, this function creates a >>>> + * new digest cache with reference count set to 1 (reference returned), sets >>>> + * it to dig_owner and consequently increments again the digest cache reference >>>> + * count. >>>> + * >>>> + * Otherwise, it simply increments the reference count of the existing >>>> + * dig_owner, since that reference is returned to the caller. >>>> + * >>>> + * Incrementing the reference count twice before calling path_put() ensures >>>> + * that the digest cache returned is valid even if the inode is evicted from >>>> + * memory (which decreases the reference count). >>>> + * >>>> + * Releasing the dig_owner_mutex lock does not mean that the digest cache is >>>> + * ready for use. digest_cache_create() callers that found a partially >>>> + * instantiated digest cache have to wait until the INIT_IN_PROGRESS bit is >>>> + * cleared by the caller that is actually creating that digest cache. >>>> + * >>>> + * Return: A new digest cache on success, NULL on error. >>>> + */ >>>> +struct digest_cache *digest_cache_create(struct dentry *dentry, >>>> + struct path *digest_list_path, >>>> + char *path_str, char *filename) >>>> +{ >>>> + struct path file_path; >>>> + struct digest_cache *digest_cache = NULL; >>>> + struct digest_cache_security *dig_sec; >>>> + struct inode *inode = d_backing_inode(digest_list_path->dentry); >>>> + bool dig_owner_exists = false; >>>> + int ret; >>>> + >>>> + if (S_ISDIR(d_backing_inode(digest_list_path->dentry)->i_mode) && >>>> + filename[0]) { >>>> + ret = vfs_path_lookup(digest_list_path->dentry, >>>> + digest_list_path->mnt, filename, 0, >>>> + &file_path); >>>> + if (ret < 0) { >>>> + pr_debug("Cannot find digest list %s/%s\n", path_str, >>>> + filename); >>>> + return NULL; >>>> + } >>>> + >>>> + digest_list_path = &file_path; >>>> + inode = d_backing_inode(file_path.dentry); >>>> + >>>> + /* >>>> + * Cannot request a digest cache for the same inode the >>>> + * digest cache is populated from. >>>> + */ >>>> + if (d_backing_inode(dentry) == inode) { >>>> + pr_debug("Cannot request a digest cache for %s and use it as digest list\n", >>>> + dentry->d_name.name); >>>> + goto out; >>>> + } >>>> + >>>> + /* No support for nested directories. */ >>>> + if (!S_ISREG(inode->i_mode)) { >>>> + pr_debug("%s is not a regular file (no support for nested directories)\n", >>>> + dentry->d_name.name); >>>> + goto out; >>>> + } >>>> + } >>>> + >>>> + dig_sec = digest_cache_get_security(inode); >>>> + if (unlikely(!dig_sec)) >>>> + goto out; >>>> + >>>> + /* Serialize check and assignment of dig_owner. */ >>>> + mutex_lock(&dig_sec->dig_owner_mutex); >>>> + if (dig_sec->dig_owner) { >>>> + /* Increment ref. count for reference returned to the caller. */ >>>> + digest_cache = digest_cache_ref(dig_sec->dig_owner); >>>> + dig_owner_exists = true; >>>> + mutex_unlock(&dig_sec->dig_owner_mutex); >>>> + goto exists; >>>> + } >>>> + >>>> + /* Ref. count is already 1 for this reference. */ >>>> + digest_cache = digest_cache_alloc_init(path_str, filename); >>>> + if (!digest_cache) { >>>> + mutex_unlock(&dig_sec->dig_owner_mutex); >>>> + goto out; >>>> + } >>>> + >>>> + /* Increment ref. count for reference set to dig_owner. */ >>>> + dig_sec->dig_owner = digest_cache_ref(digest_cache); >>>> + >>>> + /* Make the other lock contenders wait until creation complete. */ >>>> + set_bit(INIT_IN_PROGRESS, &dig_sec->dig_owner->flags); >>>> + mutex_unlock(&dig_sec->dig_owner_mutex); >>>> + >>>> + /* Creation complete, notify the other lock contenders. */ >>>> + clear_and_wake_up_bit(INIT_IN_PROGRESS, &dig_sec->dig_owner->flags); >>>> +exists: >>>> + if (dig_owner_exists) >>>> + /* Wait until creation complete. */ >>>> + wait_on_bit(&dig_sec->dig_owner->flags, INIT_IN_PROGRESS, >>>> + TASK_UNINTERRUPTIBLE); >>>> +out: >>>> + if (digest_list_path == &file_path) >>>> + path_put(&file_path); >>>> + >>>> + return digest_cache; >>>> +} >>>> + >>>> +/** >>>> + * digest_cache_new - Retrieve digest list file name and request digest cache >>>> + * @dentry: Dentry of the inode for which the digest cache will be used >>>> + * >>>> + * This function locates the default path. If it is a file, it directly creates >>>> + * a digest cache from it. Otherwise, it reads the digest list file name from >>>> + * the security.digest_list xattr and requests the creation of a digest cache >>>> + * with that file name. If security.digest_list is not found, this function >>>> + * requests the creation of a digest cache on the parent directory. >>>> + * >>>> + * Return: A new digest cache on success, NULL on error. >>>> + */ >>>> +static struct digest_cache *digest_cache_new(struct dentry *dentry) >>>> +{ >>>> + char filename[NAME_MAX + 1] = { 0 }; >>>> + struct digest_cache *digest_cache = NULL; >>>> + struct path default_path; >>>> + int ret; >>>> + >>>> + ret = kern_path(default_path_str, 0, &default_path); >>>> + if (ret < 0) { >>>> + pr_debug("Cannot find path %s\n", default_path_str); >>>> + return NULL; >>>> + } >>>> + >>>> + /* The default path is a file, no need to get xattr. */ >>>> + if (S_ISREG(d_backing_inode(default_path.dentry)->i_mode)) { >>>> + pr_debug("Default path %s is a file, not reading %s xattr\n", >>>> + default_path_str, XATTR_NAME_DIGEST_LIST); >>>> + goto create; >>>> + } else if (!S_ISDIR(d_backing_inode(default_path.dentry)->i_mode)) { >>>> + pr_debug("Default path %s must be either a file or a directory\n", >>>> + default_path_str); >>>> + goto out; >>>> + } >>>> + >>>> + ret = vfs_getxattr(&nop_mnt_idmap, dentry, XATTR_NAME_DIGEST_LIST, >>>> + filename, sizeof(filename) - 1); >>>> + if (ret <= 0) { >>>> + pr_debug("Digest list path not found for file %s, using %s\n", >>>> + dentry->d_name.name, default_path_str); >>>> + goto create; >>>> + } >>>> + >>>> + if (strchr(filename, '/')) { >>>> + pr_debug("%s xattr should contain only a file name, got: %s\n", >>>> + XATTR_NAME_DIGEST_LIST, filename); >>>> + goto out; >>>> + } >>>> + >>>> + pr_debug("Found %s xattr in %s, default path: %s, digest list: %s\n", >>>> + XATTR_NAME_DIGEST_LIST, dentry->d_name.name, default_path_str, >>>> + filename); >>>> +create: >>>> + digest_cache = digest_cache_create(dentry, &default_path, >>>> + default_path_str, filename); >>>> +out: >>>> + path_put(&default_path); >>>> + return digest_cache; >>>> +} >>>> + >>>> +/** >>>> + * digest_cache_get - Get a digest cache for a given inode >>>> + * @dentry: Dentry of the inode for which the digest cache will be used >>>> + * >>>> + * This function tries to find a digest cache from the inode security blob of >>>> + * the passed dentry (dig_user field). If a digest cache was not found, it calls >>>> + * digest_cache_new() to create a new one. In both cases, it increments the >>>> + * digest cache reference count before returning the reference to the caller. >>>> + * >>>> + * The caller is responsible to call digest_cache_put() to release the digest >>>> + * cache reference returned. >>>> + * >>>> + * Lock dig_user_mutex to protect against concurrent requests to obtain a digest >>>> + * cache for the same inode, and to make other contenders wait until the first >>>> + * requester finishes the process. >>>> + * >>>> + * Return: A digest cache on success, NULL otherwise. >>>> + */ >>>> +struct digest_cache *digest_cache_get(struct dentry *dentry) >>>> +{ >>>> + struct digest_cache_security *dig_sec; >>>> + struct digest_cache *digest_cache = NULL; >>>> + struct inode *inode = d_backing_inode(dentry); >>>> + >>>> + if (!digest_cache_enabled) >>>> + return NULL; >>>> + >>>> + dig_sec = digest_cache_get_security(inode); >>>> + if (unlikely(!dig_sec)) >>>> + return NULL; >>>> + >>>> + /* Serialize accesses to inode for which the digest cache is used. */ >>>> + mutex_lock(&dig_sec->dig_user_mutex); >>>> + if (!dig_sec->dig_user) >>>> + /* Consume extra reference from digest_cache_create(). */ >>>> + dig_sec->dig_user = digest_cache_new(dentry); >>>> + >>>> + if (dig_sec->dig_user) >>>> + /* Increment ref. count for reference returned to the caller. */ >>>> + digest_cache = digest_cache_ref(dig_sec->dig_user); >>>> + >>>> + mutex_unlock(&dig_sec->dig_user_mutex); >>>> + return digest_cache; >>>> +} >>>> +EXPORT_SYMBOL_GPL(digest_cache_get); >>>> + >>>> +/** >>>> + * digest_cache_put - Release a digest cache reference >>>> + * @digest_cache: Digest cache >>>> + * >>>> + * This function decrements the reference count of the digest cache passed as >>>> + * argument. If the reference count reaches zero, it calls digest_cache_free() >>>> + * to free the digest cache. >>>> + */ >>>> +void digest_cache_put(struct digest_cache *digest_cache) >>>> +{ >>>> + struct digest_cache *to_free; >>>> + >>>> + to_free = digest_cache_unref(digest_cache); >>>> + if (!to_free) >>>> + return; >>>> + >>>> + digest_cache_free(to_free); >>>> +} >>>> +EXPORT_SYMBOL_GPL(digest_cache_put); >>>> + >>>> +struct lsm_blob_sizes digest_cache_blob_sizes __ro_after_init = { >>>> + .lbs_inode = sizeof(struct digest_cache_security), >>>> +}; >>>> + >>>> +/** >>>> + * digest_cache_inode_alloc_security - Initialize inode security blob >>>> + * @inode: Inode for which the security blob is initialized >>>> + * >>>> + * This function initializes the digest_cache_security structure, directly >>>> + * stored in the inode security blob. >>>> + * >>>> + * Return: Zero. >>>> + */ >>>> +static int digest_cache_inode_alloc_security(struct inode *inode) >>>> +{ >>>> + struct digest_cache_security *dig_sec; >>>> + >>>> + /* The inode security blob is always allocated here. */ >>>> + dig_sec = digest_cache_get_security(inode); >>>> + mutex_init(&dig_sec->dig_owner_mutex); >>>> + mutex_init(&dig_sec->dig_user_mutex); >>>> + return 0; >>>> +} >>>> + >>>> +/** >>>> + * digest_cache_inode_free_security - Release the digest cache references >>>> + * @inode: Inode for which the digest cache references are released >>>> + * >>>> + * Since the inode is being evicted, this function releases the non-needed >>>> + * references to the digest caches stored in the digest_cache_security >>>> + * structure. >>>> + */ >>>> +static void digest_cache_inode_free_security(struct inode *inode) >>>> +{ >>>> + struct digest_cache_security *dig_sec; >>>> + >>>> + dig_sec = digest_cache_get_security(inode); >>>> + if (!dig_sec) >>>> + return; >>>> + >>>> + mutex_destroy(&dig_sec->dig_owner_mutex); >>>> + mutex_destroy(&dig_sec->dig_user_mutex); >>>> + if (dig_sec->dig_owner) >>>> + digest_cache_put(dig_sec->dig_owner); >>>> + if (dig_sec->dig_user) >>>> + digest_cache_put(dig_sec->dig_user); >>>> +} >>>> + >>>> +static struct security_hook_list digest_cache_hooks[] __ro_after_init = { >>>> + LSM_HOOK_INIT(inode_alloc_security, digest_cache_inode_alloc_security), >>>> + LSM_HOOK_INIT(inode_free_security, digest_cache_inode_free_security), >>>> +}; >>>> + >>>> +/** >>>> + * digest_cache_init_once - Initialize the digest cache structure >>>> + * @foo: Digest cache structure to initialize >>>> + * >>>> + * This function fills the digest cache structure with zeros. >>>> + */ >>>> +static void digest_cache_init_once(void *foo) >>>> +{ >>>> + struct digest_cache *digest_cache = (struct digest_cache *)foo; >>>> + >>>> + memset(digest_cache, 0, sizeof(*digest_cache)); >>>> +} >>>> + >>>> +static const struct lsm_id digest_cache_lsmid = { >>>> + .name = "digest_cache", >>>> + .id = LSM_ID_DIGEST_CACHE, >>>> +}; >>>> + >>>> +/** >>>> + * digest_cache_init - Initialize the digest_cache LSM >>>> + * >>>> + * Initialize the digest_cache LSM, by instantiating a cache for the >>>> + * digest_cache structure and by registering the digest_cache LSM hooks. >>>> + */ >>>> +static int __init digest_cache_init(void) >>>> +{ >>>> + digest_cache_cache = kmem_cache_create("digest_cache_cache", >>>> + sizeof(struct digest_cache), >>>> + 0, SLAB_PANIC, >>>> + digest_cache_init_once); >>>> + >>>> + security_add_hooks(digest_cache_hooks, ARRAY_SIZE(digest_cache_hooks), >>>> + &digest_cache_lsmid); >>>> + return 0; >>>> +} >>>> + >>>> +DEFINE_LSM(digest_cache) = { >>>> + .name = "digest_cache", >>>> + .enabled = &digest_cache_enabled, >>>> + .init = digest_cache_init, >>>> + .blobs = &digest_cache_blob_sizes, >>>> +}; >>>> diff --git a/security/security.c b/security/security.c >>>> index cbdc9bebe802..cb084ed58617 100644 >>>> --- a/security/security.c >>>> +++ b/security/security.c >>>> @@ -50,7 +50,8 @@ >>>> (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ >>>> (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0) + \ >>>> (IS_ENABLED(CONFIG_IMA) ? 1 : 0) + \ >>>> - (IS_ENABLED(CONFIG_EVM) ? 1 : 0)) >>>> + (IS_ENABLED(CONFIG_EVM) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_DIGEST_CACHE) ? 1 : 0)) >>>> >>>> /* >>>> * These are descriptions of the reasons that can be passed to the >>>> diff --git a/tools/testing/selftests/lsm/lsm_list_modules_test.c b/tools/testing/selftests/lsm/lsm_list_modules_test.c >>>> index 4d5d4cee2586..d00831edc582 100644 >>>> --- a/tools/testing/selftests/lsm/lsm_list_modules_test.c >>>> +++ b/tools/testing/selftests/lsm/lsm_list_modules_test.c >>>> @@ -128,6 +128,9 @@ TEST(correct_lsm_list_modules) >>>> case LSM_ID_EVM: >>>> name = "evm"; >>>> break; >>>> + case LSM_ID_DIGEST_CACHE: >>>> + name = "digest_cache"; >>>> + break; >>>> default: >>>> name = "INVALID"; >>>> break; >>> >>> BR, Jarkko > > BR, Jarkko
On 4/18/24 02:02, Roberto Sassu wrote: > > 72374d71c315 > > Roberto > Still FTA (fail to apply), unfortunately.
On Fri Apr 19, 2024 at 2:18 PM EEST, Bagas Sanjaya wrote: > On 4/18/24 02:02, Roberto Sassu wrote: > > > > 72374d71c315 > > > > Roberto > > > > Still FTA (fail to apply), unfortunately. Robert, quick suggestion. Maybe pick recent rc for the patch set, rebase on top of that and document that to the cover letter? In git: "git rebase <version>" when on branch with the patches applied. BR, Jarkko
On 4/19/2024 4:18 AM, Bagas Sanjaya wrote: > On 4/18/24 02:02, Roberto Sassu wrote: >> >> 72374d71c315 >> >> Roberto >> > > Still FTA (fail to apply), unfortunately. Sorry, looks like I didn't regenerate the patches after rebasing to the latest kernel. The current ones are still based on 6.8-rc3, but they still require some additional patches that I picked up. Will send the new version with Jarkko suggestions implemented. Thanks Roberto
From: Roberto Sassu <roberto.sassu@huawei.com> Integrity detection and protection has long been a desirable feature, to reach a large user base and mitigate the risk of flaws in the software and attacks. However, while solutions exist, they struggle to reach the large user base, due to requiring higher than desired constraints on performance, flexibility and configurability, that only security conscious people are willing to accept. This is where the new digest_cache LSM comes into play, it offers additional support for new and existing integrity solutions, to make them faster and easier to deploy. The full documentation with the motivation and the solution details can be found in patch 14. The IMA integration patch set will be introduced separately. Also a PoC based on the current version of IPE can be provided. v3: - Rewrite documentation, and remove the installation instructions since they are now included in the README of digest-cache-tools - Add digest cache event notifier - Drop digest_cache_was_reset(), and send instead to asynchronous notifications - Fix digest_cache LSM Kconfig style issues (suggested by Randy Dunlap) - Propagate digest cache reset to directory entries - Destroy per directory entry mutex - Introduce RESET_USER bit, to clear the dig_user pointer on set/removexattr - Replace 'file content' with 'file data' (suggested by Mimi) - Introduce per digest cache mutex and replace verif_data_lock spinlock - Track changes of security.digest_list xattr - Stop tracking file_open and use file_release instead also for file writes - Add error messages in digest_cache_create() - Load/unload testing kernel module automatically during execution of test - Add tests for digest cache event notifier - Add test for ftruncate() - Remove DIGEST_CACHE_RESET_PREFETCH_BUF command in test and clear the buffer on read instead v2: - Include the TLV parser in this patch set (from user asymmetric keys and signatures) - Move from IMA and make an independent LSM - Remove IMA-specific stuff from this patch set - Add per algorithm hash table - Expect all digest lists to be in the same directory and allow changing the default directory - Support digest lookup on directories, when there is no security.digest_list xattr - Add seq num to digest list file name, to impose ordering on directory iteration - Add a new data type DIGEST_LIST_ENTRY_DATA for the nested data in the tlv digest list format - Add the concept of verification data attached to digest caches - Add the reset mechanism to track changes on digest lists and directory containing the digest lists - Add kernel selftests v1: - Add documentation in Documentation/security/integrity-digest-cache.rst - Pass the mask of IMA actions to digest_cache_alloc() - Add a reference count to the digest cache - Remove the path parameter from digest_cache_get(), and rely on the reference count to avoid the digest cache disappearing while being used - Rename the dentry_to_check parameter of digest_cache_get() to dentry - Rename digest_cache_get() to digest_cache_new() and add digest_cache_get() to set the digest cache in the iint of the inode for which the digest cache was requested - Add dig_owner and dig_user to the iint, to distinguish from which inode the digest cache was created from, and which is using it; consequently it makes the digest cache usable to measure/appraise other digest caches (support not yet enabled) - Add dig_owner_mutex and dig_user_mutex to serialize accesses to dig_owner and dig_user until they are initialized - Enforce strong synchronization and make the contenders wait until dig_owner and dig_user are assigned to the iint the first time - Move checking IMA actions on the digest list earlier, and fail if no action were performed (digest cache not usable) - Remove digest_cache_put(), not needed anymore with the introduction of the reference count - Fail immediately in digest_cache_lookup() if the digest algorithm is not set in the digest cache - Use 64 bit mask for IMA actions on the digest list instead of 8 bit - Return NULL in the inline version of digest_cache_get() - Use list_add_tail() instead of list_add() in the iterator - Copy the digest list path to a separate buffer in digest_cache_iter_dir() - Use digest list parsers verified with Frama-C - Explicitly disable (for now) the possibility in the IMA policy to use the digest cache to measure/appraise other digest lists - Replace exit(<value>) with return <value> in manage_digest_lists.c Roberto Sassu (14): lib: Add TLV parser security: Introduce the digest_cache LSM digest_cache: Add securityfs interface digest_cache: Add hash tables and operations digest_cache: Populate the digest cache from a digest list digest_cache: Parse tlv digest lists digest_cache: Parse rpm digest lists digest_cache: Add management of verification data digest_cache: Add support for directories digest cache: Prefetch digest lists if requested digest_cache: Reset digest cache on file/directory change digest_cache: Notify digest cache events selftests/digest_cache: Add selftests for digest_cache LSM docs: Add documentation of the digest_cache LSM Documentation/security/digest_cache.rst | 763 ++++++++++++++++ Documentation/security/index.rst | 1 + MAINTAINERS | 16 + include/linux/digest_cache.h | 117 +++ include/linux/kernel_read_file.h | 1 + include/linux/tlv_parser.h | 28 + include/uapi/linux/lsm.h | 1 + include/uapi/linux/tlv_digest_list.h | 72 ++ include/uapi/linux/tlv_parser.h | 59 ++ include/uapi/linux/xattr.h | 6 + lib/Kconfig | 3 + lib/Makefile | 3 + lib/tlv_parser.c | 214 +++++ lib/tlv_parser.h | 17 + security/Kconfig | 11 +- security/Makefile | 1 + security/digest_cache/Kconfig | 33 + security/digest_cache/Makefile | 11 + security/digest_cache/dir.c | 252 ++++++ security/digest_cache/htable.c | 268 ++++++ security/digest_cache/internal.h | 290 +++++++ security/digest_cache/main.c | 570 ++++++++++++ security/digest_cache/modsig.c | 66 ++ security/digest_cache/notifier.c | 135 +++ security/digest_cache/parsers/parsers.h | 15 + security/digest_cache/parsers/rpm.c | 223 +++++ security/digest_cache/parsers/tlv.c | 299 +++++++ security/digest_cache/populate.c | 163 ++++ security/digest_cache/reset.c | 235 +++++ security/digest_cache/secfs.c | 87 ++ security/digest_cache/verif.c | 119 +++ security/security.c | 3 +- tools/testing/selftests/Makefile | 1 + .../testing/selftests/digest_cache/.gitignore | 3 + tools/testing/selftests/digest_cache/Makefile | 24 + .../testing/selftests/digest_cache/all_test.c | 815 ++++++++++++++++++ tools/testing/selftests/digest_cache/common.c | 78 ++ tools/testing/selftests/digest_cache/common.h | 135 +++ .../selftests/digest_cache/common_user.c | 47 + .../selftests/digest_cache/common_user.h | 17 + tools/testing/selftests/digest_cache/config | 1 + .../selftests/digest_cache/generators.c | 248 ++++++ .../selftests/digest_cache/generators.h | 19 + .../selftests/digest_cache/testmod/Makefile | 16 + .../selftests/digest_cache/testmod/kern.c | 564 ++++++++++++ .../selftests/lsm/lsm_list_modules_test.c | 3 + 46 files changed, 6047 insertions(+), 6 deletions(-) create mode 100644 Documentation/security/digest_cache.rst create mode 100644 include/linux/digest_cache.h create mode 100644 include/linux/tlv_parser.h create mode 100644 include/uapi/linux/tlv_digest_list.h create mode 100644 include/uapi/linux/tlv_parser.h create mode 100644 lib/tlv_parser.c create mode 100644 lib/tlv_parser.h create mode 100644 security/digest_cache/Kconfig create mode 100644 security/digest_cache/Makefile create mode 100644 security/digest_cache/dir.c create mode 100644 security/digest_cache/htable.c create mode 100644 security/digest_cache/internal.h create mode 100644 security/digest_cache/main.c create mode 100644 security/digest_cache/modsig.c create mode 100644 security/digest_cache/notifier.c create mode 100644 security/digest_cache/parsers/parsers.h create mode 100644 security/digest_cache/parsers/rpm.c create mode 100644 security/digest_cache/parsers/tlv.c create mode 100644 security/digest_cache/populate.c create mode 100644 security/digest_cache/reset.c create mode 100644 security/digest_cache/secfs.c create mode 100644 security/digest_cache/verif.c create mode 100644 tools/testing/selftests/digest_cache/.gitignore create mode 100644 tools/testing/selftests/digest_cache/Makefile create mode 100644 tools/testing/selftests/digest_cache/all_test.c create mode 100644 tools/testing/selftests/digest_cache/common.c create mode 100644 tools/testing/selftests/digest_cache/common.h create mode 100644 tools/testing/selftests/digest_cache/common_user.c create mode 100644 tools/testing/selftests/digest_cache/common_user.h create mode 100644 tools/testing/selftests/digest_cache/config create mode 100644 tools/testing/selftests/digest_cache/generators.c create mode 100644 tools/testing/selftests/digest_cache/generators.h create mode 100644 tools/testing/selftests/digest_cache/testmod/Makefile create mode 100644 tools/testing/selftests/digest_cache/testmod/kern.c