On Sat, May 06, 2017 at 01:12:10AM -0700, [email protected] wrote: > On May 5, 2017 2:26:39 AM PDT, Gary Lin <[email protected]> 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 <[email protected]> > >Cc: "H. Peter Anvin" <[email protected]> > >Cc: Thomas Gleixner <[email protected]> > >Cc: Ingo Molnar <[email protected]> > >Cc: Joey Lee <[email protected]> > >Signed-off-by: Gary Lin <[email protected]> > >--- > > 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.
Right, 2-byte is enough for secdata_offset. > Since this structure is a static ABI, it should have a way to grow it (a > length field is usually a good choice.) > sec_length serves for the purpose. It's the length of the secdata section, so we can use it to determine if there is a new field. > 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. > They are all distro-specific. sec_signer is basically the distro name. sec_distro is the distro version. sec_version is the "Security Version" defined by the distro. If grub2 is booting a "less-secure" kernel, i.e. a kernel with a lower Security Version, a warning will show to notify the user. Cheers, Gary Lin
