[PATCH 3/4] module: Support to show the current enforcement policy

[PATCH 3/4] module: Support to show the current enforcement policy

[Jia Zhang]

/sys/kernel/security/modsign/enforce gives the result of current
enforcement policy of loading module.

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

diff --git a/kernel/module.c b/kernel/module.c
index 79825ea..e3c6c8e 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2794,11 +2794,60 @@ static int module_sig_check(struct load_info *info, int flags)
 
 	return err;
 }
+
+#ifdef CONFIG_SECURITYFS
+static ssize_t modsign_enforce_read(struct file *filp, char __user *ubuf,
+				    size_t count, loff_t *offp)
+{
+	char buf[2];
+
+	sprintf(buf, "%d", !!sig_enforce);
+
+	return simple_read_from_buffer(ubuf, count, offp, buf, 1);
+}
+
+static const struct file_operations modsign_enforce_ops = {
+	.read = modsign_enforce_read,
+	.llseek = generic_file_llseek,
+};
+
+static int __init securityfs_init(void)
+{
+	struct dentry *modsign_dir;
+	struct dentry *enforce;
+
+	modsign_dir = securityfs_create_dir("modsign", NULL);
+	if (IS_ERR(modsign_dir))
+		return -1;
+
+	enforce = securityfs_create_file("enforce",
+					 S_IRUSR | S_IRGRP, modsign_dir,
+					 NULL, &modsign_enforce_ops);
+	if (IS_ERR(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 +4444,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 3/4] module: Support to show the current enforcement policy

[Jessica Yu]

+++ Jia Zhang [01/03/18 17:09 +0800]:
>/sys/kernel/security/modsign/enforce gives the result of current
>enforcement policy of loading module.
>
>Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>

Why is this being added as part of securityfs? AFAIK that's primarily used by LSMs.

And we already export sig_enforce to sysfs (See /sys/module/module/parameters/sig_enforce).
It already does exactly what your patchset tries to do, it only allows for enablement. 

Jessica

>---
> kernel/module.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index 79825ea..e3c6c8e 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -2794,11 +2794,60 @@ static int module_sig_check(struct load_info *info, int flags)
>
> 	return err;
> }
>+
>+#ifdef CONFIG_SECURITYFS
>+static ssize_t modsign_enforce_read(struct file *filp, char __user *ubuf,
>+				    size_t count, loff_t *offp)
>+{
>+	char buf[2];
>+
>+	sprintf(buf, "%d", !!sig_enforce);
>+
>+	return simple_read_from_buffer(ubuf, count, offp, buf, 1);
>+}
>+
>+static const struct file_operations modsign_enforce_ops = {
>+	.read = modsign_enforce_read,
>+	.llseek = generic_file_llseek,
>+};
>+
>+static int __init securityfs_init(void)
>+{
>+	struct dentry *modsign_dir;
>+	struct dentry *enforce;
>+
>+	modsign_dir = securityfs_create_dir("modsign", NULL);
>+	if (IS_ERR(modsign_dir))
>+		return -1;
>+
>+	enforce = securityfs_create_file("enforce",
>+					 S_IRUSR | S_IRGRP, modsign_dir,
>+					 NULL, &modsign_enforce_ops);
>+	if (IS_ERR(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 +4444,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 3/4] module: Support to show the current enforcement policy

[Jia Zhang]

On 2018/3/8 上午4:14, Jessica Yu wrote:
> +++ Jia Zhang [01/03/18 17:09 +0800]:
>> /sys/kernel/security/modsign/enforce gives the result of current
>> enforcement policy of loading module.
>>
>> Signed-off-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> 
> Why is this being added as part of securityfs? AFAIK that's primarily
> used by LSMs.

The integrity subsystem such as IMA is also located there.

> 
> And we already export sig_enforce to sysfs (See
> /sys/module/module/parameters/sig_enforce).
> It already does exactly what your patchset tries to do, it only allows
> for enablement.

I will respond this in V2.

Thanks,
Jia

> Jessica
> 
>> ---
>> kernel/module.c | 55
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 79825ea..e3c6c8e 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -2794,11 +2794,60 @@ static int module_sig_check(struct load_info
>> *info, int flags)
>>
>>     return err;
>> }
>> +
>> +#ifdef CONFIG_SECURITYFS
>> +static ssize_t modsign_enforce_read(struct file *filp, char __user
>> *ubuf,
>> +                    size_t count, loff_t *offp)
>> +{
>> +    char buf[2];
>> +
>> +    sprintf(buf, "%d", !!sig_enforce);
>> +
>> +    return simple_read_from_buffer(ubuf, count, offp, buf, 1);
>> +}
>> +
>> +static const struct file_operations modsign_enforce_ops = {
>> +    .read = modsign_enforce_read,
>> +    .llseek = generic_file_llseek,
>> +};
>> +
>> +static int __init securityfs_init(void)
>> +{
>> +    struct dentry *modsign_dir;
>> +    struct dentry *enforce;
>> +
>> +    modsign_dir = securityfs_create_dir("modsign", NULL);
>> +    if (IS_ERR(modsign_dir))
>> +        return -1;
>> +
>> +    enforce = securityfs_create_file("enforce",
>> +                     S_IRUSR | S_IRGRP, modsign_dir,
>> +                     NULL, &modsign_enforce_ops);
>> +    if (IS_ERR(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 +4444,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
>>

[PATCH v2 0/4] modsign enhancement

[Jia Zhang]

This patch series allows to disable module validity enforcement
in runtime through /sys/kernel/security/modsign/enforce interface.

Assuming CONFIG_MODULE_SIG_FORCE=y, here are the instructions to
disable the validity enforcement.

# cat /sys/kernel/security/modsign/enforce
# echo -n 0 > data
# openssl smime -sign -nocerts -noattr -binary -in data \
    -inkey <system_trusted_key> -signer <cert> -outform der \
    -out /sys/kernel/security/modsign/enforce

Now enable enforcement again on demand.

# echo 1 > /sys/kernel/security/modsign/enforce

Changelog:
v2:
- Support to disable validity enforcement in runtime.

[PATCH 1/4] 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/4] 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/4] module: Support to show the current enforcement policy

[Jia Zhang]

/sys/kernel/security/modsign/enforce gives the result of current
enforcement policy of loading module.

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

diff --git a/kernel/module.c b/kernel/module.c
index 79825ea..6b032577 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2794,11 +2794,60 @@ static int module_sig_check(struct load_info *info, int flags)
 
 	return err;
 }
+
+#ifdef CONFIG_SECURITYFS
+static ssize_t modsign_enforce_read(struct file *filp, char __user *ubuf,
+				    size_t count, loff_t *offp)
+{
+	char buf[2];
+
+	sprintf(buf, "%d", is_module_sig_enforced());
+
+	return simple_read_from_buffer(ubuf, count, offp, buf, 1);
+}
+
+static const struct file_operations modsign_enforce_ops = {
+	.read = modsign_enforce_read,
+	.llseek = generic_file_llseek,
+};
+
+static int __init securityfs_init(void)
+{
+	struct dentry *modsign_dir;
+	struct dentry *enforce;
+
+	modsign_dir = securityfs_create_dir("modsign", NULL);
+	if (IS_ERR(modsign_dir))
+		return -1;
+
+	enforce = securityfs_create_file("enforce",
+					 S_IRUSR | S_IRGRP, modsign_dir,
+					 NULL, &modsign_enforce_ops);
+	if (IS_ERR(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 +4444,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

[PATCH 4/4] 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 > data
$ openssl smime -sign -nocerts -noattr -binary -in data \
    -inkey <system_trusted_key> -signer <cert> -outform der \
    -out data.sig

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 | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 4 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 6b032577..16be198 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);
@@ -2796,6 +2802,61 @@ static int module_sig_check(struct load_info *info, int flags)
 }
 
 #ifdef CONFIG_SECURITYFS
+/*
+ * Check the intention of setting the enforcement policy.
+ *
+ * Return 1 if enabling the policy, or return 0 if disabling
+ * the policy. Note that the root privilege cannot simply
+ * disable the policy without the authentication given by a
+ * trusted key.
+ */
+static int check_enforce(char *buf, size_t count)
+{
+	u8 *p;
+
+	if (buf[0] == '1') {
+		if (count == 1 || (count == 2 && buf[1] == '\n'))
+			return 1;
+
+		return -EINVAL;
+	}
+
+	/*
+	 * 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_enforce_read(struct file *filp, char __user *ubuf,
 				    size_t count, loff_t *offp)
 {
@@ -2806,7 +2867,50 @@ static ssize_t modsign_enforce_read(struct file *filp, char __user *ubuf,
 	return simple_read_from_buffer(ubuf, count, offp, buf, 1);
 }
 
-static const struct file_operations modsign_enforce_ops = {
+static ssize_t modsign_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_enforce(buf, count);
+	if (is_module_sig_enforced() && !ret) {
+		ret = disable_enforce(buf, count);
+		if (!ret) {
+			set_module_sig_enforce(false);
+			pr_notice("Kernel module validity enforcement disabled\n");
+			ret = count;
+		}
+	} else if (!is_module_sig_enforced() && ret == 1) {
+		set_module_sig_enforce(true);
+		pr_notice("Kernel module validity enforcement enabled\n");
+		ret = count;
+	}
+
+	kfree(buf);
+
+	return ret;
+}
+
+static struct file_operations modsign_enforce_ops = {
 	.read = modsign_enforce_read,
 	.llseek = generic_file_llseek,
 };
@@ -2815,14 +2919,20 @@ static int __init securityfs_init(void)
 {
 	struct dentry *modsign_dir;
 	struct dentry *enforce;
+	umode_t mode;
 
 	modsign_dir = securityfs_create_dir("modsign", NULL);
 	if (IS_ERR(modsign_dir))
 		return -1;
 
-	enforce = securityfs_create_file("enforce",
-					 S_IRUSR | S_IRGRP, modsign_dir,
-					 NULL, &modsign_enforce_ops);
+	mode = S_IRUSR | S_IRGRP;
+	if (!is_module_sig_enforced()) {
+		modsign_enforce_ops.write = modsign_enforce_write;
+		mode |= S_IWUSR;
+	}
+
+	enforce = securityfs_create_file("enforce", mode, modsign_dir, NULL,
+					 &modsign_enforce_ops);
 	if (IS_ERR(enforce))
 		goto out;
 
-- 
1.8.3.1

Re: [PATCH v2 0/4] modsign enhancement

[Jessica Yu]

+++ Jia Zhang [08/03/18 12:26 +0800]:
>This patch series allows to disable module validity enforcement
>in runtime through /sys/kernel/security/modsign/enforce interface.
>
>Assuming CONFIG_MODULE_SIG_FORCE=y, here are the instructions to
>disable the validity enforcement.
>
># cat /sys/kernel/security/modsign/enforce
># echo -n 0 > data
># openssl smime -sign -nocerts -noattr -binary -in data \
>    -inkey <system_trusted_key> -signer <cert> -outform der \
>    -out /sys/kernel/security/modsign/enforce
>
>Now enable enforcement again on demand.
>
># echo 1 > /sys/kernel/security/modsign/enforce
>
>Changelog:
>v2:
>- Support to disable validity enforcement in runtime.

NAK - please use /sys/module/module/parameters/sig_enforce.

And I would rather keep this parameter bool_enable_only, plain and simple.
What use case do you have/why would you want to disable signature
enforcement - after having enabled it - during runtime?  None of this
is explained nor justified in the cover letter.

Thanks,

Jessica

Re: [PATCH v2 0/4] modsign enhancement

[Jia Zhang]

On 2018/3/12 下午9:28, Jessica Yu wrote:
> +++ Jia Zhang [08/03/18 12:26 +0800]:
>> This patch series allows to disable module validity enforcement
>> in runtime through /sys/kernel/security/modsign/enforce interface.
>>
>> Assuming CONFIG_MODULE_SIG_FORCE=y, here are the instructions to
>> disable the validity enforcement.
>>
>> # cat /sys/kernel/security/modsign/enforce
>> # echo -n 0 > data
>> # openssl smime -sign -nocerts -noattr -binary -in data \
>>    -inkey <system_trusted_key> -signer <cert> -outform der \
>>    -out /sys/kernel/security/modsign/enforce
>>
>> Now enable enforcement again on demand.
>>
>> # echo 1 > /sys/kernel/security/modsign/enforce
>>
>> Changelog:
>> v2:
>> - Support to disable validity enforcement in runtime.
> 
> NAK - please use /sys/module/module/parameters/sig_enforce.
> 
> And I would rather keep this parameter bool_enable_only, plain and simple.
> What use case do you have/why would you want to disable signature
> enforcement - after having enabled it - during runtime?  None of this
> is explained nor justified in the cover letter.

Because there is no way to disable it such as module.no_sig_enforce when
MODULE_SIG_FORCE=y available unless re-compiling a kernel without this
enforcement. This is inconvenient a bit. IMA and SELinux both have
cmdline control, but modsign doesn't have.

Even we really have a module.no_sig_enforce in cmdline, runtime
disablement can be used to avoid machine reboot. Sometimes machine
reboot is expensive.

If you agree, I can implement the runtime disablement via
/sys/module/module/parameters/sig_enforce. Additionally, supporting
module.no_sig_enforce when MODULE_SIG_FORCE=y is another one to be
implemented.

Thanks,
Jia

> 
> Thanks,
> 
> Jessica