Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race

[Jason A. Donenfeld]

On Tue, Jun 20, 2017 at 8:14 PM, Kees Cook  wrote:
> How about doing this:
>
>default DEBUG_KERNEL
>
> Most distro kernel select DEBUG_KERNEL because it unhides a bunch of
> other useful configs. Since it doesn't strictly _depend_ on
> DEBUG_KERNEL, I think it's probably a mistake to enforce a false
> dependency. Using it as a hint for the default seems maybe like a good
> middle ground. (And if people can't agree on that, then I guess
> "default n"...)

I didn't know you could do that with Kconfig. Great idea. I'll make
this change and submit a new patch to Ted.

Jason

Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race

[Kees Cook]

On Tue, Jun 20, 2017 at 10:50 AM, Sandy Harris  wrote:
> On Tue, Jun 20, 2017 at 5:49 AM, Jeffrey Walton  wrote:
>> On Tue, Jun 20, 2017 at 5:36 AM, Theodore Ts'o  wrote:
>>> On Tue, Jun 20, 2017 at 10:53:35AM +0200, Jason A. Donenfeld wrote:
>
 > Suppressing all messages for all configurations cast a wider net than
 > necessary. Configurations that could potentially be detected and fixed
 > likely will go unnoticed. If the problem is not brought to light, then
 > it won't be fixed.
>
>> Are there compelling reasons a single dmesg warning cannot be provided?
>>
>> A single message avoids spamming the logs. It also informs the system
>> owner of the problem. An individual or organization can then take
>> action based on their risk posture. Finally, it avoids the kernel
>> making policy decisions for a user or organization.
>
> I'd say the best solution is to have no configuration option
> specifically for these messages. Always give some, but let
> DEBUG_KERNEL control how many.
>
> If DEBUG_KERNEL is not set, emit exactly one message & ignore any
> other errors of this type. On some systems, that message may have to
> be ignored, on some it might start an incremental process where one
> problem gets fixed only to have another crop up & on some it might
> prompt the admin to explore further by compiling with DEBUG_KERNEL.
>
> If DEBUG_KERNEL is set, emit a message for every error of this type.

How about doing this:

   default DEBUG_KERNEL

Most distro kernel select DEBUG_KERNEL because it unhides a bunch of
other useful configs. Since it doesn't strictly _depend_ on
DEBUG_KERNEL, I think it's probably a mistake to enforce a false
dependency. Using it as a hint for the default seems maybe like a good
middle ground. (And if people can't agree on that, then I guess
"default n"...)

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race

[Sandy Harris]

On Tue, Jun 20, 2017 at 5:49 AM, Jeffrey Walton  wrote:
> On Tue, Jun 20, 2017 at 5:36 AM, Theodore Ts'o  wrote:
>> On Tue, Jun 20, 2017 at 10:53:35AM +0200, Jason A. Donenfeld wrote:

>>> > Suppressing all messages for all configurations cast a wider net than
>>> > necessary. Configurations that could potentially be detected and fixed
>>> > likely will go unnoticed. If the problem is not brought to light, then
>>> > it won't be fixed.

> Are there compelling reasons a single dmesg warning cannot be provided?
>
> A single message avoids spamming the logs. It also informs the system
> owner of the problem. An individual or organization can then take
> action based on their risk posture. Finally, it avoids the kernel
> making policy decisions for a user or organization.

I'd say the best solution is to have no configuration option
specifically for these messages. Always give some, but let
DEBUG_KERNEL control how many.

If DEBUG_KERNEL is not set, emit exactly one message & ignore any
other errors of this type. On some systems, that message may have to
be ignored, on some it might start an incremental process where one
problem gets fixed only to have another crop up & on some it might
prompt the admin to explore further by compiling with DEBUG_KERNEL.

If DEBUG_KERNEL is set, emit a message for every error of this type.

Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race

[Michael Ellerman]

Theodore Ts'o  writes:

> On Mon, Jun 19, 2017 at 10:57:18PM +0200, Jason A. Donenfeld wrote:
>> 
>> With rc6 already released and rc7 coming up, I'd really appreciate you
>> stepping in here and either ACKing the above commit, or giving your
>> two cents about it in case I need to roll something different.
>
> I actually had set up an earlier version of your patch for on Saturday
> while I was in Beijing.  (Like Linus, I'm attending the LinuxCon China
> conference Monday and Tuesday.)  I had even created the signed tag,
> but I didn't send the pull request to Linus because I was waiting to
> see about how discussions over the locking strategy and the spammy log
> messages on PowerPC was going to get resolved.
>
> I've since respun the commit to reflect your newer patch (see the
> random_for_linus_stable tag on random.git) and rebased the dev branch
> on top of that.  Please take a look and comment.
>
> The other open issue I want to resolve before sending a pull request
> this week is whether we want to change the default for
> CONFIG_WARN_UNSEEDED_RANDOM so that the answer is 'n'.

Yes please.

> It *is* spammy for PowerPC, because they aren't getting their CRNG

*some* powerpc machines ...

> initialized quickly enough, so several userspace processes are getting
> fork/exec'ed with an uninitialized CRNG.  That being said, it is a
> valid warning because it means that the initial stack canary for the
> first couple of PowerPC processes are being created without a fully
> initialized CRNG, which may mean that an attacker might be able to
> circumvent the stack canary on the first couple of processes.  So that
> could potentially be a real security issue on Power.  OTOH, most Power
> users aren't going to be able to do anything about the fact the stack
> canaries of the system daemons started during early boot don't have
> strong randomness, so perhaps we should disable the warning by
> default.

powerpc supports a wide range of hardware platforms, some of which are
10-15 years old, and don't have a hardware RNG.

Is there anything we can do on those machines? Seems like our only
option would be to block the boot while some more "entropy" builds up,
but that's unlikely to be popular with users.

On our newer machines (>= Power8) we have a hardware RNG which we wire
up to arch_get_random_seed_long(), so on those machines the warnings
would be valid, because they'd indicate a bug.

So I think it should be up to arches to decide whether this is turned on
via their defconfigs, and the default should be 'n' because a lot of old
hardware won't be able to do anything useful with the warnings.

cheers

Re: [kernel-hardening] Re: [PATCH] random: silence compiler warnings and fix race

[Joel Stanley]

On Tue, Jun 20, 2017 at 3:33 PM, Theodore Ts'o  wrote:
> On Mon, Jun 19, 2017 at 10:57:18PM +0200, Jason A. Donenfeld wrote:
>>
>> With rc6 already released and rc7 coming up, I'd really appreciate you
>> stepping in here and either ACKing the above commit, or giving your
>> two cents about it in case I need to roll something different.
>
> I actually had set up an earlier version of your patch for on Saturday
> while I was in Beijing.  (Like Linus, I'm attending the LinuxCon China
> conference Monday and Tuesday.)  I had even created the signed tag,
> but I didn't send the pull request to Linus because I was waiting to
> see about how discussions over the locking strategy and the spammy log
> messages on PowerPC was going to get resolved.
>
> I've since respun the commit to reflect your newer patch (see the
> random_for_linus_stable tag on random.git) and rebased the dev branch
> on top of that.  Please take a look and comment.
>
> The other open issue I want to resolve before sending a pull request
> this week is whether we want to change the default for
> CONFIG_WARN_UNSEEDED_RANDOM so that the answer is 'n'.  It *is* spammy
> for PowerPC, because they aren't getting their CRNG initialized
> quickly enough, so several userspace processes are getting
> fork/exec'ed with an uninitialized CRNG.

It's very spammy for ARM as well. I booted next-20170619 on an Aspeed
(32-bit ARM) board and by the time I made it to a shell the log buffer
contained only warnings:

[   10.452921] random: arch_pick_mmap_layout+0xa8/0xe8 get_random_u32
called with crng_init=0
[   10.461255] random: load_elf_binary+0x3c8/0x104c get_random_u32
called with crng_init=0
[   10.471464] random: arch_setup_additional_pages+0x6c/0x110
get_random_u32 called with crng_init=0
[   10.480429] random: randomize_page+0x44/0x58 get_random_u32 called
with crng_init=0
[   10.494802] random: arch_pick_mmap_layout+0xa8/0xe8 get_random_u32
called with crng_init=0
[   10.503141] random: load_elf_binary+0x3c8/0x104c get_random_u32
called with crng_init=0
[   10.511571] random: arch_setup_additional_pages+0x6c/0x110
get_random_u32 called with crng_init=0
[   10.520527] random: randomize_page+0x44/0x58 get_random_u32 called
with crng_init=0
[   10.537847] random: arch_pick_mmap_layout+0xa8/0xe8 get_random_u32
called with crng_init=0
[   10.546182] random: load_elf_binary+0x3c8/0x104c get_random_u32
called with crng_init=0
[   10.554611] random: arch_setup_additional_pages+0x6c/0x110
get_random_u32 called with crng_init=0
[   10.563563] random: randomize_page+0x44/0x58 get_random_u32 called
with crng_init=0

So +1 for defaulting CONFIG_WARN_UNSEEDED_RANDOM=n.

Cheers,

Joel


> That being said, it is a
> valid warning because it means that the initial stack canary for the
> first couple of PowerPC processes are being created without a fully
> initialized CRNG, which may mean that an attacker might be able to
> circumvent the stack canary on the first couple of processes.  So that
> could potentially be a real security issue on Power.  OTOH, most Power
> users aren't going to be able to do anything about the fact the stack
> canaries of the system daemons started during early boot don't have
> strong randomness, so perhaps we should disable the warning by
> default.
>
> Opinions?
>
> - Ted