Re: [PATCH v38 39/39] LSM: Create lsm_module_list system call

[Casey Schaufler <casey@schaufler-ca.com>]

On 10/12/2022 2:19 PM, Mickaël Salaün wrote:
>
> On 27/09/2022 22:31, Casey Schaufler wrote:
>> Create a system call to report the list of Linux Security Modules
>> that are active on the system. The list is provided as an array
>> of LSM ID numbers.
>
> With lsm_self_attr(), this would look like a dir/file structure.

I'm not sure I understand what you mean by this. I think you're suggesting
that lsm_module_list() shows the list of possibilities and lsm_self_attr()
shows the data. That's roughly correct. Note that many security modules
will never provide any data in lsm_self_attr(), and that others will
only provide it when it has been explicitly set. 

>
> Would it be useful for user space to list all the currently used LSMs
> instead of only retrieving information about a known (list of) LSM?

I believe so. User space tends to lag behind kernel features. ps(1) can report
the "current" value for any LSM without knowing which module supplied the value
today by using the /proc/self/attr/current interface. id(1) could do the same
were it not unnecessarily coded to be SELinux specific. lsm_module_list(2), like
the existing /sys/kernel/security/lsm interface, allows an application to know
if it should address the modules it knows about. It also provides the LSM order,
which could be significant to systemd, dbus, auditd or other sophisticated system
services.


> What is the use case for this syscall?

1. Identify if a specific LSM is in the list.
	Used by ps(1) to format the -Z output correctly
	Used by systemd(1) to determine which service start options to support
	Used by dbus to identify what policy to enforce at runtime
2. Identify the order of LSMs in the list.
	Will AppArmor data show up in /proc/self/attr/current, or will Smack data?

>
>
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>   arch/x86/entry/syscalls/syscall_64.tbl |  1 +
>>   include/linux/syscalls.h               |  1 +
>>   include/uapi/asm-generic/unistd.h      |  5 ++-
>>   kernel/sys_ni.c                        |  1 +
>>   security/lsm_syscalls.c                | 50 ++++++++++++++++++++++++++
>>   5 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl
>> b/arch/x86/entry/syscalls/syscall_64.tbl
>> index 56d5c5202fd0..40b35e7069a7 100644
>> --- a/arch/x86/entry/syscalls/syscall_64.tbl
>> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
>> @@ -373,6 +373,7 @@
>>   449    common    futex_waitv        sys_futex_waitv
>>   450    common    set_mempolicy_home_node   
>> sys_set_mempolicy_home_node
>>   451    common    lsm_self_attr        sys_lsm_self_attr
>> +452    common    lsm_module_list        sys_lsm_module_list
>
> As for the other syscall, this should also be in the same dedicated
> "wire syscalls" patch.
>
>
>>     #
>>   # Due to a historical design error, certain syscalls are numbered
>> differently
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 7f87ef8be546..e2e2a9e93e8c 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -1057,6 +1057,7 @@ asmlinkage long
>> sys_set_mempolicy_home_node(unsigned long start, unsigned long l
>>                           unsigned long home_node,
>>                           unsigned long flags);
>>   asmlinkage long sys_lsm_self_attr(struct lsm_ctx *ctx, size_t
>> *size, int flags);
>> +asmlinkage long sys_lsm_module_list(unsigned int *ids, size_t *size,
>> int flags);
>>     /*
>>    * Architecture-specific system calls
>> diff --git a/include/uapi/asm-generic/unistd.h
>> b/include/uapi/asm-generic/unistd.h
>> index aa66718e1b48..090617a9a53a 100644
>> --- a/include/uapi/asm-generic/unistd.h
>> +++ b/include/uapi/asm-generic/unistd.h
>> @@ -889,8 +889,11 @@ __SYSCALL(__NR_set_mempolicy_home_node,
>> sys_set_mempolicy_home_node)
>>   #define __NR_lsm_self_attr 451
>>   __SYSCALL(__NR_lsm_self_attr, sys_lsm_self_attr)
>>   +#define __NR_lsm_module_list 452
>> +__SYSCALL(__NR_lsm_module_list, sys_lsm_module_list)
>> +
>>   #undef __NR_syscalls
>> -#define __NR_syscalls 452
>> +#define __NR_syscalls 453
>
> Same here.
>
>
>>     /*
>>    * 32 bit systems traditionally used different
>> diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
>> index 0fdb0341251d..bde9e74a3473 100644
>> --- a/kernel/sys_ni.c
>> +++ b/kernel/sys_ni.c
>> @@ -264,6 +264,7 @@ COND_SYSCALL(mremap);
>>     /* security/lsm_syscalls.c */
>>   COND_SYSCALL(lsm_self_attr);
>> +COND_SYSCALL(lsm_module_list);
>>     /* security/keys/keyctl.c */
>>   COND_SYSCALL(add_key);
>> diff --git a/security/lsm_syscalls.c b/security/lsm_syscalls.c
>> index da0fab7065e2..41d9ef945ede 100644
>> --- a/security/lsm_syscalls.c
>> +++ b/security/lsm_syscalls.c
>> @@ -154,3 +154,53 @@ SYSCALL_DEFINE3(lsm_self_attr,
>>       kfree(final);
>>       return rc;
>>   }
>> +
>> +/**
>> + * lsm_module_list - Return a list of the active security modules
>> + * @ids: the LSM module ids
>> + * @size: size of @ids, updated on return
>> + * @flags: reserved for future use, must be zero
>> + *
>> + * Returns a list of the active LSM ids. On success this function
>> + * returns the number of @ids array elements. This value may be zero
>> + * if there are no LSMs active. If @size is insufficient to contain
>> + * the return data -E2BIG is returned and @size is set to the minimum
>> + * required size. In all other cases a negative value indicating the
>> + * error is returned.
>> + */
>> +SYSCALL_DEFINE3(lsm_module_list,
>> +           unsigned int __user *, ids,
>> +           size_t __user *, size,
>> +           int, flags)
>> +{
>> +    unsigned int *interum;
>> +    size_t total_size = lsm_id * sizeof(*interum);
>> +    size_t usize;
>> +    int rc;
>> +    int i;
>> +
>> +    if (get_user(usize, size))
>> +        return -EFAULT;
>> +
>> +    if (usize < total_size) {
>> +        if (put_user(total_size, size) != 0)
>> +            return -EFAULT;
>> +        return -E2BIG;
>> +    }
>> +
>> +    interum = kzalloc(total_size, GFP_KERNEL);
>> +    if (interum == NULL)
>> +        return -ENOMEM;
>> +
>> +    for (i = 0; i < lsm_id; i++)
>> +        interum[i] = lsm_idlist[i]->id;
>> +
>> +    if (copy_to_user(ids, interum, total_size) != 0 ||
>> +        put_user(total_size, size) != 0)
>> +        rc = -EFAULT;
>> +    else
>> +        rc = lsm_id;
>> +
>> +    kfree(interum);
>> +    return rc;
>> +}