Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1636268 > unrolled thread
| Started by | Gary Lin <glin@suse.com> |
|---|---|
| First post | 2017-05-05 11:30 +0200 |
| Last post | 2017-05-08 08:40 +0200 |
| Articles | 6 — 3 participants |
Back to article view | Back to linux.kernel
[RFC PATCH] x86/boot: Add the secdata section to the setup header Gary Lin <glin@suse.com> - 2017-05-05 11:30 +0200
Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header hpa@zytor.com - 2017-05-06 10:20 +0200
Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header Gary Lin <glin@suse.com> - 2017-05-08 07:00 +0200
Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header Brian Gerst <brgerst@gmail.com> - 2017-05-06 19:40 +0200
Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header hpa@zytor.com - 2017-05-06 20:50 +0200
Re: [RFC PATCH] x86/boot: Add the secdata section to the setup header Gary Lin <glin@suse.com> - 2017-05-08 08:40 +0200
| From | Gary Lin <glin@suse.com> |
|---|---|
| Date | 2017-05-05 11:30 +0200 |
| Subject | [RFC PATCH] x86/boot: Add the secdata section to the setup header |
| Message-ID | <tDHIJ-8ec-7@gated-at.bofh.it> |
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 <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>
---
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 {
--
2.12.2
[toc] | [next] | [standalone]
| From | hpa@zytor.com |
|---|---|
| Date | 2017-05-06 10:20 +0200 |
| Message-ID | <tE36x-5Bj-3@gated-at.bofh.it> |
| In reply to | #1636268 |
On May 5, 2017 2:26:39 AM PDT, Gary Lin <glin@suse.com> 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 <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>
>---
> 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. Since this structure is a static ABI, it should have a way to grow it (a length field is usually a good choice.)
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.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
[toc] | [prev] | [next] | [standalone]
| From | Gary Lin <glin@suse.com> |
|---|---|
| Date | 2017-05-08 07:00 +0200 |
| Message-ID | <tEIW5-7y9-3@gated-at.bofh.it> |
| In reply to | #1636842 |
On Sat, May 06, 2017 at 01:12:10AM -0700, hpa@zytor.com wrote:
> On May 5, 2017 2:26:39 AM PDT, Gary Lin <glin@suse.com> 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 <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>
> >---
> > 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
[toc] | [prev] | [next] | [standalone]
| From | Brian Gerst <brgerst@gmail.com> |
|---|---|
| Date | 2017-05-06 19:40 +0200 |
| Message-ID | <tEbQt-2x6-11@gated-at.bofh.it> |
| In reply to | #1636268 |
On Fri, May 5, 2017 at 5:26 AM, Gary Lin <glin@suse.com> 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 <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> > --- > 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 A more flexible way to do this would be to make sec_signer an offset to a null-terminated string. That way it can be any length, and you wouldn't need the build tool hack. -- Brian Gerst
[toc] | [prev] | [next] | [standalone]
| From | hpa@zytor.com |
|---|---|
| Date | 2017-05-06 20:50 +0200 |
| Message-ID | <tEcWe-3eo-7@gated-at.bofh.it> |
| In reply to | #1636956 |
On May 6, 2017 10:34:49 AM PDT, Brian Gerst <brgerst@gmail.com> wrote: >On Fri, May 5, 2017 at 5:26 AM, Gary Lin <glin@suse.com> 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 <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> >> --- >> 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 > >A more flexible way to do this would be to make sec_signer an offset >to a null-terminated string. That way it can be any length, and you >wouldn't need the build tool hack. > >-- >Brian Gerst Also, there is no fundamental reason this has to be a fixed structure; it could directly contain variable width entries as long as it remains parsable. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
[toc] | [prev] | [next] | [standalone]
| From | Gary Lin <glin@suse.com> |
|---|---|
| Date | 2017-05-08 08:40 +0200 |
| Message-ID | <tEKuS-mJ-5@gated-at.bofh.it> |
| In reply to | #1636956 |
On Sat, May 06, 2017 at 01:34:49PM -0400, Brian Gerst wrote: > On Fri, May 5, 2017 at 5:26 AM, Gary Lin <glin@suse.com> 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 <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> > > --- > > 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 > > A more flexible way to do this would be to make sec_signer an offset > to a null-terminated string. That way it can be any length, and you > wouldn't need the build tool hack. > I was thinking about making sec_signer a variable length string and decided to make sec_signer fixed since it's easier to match 2 fixed size variables (just convert them to u32 or u64). Besides, I also want to limit the size of sec_signer because the list of security version will be stored in the NVRAM which is quite limited (Is it possible to limit the length of a string in kconfig?). If the variable size string makes more sense, I'll update that in v2. Thanks, Gary Lin
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web