Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751753AbdFAIqw (ORCPT ); Thu, 1 Jun 2017 04:46:52 -0400 Received: from mail-it0-f54.google.com ([209.85.214.54]:36159 "EHLO mail-it0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751644AbdFAIq1 (ORCPT ); Thu, 1 Jun 2017 04:46:27 -0400 MIME-Version: 1.0 In-Reply-To: <20170601081136.ruiao3w2wfc3hftg@GaryWorkstation> References: <20170512080534.4085-1-glin@suse.com> <20170601081136.ruiao3w2wfc3hftg@GaryWorkstation> From: Ard Biesheuvel Date: Thu, 1 Jun 2017 08:46:26 +0000 Message-ID: Subject: Re: [RFC v2 PATCH] x86/boot: Add the secdata section to the setup header To: Gary Lin Cc: "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , Joey Lee Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6943 Lines: 178 On 1 June 2017 at 08:11, Gary Lin 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 >> Cc: "H. Peter Anvin" >> Cc: Thomas Gleixner >> Cc: Ingo Molnar >> Cc: Joey Lee >> Signed-off-by: Gary Lin >> --- >> 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 >>