Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline

From: Tianjia Zhang
Date: Tue Apr 28 2020 - 22:15:17 EST




On 2020/4/28 18:02, Jessica Yu wrote:
+++ Greg KH [28/04/20 09:29 +0200]:
On Tue, Apr 28, 2020 at 03:07:10PM +0800, Tianjia Zhang wrote:


On 2020/4/28 14:35, Greg KH wrote:
> On Tue, Apr 28, 2020 at 02:00:08PM +0800, Tianjia Zhang wrote:
> > This option allows to disable modsign completely at the beginning,
> > and turn off by set the kernel cmdline `no_modsig_enforce` when
> > `CONFIG_MODULE_SIG_FORCE` is enabled.
> >
> > Yet another change allows to always show the current status of
> > modsign through `/sys/module/module/parameters/sig_enforce`.
> >
> > Signed-off-by: Jia Zhang <zhang.jia@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Tianjia Zhang <tianjia.zhang@xxxxxxxxxxxxxxxxx>
> > ---
> >
> > v3 change:
> >ÂÂÂ Beautify the document description according to the recommendation.
> >
> > v2 change:
> >ÂÂÂ document this new option.
> >
> >ÂÂ Documentation/admin-guide/kernel-parameters.txt | 6 ++++++
> >ÂÂ kernel/module.cÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ | 8 ++++++++
> >ÂÂ 2 files changed, 14 insertions(+)
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 7bc83f3d9bdf..b30f013fb8c5 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -3190,6 +3190,12 @@
> >ÂÂÂÂÂÂ noirqdebugÂÂÂ [X86-32] Disables the code which attempts to detect and
> >ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ disable unhandled interrupt sources.
> > +ÂÂÂ no_modsig_enforce
> > +ÂÂÂÂÂÂÂÂÂÂÂ [KNL] When CONFIG_MODULE_SIG_FORCE is set, this option
> > +ÂÂÂÂÂÂÂÂÂÂÂ allows to disable modsign completely at the beginning.
> > +ÂÂÂÂÂÂÂÂÂÂÂ This means that modules without (valid) signatures will
> > +ÂÂÂÂÂÂÂÂÂÂÂ be loaded successfully.
> > +
>
> So now we have module.sig_enforce and this one? That feels really
> confusing, why can't you just use the existing option?
>
> And why would you want to allow the bootloader to override a kernel
> build option like this? That feels risky.
>
> thanks,
>
> greg k-h
>

If CONFIG_MODULE_SIG_FORCE is set, `module.sig_enforce` is always true and
read-only. There is indeed a risk in doing this, but it will allow the
system to boot normally in some emergency situations, such as certificate
expiration.

On the other hand, would it be a good solution to make `module.sig_enforce`
readable and writable?

Readable is fine :)

And you really can't modify the existing option to change how it works,
but my question is, why would you want to override
CONFIG_MODULE_SIG_FORCE at all? I wouldn't want my bootloader to have
the ability to change the kernel's protection model, that's a huge
security hole you are adding to the kernel that it can not protect
itself from at all.

I agree with Greg's reasoning here. We had an almost identical thread
about this two years ago:

Âhttp://lore.kernel.org/r/20180312132823.dixp7gkjypjlgymt@xxxxxxxxxxxxxxxxxxx

I generally view module signature enforcement as a one way street. You
can go from unenforced to enforced, but not the other way around. If
you are anticipating the need to load unsigned modules or undo this
protection in general, then why are you building the kernel with
CONFIG_MODULE_SIG_FORCE? It seems to defeat the purpose of enabling
this option. You could achieve the same behavior by building without
it and toggling module.sig_enforce on boot, no?

Thanks,

Jessica

I'm sorry I didn't pay attention to the previous email, your information helped me a lot, I think this scenario can be solved by toggling module.sig_enforce, thank you very much.

Thanks and best,
Tianjia