| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0c7a19cb-d270-403f-9f97-354405aba746@schaufler-ca.com>
Date: Wed, 20 Aug 2025 08:30:33 -0700
From: Casey Schaufler <casey@...aufler-ca.com>
To: Mickaël Salaün <mic@...ikod.net>,
Maxime Bélair <maxime.belair@...onical.com>
Cc: linux-security-module@...r.kernel.org, john.johansen@...onical.com,
paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com, kees@...nel.org,
stephen.smalley.work@...il.com, takedakn@...data.co.jp,
penguin-kernel@...ove.sakura.ne.jp, song@...nel.org, rdunlap@...radead.org,
linux-api@...r.kernel.org, apparmor@...ts.ubuntu.com,
linux-kernel@...r.kernel.org, Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH v5 2/3] lsm: introduce security_lsm_config_*_policy hooks
On 8/20/2025 7:21 AM, Mickaël Salaün wrote:
> On Wed, Jul 09, 2025 at 10:00:55AM +0200, Maxime Bélair wrote:
>> Define two new LSM hooks: security_lsm_config_self_policy and
>> security_lsm_config_system_policy and wire them into the corresponding
>> lsm_config_*_policy() syscalls so that LSMs can register a unified
>> interface for policy management. This initial, minimal implementation
>> only supports the LSM_POLICY_LOAD operation to limit changes.
>>
>> Signed-off-by: Maxime Bélair <maxime.belair@...onical.com>
>> ---
>> include/linux/lsm_hook_defs.h | 4 +++
>> include/linux/security.h | 20 ++++++++++++
>> include/uapi/linux/lsm.h | 8 +++++
>> security/lsm_syscalls.c | 17 ++++++++--
>> security/security.c | 60 +++++++++++++++++++++++++++++++++++
>> 5 files changed, 107 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index bf3bbac4e02a..fca490444643 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -464,3 +464,7 @@ LSM_HOOK(int, 0, bdev_alloc_security, struct block_device *bdev)
>> LSM_HOOK(void, LSM_RET_VOID, bdev_free_security, struct block_device *bdev)
>> LSM_HOOK(int, 0, bdev_setintegrity, struct block_device *bdev,
>> enum lsm_integrity_type type, const void *value, size_t size)
>> +LSM_HOOK(int, -EINVAL, lsm_config_self_policy, u32 lsm_id, u32 op,
>> + void __user *buf, size_t size, u32 flags)
>> +LSM_HOOK(int, -EINVAL, lsm_config_system_policy, u32 lsm_id, u32 op,
>> + void __user *buf, size_t size, u32 flags)
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index cc9b54d95d22..54acaee4a994 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -581,6 +581,11 @@ void security_bdev_free(struct block_device *bdev);
>> int security_bdev_setintegrity(struct block_device *bdev,
>> enum lsm_integrity_type type, const void *value,
>> size_t size);
>> +int security_lsm_config_self_policy(u32 lsm_id, u32 op, void __user *buf,
>> + size_t size, u32 flags);
>> +int security_lsm_config_system_policy(u32 lsm_id, u32 op, void __user *buf,
>> + size_t size, u32 flags);
>> +
>> #else /* CONFIG_SECURITY */
>>
>> /**
>> @@ -1603,6 +1608,21 @@ static inline int security_bdev_setintegrity(struct block_device *bdev,
>> return 0;
>> }
>>
>> +static inline int security_lsm_config_self_policy(u32 lsm_id, u32 op,
>> + void __user *buf,
>> + size_t size, u32 flags)
>> +{
>> +
>> + return -EOPNOTSUPP;
>> +}
>> +
>> +static inline int security_lsm_config_system_policy(u32 lsm_id, u32 op,
>> + void __user *buf,
>> + size_t size, u32 flags)
>> +{
>> +
>> + return -EOPNOTSUPP;
>> +}
>> #endif /* CONFIG_SECURITY */
>>
>> #if defined(CONFIG_SECURITY) && defined(CONFIG_WATCH_QUEUE)
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> index 938593dfd5da..2b9432a30cdc 100644
>> --- a/include/uapi/linux/lsm.h
>> +++ b/include/uapi/linux/lsm.h
>> @@ -90,4 +90,12 @@ struct lsm_ctx {
>> */
>> #define LSM_FLAG_SINGLE 0x0001
>>
>> +/*
>> + * LSM_POLICY_XXX definitions identify the different operations
>> + * to configure LSM policies
>> + */
>> +
>> +#define LSM_POLICY_UNDEF 0
>> +#define LSM_POLICY_LOAD 100
> Why the gap between 0 and 100?
It's conventional in LSM syscalls to start identifiers at 100.
No compelling reason other than to appease the LSM maintainer.
>
>> +
>> #endif /* _UAPI_LINUX_LSM_H */
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> index a3cb6dab8102..dd016ba6976c 100644
>> --- a/security/lsm_syscalls.c
>> +++ b/security/lsm_syscalls.c
>> @@ -122,11 +122,24 @@ SYSCALL_DEFINE3(lsm_list_modules, u64 __user *, ids, u32 __user *, size,
>> SYSCALL_DEFINE5(lsm_config_self_policy, u32, lsm_id, u32, op, void __user *,
>> buf, u32 __user *, size, u32, flags)
> Given these are a multiplexor syscalls, I'm wondering if they should not
> have common flags and LSM-specific flags. Alternatively, the op
> argument could also contains some optional flags. In either case, the
> documentation should guide LSM developers for flags that may be shared
> amongst LSMs.
>
> Examples of such flags could be to restrict the whole process instead of
> the calling thread.
>
>> {
>> - return 0;
>> + size_t usize;
>> +
>> + if (get_user(usize, size))
> Size should just be u32, not a pointer.
>
>> + return -EFAULT;
>> +
>> + return security_lsm_config_self_policy(lsm_id, op, buf, usize, flags);
>> }
>>
>> SYSCALL_DEFINE5(lsm_config_system_policy, u32, lsm_id, u32, op, void __user *,
>> buf, u32 __user *, size, u32, flags)
>> {
>> - return 0;
>> + size_t usize;
>> +
>> + if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
> I like this mandatory capability check for this specific syscall. This
> makes the semantic clearer. However, to avoid the superpower of
> CAP_SYS_ADMIN, I'm wondering how we could use the CAP_MAC_ADMIN instead.
> This syscall could require CAP_MAC_ADMIN, and current LSMs (relying on a
> filesystem interface for policy configuration) could also enforce
> CAP_SYS_ADMIN for compatibility reasons.
>
> In fact, this "system" syscall could be a "namespace" syscall, which
> would take a security/LSM namespace file descriptor as argument. If the
> namespace is not the initial namespace, any CAP_SYS_ADMIN implemented by
> current LSMs could be avoided. See
> https://lore.kernel.org/r/CAHC9VhRGMmhxbajwQNfGFy+ZFF1uN=UEBjqQZQ4UBy7yds3eVQ@mail.gmail.com
>
>> +
>> + if (get_user(usize, size))
> ditto
>
>> + return -EFAULT;
>> +
>> + return security_lsm_config_system_policy(lsm_id, op, buf, usize, flags);
>> }
>> diff --git a/security/security.c b/security/security.c
>> index fb57e8fddd91..166d7d9936d0 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -5883,6 +5883,66 @@ int security_bdev_setintegrity(struct block_device *bdev,
>> }
>> EXPORT_SYMBOL(security_bdev_setintegrity);
>>
>> +/**
>> + * security_lsm_config_self_policy() - Configure caller's LSM policies
>> + * @lsm_id: id of the LSM to target
>> + * @op: Operation to perform (one of the LSM_POLICY_XXX values)
>> + * @buf: userspace pointer to policy data
>> + * @size: size of @buf
>> + * @flags: lsm policy configuration flags
>> + *
>> + * Configure the policies of a LSM for the current domain/user. This notably
>> + * allows to update them even when the lsmfs is unavailable or restricted.
>> + * Currently, only LSM_POLICY_LOAD is supported.
>> + *
>> + * Return: Returns 0 on success, error on failure.
>> + */
>> +int security_lsm_config_self_policy(u32 lsm_id, u32 op, void __user *buf,
>> + size_t size, u32 flags)
>> +{
>> + int rc = LSM_RET_DEFAULT(lsm_config_self_policy);
>> + struct lsm_static_call *scall;
>> +
>> + lsm_for_each_hook(scall, lsm_config_self_policy) {
>> + if ((scall->hl->lsmid->id) == lsm_id) {
>> + rc = scall->hl->hook.lsm_config_self_policy(lsm_id, op, buf, size, flags);
> The lsm_id should not be passed to the hook.
>
> The LSM syscall should manage the argument copy and buffer allocation
> instead of duplicating this code in each LSM hook implementation (see
> other LSM syscalls).
>
>> + break;
>> + }
>> + }
>> +
>> + return rc;
>> +}
>> +
>> +/**
>> + * security_lsm_config_system_policy() - Configure system LSM policies
>> + * @lsm_id: id of the lsm to target
>> + * @op: Operation to perform (one of the LSM_POLICY_XXX values)
>> + * @buf: userspace pointer to policy data
>> + * @size: size of @buf
>> + * @flags: lsm policy configuration flags
>> + *
>> + * Configure the policies of a LSM for the whole system. This notably allows
>> + * to update them even when the lsmfs is unavailable or restricted. Currently,
>> + * only LSM_POLICY_LOAD is supported.
>> + *
>> + * Return: Returns 0 on success, error on failure.
>> + */
>> +int security_lsm_config_system_policy(u32 lsm_id, u32 op, void __user *buf,
>> + size_t size, u32 flags)
>> +{
>> + int rc = LSM_RET_DEFAULT(lsm_config_system_policy);
>> + struct lsm_static_call *scall;
>> +
>> + lsm_for_each_hook(scall, lsm_config_system_policy) {
>> + if ((scall->hl->lsmid->id) == lsm_id) {
>> + rc = scall->hl->hook.lsm_config_system_policy(lsm_id, op, buf, size, flags);
> ditto
>
>> + break;
>> + }
>> + }
>> +
>> + return rc;
>> +}
>> +
>> #ifdef CONFIG_PERF_EVENTS
>> /**
>> * security_perf_event_open() - Check if a perf event open is allowed
>> --
>> 2.48.1
>>
>>
Powered by blists - more mailing lists