Re: [RFC v2 PATCH] x86/boot: Add the secdata section to the setup header

[Ard Biesheuvel]

On 1 June 2017 at 08:11, Gary Lin <glin@xxxxxxxx> wrote:
> On Fri, May 12, 2017 at 04:05:34PM +0800, Gary Lin wrote:
>> A new section, secdata, in the setup header is introduced to store the
>> distro-specific security version which is designed to help the
>> bootloader to warn the user when loading a less secure or vulnerable
>> kernel. The secdata section can be presented as the following:
>>
>> struct sec_hdr {
>>       __u16 header_length;
>>       __u32 distro_version;
>>       __u16 security_version;
>> } __attribute__((packed));
>> char *signer;
>>
>> It consists of a fixed size structure and a null-terminated string.
>> "header_length" is the size of "struct sec_hdr" and can be used as the
>> offset to "signer". It also can be a kind of the "header version" to
>> detect if any new member is introduced.
>>
>> The kernel packager of the distribution can put the distro name in
>> "signer" and the distro version in "distro_version". When a severe
>> vulnerability is fixed, the packager increases "security_version" in
>> the kernel build afterward. The bootloader can maintain a list of the
>> security versions of the current kernels and only allows the kernel with
>> a higher or equal security version to boot. If the user is going to boot
>> a kernel with a lower security version, a warning should show to prevent
>> the user from loading a vulnerable kernel accidentally.
>>
>> Enabling UEFI Secure Boot is recommended when using the security version
>> or the attacker may alter the security version stealthily.
>>
> Any comment?
>

This is now entirely x86-specific. My preference would be to have a
generic solution instead.

-- 
Ard.


>> v2:
>> - Decrease the size of secdata_offset to 2 bytes since the setup header
>>   is limited to around 32KB.
>> - Restructure the secdata section. The signer is now a null-terminated
>>   string. The type of distro_version changes to u32 in case the distro
>>   uses a long version.
>> - Modify the Kconfig names and add help.
>> - Remove the signer name hack in build.c.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
>> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
>> Cc: Joey Lee <jlee@xxxxxxxx>
>> Signed-off-by: Gary Lin <glin@xxxxxxxx>
>> ---
>>  arch/x86/Kconfig                      | 28 ++++++++++++++++++++++++++++
>>  arch/x86/boot/header.S                | 14 +++++++++++++-
>>  arch/x86/boot/setup.ld                |  1 +
>>  arch/x86/boot/tools/build.c           |  1 -
>>  arch/x86/include/uapi/asm/bootparam.h |  1 +
>>  5 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 5bbdef151805..2c5539518ce0 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1817,6 +1817,34 @@ config EFI_MIXED
>>
>>          If unsure, say N.
>>
>> +config SIGNER_NAME
>> +     string "Signer name"
>> +     default ""
>> +     ---help---
>> +        This option specifies who signs or releases this kernel.
>> +
>> +config DISTRO_VERSION
>> +     int "Distribution version"
>> +     default 0
>> +     range 0 4294967295
>> +     ---help---
>> +       This option specifies the distribution version which this
>> +       kernel belongs to.
>> +
>> +config SECURITY_VERSION
>> +     int "Security version"
>> +     default 0
>> +     range 0 65535
>> +     ---help---
>> +        The security version is the version defined by the distribution
>> +        to indicate the severe security fixes. The bootloader can maintain
>> +        a list of the security versions of the current kernels. After
>> +        fixing a severe vulnerability in the kernel, the distribution can
>> +        increase the security version to notify the bootloader to update
>> +        the list. When booting a kernel with a lower security version,
>> +        the bootloader warns the user to avoid loading a vulnerable kernel
>> +        accidentally.
>> +
>>  config SECCOMP
>>       def_bool y
>>       prompt "Enable seccomp to safely compute untrusted bytecode"
>> diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S
>> index 3dd5be33aaa7..37683caf1668 100644
>> --- a/arch/x86/boot/header.S
>> +++ b/arch/x86/boot/header.S
>> @@ -301,7 +301,7 @@ _start:
>>       # Part 2 of the header, from the old setup.S
>>
>>               .ascii  "HdrS"          # header signature
>> -             .word   0x020d          # header version number (>= 0x0105)
>> +             .word   0x020e          # header version number (>= 0x0105)
>>                                       # or else old loadlin-1.5 will fail)
>>               .globl realmode_swtch
>>  realmode_swtch:      .word   0, 0            # default_switch, SETUPSEG
>> @@ -552,6 +552,7 @@ pref_address:             .quad LOAD_PHYSICAL_ADDR        # preferred load addr
>>
>>  init_size:           .long INIT_SIZE         # kernel initialization size
>>  handover_offset:     .long 0                 # Filled in by build.c
>> +secdata_offset:              .word secdata_start
>>
>>  # End of setup header #####################################################
>>
>> @@ -629,3 +630,14 @@ die:
>>  setup_corrupt:
>>       .byte   7
>>       .string "No setup signature found...\n"
>> +
>> +     .section ".secdata", "a"
>> +secdata_start:
>> +header_length:
>> +     .word   signer - secdata_start
>> +distro_version:
>> +     .long   CONFIG_DISTRO_VERSION
>> +security_version:
>> +     .word   CONFIG_SECURITY_VERSION
>> +signer:
>> +     .string CONFIG_SIGNER_NAME
>> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
>> index 96a6c7563538..43ddbaabaf7a 100644
>> --- a/arch/x86/boot/setup.ld
>> +++ b/arch/x86/boot/setup.ld
>> @@ -18,6 +18,7 @@ SECTIONS
>>       .entrytext      : { *(.entrytext) }
>>       .inittext       : { *(.inittext) }
>>       .initdata       : { *(.initdata) }
>> +     .secdata        : { *(.secdata) }
>>       __end_init = .;
>>
>>       .text           : { *(.text) }
>> diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c
>> index 0702d2531bc7..a629d6b615cf 100644
>> --- a/arch/x86/boot/tools/build.c
>> +++ b/arch/x86/boot/tools/build.c
>> @@ -287,7 +287,6 @@ static inline int reserve_pecoff_reloc_section(int c)
>>  }
>>  #endif /* CONFIG_EFI_STUB */
>>
>> -
>>  /*
>>   * Parse zoffset.h and find the entry points. We could just #include zoffset.h
>>   * but that would mean tools/build would have to be rebuilt every time. It's
>> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h
>> index 07244ea16765..32ffacfaaaff 100644
>> --- a/arch/x86/include/uapi/asm/bootparam.h
>> +++ b/arch/x86/include/uapi/asm/bootparam.h
>> @@ -85,6 +85,7 @@ struct setup_header {
>>       __u64   pref_address;
>>       __u32   init_size;
>>       __u32   handover_offset;
>> +     __u16   secdata_offset;
>>  } __attribute__((packed));
>>
>>  struct sys_desc_table {
>> --
>> 2.12.2
>>