* [PATCH v3] module: Allow to disable modsign in kernel cmdline @ 2020-04-28 6:00 Tianjia Zhang 2020-04-28 6:35 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Tianjia Zhang @ 2020-04-28 6:00 UTC (permalink / raw) To: jeyu, corbet, rdunlap, mchehab+samsung, tglx, akpm, gregkh, pawan.kumar.gupta, jgross Cc: linux-kernel, linux-doc, tianjia.zhang 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@linux.alibaba.com> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> --- 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. + no_timer_check [X86,APIC] Disables the code which tests for broken timer IRQ sources. diff --git a/kernel/module.c b/kernel/module.c index 646f1e2330d2..0e68e1286377 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -269,6 +269,14 @@ static void module_assert_mutex_or_preempt(void) static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE); module_param(sig_enforce, bool_enable_only, 0644); +#ifdef CONFIG_MODULE_SIG_FORCE +static int __init set_no_modsig_enforce(char *str) +{ + sig_enforce = false; + return 1; +} +__setup("no_modsig_enforce", set_no_modsig_enforce); +#endif /* !CONFIG_MODULE_SIG_FORCE */ /* * Export sig_enforce kernel cmdline parameter to allow other subsystems rely -- 2.17.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline 2020-04-28 6:00 [PATCH v3] module: Allow to disable modsign in kernel cmdline Tianjia Zhang @ 2020-04-28 6:35 ` Greg KH 2020-04-28 7:07 ` Tianjia Zhang 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2020-04-28 6:35 UTC (permalink / raw) To: Tianjia Zhang Cc: jeyu, corbet, rdunlap, mchehab+samsung, tglx, akpm, pawan.kumar.gupta, jgross, linux-kernel, linux-doc 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@linux.alibaba.com> > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > --- > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline 2020-04-28 6:35 ` Greg KH @ 2020-04-28 7:07 ` Tianjia Zhang 2020-04-28 7:29 ` Greg KH 0 siblings, 1 reply; 7+ messages in thread From: Tianjia Zhang @ 2020-04-28 7:07 UTC (permalink / raw) To: Greg KH Cc: jeyu, corbet, rdunlap, mchehab+samsung, tglx, akpm, pawan.kumar.gupta, jgross, linux-kernel, linux-doc 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@linux.alibaba.com> >> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> >> --- >> >> 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? Thanks and best, Tianjia ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline 2020-04-28 7:07 ` Tianjia Zhang @ 2020-04-28 7:29 ` Greg KH 2020-04-28 8:24 ` Tianjia Zhang 2020-04-28 10:02 ` Jessica Yu 0 siblings, 2 replies; 7+ messages in thread From: Greg KH @ 2020-04-28 7:29 UTC (permalink / raw) To: Tianjia Zhang Cc: jeyu, corbet, rdunlap, mchehab+samsung, tglx, akpm, pawan.kumar.gupta, jgross, linux-kernel, linux-doc 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@linux.alibaba.com> > > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> > > > --- > > > > > > 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. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline 2020-04-28 7:29 ` Greg KH @ 2020-04-28 8:24 ` Tianjia Zhang 2020-04-28 10:02 ` Jessica Yu 1 sibling, 0 replies; 7+ messages in thread From: Tianjia Zhang @ 2020-04-28 8:24 UTC (permalink / raw) To: Greg KH Cc: jeyu, corbet, rdunlap, mchehab+samsung, tglx, akpm, pawan.kumar.gupta, jgross, linux-kernel, linux-doc On 2020/4/28 15:29, Greg KH wrote: > 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@linux.alibaba.com> >>>> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> >>>> --- >>>> >>>> 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. > > thanks, > > greg k-h > Thanks for your explanation, I will reconsider whether this method is suitable. Thanks and best, Tianjia ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline 2020-04-28 7:29 ` Greg KH 2020-04-28 8:24 ` Tianjia Zhang @ 2020-04-28 10:02 ` Jessica Yu 2020-04-29 2:13 ` Tianjia Zhang 1 sibling, 1 reply; 7+ messages in thread From: Jessica Yu @ 2020-04-28 10:02 UTC (permalink / raw) To: Greg KH Cc: Tianjia Zhang, corbet, rdunlap, mchehab+samsung, tglx, akpm, pawan.kumar.gupta, jgross, linux-kernel, linux-doc +++ 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@linux.alibaba.com> >> > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> >> > > --- >> > > >> > > 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@redbean.localdomain 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] module: Allow to disable modsign in kernel cmdline 2020-04-28 10:02 ` Jessica Yu @ 2020-04-29 2:13 ` Tianjia Zhang 0 siblings, 0 replies; 7+ messages in thread From: Tianjia Zhang @ 2020-04-29 2:13 UTC (permalink / raw) To: Jessica Yu, Greg KH Cc: corbet, rdunlap, mchehab+samsung, tglx, akpm, pawan.kumar.gupta, jgross, linux-kernel, linux-doc 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@linux.alibaba.com> >>> > > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com> >>> > > --- >>> > > >>> > > 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@redbean.localdomain > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-04-29 2:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-28 6:00 [PATCH v3] module: Allow to disable modsign in kernel cmdline Tianjia Zhang 2020-04-28 6:35 ` Greg KH 2020-04-28 7:07 ` Tianjia Zhang 2020-04-28 7:29 ` Greg KH 2020-04-28 8:24 ` Tianjia Zhang 2020-04-28 10:02 ` Jessica Yu 2020-04-29 2:13 ` Tianjia Zhang
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).