* [PATCH 0/2] ima: change how MODULE_SIG_FORCE is checked on modules checking policy
@ 2017-10-20 19:19 Bruno E. O. Meneguele
2017-10-20 19:19 ` [PATCH 1/2] module: export module signature enforcement status Bruno E. O. Meneguele
2017-10-20 19:19 ` [PATCH 2/2] ima: check signature enforcement against cmdline param instead of CONFIG Bruno E. O. Meneguele
0 siblings, 2 replies; 6+ messages in thread
From: Bruno E. O. Meneguele @ 2017-10-20 19:19 UTC (permalink / raw)
To: linux-kernel
Cc: linux-security-module, linux-integrity, serge, james.l.morris,
dmitry.kasatkin, zohar, rusty, jeyu
This patchset ensure that IMA's modules checking policy:
measure func=MODULE_CHECK uid=0
rely on the correct value of CONFIG_MODULE_SIG_FORCE, since the way it
is today the code completely ignores the module.sig_enforce cmdline
param, which behaves in a OR logic with the CONFIG value
(CONFIG_MODULE_SIG_FORCE || module.sig_enforce). That said, everytime a
module would load, in the current checking code, when the kernel was not
compiled with the CONFIG set the call to init_module syscall fails with
-EACCES:
# strace -f -v modprobe <any-module> | grep init_module
init_module(0x55b9bcc9bba0, 17763, "") = -1 EACCES (Permission denied)
With this patchset the result would rely on the module.sig_enforce
cmdline as well. Once the CONFIG is not set, but the param is, the
result would be 'success', as it should be:
# strace -f -v modprobe <any-module> | grep init_module
init_module(0x7f9602d6e010, 386646, "") = 0
The patchset was tested in two different kernels: 4.13.6 (Fedora 27) and
4.14.0-rc4 (integrity-next tree)
Bruno E. O. Meneguele (2):
module: export module signature enforcement status
ima: check signature enforcement against cmdline param instead of
CONFIG
include/linux/module.h | 2 ++
kernel/module.c | 8 ++++++++
security/integrity/ima/ima_main.c | 6 +++---
3 files changed, 13 insertions(+), 3 deletions(-)
--
2.13.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] module: export module signature enforcement status
2017-10-20 19:19 [PATCH 0/2] ima: change how MODULE_SIG_FORCE is checked on modules checking policy Bruno E. O. Meneguele
@ 2017-10-20 19:19 ` Bruno E. O. Meneguele
2017-10-23 12:56 ` Mimi Zohar
2017-10-20 19:19 ` [PATCH 2/2] ima: check signature enforcement against cmdline param instead of CONFIG Bruno E. O. Meneguele
1 sibling, 1 reply; 6+ messages in thread
From: Bruno E. O. Meneguele @ 2017-10-20 19:19 UTC (permalink / raw)
To: linux-kernel
Cc: linux-security-module, linux-integrity, serge, james.l.morris,
dmitry.kasatkin, zohar, rusty, jeyu
A static variable sig_enforce is used as status var to indicate the real
value of CONFIG_MODULE_SIG_FORCE, once this one is set the var will hold
true, but if the CONFIG is not set the status var will hold whatever
value is present in the module.sig_enforce kernel cmdline param: true
when =1 and false when =0 or not present.
Considering this cmdline param take place over the CONFIG value when
it's not set, other places in the kernel could missbehave since they
would have only the CONFIG_MODULE_SIG_FORCE value to rely on. Exporting
this status var allows the kernel to rely in the effective value of
module signature enforcement, being it from CONFIG value or cmdline
param.
Signed-off-by: Bruno E. O. Meneguele <brdeoliv@redhat.com>
---
include/linux/module.h | 2 ++
kernel/module.c | 8 ++++++++
2 files changed, 10 insertions(+)
diff --git a/include/linux/module.h b/include/linux/module.h
index fe5aa3736707..ddfe17e3a85c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -806,4 +806,6 @@ static inline bool module_sig_ok(struct module *module)
}
#endif /* CONFIG_MODULE_SIG */
+bool is_module_sig_enforced(void);
+
#endif /* _LINUX_MODULE_H */
diff --git a/kernel/module.c b/kernel/module.c
index de66ec825992..b93c7ff44066 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -278,6 +278,14 @@ static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
module_param(sig_enforce, bool_enable_only, 0644);
#endif /* !CONFIG_MODULE_SIG_FORCE */
+/* Export sig_enforce kernel cmdline parameter to allow other subsystems rely
+ * on that instead of directly to CONFIG_MODULE_SIG_FORCE config. */
+bool is_module_sig_enforced(void)
+{
+ return sig_enforce;
+}
+EXPORT_SYMBOL(is_module_sig_enforced);
+
/* Block module loading/unloading? */
int modules_disabled = 0;
core_param(nomodule, modules_disabled, bint, 0);
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] ima: check signature enforcement against cmdline param instead of CONFIG
2017-10-20 19:19 [PATCH 0/2] ima: change how MODULE_SIG_FORCE is checked on modules checking policy Bruno E. O. Meneguele
2017-10-20 19:19 ` [PATCH 1/2] module: export module signature enforcement status Bruno E. O. Meneguele
@ 2017-10-20 19:19 ` Bruno E. O. Meneguele
2017-10-23 12:57 ` Mimi Zohar
1 sibling, 1 reply; 6+ messages in thread
From: Bruno E. O. Meneguele @ 2017-10-20 19:19 UTC (permalink / raw)
To: linux-kernel
Cc: linux-security-module, linux-integrity, serge, james.l.morris,
dmitry.kasatkin, zohar, rusty, jeyu
When the user requests MODULE_CHECK policy and its kernel is compiled
with CONFIG_MODULE_SIG_FORCE not set, all modules would not load, just
those loaded in initram time. One option the user would have would be
set a kernel cmdline param (module.sig_enforce) to true, but the IMA
module check code doesn't rely on this value, it checks just
CONFIG_MODULE_SIG_FORCE.
This patch solves this problem checking for the exported value of
module.sig_enforce cmdline param intead of CONFIG_MODULE_SIG_FORCE,
which holds the effective value (CONFIG || param).
Signed-off-by: Bruno E. O. Meneguele <brdeoliv@redhat.com>
---
security/integrity/ima/ima_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index e4ab8ef8016e..d11a7fcc5c8b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -356,12 +356,12 @@ void ima_post_path_mknod(struct dentry *dentry)
*/
int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
{
+ bool sig_enforce = is_module_sig_enforced();
+
if (!file && read_id == READING_MODULE) {
-#ifndef CONFIG_MODULE_SIG_FORCE
- if ((ima_appraise & IMA_APPRAISE_MODULES) &&
+ if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
(ima_appraise & IMA_APPRAISE_ENFORCE))
return -EACCES; /* INTEGRITY_UNKNOWN */
-#endif
return 0; /* We rely on module signature checking */
}
return 0;
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] module: export module signature enforcement status
2017-10-20 19:19 ` [PATCH 1/2] module: export module signature enforcement status Bruno E. O. Meneguele
@ 2017-10-23 12:56 ` Mimi Zohar
2017-10-23 16:24 ` Bruno E. O. Meneguele
0 siblings, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2017-10-23 12:56 UTC (permalink / raw)
To: Bruno E. O. Meneguele, linux-kernel
Cc: linux-security-module, linux-integrity, serge, james.l.morris,
dmitry.kasatkin, rusty, jeyu
On Fri, 2017-10-20 at 17:19 -0200, Bruno E. O. Meneguele wrote:
> A static variable sig_enforce is used as status var to indicate the real
> value of CONFIG_MODULE_SIG_FORCE, once this one is set the var will hold
> true, but if the CONFIG is not set the status var will hold whatever
> value is present in the module.sig_enforce kernel cmdline param: true
> when =1 and false when =0 or not present.
>
> Considering this cmdline param take place over the CONFIG value when
> it's not set, other places in the kernel could missbehave since they
^misbehave
> would have only the CONFIG_MODULE_SIG_FORCE value to rely on. Exporting
> this status var allows the kernel to rely in the effective value of
> module signature enforcement, being it from CONFIG value or cmdline
> param.
Thanks! There's a minor checkpatch warning below.
> Signed-off-by: Bruno E. O. Meneguele <brdeoliv@redhat.com>
> ---
> include/linux/module.h | 2 ++
> kernel/module.c | 8 ++++++++
> 2 files changed, 10 insertions(+)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index fe5aa3736707..ddfe17e3a85c 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -806,4 +806,6 @@ static inline bool module_sig_ok(struct module *module)
> }
> #endif /* CONFIG_MODULE_SIG */
>
> +bool is_module_sig_enforced(void);
> +
> #endif /* _LINUX_MODULE_H */
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec825992..b93c7ff44066 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -278,6 +278,14 @@ static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> module_param(sig_enforce, bool_enable_only, 0644);
> #endif /* !CONFIG_MODULE_SIG_FORCE */
>
> +/* Export sig_enforce kernel cmdline parameter to allow other subsystems rely
> + * on that instead of directly to CONFIG_MODULE_SIG_FORCE config. */
Checkpatch complains. Please use the normal format:
/*
*
*/
> +bool is_module_sig_enforced(void)
> +{
> + return sig_enforce;
> +}
> +EXPORT_SYMBOL(is_module_sig_enforced);
> +
> /* Block module loading/unloading? */
> int modules_disabled = 0;
> core_param(nomodule, modules_disabled, bint, 0);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] ima: check signature enforcement against cmdline param instead of CONFIG
2017-10-20 19:19 ` [PATCH 2/2] ima: check signature enforcement against cmdline param instead of CONFIG Bruno E. O. Meneguele
@ 2017-10-23 12:57 ` Mimi Zohar
0 siblings, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2017-10-23 12:57 UTC (permalink / raw)
To: Bruno E. O. Meneguele, linux-kernel
Cc: linux-security-module, linux-integrity, serge, james.l.morris,
dmitry.kasatkin, rusty, jeyu
On Fri, 2017-10-20 at 17:19 -0200, Bruno E. O. Meneguele wrote:
> When the user requests MODULE_CHECK policy and its kernel is compiled
> with CONFIG_MODULE_SIG_FORCE not set, all modules would not load, just
> those loaded in initram time. One option the user would have would be
> set a kernel cmdline param (module.sig_enforce) to true, but the IMA
> module check code doesn't rely on this value, it checks just
> CONFIG_MODULE_SIG_FORCE.
>
> This patch solves this problem checking for the exported value of
> module.sig_enforce cmdline param intead of CONFIG_MODULE_SIG_FORCE,
> which holds the effective value (CONFIG || param).
>
> Signed-off-by: Bruno E. O. Meneguele <brdeoliv@redhat.com>
Thanks!
> ---
> security/integrity/ima/ima_main.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index e4ab8ef8016e..d11a7fcc5c8b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -356,12 +356,12 @@ void ima_post_path_mknod(struct dentry *dentry)
> */
> int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> {
> + bool sig_enforce = is_module_sig_enforced();
> +
> if (!file && read_id == READING_MODULE) {
> -#ifndef CONFIG_MODULE_SIG_FORCE
> - if ((ima_appraise & IMA_APPRAISE_MODULES) &&
> + if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> (ima_appraise & IMA_APPRAISE_ENFORCE))
> return -EACCES; /* INTEGRITY_UNKNOWN */
> -#endif
> return 0; /* We rely on module signature checking */
> }
> return 0;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] module: export module signature enforcement status
2017-10-23 12:56 ` Mimi Zohar
@ 2017-10-23 16:24 ` Bruno E. O. Meneguele
0 siblings, 0 replies; 6+ messages in thread
From: Bruno E. O. Meneguele @ 2017-10-23 16:24 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-kernel, linux-security-module, linux-integrity, serge,
james.l.morris, dmitry.kasatkin, rusty, jeyu
[-- Attachment #1: Type: text/plain, Size: 2526 bytes --]
On 23-10, Mimi Zohar wrote:
> On Fri, 2017-10-20 at 17:19 -0200, Bruno E. O. Meneguele wrote:
> > A static variable sig_enforce is used as status var to indicate the real
> > value of CONFIG_MODULE_SIG_FORCE, once this one is set the var will hold
> > true, but if the CONFIG is not set the status var will hold whatever
> > value is present in the module.sig_enforce kernel cmdline param: true
> > when =1 and false when =0 or not present.
> >
> > Considering this cmdline param take place over the CONFIG value when
> > it's not set, other places in the kernel could missbehave since they
>
> ^misbehave
>
right.
> > would have only the CONFIG_MODULE_SIG_FORCE value to rely on. Exporting
> > this status var allows the kernel to rely in the effective value of
> > module signature enforcement, being it from CONFIG value or cmdline
> > param.
>
> Thanks! There's a minor checkpatch warning below.
>
> > Signed-off-by: Bruno E. O. Meneguele <brdeoliv@redhat.com>
> > ---
> > include/linux/module.h | 2 ++
> > kernel/module.c | 8 ++++++++
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index fe5aa3736707..ddfe17e3a85c 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -806,4 +806,6 @@ static inline bool module_sig_ok(struct module *module)
> > }
> > #endif /* CONFIG_MODULE_SIG */
> >
> > +bool is_module_sig_enforced(void);
> > +
> > #endif /* _LINUX_MODULE_H */
> > diff --git a/kernel/module.c b/kernel/module.c
> > index de66ec825992..b93c7ff44066 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -278,6 +278,14 @@ static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> > module_param(sig_enforce, bool_enable_only, 0644);
> > #endif /* !CONFIG_MODULE_SIG_FORCE */
> >
> > +/* Export sig_enforce kernel cmdline parameter to allow other subsystems rely
> > + * on that instead of directly to CONFIG_MODULE_SIG_FORCE config. */
>
> Checkpatch complains. Please use the normal format:
> /*
> *
> */
>
Oh, sure, sorry for that! I'll resend a v2 of the patchset in a minute.
Thanks.
> > +bool is_module_sig_enforced(void)
> > +{
> > + return sig_enforce;
> > +}
> > +EXPORT_SYMBOL(is_module_sig_enforced);
> > +
> > /* Block module loading/unloading? */
> > int modules_disabled = 0;
> > core_param(nomodule, modules_disabled, bint, 0);
>
--
bmeneg
PGP Key: http://bmeneg.com/pubkey.txt
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-10-23 16:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 19:19 [PATCH 0/2] ima: change how MODULE_SIG_FORCE is checked on modules checking policy Bruno E. O. Meneguele
2017-10-20 19:19 ` [PATCH 1/2] module: export module signature enforcement status Bruno E. O. Meneguele
2017-10-23 12:56 ` Mimi Zohar
2017-10-23 16:24 ` Bruno E. O. Meneguele
2017-10-20 19:19 ` [PATCH 2/2] ima: check signature enforcement against cmdline param instead of CONFIG Bruno E. O. Meneguele
2017-10-23 12:57 ` Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).