Call is_module_sig_enforced() instead.
Signed-off-by: Jia Zhang <[email protected]>
---
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
If module signature verification check is enabled but the
validity enforcement is configured to be disabled, it should
be allowed to enable it. Once enabled, it is disallowed to
disable it.
Signed-off-by: Jia Zhang <[email protected]>
---
kernel/module.c | 39 ++++++++++++++++++++++++++++++++++++---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index e3c6c8e..89704df 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2806,8 +2806,37 @@ static ssize_t modsign_enforce_read(struct file *filp, char __user *ubuf,
return simple_read_from_buffer(ubuf, count, offp, buf, 1);
}
+#ifndef CONFIG_MODULE_SIG_FORCE
+static ssize_t modsign_enforce_write(struct file *filp,
+ const char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ char buf;
+ ssize_t ret;
+
+ if (*offp > 1)
+ return -EFBIG;
+
+ ret = simple_write_to_buffer(&buf, 1, offp, ubuf, count);
+ if (ret > 0) {
+ if (buf != '1')
+ return -EINVAL;
+
+ sig_enforce = true;
+ pr_notice_once("Kernel module validity enforcement enabled\n");
+
+ ret = count;
+ }
+
+ return ret;
+}
+#endif
+
static const struct file_operations modsign_enforce_ops = {
.read = modsign_enforce_read,
+#ifndef CONFIG_MODULE_SIG_FORCE
+ .write = modsign_enforce_write,
+#endif
.llseek = generic_file_llseek,
};
@@ -2815,14 +2844,18 @@ 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 (!sig_enforce)
+ 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
This entry point currently includes the procfs initialization,
and will include a securityfs initialization.
Signed-off-by: Jia Zhang <[email protected]>
---
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
/sys/kernel/security/modsign/enforce gives the result of current
enforcement policy of loading module.
Signed-off-by: Jia Zhang <[email protected]>
---
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
Hi Jessica,
Could you review this patch series?
Thanks,
Jia
On 2018/3/1 下午5:09, Jia Zhang wrote:
> Call is_module_sig_enforced() instead.
>
> Signed-off-by: Jia Zhang <[email protected]>
> ---
> 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;
>
+++ 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 <[email protected]>
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
>
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 <[email protected]>
>
> 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
>>