Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751710AbdFAJ4w (ORCPT ); Thu, 1 Jun 2017 05:56:52 -0400 Received: from smtp.nue.novell.com ([195.135.221.5]:42668 "EHLO smtp.nue.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751675AbdFAJ4u (ORCPT ); Thu, 1 Jun 2017 05:56:50 -0400 Date: Thu, 1 Jun 2017 17:56:29 +0800 From: Gary Lin To: Ard Biesheuvel Cc: "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , Joey Lee Subject: Re: [RFC v2 PATCH] x86/boot: Add the secdata section to the setup header Message-ID: <20170601095629.wwgfedu4w3j7ff2j@GaryWorkstation> References: <20170512080534.4085-1-glin@suse.com> <20170601081136.ruiao3w2wfc3hftg@GaryWorkstation> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7650 Lines: 187 On Thu, Jun 01, 2017 at 08:46:26AM +0000, Ard Biesheuvel wrote: > 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. > That's why I proposed to use the PE/COFF header in the beginning, but it's hard to clear the concern about the potential change in the PE/COFF header. So far, I'm not aware of other generic solution to put those fields, so I'm planning to work on x86 and arm64 separately. Gary Lin > -- > 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 > >> >