Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On Wed, Apr 24, 2019 at 4:36 AM Kees Cook wrote:
>
> On Thu, Apr 11, 2019 at 6:39 PM Masahiro Yamada
> wrote:
> >
> > On Fri, Apr 12, 2019 at 3:01 AM Kees Cook wrote:
> > >
> > > Right now kernel hardening options are scattered around various Kconfig
> > > files. This can be a central place to collect these kinds of options
> > > going forward. This is initially populated with the memory initialization
> > > options from the gcc-plugins.
> > >
> > > Signed-off-by: Kees Cook
> > > ---
> > > scripts/gcc-plugins/Kconfig | 74 +++--
> > > security/Kconfig| 2 +
> > > security/Kconfig.hardening | 93 +
> > > 3 files changed, 102 insertions(+), 67 deletions(-)
> > > create mode 100644 security/Kconfig.hardening
> > >
> > > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > > index 74271dba4f94..84d471dea2b7 100644
> > > --- a/scripts/gcc-plugins/Kconfig
> > > +++ b/scripts/gcc-plugins/Kconfig
> > > @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> > > An arch should select this symbol if it supports building with
> > > GCC plugins.
> > >
> > > -menuconfig GCC_PLUGINS
> > > - bool "GCC plugins"
> > > +config GCC_PLUGINS
> > > + bool
> > > depends on HAVE_GCC_PLUGINS
> > > depends on PLUGIN_HOSTCC != ""
> > > + default y
> > > help
> > > GCC plugins are loadable modules that provide extra features to
> > > the
> > > compiler. They are useful for runtime instrumentation and
> > > static analysis.
> > > @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
> > >
> > > if GCC_PLUGINS
> > >
> > > +menu "GCC plugins"
> > > +
> >
> >
> >
> > Just a tip to save "if" ... "endif" block.
> >
> >
> > If you like, you can write like follows:
> >
> >
> > menu "GCC plugins"
> > depends on GCC_PLUGINS
> >
> >
> >
> > endmenu
>
> Ah yes, thanks! Adjusted.
>
> > > +menu "Memory initialization"
> > > +
> > > +choice
> > > + prompt "Initialize kernel stack variables at function entry"
> > > + depends on GCC_PLUGINS
> >
> > On second thought,
> > this 'depends on' is unnecessary
> > because INIT_STACK_NONE should be always visible.
>
> Oh yes, excellent point. Adjusted.
>
> > Another behavior change is
> > GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
> > and in the compile-test coverage.
>
> I could set the defaults based on CONFIG_COMPILE_TEST, though? I.e.:
>
> prompt "Initialize kernel stack variables at function entry"
> default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
> default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
> default INIT_STACK_NONE
Looks a good idea to me.
Thanks.
--
Best Regards
Masahiro Yamada
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On Thu, Apr 11, 2019 at 6:39 PM Masahiro Yamada
wrote:
>
> On Fri, Apr 12, 2019 at 3:01 AM Kees Cook wrote:
> >
> > Right now kernel hardening options are scattered around various Kconfig
> > files. This can be a central place to collect these kinds of options
> > going forward. This is initially populated with the memory initialization
> > options from the gcc-plugins.
> >
> > Signed-off-by: Kees Cook
> > ---
> > scripts/gcc-plugins/Kconfig | 74 +++--
> > security/Kconfig| 2 +
> > security/Kconfig.hardening | 93 +
> > 3 files changed, 102 insertions(+), 67 deletions(-)
> > create mode 100644 security/Kconfig.hardening
> >
> > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > index 74271dba4f94..84d471dea2b7 100644
> > --- a/scripts/gcc-plugins/Kconfig
> > +++ b/scripts/gcc-plugins/Kconfig
> > @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> > An arch should select this symbol if it supports building with
> > GCC plugins.
> >
> > -menuconfig GCC_PLUGINS
> > - bool "GCC plugins"
> > +config GCC_PLUGINS
> > + bool
> > depends on HAVE_GCC_PLUGINS
> > depends on PLUGIN_HOSTCC != ""
> > + default y
> > help
> > GCC plugins are loadable modules that provide extra features to
> > the
> > compiler. They are useful for runtime instrumentation and static
> > analysis.
> > @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
> >
> > if GCC_PLUGINS
> >
> > +menu "GCC plugins"
> > +
>
>
>
> Just a tip to save "if" ... "endif" block.
>
>
> If you like, you can write like follows:
>
>
> menu "GCC plugins"
> depends on GCC_PLUGINS
>
>
>
> endmenu
Ah yes, thanks! Adjusted.
> > +menu "Memory initialization"
> > +
> > +choice
> > + prompt "Initialize kernel stack variables at function entry"
> > + depends on GCC_PLUGINS
>
> On second thought,
> this 'depends on' is unnecessary
> because INIT_STACK_NONE should be always visible.
Oh yes, excellent point. Adjusted.
> Another behavior change is
> GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
> and in the compile-test coverage.
I could set the defaults based on CONFIG_COMPILE_TEST, though? I.e.:
prompt "Initialize kernel stack variables at function entry"
default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
default INIT_STACK_NONE
>
> It will be disabled for all{yes,mod}config with this patch.
>
> This is up to you. Just FYI.
Thanks for looking this over!
--
Kees Cook
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On 16.04.2019 16:56, Kees Cook wrote:
> On Tue, Apr 16, 2019 at 8:55 AM Alexander Popov wrote:
>>
>> On 16.04.2019 7:02, Kees Cook wrote:
>>> On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov
>>> wrote:
What do you think about some separator between memory initialization
options and
CONFIG_CRYPTO?
>>>
>>> This was true before too
>>
>> Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
>> config options stick together.
>>
>> I've created a patch to fix that. What do you think about it?
>> I can send it to LKML separately.
>>
>>
>> From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
>> From: Alexander Popov
>> Date: Tue, 16 Apr 2019 16:09:40 +0300
>> Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
>> generated config
>>
>> Currently menu blocks start with a pretty header but end with nothing in
>> the generated config. So next config options stick together with the
>> options from the menu block.
>>
>> Let's terminate menu blocks with a newline in the generated config.
>>
>> Signed-off-by: Alexander Popov
>> ---
>> scripts/kconfig/confdata.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>> index 08ba146..1459153 100644
>> --- a/scripts/kconfig/confdata.c
>> +++ b/scripts/kconfig/confdata.c
>> @@ -888,6 +888,8 @@ int conf_write(const char *name)
>> if (menu->next)
>> menu = menu->next;
>> else while ((menu = menu->parent)) {
>> + if (!menu->sym && menu_is_visible(menu))
>> + fprintf(out, "\n");
>> if (menu->next) {
>> menu = menu->next;
>> break;
>
> Seems fine to me. I defer to Masahiro, though. :)
Hi! I've sent this patch separately to LKML:
https://lore.kernel.org/lkml/1555669773-9766-1-git-send-email-alex.po...@linux.com/T/#u
Best regards,
Alexander
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On Tue, Apr 16, 2019 at 8:55 AM Alexander Popov wrote:
>
> On 16.04.2019 7:02, Kees Cook wrote:
> > On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov
> > wrote:
> >>
> >> What do you think about some separator between memory initialization
> >> options and
> >> CONFIG_CRYPTO?
> >
> > This was true before too
>
> Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
> config options stick together.
>
> I've created a patch to fix that. What do you think about it?
> I can send it to LKML separately.
>
>
> From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
> From: Alexander Popov
> Date: Tue, 16 Apr 2019 16:09:40 +0300
> Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
> generated config
>
> Currently menu blocks start with a pretty header but end with nothing in
> the generated config. So next config options stick together with the
> options from the menu block.
>
> Let's terminate menu blocks with a newline in the generated config.
>
> Signed-off-by: Alexander Popov
> ---
> scripts/kconfig/confdata.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 08ba146..1459153 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -888,6 +888,8 @@ int conf_write(const char *name)
> if (menu->next)
> menu = menu->next;
> else while ((menu = menu->parent)) {
> + if (!menu->sym && menu_is_visible(menu))
> + fprintf(out, "\n");
> if (menu->next) {
> menu = menu->next;
> break;
Seems fine to me. I defer to Masahiro, though. :)
--
Kees Cook
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On 16.04.2019 7:02, Kees Cook wrote:
> On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov wrote:
>>
>> What do you think about some separator between memory initialization options
>> and
>> CONFIG_CRYPTO?
>
> This was true before too
Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
config options stick together.
I've created a patch to fix that. What do you think about it?
I can send it to LKML separately.
>From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
From: Alexander Popov
Date: Tue, 16 Apr 2019 16:09:40 +0300
Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
generated config
Currently menu blocks start with a pretty header but end with nothing in
the generated config. So next config options stick together with the
options from the menu block.
Let's terminate menu blocks with a newline in the generated config.
Signed-off-by: Alexander Popov
---
scripts/kconfig/confdata.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 08ba146..1459153 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -888,6 +888,8 @@ int conf_write(const char *name)
if (menu->next)
menu = menu->next;
else while ((menu = menu->parent)) {
+ if (!menu->sym && menu_is_visible(menu))
+ fprintf(out, "\n");
if (menu->next) {
menu = menu->next;
break;
--
2.7.4
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov wrote: > > On 11.04.2019 21:01, Kees Cook wrote: > > Right now kernel hardening options are scattered around various Kconfig > > files. This can be a central place to collect these kinds of options > > going forward. This is initially populated with the memory initialization > > options from the gcc-plugins. > > > > Signed-off-by: Kees Cook > > Hello Kees, hello everyone! > > After applying this series the kernel config looks like that: > > ... > ... > CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor" > > # > # Kernel hardening options > # > > # > # Memory initialization > # > CONFIG_INIT_STACK_NONE=y > # CONFIG_GCC_PLUGIN_STRUCTLEAK_USER is not set > # CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF is not set > # CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL is not set > # CONFIG_GCC_PLUGIN_STACKLEAK is not set > CONFIG_CRYPTO=y > > # > # Crypto core or helper > # > CONFIG_CRYPTO_ALGAPI=y > ... > ... > > What do you think about some separator between memory initialization options > and > CONFIG_CRYPTO? This was true before too: ... # CONFIG_DEFAULT_SECURITY_DAC is not set CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor" CONFIG_XOR_BLOCKS=y CONFIG_ASYNC_CORE=y CONFIG_ASYNC_MEMCPY=y CONFIG_ASYNC_XOR=y CONFIG_ASYNC_PQ=y CONFIG_ASYNC_RAID6_RECOV=y CONFIG_CRYPTO=y ... Perhaps crypto/Kconfig's comment line could move to the top of the file? comment "Crypto core or helper" is what generates the separator... -- Kees Cook
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On 11.04.2019 21:01, Kees Cook wrote: > Right now kernel hardening options are scattered around various Kconfig > files. This can be a central place to collect these kinds of options > going forward. This is initially populated with the memory initialization > options from the gcc-plugins. > > Signed-off-by: Kees Cook Hello Kees, hello everyone! After applying this series the kernel config looks like that: ... ... CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor" # # Kernel hardening options # # # Memory initialization # CONFIG_INIT_STACK_NONE=y # CONFIG_GCC_PLUGIN_STRUCTLEAK_USER is not set # CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF is not set # CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL is not set # CONFIG_GCC_PLUGIN_STACKLEAK is not set CONFIG_CRYPTO=y # # Crypto core or helper # CONFIG_CRYPTO_ALGAPI=y ... ... What do you think about some separator between memory initialization options and CONFIG_CRYPTO? Best regards, Alexander
Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
On Fri, Apr 12, 2019 at 3:01 AM Kees Cook wrote:
>
> Right now kernel hardening options are scattered around various Kconfig
> files. This can be a central place to collect these kinds of options
> going forward. This is initially populated with the memory initialization
> options from the gcc-plugins.
>
> Signed-off-by: Kees Cook
> ---
> scripts/gcc-plugins/Kconfig | 74 +++--
> security/Kconfig| 2 +
> security/Kconfig.hardening | 93 +
> 3 files changed, 102 insertions(+), 67 deletions(-)
> create mode 100644 security/Kconfig.hardening
>
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index 74271dba4f94..84d471dea2b7 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> An arch should select this symbol if it supports building with
> GCC plugins.
>
> -menuconfig GCC_PLUGINS
> - bool "GCC plugins"
> +config GCC_PLUGINS
> + bool
> depends on HAVE_GCC_PLUGINS
> depends on PLUGIN_HOSTCC != ""
> + default y
> help
> GCC plugins are loadable modules that provide extra features to the
> compiler. They are useful for runtime instrumentation and static
> analysis.
> @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
>
> if GCC_PLUGINS
>
> +menu "GCC plugins"
> +
Just a tip to save "if" ... "endif" block.
If you like, you can write like follows:
menu "GCC plugins"
depends on GCC_PLUGINS
endmenu
instead of
if GCC_PLUGINS
menu "GCC plugins"
endmenu
fi
> +menu "Memory initialization"
> +
> +choice
> + prompt "Initialize kernel stack variables at function entry"
> + depends on GCC_PLUGINS
On second thought,
this 'depends on' is unnecessary
because INIT_STACK_NONE should be always visible.
> + default INIT_STACK_NONE
> + help
> + This option enables initialization of stack variables at
> + function entry time. This has the possibility to have the
> + greatest coverage (since all functions can have their
> + variables initialized), but the performance impact depends
> + on the function calling complexity of a given workload's
> + syscalls.
> +
> + This chooses the level of coverage over classes of potentially
> + uninitialized variables. The selected class will be
> + initialized before use in a function.
> +
> + config INIT_STACK_NONE
> + bool "no automatic initialization (weakest)"
> + help
> + Disable automatic stack variable initialization.
> + This leaves the kernel vulnerable to the standard
> + classes of uninitialized stack variable exploits
> + and information exposures.
> +
> + config GCC_PLUGIN_STRUCTLEAK_USER
> + bool "zero-init structs marked for userspace (weak)"
> + depends on GCC_PLUGINS
> + select GCC_PLUGIN_STRUCTLEAK
> + help
> + Zero-initialize any structures on the stack containing
> + a __user attribute. This can prevent some classes of
> + uninitialized stack variable exploits and information
> + exposures, like CVE-2013-2141:
> + https://git.kernel.org/linus/b9e146d8eb3b9eca
> +
> + config GCC_PLUGIN_STRUCTLEAK_BYREF
> + bool "zero-init structs passed by reference (strong)"
> + depends on GCC_PLUGINS
> + select GCC_PLUGIN_STRUCTLEAK
> + help
> + Zero-initialize any structures on the stack that may
> + be passed by reference and had not already been
> + explicitly initialized. This can prevent most classes
> + of uninitialized stack variable exploits and information
> + exposures, like CVE-2017-1000410:
> + https://git.kernel.org/linus/06e7e776ca4d3654
> +
> + config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> + bool "zero-init anything passed by reference (very strong)"
> + depends on GCC_PLUGINS
> + select GCC_PLUGIN_STRUCTLEAK
> + help
> + Zero-initialize any stack variables that may be passed
> + by reference and had not already been explicitly
> + initialized. This is intended to eliminate all classes
> + of uninitialized stack variable exploits and information
> + exposures.
> +
> +endchoice
Another behavior change is
GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
and in the compile-test coverage.
It will be disabled for all{yes,mod}config with this patch.
This is up to you. Just FYI.
--
Best Regards
Masahiro Yamada
[PATCH v2 1/3] security: Create "kernel hardening" config area
Right now kernel hardening options are scattered around various Kconfig files. This can be a central place to collect these kinds of options going forward. This is initially populated with the memory initialization options from the gcc-plugins. Signed-off-by: Kees Cook --- scripts/gcc-plugins/Kconfig | 74 +++-- security/Kconfig| 2 + security/Kconfig.hardening | 93 + 3 files changed, 102 insertions(+), 67 deletions(-) create mode 100644 security/Kconfig.hardening diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig index 74271dba4f94..84d471dea2b7 100644 --- a/scripts/gcc-plugins/Kconfig +++ b/scripts/gcc-plugins/Kconfig @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS An arch should select this symbol if it supports building with GCC plugins. -menuconfig GCC_PLUGINS - bool "GCC plugins" +config GCC_PLUGINS + bool depends on HAVE_GCC_PLUGINS depends on PLUGIN_HOSTCC != "" + default y help GCC plugins are loadable modules that provide extra features to the compiler. They are useful for runtime instrumentation and static analysis. @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS if GCC_PLUGINS +menu "GCC plugins" + config GCC_PLUGIN_CYC_COMPLEXITY bool "Compute the cyclomatic complexity of a function" if EXPERT depends on !COMPILE_TEST# too noisy @@ -66,71 +69,6 @@ config GCC_PLUGIN_LATENT_ENTROPY * https://grsecurity.net/ * https://pax.grsecurity.net/ -config GCC_PLUGIN_STRUCTLEAK - bool "Zero initialize stack variables" - help - While the kernel is built with warnings enabled for any missed - stack variable initializations, this warning is silenced for - anything passed by reference to another function, under the - occasionally misguided assumption that the function will do - the initialization. As this regularly leads to exploitable - flaws, this plugin is available to identify and zero-initialize - such variables, depending on the chosen level of coverage. - - This plugin was originally ported from grsecurity/PaX. More - information at: - * https://grsecurity.net/ - * https://pax.grsecurity.net/ - -choice - prompt "Coverage" - depends on GCC_PLUGIN_STRUCTLEAK - default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL - help - This chooses the level of coverage over classes of potentially - uninitialized variables. The selected class will be - zero-initialized before use. - - config GCC_PLUGIN_STRUCTLEAK_USER - bool "structs marked for userspace" - help - Zero-initialize any structures on the stack containing - a __user attribute. This can prevent some classes of - uninitialized stack variable exploits and information - exposures, like CVE-2013-2141: - https://git.kernel.org/linus/b9e146d8eb3b9eca - - config GCC_PLUGIN_STRUCTLEAK_BYREF - bool "structs passed by reference" - help - Zero-initialize any structures on the stack that may - be passed by reference and had not already been - explicitly initialized. This can prevent most classes - of uninitialized stack variable exploits and information - exposures, like CVE-2017-1000410: - https://git.kernel.org/linus/06e7e776ca4d3654 - - config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL - bool "anything passed by reference" - help - Zero-initialize any stack variables that may be passed - by reference and had not already been explicitly - initialized. This is intended to eliminate all classes - of uninitialized stack variable exploits and information - exposures. - -endchoice - -config GCC_PLUGIN_STRUCTLEAK_VERBOSE - bool "Report forcefully initialized variables" - depends on GCC_PLUGIN_STRUCTLEAK - depends on !COMPILE_TEST# too noisy - help - This option will cause a warning to be printed each time the - structleak plugin finds a variable it thinks needs to be - initialized. Since not all existing initializers are detected - by the plugin, this can produce false positive warnings. - config GCC_PLUGIN_RANDSTRUCT bool "Randomize layout of sensitive kernel structures" select MODVERSIONS if MODULES @@ -226,4 +164,6 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK bool depends on GCC_PLUGINS && ARM +endmenu + endif diff --git a/security/Kconfig b/security/Kconfig index 1d6463fb1450..7aec8d094ce2 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -249,5 +249,7 @@ config LSM
