[PATCH v3 0/3][RESEND] modsign enhancement

[PATCH v3 0/3][RESEND] modsign enhancement

[Jia Zhang]

This patch series allows to disable module validity enforcement
in runtime through the control switch located in securityfs.

In order to keep /sys/module/module/parameters/sig_enforce simple,
the disablement switch is located at
/sys/kernel/security/modsign/disable_enforce.

Assuming CONFIG_MODULE_SIG_FORCE=n, here are the instructions to
test this control switch.

# cat /sys/module/module/parameters/sig_enforce
N
# echo 1 > /sys/module/module/parameters/sig_enforce
# cat /sys/module/module/parameters/sig_enforce
Y
# echo -n 0 > no_sig_enforce
# openssl smime -sign -nocerts -noattr -binary -in no_sig_enforce \
    -inkey <system_trusted_key> -signer <cert> -outform der \
    -out /sys/kernel/security/modsign/disable_enforce
# cat /sys/module/module/parameters/sig_enforce
N

Changelog:
v3:
- The control switch now doesn't support showing the status of sig_enforce.

v2:
- Support to disable validity enforcement in runtime.

[PATCH 1/3] module: Do not access sig_enforce directly

[Jia Zhang]

Call is_module_sig_enforced() instead.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 kernel/module.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index ad2d420..003d0ab 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2789,7 +2789,7 @@ static int module_sig_check(struct load_info *info, int flags)
 	}
 
 	/* Not having a signature is only an error if we're strict. */
-	if (err == -ENOKEY && !sig_enforce)
+	if (err == -ENOKEY && !is_module_sig_enforced())
 		err = 0;
 
 	return err;
-- 
1.8.3.1

[PATCH 2/3] module: Create the entry point initialize_module()

[Jia Zhang]

This entry point currently includes the procfs initialization,
and will include a securityfs initialization.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 kernel/module.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/kernel/module.c b/kernel/module.c
index 003d0ab..79825ea 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -4243,7 +4243,11 @@ static int __init proc_modules_init(void)
 	proc_create("modules", 0, NULL, &proc_modules_operations);
 	return 0;
 }
-module_init(proc_modules_init);
+#else	/* CONFIG_PROC_FS */
+static int __init proc_modules_init(void)
+{
+        return 0;
+}
 #endif
 
 /* Given an address, look for it in the module exception tables. */
@@ -4388,3 +4392,11 @@ void module_layout(struct module *mod,
 }
 EXPORT_SYMBOL(module_layout);
 #endif
+
+static int __init initialize_module(void)
+{
+	proc_modules_init();
+
+	return 0;
+}
+module_init(initialize_module);
-- 
1.8.3.1

[PATCH 3/3] module: Support to disable validity enforcement in runtime

[Jia Zhang]

In order to disable the module validity enforcement, writing
a PKCS#7 signature corresponding the signed content '0' is
required. Given a simple way to archive this:

$ echo -n 0 > no_sig_enforce
$ openssl smime -sign -nocerts -noattr -binary \
    -in no_sig_enforce -inkey <system_trusted_key> \
    -signer <cert> -outform der -out no_sig_enforce.p7s
$ sudo cat no_sig_enforce.p7s \
    > /sys/kernel/security/modsign/disable_enforce

Note that the signing key must be a trust key located in
system trusted keyring. So even the root privilige cannot
simply disable the enforcement.

Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
---
 kernel/module.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/kernel/module.c b/kernel/module.c
index 79825ea..3dd35ac 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -64,6 +64,7 @@
 #include <linux/bsearch.h>
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
+#include <linux/verification.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -288,6 +289,11 @@ bool is_module_sig_enforced(void)
 }
 EXPORT_SYMBOL(is_module_sig_enforced);
 
+static void set_module_sig_enforce(bool enforce)
+{
+	sig_enforce = enforce;
+}
+
 /* Block module loading/unloading? */
 int modules_disabled = 0;
 core_param(nomodule, modules_disabled, bint, 0);
@@ -2794,11 +2800,139 @@ static int module_sig_check(struct load_info *info, int flags)
 
 	return err;
 }
+
+#ifdef CONFIG_SECURITYFS
+/*
+ * Check the input for disabling enforcement policy.
+ *
+ * Return 0 if inteding to disabling the policy. Note that the root
+ * privilege cannot simply disable the policy without the
+ * authentication given by a trusted key.
+ */
+static int check_disable_enforce(char *buf, size_t count)
+{
+	u8 *p;
+
+	/*
+	 * In order to disable the enforcement policy, a PKCS#7 signature
+	 * is supplied.
+	 *
+	 * Assuming ASN.1 encoding supplied, the minimal length would be
+	 * 4-byte header plus at least 256-byte payload.
+	 */
+	if (count < 260)
+		return -EINVAL;
+
+	p = (u8 *)buf;
+
+	/* The primitive type must be a sequnce */
+	if (p[0] != 0x30 || p[1] != 0x82)
+		return -EINVAL;
+
+	/* Match up the length of the supplied buffer */
+	if (be16_to_cpup((__be16 *)(p + 2)) != count - 4)
+		return -EINVAL;
+
+	return 0;
+}
+
+/*
+ * Disable the enforceme and verify the supplied PKCS#7 signature.
+ * The signed content is simply the charactoror '0'.
+ */
+static int disable_enforce(void *pkcs7, size_t pkcs7_len)
+{
+	char data = '0';
+
+	return verify_pkcs7_signature(&data, sizeof(data), pkcs7, pkcs7_len,
+				      NULL, VERIFYING_UNSPECIFIED_SIGNATURE,
+				      NULL, NULL);
+}
+
+static ssize_t modsign_disable_enforce_write(struct file *filp,
+					     const char __user *ubuf,
+					     size_t count, loff_t *offp)
+{
+	char *buf;
+	ssize_t ret;
+	size_t max_buf_size = 1 << MAX_ORDER;
+
+	if (*offp > 1)
+		return -EFBIG;
+
+	if (count > max_buf_size)
+		return -EFBIG;
+
+	buf = kmalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = simple_write_to_buffer(buf, count, offp, ubuf, count);
+	if (ret <= 0) {
+		kfree(buf);
+		return ret;
+	}
+
+	ret = check_disable_enforce(buf, count);
+	if (!ret) {
+		if (is_module_sig_enforced()) {
+			ret = disable_enforce(buf, count);
+			if (!ret) {
+				set_module_sig_enforce(false);
+				pr_notice("Kernel module validity enforcement disabled\n");
+				ret = count;
+			}
+		} else
+			ret = count;
+	}
+
+	kfree(buf);
+
+	return ret;
+}
+
+static const struct file_operations modsign_disable_enforce_ops = {
+	.write = modsign_disable_enforce_write,
+	.llseek = generic_file_llseek,
+};
+
+static int __init securityfs_init(void)
+{
+	struct dentry *modsign_dir;
+	struct dentry *disable_enforce;
+
+	modsign_dir = securityfs_create_dir("modsign", NULL);
+	if (IS_ERR(modsign_dir))
+		return -1;
+
+	disable_enforce = securityfs_create_file("disable_enforce", S_IWUSR,
+						 modsign_dir, NULL,
+						 &modsign_disable_enforce_ops);
+	if (IS_ERR(disable_enforce))
+		goto out;
+
+	return 0;
+out:
+	securityfs_remove(modsign_dir);
+
+	return -1;
+}
+#else /* !CONFIG_SECURITYFS */
+static int __init securityfs_init(void)
+{
+	return 0;
+}
+#endif
 #else /* !CONFIG_MODULE_SIG */
 static int module_sig_check(struct load_info *info, int flags)
 {
 	return 0;
 }
+
+static int __init securityfs_init(void)
+{
+	return 0;
+}
 #endif /* !CONFIG_MODULE_SIG */
 
 /* Sanity checks against invalid binaries, wrong arch, weird elf version. */
@@ -4395,8 +4529,14 @@ void module_layout(struct module *mod,
 
 static int __init initialize_module(void)
 {
+	int ret;
+
 	proc_modules_init();
 
+	ret = securityfs_init();
+	if (unlikely(ret))
+		return ret;
+
 	return 0;
 }
 module_init(initialize_module);
-- 
1.8.3.1

Re: [PATCH v3 0/3][RESEND] modsign enhancement

[Jessica Yu]

+++ Jia Zhang [24/03/18 10:59 +0800]:
>This patch series allows to disable module validity enforcement
>in runtime through the control switch located in securityfs.
>
>In order to keep /sys/module/module/parameters/sig_enforce simple,
>the disablement switch is located at
>/sys/kernel/security/modsign/disable_enforce.
>
>Assuming CONFIG_MODULE_SIG_FORCE=n, here are the instructions to
>test this control switch.
>
># cat /sys/module/module/parameters/sig_enforce
>N
># echo 1 > /sys/module/module/parameters/sig_enforce
># cat /sys/module/module/parameters/sig_enforce
>Y
># echo -n 0 > no_sig_enforce
># openssl smime -sign -nocerts -noattr -binary -in no_sig_enforce \
>    -inkey <system_trusted_key> -signer <cert> -outform der \
>    -out /sys/kernel/security/modsign/disable_enforce
># cat /sys/module/module/parameters/sig_enforce
>N

I'm not convinced we need this. And neither the use case nor the
motivation is explained in the cover letter :-(

The way I see it - the only time you'd actually use this is in the
situation where you have *already* enabled sig_enforce, and then later
you change your mind - meaning you wanted to load unsigned modules
after all. And if you ever plan on loading unsigned modules, why would
you have enabled sig_enforce in the first place? If you want to keep
the option of loading unsigned modules, don't have sig_enforce or
CONFIG_MODULE_SIG_FORCE enabled.

[ CC'd Rusty in case he has some thoughts on this ]

Jessica

Re: [PATCH v3 0/3][RESEND] modsign enhancement

[Jia Zhang]

On 2018/3/28 上午6:11, Jessica Yu wrote:
> +++ Jia Zhang [24/03/18 10:59 +0800]:
>> This patch series allows to disable module validity enforcement
>> in runtime through the control switch located in securityfs.
>>
>> In order to keep /sys/module/module/parameters/sig_enforce simple,
>> the disablement switch is located at
>> /sys/kernel/security/modsign/disable_enforce.
>>
>> Assuming CONFIG_MODULE_SIG_FORCE=n, here are the instructions to
>> test this control switch.
>>
>> # cat /sys/module/module/parameters/sig_enforce
>> N
>> # echo 1 > /sys/module/module/parameters/sig_enforce
>> # cat /sys/module/module/parameters/sig_enforce
>> Y
>> # echo -n 0 > no_sig_enforce
>> # openssl smime -sign -nocerts -noattr -binary -in no_sig_enforce \
>>    -inkey <system_trusted_key> -signer <cert> -outform der \
>>    -out /sys/kernel/security/modsign/disable_enforce
>> # cat /sys/module/module/parameters/sig_enforce
>> N
> 
> I'm not convinced we need this. And neither the use case nor the
> motivation is explained in the cover letter :-(
> 
> The way I see it - the only time you'd actually use this is in the
> situation where you have *already* enabled sig_enforce, and then later
> you change your mind - meaning you wanted to load unsigned modules
> after all. And if you ever plan on loading unsigned modules, why would
> you have enabled sig_enforce in the first place? If you want to keep

Similar SELinux, this is just providing a runtime switch to disable
modsign temporarily in a safer way. At least it is cannot be simply
disabled.

Thanks,
Jia

> the option of loading unsigned modules, don't have sig_enforce or
> CONFIG_MODULE_SIG_FORCE enabled.
> 
> [ CC'd Rusty in case he has some thoughts on this ]
> 
> Jessica