mbox series

[v4,00/14] security: digest_cache LSM

Message ID 20240415142436.2545003-1-roberto.sassu@huaweicloud.com
Headers show
Series security: digest_cache LSM | expand

Message

Roberto Sassu April 15, 2024, 2:24 p.m. UTC
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

Comments

Roberto Sassu April 17, 2024, 5 p.m. UTC | #1
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
Bagas Sanjaya April 19, 2024, 11:18 a.m. UTC | #2
On 4/18/24 02:02, Roberto Sassu wrote:
> 
> 72374d71c315
> 
> Roberto
> 

Still FTA (fail to apply), unfortunately.
Jarkko Sakkinen April 19, 2024, 8:05 p.m. UTC | #3
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
Roberto Sassu April 19, 2024, 11:29 p.m. UTC | #4
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
Paul Moore June 18, 2024, 11:20 p.m. UTC | #5
On Mon, Apr 15, 2024 at 10:25 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> 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.

I'm not sure we want to implement a cache as a LSM.  I'm sure it would
work, but historically LSMs have provided some form of access control,
measurement, or other traditional security service.  A digest cache,
while potentially useful for a variety of security related
applications, is not a security service by itself, it is simply a file
digest storage mechanism.

I think it's fine if an individual LSM wants to implement a file
digest cache as part of its own functionality, but a generalized file
digest cache seems like something that should be part of the general
kernel, and not implemented as a LSM.
Roberto Sassu June 19, 2024, 7:59 a.m. UTC | #6
On Tue, 2024-06-18 at 19:20 -0400, Paul Moore wrote:
> On Mon, Apr 15, 2024 at 10:25 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > 
> > 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.
> 
> I'm not sure we want to implement a cache as a LSM.  I'm sure it would
> work, but historically LSMs have provided some form of access control,
> measurement, or other traditional security service.  A digest cache,
> while potentially useful for a variety of security related
> applications, is not a security service by itself, it is simply a file
> digest storage mechanism.

Uhm, currently the digest_cache LSM is heavily based on the LSM
infrastructure:

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),
	LSM_HOOK_INIT(path_truncate, digest_cache_path_truncate),
	LSM_HOOK_INIT(file_release, digest_cache_file_release),
	LSM_HOOK_INIT(inode_unlink, digest_cache_inode_unlink),
	LSM_HOOK_INIT(inode_rename, digest_cache_inode_rename),
	LSM_HOOK_INIT(inode_post_setxattr, digest_cache_inode_post_setxattr),
	LSM_HOOK_INIT(inode_post_removexattr,
		      digest_cache_inode_post_removexattr),
};

struct lsm_blob_sizes digest_cache_blob_sizes __ro_after_init = {
	.lbs_inode = sizeof(struct digest_cache_security),
	.lbs_file = sizeof(struct digest_cache *),
};

Sure, there could be a different indexing mechanism, although using the
inode security blob seems quite efficient, since resolving the path is
sufficient to find a digest cache.

Also, registering to inode_alloc/free_security allows the digest_cache
LSM to dynamically deallocate data when it is not necessary. In
addition to that, there are a number of hooks to determine whether a
digest cache should be refreshed or not.

In the past, it was part of IMA and known as IMA Digest Lists, and as a
separate module, called DIGLIM.

Both required explicit loading of the file digests are extract from to
the kernel through securityfs. Loading was done by an rpm plugin,
invoked when software is installd/removed.

That didn't look a good idea. DIGLIM does not know when the system is
under memory pressure and when digests can be evicted from memory. All
digests needed to be loaded, leading to having a big database.

I think this shortcoming has now been effectively solved by attaching
the digests to the filesystem. Digests are always there, loadable on
demand, unloadable by the system under memory pressure.

> I think it's fine if an individual LSM wants to implement a file
> digest cache as part of its own functionality, but a generalized file
> digest cache seems like something that should be part of the general
> kernel, and not implemented as a LSM.

If we keep the same design as now, it would be anyway connected to the
filesystem, but reusing the LSM infrastructure makes it very easy as I
don't require any change anywhere else.

Sure, it is not doing access control, but I haven't find another good
way to achieve the same. Do you have anything more specific in mind?

Thanks

Roberto
Paul Moore June 19, 2024, 3:49 p.m. UTC | #7
On Wed, Jun 19, 2024 at 3:59 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
> On Tue, 2024-06-18 at 19:20 -0400, Paul Moore wrote:
> > On Mon, Apr 15, 2024 at 10:25 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > >
> > > 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.
> >
> > I'm not sure we want to implement a cache as a LSM.  I'm sure it would
> > work, but historically LSMs have provided some form of access control,
> > measurement, or other traditional security service.  A digest cache,
> > while potentially useful for a variety of security related
> > applications, is not a security service by itself, it is simply a file
> > digest storage mechanism.
>
> Uhm, currently the digest_cache LSM is heavily based on the LSM
> infrastructure:

I understand that, but as I said previously, I don't believe that we
want to support a LSM which exists solely to provide a file digest
cache.  LSMs should be based around the idea of some type of access
control, security monitoring, etc.

Including a file digest cache in IMA, or implementing it as a
standalone piece of kernel functionality, are still options.  If you
want to pursue this, I would suggest that including the digest cache
as part of IMA would be the easier of the two options; if it proves to
be generally useful outside of IMA, it can always be abstracted out to
a general kernel module/subsystem.
Roberto Sassu June 19, 2024, 3:55 p.m. UTC | #8
On Wed, 2024-06-19 at 11:49 -0400, Paul Moore wrote:
> On Wed, Jun 19, 2024 at 3:59 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Tue, 2024-06-18 at 19:20 -0400, Paul Moore wrote:
> > > On Mon, Apr 15, 2024 at 10:25 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > 
> > > > 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.
> > > 
> > > I'm not sure we want to implement a cache as a LSM.  I'm sure it would
> > > work, but historically LSMs have provided some form of access control,
> > > measurement, or other traditional security service.  A digest cache,
> > > while potentially useful for a variety of security related
> > > applications, is not a security service by itself, it is simply a file
> > > digest storage mechanism.
> > 
> > Uhm, currently the digest_cache LSM is heavily based on the LSM
> > infrastructure:
> 
> I understand that, but as I said previously, I don't believe that we
> want to support a LSM which exists solely to provide a file digest
> cache.  LSMs should be based around the idea of some type of access
> control, security monitoring, etc.
> 
> Including a file digest cache in IMA, or implementing it as a
> standalone piece of kernel functionality, are still options.  If you
> want to pursue this, I would suggest that including the digest cache
> as part of IMA would be the easier of the two options; if it proves to
> be generally useful outside of IMA, it can always be abstracted out to
> a general kernel module/subsystem.

Ok. I thought about IPE and eBPF as potential users. But if you think
that adding as part of IMA would be easier, I could try to pursue that.

Thanks

Roberto
Paul Moore June 19, 2024, 4:34 p.m. UTC | #9
On Wed, Jun 19, 2024 at 11:55 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
> On Wed, 2024-06-19 at 11:49 -0400, Paul Moore wrote:
> > On Wed, Jun 19, 2024 at 3:59 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > On Tue, 2024-06-18 at 19:20 -0400, Paul Moore wrote:
> > > > On Mon, Apr 15, 2024 at 10:25 AM Roberto Sassu
> > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > >
> > > > > 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.
> > > >
> > > > I'm not sure we want to implement a cache as a LSM.  I'm sure it would
> > > > work, but historically LSMs have provided some form of access control,
> > > > measurement, or other traditional security service.  A digest cache,
> > > > while potentially useful for a variety of security related
> > > > applications, is not a security service by itself, it is simply a file
> > > > digest storage mechanism.
> > >
> > > Uhm, currently the digest_cache LSM is heavily based on the LSM
> > > infrastructure:
> >
> > I understand that, but as I said previously, I don't believe that we
> > want to support a LSM which exists solely to provide a file digest
> > cache.  LSMs should be based around the idea of some type of access
> > control, security monitoring, etc.
> >
> > Including a file digest cache in IMA, or implementing it as a
> > standalone piece of kernel functionality, are still options.  If you
> > want to pursue this, I would suggest that including the digest cache
> > as part of IMA would be the easier of the two options; if it proves to
> > be generally useful outside of IMA, it can always be abstracted out to
> > a general kernel module/subsystem.
>
> Ok. I thought about IPE and eBPF as potential users. But if you think
> that adding as part of IMA would be easier, I could try to pursue that.

It isn't clear to me how this would interact with IPE and/or eBPF, but
if you believe there is value there I would encourage you to work with
those subsystem maintainers.  If the consensus is that a general file
digest cache is useful then you should pursue the digest cache as a
kernel subsystem, just not a LSM.
Roberto Sassu June 19, 2024, 4:37 p.m. UTC | #10
On Wed, 2024-06-19 at 12:34 -0400, Paul Moore wrote:
> On Wed, Jun 19, 2024 at 11:55 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Wed, 2024-06-19 at 11:49 -0400, Paul Moore wrote:
> > > On Wed, Jun 19, 2024 at 3:59 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > On Tue, 2024-06-18 at 19:20 -0400, Paul Moore wrote:
> > > > > On Mon, Apr 15, 2024 at 10:25 AM Roberto Sassu
> > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > 
> > > > > > 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.
> > > > > 
> > > > > I'm not sure we want to implement a cache as a LSM.  I'm sure it would
> > > > > work, but historically LSMs have provided some form of access control,
> > > > > measurement, or other traditional security service.  A digest cache,
> > > > > while potentially useful for a variety of security related
> > > > > applications, is not a security service by itself, it is simply a file
> > > > > digest storage mechanism.
> > > > 
> > > > Uhm, currently the digest_cache LSM is heavily based on the LSM
> > > > infrastructure:
> > > 
> > > I understand that, but as I said previously, I don't believe that we
> > > want to support a LSM which exists solely to provide a file digest
> > > cache.  LSMs should be based around the idea of some type of access
> > > control, security monitoring, etc.
> > > 
> > > Including a file digest cache in IMA, or implementing it as a
> > > standalone piece of kernel functionality, are still options.  If you
> > > want to pursue this, I would suggest that including the digest cache
> > > as part of IMA would be the easier of the two options; if it proves to
> > > be generally useful outside of IMA, it can always be abstracted out to
> > > a general kernel module/subsystem.
> > 
> > Ok. I thought about IPE and eBPF as potential users. But if you think
> > that adding as part of IMA would be easier, I could try to pursue that.
> 
> It isn't clear to me how this would interact with IPE and/or eBPF, but
> if you believe there is value there I would encourage you to work with
> those subsystem maintainers.  If the consensus is that a general file
> digest cache is useful then you should pursue the digest cache as a
> kernel subsystem, just not a LSM.

Making it a kernel subsystem would likely mean replicating what the LSM
infrastructure is doing, inode (security) blob and being notified about
file/directory changes.

I guess I will go for the IMA route...

Roberto
Paul Moore June 19, 2024, 6:43 p.m. UTC | #11
On Wed, Jun 19, 2024 at 12:38 PM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> Making it a kernel subsystem would likely mean replicating what the LSM
> infrastructure is doing, inode (security) blob and being notified about
> file/directory changes.

Just because the LSM framework can be used for something, perhaps it
even makes the implementation easier, it doesn't mean the framework
should be used for everything.
Roberto Sassu June 20, 2024, 9:16 a.m. UTC | #12
On Thu, 2024-06-20 at 11:12 +0200, Roberto Sassu wrote:
> On Wed, 2024-06-19 at 14:43 -0400, Paul Moore wrote:
> > On Wed, Jun 19, 2024 at 12:38 PM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > 
> > > Making it a kernel subsystem would likely mean replicating what the LSM
> > > infrastructure is doing, inode (security) blob and being notified about
> > > file/directory changes.
> > 
> > Just because the LSM framework can be used for something, perhaps it
> > even makes the implementation easier, it doesn't mean the framework
> > should be used for everything.
> 
> It is supporting 3 LSMs: IMA, IPE and BPF LSM.
> 
> That makes it a clear target for the security subsystem, and as you
> suggested to start for IMA, if other kernel subsystems require them, we

If other kernel subsystems require the same functionality,

Roberto

> can make it as an independent subsystem.
> 
> Starting from IMA means that we are mixing two different things in the
> inode security blob, and I'm not sure that it is more straightforward
> than making the digest_cache LSM require the space it needs and be
> notified about security events.
> 
> Thanks
> 
> Roberto
>
Roberto Sassu June 20, 2024, 3:14 p.m. UTC | #13
On Thu, 2024-06-20 at 10:48 -0400, Paul Moore wrote:
> On Thu, Jun 20, 2024 at 5:12 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Wed, 2024-06-19 at 14:43 -0400, Paul Moore wrote:
> > > On Wed, Jun 19, 2024 at 12:38 PM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > 
> > > > Making it a kernel subsystem would likely mean replicating what the LSM
> > > > infrastructure is doing, inode (security) blob and being notified about
> > > > file/directory changes.
> > > 
> > > Just because the LSM framework can be used for something, perhaps it
> > > even makes the implementation easier, it doesn't mean the framework
> > > should be used for everything.
> > 
> > It is supporting 3 LSMs: IMA, IPE and BPF LSM.
> > 
> > That makes it a clear target for the security subsystem, and as you
> > suggested to start for IMA, if other kernel subsystems require them, we
> > can make it as an independent subsystem.
> 
> Have you discussed the file digest cache functionality with either the
> IPE or BPF LSM maintainers?  While digest_cache may support these

Well, yes. I was in a discussion since long time ago with Deven and
Fan. The digest_cache LSM is listed in the Use Case section of the IPE
cover letter:

https://lore.kernel.org/linux-integrity/1716583609-21790-1-git-send-email-wufan@linux.microsoft.com/

I also developed an IPE module back in the DIGLIM days:

https://lore.kernel.org/linux-integrity/a16a628b9e21433198c490500a987121@huawei.com/

As for eBPF, I just need to make the digest_cache LSM API callable by
eBPF programs, very likely not requiring any change on the eBPF
infrastructure itself. As an example of the modification needed, you
could have a look at:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/trace/bpf_trace.c?h=v6.10-rc4#n1381


Once the digest_cache LSM API is exposed in eBPF, you could write a
simple file integrity check (taken from my DIGLIM eBPF), not tested:

SEC("lsm.s/bprm_creds_for_exec")
int BPF_PROG(exec, struct linux_binprm *bprm)
{
	u8 digest[MAX_DIGEST_SIZE] = { 0 };
	digest_cache_found_t found;
	struct digest_cache;
	int algo;

	algo = bpf_ima_file_hash(bprm->file, digest, sizeof(digest));
	if (algo < 0)
		return -EPERM;

	digest_cache = bpf_digest_cache_get(bprm->file->f_path.dentry);
	if (!digest_cache)
		return -EPERM;

	found = bpf_digest_cache_lookup(bprm->file->f_path.dentry,
					digest_cache, digest, algo);

	bpf_digest_cache_put(digest_cache);
	return found ? 0 : -EPERM;
}

Roberto

> LSMs, I don't recall seeing any comments from the other LSM
> developers; if you are going to advocate for this as something outside
> of IMA, it would be good to see a show of support for the other LSMs.
>
Roberto Sassu June 20, 2024, 4:54 p.m. UTC | #14
On Thu, 2024-06-20 at 11:32 -0500, Dr. Greg wrote:
> On Wed, Jun 19, 2024 at 06:37:49PM +0200, Roberto Sassu wrote:
> 
> Good morning Roberto, I hope your week is going well, greetings to
> everyone copied else as well.
> 
> > On Wed, 2024-06-19 at 12:34 -0400, Paul Moore wrote:
> > > On Wed, Jun 19, 2024 at 11:55???AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > On Wed, 2024-06-19 at 11:49 -0400, Paul Moore wrote:
> > > > > On Wed, Jun 19, 2024 at 3:59???AM Roberto Sassu
> > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > On Tue, 2024-06-18 at 19:20 -0400, Paul Moore wrote:
> > > > > > > On Mon, Apr 15, 2024 at 10:25???AM Roberto Sassu
> > > > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > 
> > > > > > > > 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.
> > > > > > > 
> > > > > > > I'm not sure we want to implement a cache as a LSM.  I'm sure it would
> > > > > > > work, but historically LSMs have provided some form of access control,
> > > > > > > measurement, or other traditional security service.  A digest cache,
> > > > > > > while potentially useful for a variety of security related
> > > > > > > applications, is not a security service by itself, it is simply a file
> > > > > > > digest storage mechanism.
> > > > > > 
> > > > > > Uhm, currently the digest_cache LSM is heavily based on the LSM
> > > > > > infrastructure:
> > > > > 
> > > > > I understand that, but as I said previously, I don't believe that we
> > > > > want to support a LSM which exists solely to provide a file digest
> > > > > cache.  LSMs should be based around the idea of some type of access
> > > > > control, security monitoring, etc.
> > > > > 
> > > > > Including a file digest cache in IMA, or implementing it as a
> > > > > standalone piece of kernel functionality, are still options.  If you
> > > > > want to pursue this, I would suggest that including the digest cache
> > > > > as part of IMA would be the easier of the two options; if it proves to
> > > > > be generally useful outside of IMA, it can always be abstracted out to
> > > > > a general kernel module/subsystem.
> > > > 
> > > > Ok. I thought about IPE and eBPF as potential users. But if you think
> > > > that adding as part of IMA would be easier, I could try to pursue that.
> > > 
> > > It isn't clear to me how this would interact with IPE and/or eBPF, but
> > > if you believe there is value there I would encourage you to work with
> > > those subsystem maintainers.  If the consensus is that a general file
> > > digest cache is useful then you should pursue the digest cache as a
> > > kernel subsystem, just not a LSM.
> 
> > Making it a kernel subsystem would likely mean replicating what the LSM
> > infrastructure is doing, inode (security) blob and being notified about
> > file/directory changes.
> > 
> > I guess I will go for the IMA route...
> 
> This thread brings up an issue that we have been thinking about but
> has been on the back burner.
> 
> Roberto, I'm assuming you have seen our TSEM submissions go by.  Our
> V4 release will be immediately after the Fourth of July holiday week
> here in the states.
> 
> Since TSEM implements a generic security modeling framework for the
> kernel, it ends up implementing a superset of IMA functionality.  That
> required us to implement our own file digest generation and cacheing
> infrastructure.
> 
> Given the trajectory that things are on with respect to security,
> there is only going to be more demand for file digests and their
> associated cacheing.  Doesn't seem like it makes a lot of sense to
> have multiple teams replicating what is largely the same
> functionality.
> 
> If your group would have interest, we would certainly be willing to
> entertain conversations on how we could collaborate to brew up
> something that would be of mutual benefit to everyone who has a need
> for this type of infrastructure.

Hi Greg

sure, I would be happy to give you more details on how the digest_cache
LSM works and how you could make use of it.

> As you noted, consumers of the BPF LSM would also be a clear candidate
> for generic infrastructure.  One of the issues blocking a BPF based
> integrity implementation is that BPF itself is not going to be able
> generate digests on its own.  So it would seem to make sense to have
> whatever gets built have a kfunc accessible API.  Plenty of other
> additional warts on that front as well but getting access to digests
> is the necessary starting point.

Yes, adding a few kfuncs is what I had in mind.

> Given what we have seen with IMA's challenge with respect to overlayfs
> issues and file versioning issues in general, it would seem to be of
> profit to have all these issues addressed uniformally and in one
> place.
> 
> Since virtually everything that is accessing this infrastructure is
> going to be an LSM, we would envision API's out of a common
> infrastructure, invoked by the event handlers of the various LSM's
> interested in integrity information, driving the cache generation and
> maintenance.  That would seem to have all of the benefits of being
> implemented by the LSM infrastructure without necessarily being an
> 'LSM' in and of itself.

Yes, this is exactly how it works. There is a generic API for users to
get a digest cache and query it. The LSM infrastructure is needed for
attaching data to an inode and for being notified of file backend
changes.

The digest_cache LSM makes it transparent for its users the process of
retrieving the reference digest for a given file whose integrity should
be checked, and allows those users to simply query the calculated file
digest.

> We assume that everyone would want to maintain the O(1) lookup
> characteristics of what the LSM inode blob mechanism provides.  We
> would presume that a common cacheing architecture would return a
> pointer to the structure that the digest cache maintains describing
> the various digests associated with the contents of a file, as there
> will be a need for multiple digest support, when an LSM hands the
> cache an inode referencing a file.  An LSM could then place that
> pointer in its own inode blob for future reference.

That was what initially thought and implemented. But I realized that
pinning the digest cache to other inode security blobs makes it more
difficult to free the digest cache (when its reference count goes to
zero).

I opted instead for releasing a digest cache when not in use, and for
introducing a notification mechanism, similar to what LSMs use to
notify about policy changes, which reports when the file backend
changes, so that LSMs can invalidate their decision based on the
affected digest cache. This mechanism is already working in IMA:

https://lore.kernel.org/linux-integrity/20240415161044.2572438-10-roberto.sassu@huaweicloud.com/

> Either that, probably better, stick a pointer into the inode structure
> itself that references it's digest cache object and it would get
> populated by the first event that opens the associated file.

Yes, the digest cache pointer is stored both in the inode that should
be verified with the digest cache, and in the inode the digest cache
was created from. The first reference avoids retrieving the link file-
package every time a digest cache is requested for that inode.

> > Roberto
> 
> So an open invitation to anyone that would want to discuss
> requirements around a common implementation.
> 
> Have a good weekend.

Thanks, the same to you.

Roberto

> As always,
> Dr. Greg
> 
> The Quixote Project - Flailing at the Travails of Cybersecurity
>               https://github.com/Quixote-Project
Paul Moore June 20, 2024, 5:13 p.m. UTC | #15
On Thu, Jun 20, 2024 at 1:06 PM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
> On Thu, 2024-06-20 at 12:51 -0400, Paul Moore wrote:
> > On Thu, Jun 20, 2024 at 12:31 PM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > On Thu, 2024-06-20 at 12:08 -0400, Paul Moore wrote:
> > > > On Thu, Jun 20, 2024 at 11:14 AM Roberto Sassu
> > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > On Thu, 2024-06-20 at 10:48 -0400, Paul Moore wrote:
> > > > > > On Thu, Jun 20, 2024 at 5:12 AM Roberto Sassu
> > > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > On Wed, 2024-06-19 at 14:43 -0400, Paul Moore wrote:
> > > > > > > > On Wed, Jun 19, 2024 at 12:38 PM Roberto Sassu
> > > > > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > >
> > > > > > > > > Making it a kernel subsystem would likely mean replicating what the LSM
> > > > > > > > > infrastructure is doing, inode (security) blob and being notified about
> > > > > > > > > file/directory changes.
> > > > > > > >
> > > > > > > > Just because the LSM framework can be used for something, perhaps it
> > > > > > > > even makes the implementation easier, it doesn't mean the framework
> > > > > > > > should be used for everything.
> > > > > > >
> > > > > > > It is supporting 3 LSMs: IMA, IPE and BPF LSM.
> > > > > > >
> > > > > > > That makes it a clear target for the security subsystem, and as you
> > > > > > > suggested to start for IMA, if other kernel subsystems require them, we
> > > > > > > can make it as an independent subsystem.
> > > > > >
> > > > > > Have you discussed the file digest cache functionality with either the
> > > > > > IPE or BPF LSM maintainers?  While digest_cache may support these
> > > > >
> > > > > Well, yes. I was in a discussion since long time ago with Deven and
> > > > > Fan. The digest_cache LSM is listed in the Use Case section of the IPE
> > > > > cover letter:
> > > > >
> > > > > https://lore.kernel.org/linux-integrity/1716583609-21790-1-git-send-email-wufan@linux.microsoft.com/
> > > >
> > > > I would hope to see more than one sentence casually mentioning that
> > > > there might be some integration in the future.
> > >
> > > Sure, I can work more with Fan to do a proper integration.
> >
> > That seems like a good pre-requisite for turning digest_cache into a
> > general purpose subsystem.
> >
> > > > > I also developed an IPE module back in the DIGLIM days:
> > > > >
> > > > > https://lore.kernel.org/linux-integrity/a16a628b9e21433198c490500a987121@huawei.com/
> > > >
> > > > That looks like more of an fs-verity integration to me.  Yes, of
> > > > course there would be IPE changes to accept a signature/digest from a
> > > > digest cache, but that should be minor.
> > >
> > > True, but IPE will also benefit from not needing to specify every
> > > digest in the policy.
> >
> > Sure, but that isn't really that important from a code integration
> > perspective, that's an admin policy issue.  I expect there would be
> > much more integration work with fs-verity than with IPE, and I think
> > the fs-verity related work might be a challenge.
>
> Uhm, not sure what you mean, but I don't plan to touch fsverity. There
> was already work to get the fsverity digest. All I would need to do
> from my side is to request a digest cache for the inode being verified
> by IPE and to query the fsverity digest.

So your proposed file digest cache wouldn't be used as a replacement
for the fs-verity digest?  Hmm.  I'll leave this up to you and Fan
(current IPE maintainer), but I'm not sure how much value this would
have for IPE, especially since I believe IPE's fs-verity support is
mostly around fs-verity signatures.

> Of course IPE should also capture kernel reads and verify the file
> containing the reference digests, used to build the digest cache.
>
> > > Also, the design choice of attaching the digest cache to the inode
> > > helps LSMs like IPE that don't have a per inode cache on their own.
> > > Sure, IPE would have to do a digest lookup every time, but at least on
> > > an already populated hash table.
> >
> > Just because you need to attach some state to an inode does not mean a
> > file digest cache must be a LSM.  It could be integrated into the VFS
> > or it could be a separate subsystem; either way it could provide an
> > API (either through well defined data structures or functions) that
> > could be used by various LSMs and filesystems that provide integrity
> > protection.
>
> Given that IMA solved the same problem after 15 years, when it became
> an LSM, I'm not super optimistic on that. But if VFS people or other
> subsystem maintainers would be open for such alternative, I can give it
> a try.

I think you should, because I'm not currently supportive of
digest_cache as a standalone LSM.
Roberto Sassu June 21, 2024, 7:10 a.m. UTC | #16
On Thu, 2024-06-20 at 13:13 -0400, Paul Moore wrote:
> On Thu, Jun 20, 2024 at 1:06 PM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Thu, 2024-06-20 at 12:51 -0400, Paul Moore wrote:
> > > On Thu, Jun 20, 2024 at 12:31 PM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > On Thu, 2024-06-20 at 12:08 -0400, Paul Moore wrote:
> > > > > On Thu, Jun 20, 2024 at 11:14 AM Roberto Sassu
> > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > On Thu, 2024-06-20 at 10:48 -0400, Paul Moore wrote:
> > > > > > > On Thu, Jun 20, 2024 at 5:12 AM Roberto Sassu
> > > > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > On Wed, 2024-06-19 at 14:43 -0400, Paul Moore wrote:
> > > > > > > > > On Wed, Jun 19, 2024 at 12:38 PM Roberto Sassu
> > > > > > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > > > > > 
> > > > > > > > > > Making it a kernel subsystem would likely mean replicating what the LSM
> > > > > > > > > > infrastructure is doing, inode (security) blob and being notified about
> > > > > > > > > > file/directory changes.
> > > > > > > > > 
> > > > > > > > > Just because the LSM framework can be used for something, perhaps it
> > > > > > > > > even makes the implementation easier, it doesn't mean the framework
> > > > > > > > > should be used for everything.
> > > > > > > > 
> > > > > > > > It is supporting 3 LSMs: IMA, IPE and BPF LSM.
> > > > > > > > 
> > > > > > > > That makes it a clear target for the security subsystem, and as you
> > > > > > > > suggested to start for IMA, if other kernel subsystems require them, we
> > > > > > > > can make it as an independent subsystem.
> > > > > > > 
> > > > > > > Have you discussed the file digest cache functionality with either the
> > > > > > > IPE or BPF LSM maintainers?  While digest_cache may support these
> > > > > > 
> > > > > > Well, yes. I was in a discussion since long time ago with Deven and
> > > > > > Fan. The digest_cache LSM is listed in the Use Case section of the IPE
> > > > > > cover letter:
> > > > > > 
> > > > > > https://lore.kernel.org/linux-integrity/1716583609-21790-1-git-send-email-wufan@linux.microsoft.com/
> > > > > 
> > > > > I would hope to see more than one sentence casually mentioning that
> > > > > there might be some integration in the future.
> > > > 
> > > > Sure, I can work more with Fan to do a proper integration.
> > > 
> > > That seems like a good pre-requisite for turning digest_cache into a
> > > general purpose subsystem.
> > > 
> > > > > > I also developed an IPE module back in the DIGLIM days:
> > > > > > 
> > > > > > https://lore.kernel.org/linux-integrity/a16a628b9e21433198c490500a987121@huawei.com/
> > > > > 
> > > > > That looks like more of an fs-verity integration to me.  Yes, of
> > > > > course there would be IPE changes to accept a signature/digest from a
> > > > > digest cache, but that should be minor.
> > > > 
> > > > True, but IPE will also benefit from not needing to specify every
> > > > digest in the policy.
> > > 
> > > Sure, but that isn't really that important from a code integration
> > > perspective, that's an admin policy issue.  I expect there would be
> > > much more integration work with fs-verity than with IPE, and I think
> > > the fs-verity related work might be a challenge.
> > 
> > Uhm, not sure what you mean, but I don't plan to touch fsverity. There
> > was already work to get the fsverity digest. All I would need to do
> > from my side is to request a digest cache for the inode being verified
> > by IPE and to query the fsverity digest.
> 
> So your proposed file digest cache wouldn't be used as a replacement
> for the fs-verity digest?  Hmm.  I'll leave this up to you and Fan
> (current IPE maintainer), but I'm not sure how much value this would
> have for IPE, especially since I believe IPE's fs-verity support is
> mostly around fs-verity signatures.

The digest_cache (LSM) would store fsverity digests in the hash table.
At the moment, there is no available source for those, but they could
be included in RPM headers like regular digests.

IPE will compare the digest from fsverity with the digest queried on
the digest cache.

Roberto

> > Of course IPE should also capture kernel reads and verify the file
> > containing the reference digests, used to build the digest cache.
> > 
> > > > Also, the design choice of attaching the digest cache to the inode
> > > > helps LSMs like IPE that don't have a per inode cache on their own.
> > > > Sure, IPE would have to do a digest lookup every time, but at least on
> > > > an already populated hash table.
> > > 
> > > Just because you need to attach some state to an inode does not mean a
> > > file digest cache must be a LSM.  It could be integrated into the VFS
> > > or it could be a separate subsystem; either way it could provide an
> > > API (either through well defined data structures or functions) that
> > > could be used by various LSMs and filesystems that provide integrity
> > > protection.
> > 
> > Given that IMA solved the same problem after 15 years, when it became
> > an LSM, I'm not super optimistic on that. But if VFS people or other
> > subsystem maintainers would be open for such alternative, I can give it
> > a try.
> 
> I think you should, because I'm not currently supportive of
> digest_cache as a standalone LSM.
>