lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 07 Sep 2017 14:16:21 -0700
From:   hpa@...or.com
To:     Gary Lin <glin@...e.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>
CC:     "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joey Lee <jlee@...e.com>
Subject: Re: [RFC v2 PATCH] x86/boot: Add the secdata section to the setup header

On September 7, 2017 2:44:51 AM PDT, Gary Lin <glin@...e.com> wrote:
>On Thu, Jun 01, 2017 at 08:46:26AM +0000, Ard Biesheuvel wrote:
>> On 1 June 2017 at 08:11, Gary Lin <glin@...e.com> 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.
>> 
>After check the headers again, another idea came to my mind: the MS-DOS
>stub. It's designed to show a warning while the image is loaded in
>DOS(*),
>but I wonder if it still matters. In the x86 linux efi header, the stub
>is just a 3-lines message, while arm64 completely ignores the stub.
>
>Since there is a offset to the PE header at 0x3c, we can theoretically
>put any thing between 0x40 and the PE header without affecting the
>current settings.
>
>HPA,
>
>Does the MS-DOS stub raise any concern to you?
>
>Thanks,
>
>Gary Lin
>
>(*)
>https://msdn.microsoft.com/zh-tw/library/windows/desktop/ms680547(v=vs.85).aspx#ms-dos_stub__image_only_
>
>> -- 
>> 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@...aro.org>
>> >> Cc: "H. Peter Anvin" <hpa@...or.com>
>> >> Cc: Thomas Gleixner <tglx@...utronix.de>
>> >> Cc: Ingo Molnar <mingo@...hat.com>
>> >> Cc: Joey Lee <jlee@...e.com>
>> >> Signed-off-by: Gary Lin <glin@...e.com>
>> >> ---
>> >>  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
>> >>
>> 

I really don't think that is a good idea.  I would much rather keep this in a space we fully own.
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Powered by blists - more mailing lists