Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932080AbdIGJpF (ORCPT ); Thu, 7 Sep 2017 05:45:05 -0400 Received: from victor.provo.novell.com ([137.65.250.26]:38637 "EHLO prv3-mh.provo.novell.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754919AbdIGJpD (ORCPT ); Thu, 7 Sep 2017 05:45:03 -0400 Date: Thu, 7 Sep 2017 17:44:51 +0800 From: Gary Lin To: Ard Biesheuvel , "H. Peter Anvin" Cc: "x86@kernel.org" , "linux-kernel@vger.kernel.org" , Thomas Gleixner , Ingo Molnar , Joey Lee Subject: Re: [RFC v2 PATCH] x86/boot: Add the secdata section to the setup header Message-ID: <20170907094451.2h2cbxpfmtga7buf@localhost> 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: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7994 Lines: 199 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. > 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 > >> 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 > >> >