[PATCH v4 0/4] Certificate insertion support for x86 bzImages

[PATCH v4 0/4] Certificate insertion support for x86 bzImages

[Mehmet Kayaalp]

These patches add support for modifying the reserved space for extra
certificates in a compressed bzImage in x86. This allows separating the
system keyring certificate from the kernel build process. After the kernel
image is distributed, the insert-sys-cert script can be used to insert the
certificate for x86.

Changes:

v4:
* Applied checkpatch.pl suggestions (2/4, 3/4)
* Cleaned up the commit messages (1/4, 2/4)
* Added the build file to .gitignore (1/4)

v3:
* Rewrote 1/4 to insert incompressible bytes are at build time. Previous
solution required changes to <arch>/boot/Makefile's for modifying the
vmlinux file after linking, and did not work well with cross compilation.
* Added 2/4 for ELF class-independent processing of vmlinux file, in case
the script was compiled for 64-bit and the kernel was compiled for 32-bit.
* Reordered 3/4, added x86 bzImage boot version (>=2.08) verification.

v2:
* Rebased arch/boot/x86/Makefile patch (removed in v3)

Mehmet Kayaalp (4):
  KEYS: Insert incompressible bytes to reserve space in bzImage
  KEYS: Add ELF class-independent certificate insertion support
  KEYS: Support for inserting a certificate into x86 bzImage
  KEYS: Print insert-sys-cert information to stdout instead of stderr

 certs/.gitignore            |   1 +
 certs/Makefile              |  21 +-
 certs/system_certificates.S |   2 +-
 scripts/Makefile            |   1 +
 scripts/insert-sys-cert.c   | 453 +++++++++++++++++++++++++++++++++-----------
 5 files changed, 361 insertions(+), 117 deletions(-)

-- 
2.7.4

[PATCH v4 1/4] KEYS: Insert incompressible bytes to reserve space in bzImage

[Mehmet Kayaalp]

Include a random filled binary in vmlinux at the space reserved with
CONFIG_SYSTEM_EXTRA_CERTIFICATE. This results in an uncompressed reserved
area inside the bzImage as well, so that it can be replaced with an actual
certificate later (after the bzImage is distributed).

The bzImage contains a stripped ELF file with one section containing the
compressed vmlinux. If the reserved space is initially filled with zeros,
certificate insertion will cause a size increase in the compressed vmlinux.
In that case, reconstructing the bzImage would require relocation. To avoid
this situation, the reserved space is initially filled with random bytes.
Since a certificate contains some compressible bytes, after insertion the
vmlinux will hopefully be compressed to a smaller size.

Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
---
 certs/.gitignore            |  1 +
 certs/Makefile              | 21 ++++++++++++++++++---
 certs/system_certificates.S |  2 +-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/certs/.gitignore b/certs/.gitignore
index f51aea4..4ecc8dd 100644
--- a/certs/.gitignore
+++ b/certs/.gitignore
@@ -2,3 +2,4 @@
 # Generated files
 #
 x509_certificate_list
+extra_cert_placeholder
diff --git a/certs/Makefile b/certs/Makefile
index 4119bb3..ad04feb 100644
--- a/certs/Makefile
+++ b/certs/Makefile
@@ -15,7 +15,12 @@ ifeq ($(CONFIG_SYSTEM_TRUSTED_KEYRING),y)
 $(eval $(call config_filename,SYSTEM_TRUSTED_KEYS))
 
 # GCC doesn't include .incbin files in -MD generated dependencies (PR#66871)
-$(obj)/system_certificates.o: $(obj)/x509_certificate_list
+ifeq ($(CONFIG_SYSTEM_EXTRA_CERTIFICATE),y)
+system_certs_incbin = $(obj)/x509_certificate_list $(obj)/extra_cert_placeholder
+else
+system_certs_incbin = $(obj)/x509_certificate_list
+endif
+$(obj)/system_certificates.o: $(system_certs_incbin)
 
 # Cope with signing_key.x509 existing in $(srctree) not $(objtree)
 AFLAGS_system_certificates.o := -I$(srctree)
@@ -23,12 +28,22 @@ AFLAGS_system_certificates.o := -I$(srctree)
 quiet_cmd_extract_certs  = EXTRACT_CERTS   $(patsubst "%",%,$(2))
       cmd_extract_certs  = scripts/extract-cert $(2) $@ || ( rm $@; exit 1)
 
-targets += x509_certificate_list
+targets += $(system_certs_incbin)
 $(obj)/x509_certificate_list: scripts/extract-cert $(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(SYSTEM_TRUSTED_KEYS_FILENAME) FORCE
 	$(call if_changed,extract_certs,$(SYSTEM_TRUSTED_KEYS_SRCPREFIX)$(CONFIG_SYSTEM_TRUSTED_KEYS))
+
+ifeq ($(CONFIG_SYSTEM_EXTRA_CERTIFICATE),y)
+# Generate incompressible bytes. Use seed to make it reproducible
+quiet_cmd_placeholder = EXTRA_CERT_PLACEHOLDER
+      cmd_placeholder = perl -e 'srand(0); printf("%c", int(rand(256))) for (1..$(2))' > $@
+
+$(obj)/extra_cert_placeholder: FORCE
+	$(call if_changed,placeholder,$(CONFIG_SYSTEM_EXTRA_CERTIFICATE_SIZE))
+endif
+
 endif
 
-clean-files := x509_certificate_list .x509.list
+clean-files := $(system_certs_incbin) .x509.list
 
 ifeq ($(CONFIG_MODULE_SIG),y)
 ###############################################################################
diff --git a/certs/system_certificates.S b/certs/system_certificates.S
index c9ceb71..02b9222 100644
--- a/certs/system_certificates.S
+++ b/certs/system_certificates.S
@@ -17,7 +17,7 @@ __cert_list_end:
 	.globl VMLINUX_SYMBOL(system_extra_cert)
 	.size system_extra_cert, CONFIG_SYSTEM_EXTRA_CERTIFICATE_SIZE
 VMLINUX_SYMBOL(system_extra_cert):
-	.fill CONFIG_SYSTEM_EXTRA_CERTIFICATE_SIZE, 1, 0
+	.incbin "certs/extra_cert_placeholder"
 
 	.align 4
 	.globl VMLINUX_SYMBOL(system_extra_cert_used)
-- 
2.7.4

Re: [PATCH v4 1/4] KEYS: Insert incompressible bytes to reserve space in bzImage

[Henrique de Moraes Holschuh]

On Thu, 20 Apr 2017, Mehmet Kayaalp wrote:
> Include a random filled binary in vmlinux at the space reserved with
> CONFIG_SYSTEM_EXTRA_CERTIFICATE. This results in an uncompressed reserved

Random data is not always going to be completely incompressible.  And
just how much it could be compressed also depends on the compression
engine.

Failures here would be quite annoying, even if they would be rare (not
just due to the randomness factor, but also depending on just how
overprovisioned the space reserved for the extra certificate was when
compared with the real certificate size).

Maybe it would be safer if you test it for incompressability once you
generated the random data (using the same compression engine that the
image will use)?  If it fails, add some overprovisioning and retry...

Alternatively, you could ship a static file with random data that has
been tested to be uncompressible "enough" for every currently supported
compression engine, maybe with a bit of a safety margin just in case a
future compression engine does somewhat better...

-- 
  Henrique Holschuh

Re: [PATCH v4 1/4] KEYS: Insert incompressible bytes to reserve space in bzImage

[Mehmet Kayaalp]

> On Apr 20, 2017, at 7:13 PM, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> 
> On Thu, 20 Apr 2017, Mehmet Kayaalp wrote:
>> Include a random filled binary in vmlinux at the space reserved with
>> CONFIG_SYSTEM_EXTRA_CERTIFICATE. This results in an uncompressed reserved
> 
> Random data is not always going to be completely incompressible.  And
> just how much it could be compressed also depends on the compression
> engine.

It's almost impossible to compress random data. The compression 
algorithm is looking for a pattern, and there is none. Plus, the format of 
the compression adds an overhead.

    for n in `seq 8 24`; do \
    perl -e 'srand(0); printf("%c", int(rand(256))) for (1..(1<<$ARGV[0]))' \
    $n | bzip2 -9 -c -v > /dev/null; \
    done
  (stdin):  0.615:1, 13.000 bits/byte, -62.50% saved, 256 in, 416 out.
  (stdin):  0.760:1, 10.531 bits/byte, -31.64% saved, 512 in, 674 out.
  (stdin):  0.780:1, 10.250 bits/byte, -28.12% saved, 1024 in, 1312 out.
  (stdin):  0.831:1,  9.629 bits/byte, -20.36% saved, 2048 in, 2465 out.
  (stdin):  0.895:1,  8.939 bits/byte, -11.74% saved, 4096 in, 4577 out.
  (stdin):  0.947:1,  8.449 bits/byte, -5.62% saved, 8192 in, 8652 out.
  (stdin):  0.972:1,  8.232 bits/byte, -2.90% saved, 16384 in, 16859 out.
  (stdin):  0.985:1,  8.122 bits/byte, -1.52% saved, 32768 in, 33266 out.
  (stdin):  0.990:1,  8.079 bits/byte, -0.99% saved, 65536 in, 66187 out.
  (stdin):  0.993:1,  8.060 bits/byte, -0.75% saved, 131072 in, 132055 out.
  (stdin):  0.994:1,  8.048 bits/byte, -0.60% saved, 262144 in, 263720 out.
  (stdin):  0.995:1,  8.042 bits/byte, -0.53% saved, 524288 in, 527067 out.
  (stdin):  0.996:1,  8.036 bits/byte, -0.45% saved, 1048576 in, 1053269 out.
  (stdin):  0.996:1,  8.036 bits/byte, -0.45% saved, 2097152 in, 2106558 out.
  (stdin):  0.996:1,  8.035 bits/byte, -0.44% saved, 4194304 in, 4212741 out.
  (stdin):  0.996:1,  8.035 bits/byte, -0.44% saved, 8388608 in, 8425198 out.
  (stdin):  0.996:1,  8.035 bits/byte, -0.44% saved, 16777216 in, 16850429 out.

> Failures here would be quite annoying, even if they would be rare (not
> just due to the randomness factor, but also depending on just how
> overprovisioned the space reserved for the extra certificate was when
> compared with the real certificate size).

I wanted to avoid the randomness with the seed. For a given size, you 
will get the same random payload. There will be over-provisioning, but
primarily because we don't know the size of the certificate to be inserted,
and how much it will compress.

> Maybe it would be safer if you test it for incompressability once you
> generated the random data (using the same compression engine that the
> image will use)?  If it fails, add some overprovisioning and retry...

The actual amount of over-provisioning will depend on the size and the
compressibility of the certificate to be inserted. If you have a longer CN,
it will compress more.

> Alternatively, you could ship a static file with random data that has
> been tested to be uncompressible "enough" for every currently supported
> compression engine, maybe with a bit of a safety margin just in case a
> future compression engine does somewhat better...

The seed makes it static for a given size, and I tested it to be
incompressible. But I don't know about the safety margin. Even without the
compression, the reserved size is not accurate. If you reserve 4096 bytes,
the DER encoded certificate inserted is not going to be exactly 4096 either
(for reference, the built-in certificate is 1346 bytes). Compression makes it 
a little more inaccurate, but is over-provisioning several hundreds of bytes 
a concern when the bzImage is several megabytes?

Unless you have a compression method that detects how the pseudo-random 
bytes are generated and encodes the recipe to generate it again, you can't 
achieve positive compression rates. I'm sure there is a proof for true random
generation involving the Shannon entropy. 

>  Henrique Holschuh

Mehmet

Re: [PATCH v4 1/4] KEYS: Insert incompressible bytes to reserve space in bzImage

[Henrique de Moraes Holschuh]

On Thu, 20 Apr 2017, Mehmet Kayaalp wrote:
> > On Apr 20, 2017, at 7:13 PM, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> > On Thu, 20 Apr 2017, Mehmet Kayaalp wrote:
> >> Include a random filled binary in vmlinux at the space reserved with
> >> CONFIG_SYSTEM_EXTRA_CERTIFICATE. This results in an uncompressed reserved

...

> > Alternatively, you could ship a static file with random data that has
> > been tested to be uncompressible "enough" for every currently supported
> > compression engine, maybe with a bit of a safety margin just in case a
> > future compression engine does somewhat better...
> 
> The seed makes it static for a given size, and I tested it to be
> incompressible. But I don't know about the safety margin. Even without the

If you tested the result to be incompressible enough, it is fine with me.

> compression, the reserved size is not accurate. If you reserve 4096 bytes,
> the DER encoded certificate inserted is not going to be exactly 4096 either
> (for reference, the built-in certificate is 1346 bytes). Compression makes it 
> a little more inaccurate, but is over-provisioning several hundreds of bytes 
> a concern when the bzImage is several megabytes?

Maybe for embedded, but in that case any overprovisioning would already
be too much, and one has to fix the issue in some other way.

-- 
  Henrique Holschuh

[PATCH v4 2/4] KEYS: Add ELF class-independent certificate insertion support

[Mehmet Kayaalp]

Use ELF class-independent GElf API for processing the kernel binary. This
patch adds support for compiling the script for 64-bit and the kernel for
32-bit (e.g. make ARCH=i386 on x86-64).

Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
---
 scripts/Makefile          |   1 +
 scripts/insert-sys-cert.c | 215 +++++++++++++++++++++++-----------------------
 2 files changed, 109 insertions(+), 107 deletions(-)

diff --git a/scripts/Makefile b/scripts/Makefile
index 1d80897..5633f2c 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -25,6 +25,7 @@ HOSTCFLAGS_sortextable.o = -I$(srctree)/tools/include
 HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
 HOSTLOADLIBES_sign-file = -lcrypto
 HOSTLOADLIBES_extract-cert = -lcrypto
+HOSTLOADLIBES_insert-sys-cert = -lelf
 
 always		:= $(hostprogs-y) $(hostprogs-m)
 
diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
index 8902836..86a3d41 100644
--- a/scripts/insert-sys-cert.c
+++ b/scripts/insert-sys-cert.c
@@ -24,7 +24,8 @@
 #include <sys/mman.h>
 #include <fcntl.h>
 #include <unistd.h>
-#include <elf.h>
+#include <gelf.h>
+#include <linux/types.h>
 
 #define CERT_SYM  "system_extra_cert"
 #define USED_SYM  "system_extra_cert_used"
@@ -34,18 +35,6 @@
 #define warn(format, args...) fprintf(stdout, "WARNING: " format, ## args)
 #define  err(format, args...) fprintf(stderr, "ERROR:   " format, ## args)
 
-#if UINTPTR_MAX == 0xffffffff
-#define CURRENT_ELFCLASS ELFCLASS32
-#define Elf_Ehdr	Elf32_Ehdr
-#define Elf_Shdr	Elf32_Shdr
-#define Elf_Sym		Elf32_Sym
-#else
-#define CURRENT_ELFCLASS ELFCLASS64
-#define Elf_Ehdr	Elf64_Ehdr
-#define Elf_Shdr	Elf64_Shdr
-#define Elf_Sym		Elf64_Sym
-#endif
-
 static unsigned char endianness(void)
 {
 	uint16_t two_byte = 0x00FF;
@@ -65,22 +54,17 @@ struct sym {
 	int size;
 };
 
-static unsigned long get_offset_from_address(Elf_Ehdr *hdr, unsigned long addr)
+static unsigned long get_offset_from_address(Elf *elf, unsigned long addr)
 {
-	Elf_Shdr *x;
-	unsigned int i, num_sections;
-
-	x = (void *)hdr + hdr->e_shoff;
-	if (hdr->e_shnum == SHN_UNDEF)
-		num_sections = x[0].sh_size;
-	else
-		num_sections = hdr->e_shnum;
-
-	for (i = 1; i < num_sections; i++) {
-		unsigned long start = x[i].sh_addr;
-		unsigned long end = start + x[i].sh_size;
-		unsigned long offset = x[i].sh_offset;
-
+	Elf_Scn *scn = NULL;
+	GElf_Shdr shdr;
+	unsigned long start, end, offset;
+
+	while ((scn = elf_nextscn(elf, scn)) != NULL) {
+		gelf_getshdr(scn, &shdr);
+		start = shdr.sh_addr;
+		end = start + shdr.sh_size;
+		offset = shdr.sh_offset;
 		if (addr >= start && addr <= end)
 			return addr - start + offset;
 	}
@@ -90,7 +74,7 @@ static unsigned long get_offset_from_address(Elf_Ehdr *hdr, unsigned long addr)
 
 #define LINE_SIZE 100
 
-static void get_symbol_from_map(Elf_Ehdr *hdr, FILE *f, char *name,
+static void get_symbol_from_map(Elf *elf, void *base, FILE *f, char *name,
 				struct sym *s)
 {
 	char l[LINE_SIZE];
@@ -125,76 +109,65 @@ static void get_symbol_from_map(Elf_Ehdr *hdr, FILE *f, char *name,
 	s->address = strtoul(l, NULL, 16);
 	if (s->address == 0)
 		return;
-	s->offset = get_offset_from_address(hdr, s->address);
+	s->offset = get_offset_from_address(elf, s->address);
 	s->name = name;
-	s->content = (void *)hdr + s->offset;
-}
-
-static Elf_Sym *find_elf_symbol(Elf_Ehdr *hdr, Elf_Shdr *symtab, char *name)
-{
-	Elf_Sym *sym, *symtab_start;
-	char *strtab, *symname;
-	unsigned int link;
-	Elf_Shdr *x;
-	int i, n;
-
-	x = (void *)hdr + hdr->e_shoff;
-	link = symtab->sh_link;
-	symtab_start = (void *)hdr + symtab->sh_offset;
-	n = symtab->sh_size / symtab->sh_entsize;
-	strtab = (void *)hdr + x[link].sh_offset;
-
-	for (i = 0; i < n; i++) {
-		sym = &symtab_start[i];
-		symname = strtab + sym->st_name;
-		if (strcmp(symname, name) == 0)
-			return sym;
-	}
-	err("Unable to find symbol: %s\n", name);
-	return NULL;
+	s->content = (void *)base + s->offset;
 }
 
-static void get_symbol_from_table(Elf_Ehdr *hdr, Elf_Shdr *symtab,
+static void get_symbol_from_table(Elf *elf, Elf_Scn *symtab, void *base,
 				  char *name, struct sym *s)
 {
-	Elf_Shdr *sec;
-	int secndx;
-	Elf_Sym *elf_sym;
-	Elf_Shdr *x;
+	GElf_Shdr shdr;
+	Elf_Data *data;
+	int count;
+	int i;
+	GElf_Sym sym;
+	char *symname;
+	int found = 0;
+	size_t secndx;
+	Elf_Scn *sec;
+
+	gelf_getshdr(symtab, &shdr);
+	data = elf_getdata(symtab, NULL);
+	count = shdr.sh_size / shdr.sh_entsize;
+
+	for (i = 0; i < count; i++) {
+		gelf_getsym(data, i, &sym);
+		symname = elf_strptr(elf, shdr.sh_link, sym.st_name);
+		if (strcmp(symname, name) == 0) {
+			found = 1;
+			break;
+		}
+	}
 
-	x = (void *)hdr + hdr->e_shoff;
 	s->size = 0;
 	s->address = 0;
 	s->offset = 0;
-	elf_sym = find_elf_symbol(hdr, symtab, name);
-	if (!elf_sym)
+	if (!found)
 		return;
-	secndx = elf_sym->st_shndx;
+	secndx = sym.st_shndx;
 	if (!secndx)
 		return;
-	sec = &x[secndx];
-	s->size = elf_sym->st_size;
-	s->address = elf_sym->st_value;
-	s->offset = s->address - sec->sh_addr
-			       + sec->sh_offset;
+	sec = elf_getscn(elf, secndx);
+	gelf_getshdr(sec, &shdr);
+	s->size = sym.st_size;
+	s->address = sym.st_value;
+	s->offset = s->address - shdr.sh_addr
+			       + shdr.sh_offset;
 	s->name = name;
-	s->content = (void *)hdr + s->offset;
+	s->content = base + s->offset;
 }
 
-static Elf_Shdr *get_symbol_table(Elf_Ehdr *hdr)
+static Elf_Scn *get_symbol_table(Elf *elf)
 {
-	Elf_Shdr *x;
-	unsigned int i, num_sections;
-
-	x = (void *)hdr + hdr->e_shoff;
-	if (hdr->e_shnum == SHN_UNDEF)
-		num_sections = x[0].sh_size;
-	else
-		num_sections = hdr->e_shnum;
+	Elf_Scn *scn = NULL;
+	GElf_Shdr shdr;
 
-	for (i = 1; i < num_sections; i++)
-		if (x[i].sh_type == SHT_SYMTAB)
-			return &x[i];
+	while ((scn = elf_nextscn(elf, scn)) != NULL) {
+		gelf_getshdr(scn, &shdr);
+		if (shdr.sh_type == SHT_SYMTAB)
+			return scn;
+	}
 	return NULL;
 }
 
@@ -257,7 +230,7 @@ static char *read_file(char *file_name, int *size)
 	return buf;
 }
 
-static void print_sym(Elf_Ehdr *hdr, struct sym *s)
+static void print_sym(struct sym *s)
 {
 	info("sym:    %s\n", s->name);
 	info("addr:   0x%lx\n", s->address);
@@ -277,14 +250,15 @@ int main(int argc, char **argv)
 	char *cert_file = NULL;
 	int vmlinux_size;
 	int cert_size;
-	Elf_Ehdr *hdr;
+	char *vmlinux;
 	char *cert;
 	FILE *system_map;
-	unsigned long *lsize;
 	int *used;
 	int opt;
-	Elf_Shdr *symtab = NULL;
 	struct sym cert_sym, lsize_sym, used_sym;
+	Elf *elf;
+	GElf_Ehdr hdr_s, *hdr;
+	Elf_Scn *symtab = NULL;
 
 	while ((opt = getopt(argc, argv, "b:c:s:")) != -1) {
 		switch (opt) {
@@ -311,12 +285,24 @@ int main(int argc, char **argv)
 	if (!cert)
 		exit(EXIT_FAILURE);
 
-	hdr = map_file(vmlinux_file, &vmlinux_size);
-	if (!hdr)
+	vmlinux = map_file(vmlinux_file, &vmlinux_size);
+	if (!vmlinux)
 		exit(EXIT_FAILURE);
 
-	if (vmlinux_size < sizeof(*hdr)) {
-		err("Invalid ELF file.\n");
+	if (elf_version(EV_CURRENT) == EV_NONE) {
+		err("Init libelf failed.\n");
+		exit(EXIT_FAILURE);
+	}
+
+	elf = elf_memory(vmlinux, vmlinux_size);
+	if (!elf) {
+		err("Unable to read Elf: %s\n", elf_errmsg(elf_errno()));
+		exit(EXIT_FAILURE);
+	}
+
+	hdr = gelf_getehdr(elf, &hdr_s);
+	if (!hdr) {
+		err("Unable to read Elf_hdr: %s\n", elf_errmsg(elf_errno()));
 		exit(EXIT_FAILURE);
 	}
 
@@ -328,11 +314,6 @@ int main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
-	if (hdr->e_ident[EI_CLASS] != CURRENT_ELFCLASS) {
-		err("ELF class mismatch.\n");
-		exit(EXIT_FAILURE);
-	}
-
 	if (hdr->e_ident[EI_DATA] != endianness()) {
 		err("ELF endian mismatch.\n");
 		exit(EXIT_FAILURE);
@@ -343,7 +324,7 @@ int main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
-	symtab = get_symbol_table(hdr);
+	symtab = get_symbol_table(elf);
 	if (!symtab) {
 		warn("Could not find the symbol table.\n");
 		if (!system_map_file) {
@@ -357,27 +338,32 @@ int main(int argc, char **argv)
 			perror(system_map_file);
 			exit(EXIT_FAILURE);
 		}
-		get_symbol_from_map(hdr, system_map, CERT_SYM, &cert_sym);
-		get_symbol_from_map(hdr, system_map, USED_SYM, &used_sym);
-		get_symbol_from_map(hdr, system_map, LSIZE_SYM, &lsize_sym);
+		get_symbol_from_map(elf, vmlinux, system_map,
+				    CERT_SYM, &cert_sym);
+		get_symbol_from_map(elf, vmlinux, system_map,
+				    USED_SYM, &used_sym);
+		get_symbol_from_map(elf, vmlinux, system_map,
+				    LSIZE_SYM, &lsize_sym);
 		cert_sym.size = used_sym.address - cert_sym.address;
 	} else {
 		info("Symbol table found.\n");
 		if (system_map_file)
 			warn("System.map is ignored.\n");
-		get_symbol_from_table(hdr, symtab, CERT_SYM, &cert_sym);
-		get_symbol_from_table(hdr, symtab, USED_SYM, &used_sym);
-		get_symbol_from_table(hdr, symtab, LSIZE_SYM, &lsize_sym);
+		get_symbol_from_table(elf, symtab, vmlinux,
+				      CERT_SYM, &cert_sym);
+		get_symbol_from_table(elf, symtab, vmlinux,
+				      USED_SYM, &used_sym);
+		get_symbol_from_table(elf, symtab, vmlinux,
+				      LSIZE_SYM, &lsize_sym);
 	}
 
 	if (!cert_sym.offset || !lsize_sym.offset || !used_sym.offset)
 		exit(EXIT_FAILURE);
 
-	print_sym(hdr, &cert_sym);
-	print_sym(hdr, &used_sym);
-	print_sym(hdr, &lsize_sym);
+	print_sym(&cert_sym);
+	print_sym(&used_sym);
+	print_sym(&lsize_sym);
 
-	lsize = (unsigned long *)lsize_sym.content;
 	used = (int *)used_sym.content;
 
 	if (cert_sym.size < cert_size) {
@@ -400,11 +386,26 @@ int main(int argc, char **argv)
 		memset(cert_sym.content + cert_size,
 			0, cert_sym.size - cert_size);
 
-	*lsize = *lsize + cert_size - *used;
+	if (hdr->e_ident[EI_CLASS] == ELFCLASS64) {
+		u64 *lsize;
+
+		lsize = (u64 *)lsize_sym.content;
+		*lsize = *lsize + cert_size - *used;
+	} else {
+		u32 *lsize;
+
+		lsize = (u32 *)lsize_sym.content;
+		*lsize = *lsize + cert_size - *used;
+	}
 	*used = cert_size;
 	info("Inserted the contents of %s into %lx.\n", cert_file,
 						cert_sym.address);
 	info("Used %d bytes out of %d bytes reserved.\n", *used,
 						 cert_sym.size);
+	if (munmap(vmlinux, vmlinux_size) == -1) {
+		perror(vmlinux_file);
+		exit(EXIT_FAILURE);
+	}
+
 	exit(EXIT_SUCCESS);
 }
-- 
2.7.4

[PATCH v4 3/4] KEYS: Support for inserting a certificate into x86 bzImage

[Mehmet Kayaalp]

The config option SYSTEM_EXTRA_CERTIFICATE (introduced in c4c361059585)
reserves space in vmlinux file, which is compressed to create the
self-extracting bzImage. This patch adds the capability of extracting the
vmlinux, inserting the certificate, and repackaging the result into a
bzImage.

It only works if the resulting compressed vmlinux is smaller than the
original. Otherwise re-linking would be required. To make the reserved
space allocate actual space in bzImage, incompressible bytes are
inserted into the vmlinux as a placeholder for the extra certificate.
After receiving a bzImage that is created this way, the actual
certificate can be inserted into the bzImage:

	scripts/insert-sys-cert -s <System.map> -z <bzImage> -c <certfile>

Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
---
 scripts/insert-sys-cert.c | 236 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 231 insertions(+), 5 deletions(-)

diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
index 86a3d41..f558616 100644
--- a/scripts/insert-sys-cert.c
+++ b/scripts/insert-sys-cert.c
@@ -7,7 +7,8 @@
  * This software may be used and distributed according to the terms
  * of the GNU General Public License, incorporated herein by reference.
  *
- * Usage: insert-sys-cert [-s <System.map> -b <vmlinux> -c <certfile>
+ * Usage: insert-sys-cert [-s <System.map>] -b <vmlinux> -c <certfile>
+ *                        [-s <System.map>] -z <bzImage> -c <certfile>
  */
 
 #define _GNU_SOURCE
@@ -230,6 +231,208 @@ static char *read_file(char *file_name, int *size)
 	return buf;
 }
 
+#define BOOT_FLAG		0xAA55
+#define MAGIC			0x53726448
+
+#define BOOT_FLAG_O		0x1FE
+#define MAGIC_O			0x202
+#define VERSION_O		0x206
+#define SETUP_SECTS_O		0x1F1
+#define PAYLOAD_OFFSET_O	0x248
+#define PAYLOAD_LENGTH_O	0x24C
+
+static int image_supported(char *bzimage, int bzimage_size)
+{
+	u16 boot_flag;
+	u32 magic;
+	u16 version;
+
+	if (bzimage_size < 1024) {
+		err("Invalid bzImage: File is too small\n");
+		return 0;
+	}
+
+	boot_flag = *((u16 *)&bzimage[BOOT_FLAG_O]);
+	magic = *((u32 *)&bzimage[MAGIC_O]);
+	version = *((u16 *)&bzimage[VERSION_O]);
+
+	if (boot_flag != BOOT_FLAG || magic != MAGIC) {
+		err("Invalid bzImage: Magic mismatch\n");
+		return 0;
+	}
+
+	if (version < 0x208) {
+		err("Invalid bzImage: Boot version <2.08 not supported\n");
+		return 0;
+	}
+
+	return 1;
+}
+
+static void get_payload_info(char *bzimage, int *offset, int *size)
+{
+	unsigned int system_offset;
+	unsigned char setup_sectors;
+
+	setup_sectors = bzimage[SETUP_SECTS_O] + 1;
+	system_offset = setup_sectors * 512;
+	*offset = system_offset + *((int *)&bzimage[PAYLOAD_OFFSET_O]);
+	*size = *((int *)&bzimage[PAYLOAD_LENGTH_O]);
+}
+
+static void update_payload_info(char *bzimage, int new_size)
+{
+	int offset, size;
+
+	get_payload_info(bzimage, &offset, &size);
+	*((int *)&bzimage[PAYLOAD_LENGTH_O]) = new_size;
+	if (new_size < size)
+		memset(bzimage + offset + new_size, 0, size - new_size);
+}
+
+struct zipper {
+	unsigned char pattern[10];
+	int length;
+	char *command;
+	char *compress;
+};
+
+struct zipper zippers[] = {
+	{{0x7F, 'E', 'L', 'F'},
+	 4, "cat", "cat"},
+	{{0x1F, 0x8B},
+	 2, "gunzip", "gzip -n -f -9"},
+	{{0xFD, '7', 'z', 'X', 'Z', 0},
+	 6, "unxz", "xz"},
+	{{'B', 'Z', 'h'},
+	 3, "bunzip2", "bzip2 -9"},
+	{{0xFF, 'L', 'Z', 'M', 'A', 0},
+	 6, "unlzma", "lzma -9"},
+	{{0xD3, 'L', 'Z', 'O', 0, '\r', '\n', 0x20, '\n'},
+	 9, "lzop -d", "lzop -9"}
+};
+
+static struct zipper *get_zipper(char *p)
+{
+	int i;
+
+	for (i = 0; i < sizeof(zippers) / sizeof(struct zipper); i++) {
+		if (memcmp(p, zippers[i].pattern, zippers[i].length) == 0)
+			return &zippers[i];
+	}
+	return NULL;
+}
+
+/*
+ * This only works for x86 bzImage
+ */
+static void extract_vmlinux(char *bzimage, int bzimage_size,
+			    char **file, struct zipper **zipper)
+{
+	int r;
+	char src[15] = "vmlinux-XXXXXX";
+	char dest[15] = "vmlinux-XXXXXX";
+	char cmd[100];
+	int src_fd, dest_fd;
+	int offset, size;
+	struct zipper *z;
+
+	if (!image_supported(bzimage, bzimage_size))
+		return;
+
+	get_payload_info(bzimage, &offset, &size);
+	z = get_zipper(bzimage + offset);
+	if (!z) {
+		err("Unable to determine the compression of vmlinux\n");
+		return;
+	}
+
+	src_fd = mkstemp(src);
+	if (src_fd == -1) {
+		perror("Could not create temp file");
+		return;
+	}
+
+	r = write(src_fd, bzimage + offset, size);
+	if (r != size) {
+		perror("Could not write vmlinux");
+		return;
+	}
+	dest_fd = mkstemp(dest);
+	if (dest_fd == -1) {
+		perror("Could not create temp file");
+		return;
+	}
+
+	snprintf(cmd, sizeof(cmd), "%s <%s >%s", z->command, src, dest);
+	info("Executing: %s\n", cmd);
+	r = system(cmd);
+	if (r != 0)
+		warn("Possible errors when extracting\n");
+
+	r = remove(src);
+	if (r != 0)
+		perror(src);
+
+	*file = strdup(dest);
+	*zipper = z;
+}
+
+static void repack_image(char *bzimage, int bzimage_size,
+			 char *vmlinux_file, struct zipper *z)
+{
+	char tmp[15] = "vmlinux-XXXXXX";
+	char cmd[100];
+	int fd;
+	struct stat st;
+	int new_size;
+	int r;
+	int offset, size;
+
+	get_payload_info(bzimage, &offset, &size);
+
+	fd = mkstemp(tmp);
+	if (fd == -1) {
+		perror("Could not create temp file");
+		return;
+	}
+	snprintf(cmd, sizeof(cmd), "%s <%s >%s",
+		 z->compress, vmlinux_file, tmp);
+
+	info("Executing: %s\n", cmd);
+	r = system(cmd);
+	if (r != 0)
+		warn("Possible errors when compressing\n");
+
+	r = remove(vmlinux_file);
+	if (r != 0)
+		perror(vmlinux_file);
+
+	if (fstat(fd, &st)) {
+		perror("Could not determine file size");
+		close(fd);
+	}
+	new_size = st.st_size;
+	if (new_size > size) {
+		err("Increase in compressed size is not supported.\n");
+		err("Old size was %d, new size is %d\n", size, new_size);
+		exit(EXIT_FAILURE);
+	}
+
+	r = read(fd, bzimage + offset, new_size);
+	if (r != new_size)
+		perror(tmp);
+
+	r = remove(tmp);
+	if (r != 0)
+		perror(tmp);
+
+	/* x86 specific patching of bzimage */
+	update_payload_info(bzimage, new_size);
+
+	/* TODO: update CRC */
+}
+
 static void print_sym(struct sym *s)
 {
 	info("sym:    %s\n", s->name);
@@ -240,18 +443,23 @@ static void print_sym(struct sym *s)
 
 static void print_usage(char *e)
 {
-	printf("Usage %s [-s <System.map>] -b <vmlinux> -c <certfile>\n", e);
+	printf("Usage: %s [-s <System.map>] -b <vmlinux> -c <certfile>\n", e);
+	printf("       %s [-s <System.map>] -z <bzImage> -c <certfile>\n", e);
 }
 
 int main(int argc, char **argv)
 {
 	char *system_map_file = NULL;
 	char *vmlinux_file = NULL;
+	char *bzimage_file = NULL;
 	char *cert_file = NULL;
 	int vmlinux_size;
+	int bzimage_size;
 	int cert_size;
 	char *vmlinux;
 	char *cert;
+	char *bzimage = NULL;
+	struct zipper *z = NULL;
 	FILE *system_map;
 	int *used;
 	int opt;
@@ -260,7 +468,7 @@ int main(int argc, char **argv)
 	GElf_Ehdr hdr_s, *hdr;
 	Elf_Scn *symtab = NULL;
 
-	while ((opt = getopt(argc, argv, "b:c:s:")) != -1) {
+	while ((opt = getopt(argc, argv, "b:z:c:s:")) != -1) {
 		switch (opt) {
 		case 's':
 			system_map_file = optarg;
@@ -268,6 +476,9 @@ int main(int argc, char **argv)
 		case 'b':
 			vmlinux_file = optarg;
 			break;
+		case 'z':
+			bzimage_file = optarg;
+			break;
 		case 'c':
 			cert_file = optarg;
 			break;
@@ -276,7 +487,9 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (!vmlinux_file || !cert_file) {
+	if (!cert_file ||
+	    (!vmlinux_file && !bzimage_file) ||
+	    (vmlinux_file && bzimage_file)) {
 		print_usage(argv[0]);
 		exit(EXIT_FAILURE);
 	}
@@ -285,6 +498,16 @@ int main(int argc, char **argv)
 	if (!cert)
 		exit(EXIT_FAILURE);
 
+	if (bzimage_file) {
+		bzimage = map_file(bzimage_file, &bzimage_size);
+		if (!bzimage)
+			exit(EXIT_FAILURE);
+
+		extract_vmlinux(bzimage, bzimage_size, &vmlinux_file, &z);
+		if (!vmlinux_file)
+			exit(EXIT_FAILURE);
+	}
+
 	vmlinux = map_file(vmlinux_file, &vmlinux_size);
 	if (!vmlinux)
 		exit(EXIT_FAILURE);
@@ -372,7 +595,7 @@ int main(int argc, char **argv)
 	}
 
 	/* If the existing cert is the same, don't overwrite */
-	if (cert_size == *used &&
+	if (cert_size > 0 && cert_size == *used &&
 	    strncmp(cert_sym.content, cert, cert_size) == 0) {
 		warn("Certificate was already inserted.\n");
 		exit(EXIT_SUCCESS);
@@ -407,5 +630,8 @@ int main(int argc, char **argv)
 		exit(EXIT_FAILURE);
 	}
 
+	if (bzimage)
+		repack_image(bzimage, bzimage_size, vmlinux_file, z);
+
 	exit(EXIT_SUCCESS);
 }
-- 
2.7.4

[PATCH v4 4/4] KEYS: Print insert-sys-cert information to stdout instead of stderr

[Mehmet Kayaalp]

Detailed INFO output should go to stdout instead of stderr.

Signed-off-by: Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com>
---
 scripts/insert-sys-cert.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/insert-sys-cert.c b/scripts/insert-sys-cert.c
index f558616..56c5482 100644
--- a/scripts/insert-sys-cert.c
+++ b/scripts/insert-sys-cert.c
@@ -32,7 +32,7 @@
 #define USED_SYM  "system_extra_cert_used"
 #define LSIZE_SYM "system_certificate_list_size"
 
-#define info(format, args...) fprintf(stderr, "INFO:    " format, ## args)
+#define info(format, args...) fprintf(stdout, "INFO:    " format, ## args)
 #define warn(format, args...) fprintf(stdout, "WARNING: " format, ## args)
 #define  err(format, args...) fprintf(stderr, "ERROR:   " format, ## args)
 
-- 
2.7.4

Re: [PATCH v4 3/4] KEYS: Support for inserting a certificate into x86 bzImage

[David Howells]

Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com> wrote:

> +	/* TODO: update CRC */

Is this bit missing?

David

Re: [PATCH v4 3/4] KEYS: Support for inserting a certificate into x86 bzImage

[Mehmet Kayaalp]

> On Apr 27, 2017, at 9:54 AM, David Howells <dhowells@redhat.com> wrote:
> 
> Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com> wrote:
> 
>> +	/* TODO: update CRC */
> 
> Is this bit missing?

I didn't add it, since I wasn't sure it was still used with secure boot. The CRC 
code is implemented in multiple places, but not exposed to the tools/scripts.
Should I make the crc32() in tools/pcmcia/crc32hash.c non-static, maybe?

Mehmet 

Re: [PATCH v4 3/4] KEYS: Support for inserting a certificate into x86 bzImage

[David Howells]

Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com> wrote:

> >> +	/* TODO: update CRC */
> > 
> > Is this bit missing?
> 
> I didn't add it, since I wasn't sure it was still used with secure boot.

I'm not sure why secure boot would skip it if normal boot does it.  You'll
have to trawl the boot wrapper and kernel arch setup code to answer that one.

> The CRC code is implemented in multiple places, but not exposed to the
> tools/scripts.  Should I make the crc32() in tools/pcmcia/crc32hash.c
> non-static, maybe?

Making it non-static won't help since it's a standalone program.  You could
just shift it to scripts or something.

David

Re: [PATCH v4 4/4] KEYS: Print insert-sys-cert information to stdout instead of stderr

[David Howells]

Mehmet Kayaalp <mkayaalp@linux.vnet.ibm.com> wrote:

> +#define info(format, args...) fprintf(stdout, "INFO:    " format, ## args)

Btw, you really ought to be using standard varargs macros:

	#define info(format, ...) fprintf(stdout, "INFO:    " format, ##__VA_LIST__)

But don't worry about that for now.

David