On Tue, Feb 11, 2025 at 1:22 PM <kippndavis.work@xxxxxxx> wrote:
>
> From: "Kipp N. Davis" <kippndavis.work@xxxxxxx>
>
> Although the LSM hooks for loading kernel modules were later generalized
> to cover loading other kinds of files, SELinux didn't implement
> corresponding permission checks, leaving only the module case covered.
> Define and add new permission checks for these other cases.
>
> Signed-off-by: Cameron K. Williams <ckwilliams.work@xxxxxxxxx>
> Signed-off-by: Kipp N. Davis <kippndavis.work@xxxxxxx>
Acked-by: Stephen Smalley <stephen.smalley.work@xxxxxxxxx>
>
> ---
> Changes in v2:
> - Removed the `-EACCES` return in default case in
> selinux_kernel_read_file() and selinux_kernel_load_from_file(),
> reverting previous fallback behavior.
> - Added a compile-time check in these functions to catch new
> READING/LOADING_XXX entries.
>
> Thanks for your review! I've adjusted the default case so as to not
> return an error and depart from pre-existing logic. I first tried to use
> the pre-processor #errors but failed with the READING/LOADING_MAX_ID
> enums, so I opted for BUILD_BUG_ON_MSG as a compile-time check with
> those same enums instead to catch new entries.
> ---
> security/selinux/hooks.c | 56 +++++++++++++++++++++++------
> security/selinux/include/classmap.h | 4 ++-
> 2 files changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7b867dfec88b..67bf37693cd7 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4096,7 +4096,7 @@ static int selinux_kernel_module_request(char *kmod_name)
> SYSTEM__MODULE_REQUEST, &ad);
> }
>
> -static int selinux_kernel_module_from_file(struct file *file)
> +static int selinux_kernel_load_from_file(struct file *file, u32 requested)
> {
> struct common_audit_data ad;
> struct inode_security_struct *isec;
> @@ -4104,12 +4104,9 @@ static int selinux_kernel_module_from_file(struct file *file)
> u32 sid = current_sid();
> int rc;
>
> - /* init_module */
> if (file == NULL)
> return avc_has_perm(sid, sid, SECCLASS_SYSTEM,
> - SYSTEM__MODULE_LOAD, NULL);
> -
> - /* finit_module */
> + requested, NULL);
>
> ad.type = LSM_AUDIT_DATA_FILE;
> ad.u.file = file;
> @@ -4123,7 +4120,7 @@ static int selinux_kernel_module_from_file(struct file *file)
>
> isec = inode_security(file_inode(file));
> return avc_has_perm(sid, isec->sid, SECCLASS_SYSTEM,
> - SYSTEM__MODULE_LOAD, &ad);
> + requested, &ad);
> }
>
> static int selinux_kernel_read_file(struct file *file,
> @@ -4131,10 +4128,32 @@ static int selinux_kernel_read_file(struct file *file,
> bool contents)
> {
> int rc = 0;
> -
> + BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
> + "New kernel_read_file_id introduced; update SELinux!");
> switch (id) {
> + case READING_FIRMWARE:
> + rc = selinux_kernel_load_from_file(contents ? file : NULL,
> + SYSTEM__FIRMWARE_LOAD);
> + break;
> case READING_MODULE:
> - rc = selinux_kernel_module_from_file(contents ? file : NULL);
> + rc = selinux_kernel_load_from_file(contents ? file : NULL,
> + SYSTEM__MODULE_LOAD);
> + break;
> + case READING_KEXEC_IMAGE:
> + rc = selinux_kernel_load_from_file(contents ? file : NULL,
> + SYSTEM__KEXEC_IMAGE_LOAD);
> + break;
> + case READING_KEXEC_INITRAMFS:
> + rc = selinux_kernel_load_from_file(contents ? file : NULL,
> + SYSTEM__KEXEC_INITRAMFS_LOAD);
> + break;
> + case READING_POLICY:
> + rc = selinux_kernel_load_from_file(contents ? file : NULL,
> + SYSTEM__POLICY_LOAD);
> + break;
> + case READING_X509_CERTIFICATE:
> + rc = selinux_kernel_load_from_file(contents ? file : NULL,
> + SYSTEM__X509_CERTIFICATE_LOAD);
> break;
> default:
> break;
> @@ -4146,10 +4165,27 @@ static int selinux_kernel_read_file(struct file *file,
> static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
> {
> int rc = 0;
> -
> + BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
> + "New kernel_load_data_id introduced; update SELinux!");
> switch (id) {
> + case LOADING_FIRMWARE:
> + rc = selinux_kernel_load_from_file(NULL, SYSTEM__FIRMWARE_LOAD);
> + break;
> case LOADING_MODULE:
> - rc = selinux_kernel_module_from_file(NULL);
> + rc = selinux_kernel_load_from_file(NULL, SYSTEM__MODULE_LOAD);
> + break;
> + case LOADING_KEXEC_IMAGE:
> + rc = selinux_kernel_load_from_file(NULL, SYSTEM__KEXEC_IMAGE_LOAD);
> + break;
> + case LOADING_KEXEC_INITRAMFS:
> + rc = selinux_kernel_load_from_file(NULL, SYSTEM__KEXEC_INITRAMFS_LOAD);
> + break;
> + case LOADING_POLICY:
> + rc = selinux_kernel_load_from_file(NULL, SYSTEM__POLICY_LOAD);
> + break;
> + case LOADING_X509_CERTIFICATE:
> + rc = selinux_kernel_load_from_file(NULL,
> + SYSTEM__X509_CERTIFICATE_LOAD);
> break;
> default:
> break;
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index 03e82477dce9..cfac41d12f7d 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -63,7 +63,9 @@ const struct security_class_mapping secclass_map[] = {
> { "process2", { "nnp_transition", "nosuid_transition", NULL } },
> { "system",
> { "ipc_info", "syslog_read", "syslog_mod", "syslog_console",
> - "module_request", "module_load", NULL } },
> + "module_request", "module_load", "firmware_load",
> + "kexec_image_load", "kexec_initramfs_load", "policy_load",
> + "x509_certificate_load", NULL } },
> { "capability", { COMMON_CAP_PERMS, NULL } },
> { "filesystem",
> { "mount", "remount", "unmount", "getattr", "relabelfrom",
> --
> 2.48.1
>
