diff mbox series

selinux: add permission checks for loading other kinds of kernel files

Message ID 20250205205909.19515-1-kippndavis.work@gmx.com (mailing list archive)
State Changes Requested
Delegated to: Paul Moore
Headers show
Series selinux: add permission checks for loading other kinds of kernel files | expand

Commit Message

kippndavis.work@gmx.com Feb. 5, 2025, 8:59 p.m. UTC 
From: "Kipp N. Davis" <kippndavis.work@gmx.com>

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@gmail.com>
Signed-off-by: Kipp N. Davis <kippndavis.work@gmx.com>
---
 security/selinux/hooks.c            | 54 ++++++++++++++++++++++++-----
 security/selinux/include/classmap.h |  4 ++-
 2 files changed, 49 insertions(+), 9 deletions(-)

--
2.48.1

Comments

Paul Moore Feb. 7, 2025, 9:08 p.m. UTC | #1 
On Feb  5, 2025 kippndavis.work@gmx.com wrote:
> 
> 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@gmail.com>
> Signed-off-by: Kipp N. Davis <kippndavis.work@gmx.com>
> ---
>  security/selinux/hooks.c            | 54 ++++++++++++++++++++++++-----
>  security/selinux/include/classmap.h |  4 ++-
>  2 files changed, 49 insertions(+), 9 deletions(-)

Thanks for putting this patch together, and double thank you for the
tests too!  If you've got the time, it would be great if you could
submit a patch/PR to update notebook too:

 * https://github.com/SELinuxProject/selinux-notebook

Some small comments below ...
 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7b867dfec88b..e96ade50c137 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4133,10 +4130,33 @@ static int selinux_kernel_read_file(struct file *file,
>  	int rc = 0;
> 
>  	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:
> +		pr_err("SELinux:  kernel_read_file_id unknown");
> +		rc = -EACCES;

If we can't come up with a way to catch new READING_XXX entries at
compile time, e.g. socket address families or capabilities (grep for
"#error" in security/selinux/hooks.c), I'm not sure we want to return an
error here, both because we haven't previously and it ignores the
loaded policy's UNK_PERMS setting.

This also applies to selinux_kernel_load_data().

>  		break;
>  	}

--
paul-moore.com
diff mbox series

Patch

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7b867dfec88b..e96ade50c137 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,
@@ -4133,10 +4130,33 @@  static int selinux_kernel_read_file(struct file *file,
 	int rc = 0;

 	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:
+		pr_err("SELinux:  kernel_read_file_id unknown");
+		rc = -EACCES;
 		break;
 	}

@@ -4148,10 +4168,28 @@  static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
 	int rc = 0;

 	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:
+		pr_err("SELinux:  kernel_read_file_id unknown");
+		rc = -EACCES;
 		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",