| 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
| ||
|
Message-ID: <E33A1188-7198-4C10-81B8-902644B1AAC6@zytor.com>
Date: Sat, 06 May 2017 01:12:10 -0700
From: hpa@...or.com
To: Gary Lin <glin@...e.com>, x86@...nel.org
CC: linux-kernel@...r.kernel.org,
Ard Biesheuvel <ard.biesheuvel@...aro.org>,
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 May 5, 2017 2:26:39 AM PDT, 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
>+sec_distro:
>+ .word CONFIG_SEC_DISTRO
>+sec_version:
>+ .word CONFIG_SEC_VERSION
>+secdata_end:
>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..ec4b311d7b2d 100644
>--- a/arch/x86/boot/tools/build.c
>+++ b/arch/x86/boot/tools/build.c
>@@ -287,6 +287,15 @@ static inline int reserve_pecoff_reloc_section(int
>c)
> }
> #endif /* CONFIG_EFI_STUB */
>
>+static void security_fields_update(void)
>+{
>+ unsigned int security_offset;
>+ char *dest;
>+
>+ security_offset = get_unaligned_le32(&buf[0x268]);
>+ dest = (char *)&buf[security_offset + 4];
>+ strncpy(dest, CONFIG_SEC_SIGNER, 8);
>+}
>
> /*
>* Parse zoffset.h and find the entry points. We could just #include
>zoffset.h
>@@ -401,6 +410,8 @@ int main(int argc, char ** argv)
>
> efi_stub_entry_update();
>
>+ security_fields_update();
>+
> crc = partial_crc32(buf, i, crc);
> if (fwrite(buf, 1, i, dest) != i)
> die("Writing setup failed");
>diff --git a/arch/x86/include/uapi/asm/bootparam.h
>b/arch/x86/include/uapi/asm/bootparam.h
>index 07244ea16765..713ae5d933a4 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;
>+ __u32 secdata_offset;
> } __attribute__((packed));
>
> struct sys_desc_table {
There is no need to burn a 4-byte field if this is inside the setup area, as that is limited to ~32K anyway. Since this structure is a static ABI, it should have a way to grow it (a length field is usually a good choice.)
What are the semantics of these fields? If they are supposed to be distribution-independent, they need semantics defined, otherwise they might better be a distro-specific use field.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
Powered by blists - more mailing lists