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

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] [day] [month] [year] [list]

Message-ID: <20170508063505.4jom257jy6o3nsoo@GaryWorkstation>
Date:   Mon, 8 May 2017 14:35:05 +0800
From:   Gary Lin <glin@...e.com>
To:     Brian Gerst <brgerst@...il.com>
Cc:     the arch/x86 maintainers <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joey Lee <jlee@...e.com>
Subject: Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header

On Sat, May 06, 2017 at 01:34:49PM -0400, Brian Gerst wrote:
> On Fri, May 5, 2017 at 5:26 AM, Gary Lin <glin@...e.com> wrote:
> > This is a different approach to replace my previous implementation of
> > Security Version(*). Instead of using the fields in the PE/COFF header,
> > this commit adds secdata_offset in the setup header for the file offset
> > of secdata. Currently, the secdata section contains the signer's name,
> > the distro version, and the security version as defined in the wiki page.
> > Since we don't rely on the PE/COFF header anymore, the size of signer is
> > increased to 8 bytes to store more characters.
> >
> > Since this is just a tentative patch, I haven't started the other parts
> > (shim and mokutil) yet, and it's flexible to change. Any comment and
> > suggestion are welcome.
> >
> > (*) https://github.com/lcp/shim/wiki/Security-Version
> >
> > 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                      | 14 ++++++++++++++
> >  arch/x86/boot/header.S                | 15 ++++++++++++++-
> >  arch/x86/boot/setup.ld                |  1 +
> >  arch/x86/boot/tools/build.c           | 11 +++++++++++
> >  arch/x86/include/uapi/asm/bootparam.h |  1 +
> >  5 files changed, 41 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index 5bbdef151805..09f99cd1e699 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -1817,6 +1817,20 @@ config EFI_MIXED
> >
> >            If unsure, say N.
> >
> > +config SEC_SIGNER
> > +       string "The signer name"
> > +       default "none"
> > +
> > +config SEC_DISTRO
> > +       int "The distro version"
> > +       default 0
> > +       range 0 65535
> > +
> > +config SEC_VERSION
> > +       int "The security version"
> > +       default 0
> > +       range 0 65535
> > +
> >  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..f751790f1f44 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:                .long secdata_start
> >
> >  # End of setup header #####################################################
> >
> > @@ -629,3 +630,15 @@ die:
> >  setup_corrupt:
> >         .byte   7
> >         .string "No setup signature found...\n"
> > +
> > +       .section ".secdata", "a"
> > +secdata_start:
> > +sec_length:
> > +       .long   secdata_end - secdata_start
> > +sec_signer:
> > +       .quad   0                               # Filled by build.c
> 
> A more flexible way to do this would be to make sec_signer an offset
> to a null-terminated string.  That way it can be any length, and you
> wouldn't need the build tool hack.
> 
I was thinking about making sec_signer a variable length string and decided
to make sec_signer fixed since it's easier to match 2 fixed size
variables (just convert them to u32 or u64). Besides, I also want to
limit the size of sec_signer because the list of security version will
be stored in the NVRAM which is quite limited (Is it possible to limit
the length of a string in kconfig?).

If the variable size string makes more sense, I'll update that in v2.

Thanks,

Gary Lin