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

[joeyli <jlee@suse.com>]

Hi,

On Mon, Jul 10, 2017 at 11:24:44AM +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.
> 
> (For more details: https://github.com/lcp/shim/wiki/Security-Version)
> 
> 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 <ard.biesheuvel@linaro.org>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Joey Lee <jlee@suse.com>
> Signed-off-by: Gary Lin <glin@suse.com>

I have reviewed and tested this patch. Please feel free to add:

Signed-off-by: Joey Lee <jlee@suse.com>

Thanks
Joey Lee

> ---
>  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 316152f72bb9..043ff86828a6 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1828,6 +1828,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 2ed8f0c25def..c62e0baf2d89 100644
> --- a/arch/x86/boot/header.S
> +++ b/arch/x86/boot/header.S
> @@ -300,7 +300,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
> @@ -551,6 +551,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 #####################################################
>  
> @@ -628,3 +629,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 ddef37b16af2..c46763929a6b 100644
> --- a/arch/x86/include/uapi/asm/bootparam.h
> +++ b/arch/x86/include/uapi/asm/bootparam.h
> @@ -84,6 +84,7 @@ struct setup_header {
>  	__u64	pref_address;
>  	__u32	init_size;
>  	__u32	handover_offset;
> +	__u16	secdata_offset;
>  } __attribute__((packed));
>  
>  struct sys_desc_table {
> -- 
> 2.13.2