Re: [PATCH 0/5] security, efi: Set lockdown if in secure boot mode

[Ard Biesheuvel]

On 31 May 2017 at 13:33, David Howells <dhowells@xxxxxxxxxx> wrote:
> Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>
>> No, I am fine with keeping this as a single series. I don't want
>> anything under drivers/efi to imply policy regarding lockdown. Kernel
>> lockdown should be a feature that lives somewhere else, and which
>> contains a CONFIG_ option that implies 'lockdown is enabled by default
>> when UEFI secure boot is detected.' The code that gets added to
>> drivers/efi should only concern itself with establishing whether
>> secure boot is in effect or not (and can hence be enabled
>> unconditionally)
>> ...
>> So what I would prefer is to separate this from the EFI code,
>
> In that case I don't know where to connect the UEFI secure boot with the
> lockdown code.
>
> I was under the impression that you wanted the switch-statement that I had in
> x86 setup.c moved to the efi code (as I've done in patch 1).  Was I wrong in
> that assessment and that you actually wanted it, say, in security?
>

No, that patch, and the patch that sets the EFI_SECURE_BOOT flag are
perfectly fine. I just think it should be the lockdown code that
contains the efi_enabled(EFI_SECURE_BOOT) check. Note that linux/efi.h
does the right thing in case CONFIG_EFI is not defined.

> I don't think that the non-EFI core code should know about UEFI secure boot
> mode.  Either the arch needs to implement the connection or the EFI code needs
> to implement it.  In the former is preferred, I should drop patch 1.
>
>> ... and perhaps print something like
>>
>> lockdown: Kernel lockdown policy in effect due to xxx
>
> I'm okay with printing that instead.
>
>> and print a subsequent line for every lockdown feature that is enabled, e.g.,
>>
>> lockdown: disabling MSRs
>> lockdown: disabling hibernate support
>
> That could add a lot of lines to the boot output:-/
>

Why is that a bad thing?