[PATCH 0/2] evaluate integrity of a barebox image

[PATCH 0/2] evaluate integrity of a barebox image

[Ulrich Ölmann]

These two patches provide functionality to assess if a barebox image is
corrupted or not by embedding and checking a hash value. This can be of interest
if for example the image has been loaded from unreliable storage.

Jan Luebbe (2):
  images: write checksum into barebox header
  images: add function to verify checksum in barebox header

 common/Kconfig         |   9 ++
 common/Makefile        |   1 +
 common/barebox-image.c |  85 +++++++++++++++++++
 images/Makefile        |   3 +-
 include/common.h       |   9 ++
 include/filetype.h     |   2 +
 scripts/.gitignore     |   1 +
 scripts/Makefile       |   1 +
 scripts/bareboxcrc.c   | 219 +++++++++++++++++++++++++++++++++++++++++++++++++
 9 files changed, 329 insertions(+), 1 deletion(-)
 create mode 100644 common/barebox-image.c
 create mode 100644 scripts/bareboxcrc.c

-- 
2.8.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] images: write checksum into barebox header

[Ulrich Ölmann]

From: Jan Luebbe <jlu@pengutronix.de>

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
---
 images/Makefile      |   3 +-
 include/filetype.h   |   2 +
 scripts/.gitignore   |   1 +
 scripts/Makefile     |   1 +
 scripts/bareboxcrc.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 225 insertions(+), 1 deletion(-)
 create mode 100644 scripts/bareboxcrc.c

diff --git a/images/Makefile b/images/Makefile
index da9cc8d396ee..812074e624b0 100644
--- a/images/Makefile
+++ b/images/Makefile
@@ -71,7 +71,8 @@ quiet_cmd_pblx ?= PBLX    $@
       cmd_pblx ?= cat $(obj)/$(patsubst %.pblx,%.pblb,$(2)) > $@; \
 		  $(call size_append, $(obj)/barebox.z) >> $@; \
 		  cat $(obj)/barebox.z >> $@; \
-		  $(objtree)/scripts/fix_size -f $@
+		  $(objtree)/scripts/fix_size -f $@; \
+		  $(objtree)/scripts/bareboxcrc $@
 
 $(obj)/%.pblx: $(obj)/%.pblb $(obj)/barebox.z FORCE
 	$(call if_changed,pblx,$(@F))
diff --git a/include/filetype.h b/include/filetype.h
index e87ca174a89d..ec0e01f64f9f 100644
--- a/include/filetype.h
+++ b/include/filetype.h
@@ -51,6 +51,8 @@ enum filetype is_fat_or_mbr(const unsigned char *sector, unsigned long *bootsec)
 int is_fat_boot_sector(const void *_buf);
 bool filetype_is_barebox_image(enum filetype ft);
 
+#define BAREBOX_CRC_MAGIC_VALUE		"crc"
+
 #define ARM_HEAD_SIZE			0x30
 #define ARM_HEAD_MAGICWORD_OFFSET	0x20
 #define ARM_HEAD_SIZE_OFFSET		0x2C
diff --git a/scripts/.gitignore b/scripts/.gitignore
index 533bffd97d36..d11ac7f97f32 100644
--- a/scripts/.gitignore
+++ b/scripts/.gitignore
@@ -2,6 +2,7 @@ bin2c
 mkimage
 fix_size
 bareboxenv
+bareboxcrc
 bareboxcrc32
 kernel-install
 bareboximd
diff --git a/scripts/Makefile b/scripts/Makefile
index a5c16b2f3133..1683d38c222d 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -8,6 +8,7 @@ hostprogs-y                      += bin2c
 hostprogs-y                      += mkimage
 hostprogs-y                      += fix_size
 hostprogs-y                      += bareboxenv
+hostprogs-y                      += bareboxcrc
 hostprogs-y                      += bareboxcrc32
 hostprogs-y                      += kernel-install
 hostprogs-$(CONFIG_IMD)          += bareboximd
diff --git a/scripts/bareboxcrc.c b/scripts/bareboxcrc.c
new file mode 100644
index 000000000000..37cb9b0a6ce8
--- /dev/null
+++ b/scripts/bareboxcrc.c
@@ -0,0 +1,219 @@
+/*
+ * bareboxcrc.c
+ *
+ * Copyright (C) 2016 Jan Luebbe <j.luebbe@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#define _DEFAULT_SOURCE
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <stdint.h>
+#include <limits.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <getopt.h>
+#include <endian.h>
+
+#include "compiler.h"
+
+#include "../include/filetype.h"
+#include "../crypto/crc32.c"
+
+#define EX_SUCCESS		0
+#define EX_NO_CHECKSUM		1
+#define EX_CHECKSUM_FAULTY	2
+#define EX_NO_BAREBOX_HEADER	3
+#define EX_IMAGE_TOO_SMALL	4
+#define EX_IMAGE_TOO_BIG	5
+#define EX_LOCATION_OCCUPIED	6
+#define EX_OUT_OF_MEMORY	7
+#define EX_IO_ERROR		8
+#define EX_WRONG_USAGE		9
+
+void usage(char *prgname)
+{
+	fprintf(stderr, "Usage : %s [OPTION] FILE\n"
+		"\n"
+		"options:\n"
+		"  -c        verify checksum\n"
+		"  -f        force\n",
+		prgname);
+}
+
+int get_barebox_head_size(const char *head)
+{
+	if (!strcmp(head + ARM_HEAD_MAGICWORD_OFFSET, "barebox"))
+		return ARM_HEAD_SIZE;
+	else if (!strcmp(head + MIPS_HEAD_MAGICWORD_OFFSET, "barebox"))
+		return MIPS_HEAD_SIZE;
+
+	return 0;
+}
+
+int checksum(FILE *image, bool check, bool force, off_t size)
+{
+	void *buf = malloc(size);
+	int ret = EX_SUCCESS;
+	size_t head_size;
+	uint32_t crc = 0;
+	uint32_t *p;
+
+	if (!buf) {
+		fprintf(stderr, "malloc: out of memory\n");
+		return EX_OUT_OF_MEMORY;
+	}
+
+	if (fseeko(image, 0, SEEK_SET) == -1) {
+		perror("fseeko");
+		ret = EX_IO_ERROR;
+		goto out;
+	}
+
+	if (fread(buf, 1, size, image) < size) {
+		perror("fread");
+		ret = EX_IO_ERROR;
+		goto out;
+	}
+
+	head_size = get_barebox_head_size(buf);
+	if (!head_size) {
+		fprintf(stderr, "image does not contain a barebox header\n");
+		ret = EX_NO_BAREBOX_HEADER;
+		goto out;
+	}
+
+	if (size < head_size + 2 * sizeof(uint32_t)) {
+		fprintf(stderr, "image is too small\n");
+		ret = EX_IMAGE_TOO_SMALL;
+		goto out;
+	}
+
+	p = buf + head_size;
+
+	if (check) {
+		if (strcmp((const char*) p, BAREBOX_CRC_MAGIC_VALUE)) {
+			fprintf(stderr, "image does not contain a checksum\n");
+			ret = EX_NO_CHECKSUM;
+			goto out;
+		}
+	} else {
+		if (!force && (*p != 0x55555555 || *(p+1) != 0x55555555)) {
+			fprintf(stderr, "location for magic value and CRC seems already "
+					"to be occupied by something else\n");
+			ret = EX_LOCATION_OCCUPIED;
+			goto out;
+		}
+
+		if (fseeko(image, head_size, SEEK_SET) == -1) {
+			perror("fseeko");
+			ret = EX_IO_ERROR;
+			goto out;
+		}
+
+		if (fwrite(BAREBOX_CRC_MAGIC_VALUE, sizeof(uint32_t), 1, image) != 1) {
+			perror("fwrite");
+			ret = EX_IO_ERROR;
+			goto out;
+		}
+
+		sprintf((char*) p, "%s", BAREBOX_CRC_MAGIC_VALUE);
+	}
+
+	crc = crc32(crc, buf, head_size + sizeof(uint32_t));
+	crc = crc32(crc, buf + head_size + 2 * sizeof(uint32_t),
+			size - (head_size + 2 * sizeof(uint32_t)));
+	crc = htobe32(crc);
+
+	if (check) {
+		if (crc != *(p + 1)) {
+			fprintf(stderr, "embedded checksum is faulty\n");
+			ret = EX_CHECKSUM_FAULTY;
+		}
+	} else {
+		if (fwrite(&crc, sizeof(uint32_t), 1, image) != 1) {
+			perror("fwrite");
+			ret = EX_IO_ERROR;
+		}
+	}
+
+out:
+	free(buf);
+	return ret;
+}
+
+int main(int argc, char *argv[])
+{
+	FILE *image;
+	bool check = false;
+	bool force = false;
+	int opt;
+	int ret;
+	off_t size;
+
+	while((opt = getopt(argc, argv, "cf")) != -1) {
+		switch (opt) {
+		case 'c':
+			check = true;
+			break;
+		case 'f':
+			force = true;
+			break;
+		}
+	}
+
+	if (optind >= argc) {
+		usage(argv[0]);
+		exit(EX_WRONG_USAGE);
+	}
+
+	image = fopen(argv[optind], check ? "r" : "r+");
+	if (image == NULL) {
+		perror("fopen");
+		exit(EX_IO_ERROR);
+	}
+
+	if (fseeko(image, 0, SEEK_END) == -1) {
+		perror("fseeko");
+		exit(EX_IO_ERROR);
+	}
+	size = ftello(image);
+	if (size == -1) {
+		perror("ftello");
+		exit(EX_IO_ERROR);
+	}
+	if (size > 0xa00000) {
+		fprintf(stderr, "error: image should be smaller than 10 MiB\n");
+		exit(EX_IMAGE_TOO_BIG);
+	}
+
+	ret = checksum(image, check, force, size);
+	if (ret)
+		exit(ret);
+
+	if (fclose(image) != 0) {
+		perror("fclose");
+		exit(EX_IO_ERROR);
+	}
+
+	exit(EX_SUCCESS);
+}
-- 
2.8.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] images: add function to verify checksum in barebox header

[Ulrich Ölmann]

From: Jan Luebbe <jlu@pengutronix.de>

Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
Signed-off-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
---
 common/Kconfig         |  9 ++++++
 common/Makefile        |  1 +
 common/barebox-image.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/common.h       |  9 ++++++
 4 files changed, 104 insertions(+)
 create mode 100644 common/barebox-image.c

diff --git a/common/Kconfig b/common/Kconfig
index 928db0a159e1..bebbb614509d 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -580,6 +580,15 @@ config IMD_TARGET
 	depends on IMD
 	depends on !SANDBOX
 
+config VERIFY_EMBEDDED_CRC
+	prompt "Verify the CRC checksum embedded into barebox images"
+	bool
+	select CRC32
+	help
+	  If you say Y here, the CRC checksum that is embedded into every
+	  barebox image directly after the header can be verified to guarantee
+	  the integrity of the image.
+
 config KERNEL_INSTALL_TARGET
 	bool
 	depends on !SANDBOX
diff --git a/common/Makefile b/common/Makefile
index 99681e21215b..0106f0fe7bd9 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_STATE)		+= state.o
 obj-$(CONFIG_RATP)		+= ratp.o
 obj-$(CONFIG_UIMAGE)		+= image.o uimage.o
 obj-$(CONFIG_FITIMAGE)		+= image-fit.o
+obj-$(CONFIG_VERIFY_EMBEDDED_CRC) += barebox-image.o
 obj-$(CONFIG_MENUTREE)		+= menutree.o
 obj-$(CONFIG_EFI_GUID)		+= efi-guid.o
 obj-$(CONFIG_EFI_DEVICEPATH)	+= efi-devicepath.o
diff --git a/common/barebox-image.c b/common/barebox-image.c
new file mode 100644
index 000000000000..733d9ea2ecdf
--- /dev/null
+++ b/common/barebox-image.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2016 Pengutronix, Jan Luebbe <j.luebbe@pengutronix.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include <common.h>
+#include <io.h>
+#include <filetype.h>
+
+static inline size_t get_barebox_head_size(const void *buf)
+{
+	if (is_barebox_arm_head(buf))
+		return ARM_HEAD_SIZE;
+	else if (is_barebox_mips_head(buf))
+		return MIPS_HEAD_SIZE;
+
+	return 0;
+}
+
+static inline size_t get_barebox_head_size_offset(const void *buf)
+{
+	if (is_barebox_arm_head(buf))
+		return ARM_HEAD_SIZE_OFFSET;
+	else if (is_barebox_mips_head(buf))
+		return MIPS_HEAD_SIZE_OFFSET;
+
+	return 0;
+}
+
+bool barebox_verify_image(void *buf, size_t bufsize)
+{
+	unsigned int *psize;
+	size_t head_size;
+	uint32_t crc = 0;
+	uint32_t *p;
+
+	if (bufsize < ARM_HEAD_SIZE) {
+		pr_err("buffer is smaller than ARM_HEAD_SIZE\n");
+		return false;
+	}
+
+	head_size = get_barebox_head_size(buf);
+	if (!head_size) {
+		pr_err("image does not contain a barebox header\n");
+		return false;
+	}
+
+	psize = buf + get_barebox_head_size_offset(buf);
+
+	if (bufsize < *psize) {
+		pr_err("buffer is too small for image\n");
+		return false;
+	}
+
+	if (*psize < head_size + 2 * sizeof(uint32_t)) {
+		pr_err("image is too small\n");
+		return false;
+	}
+
+	p = buf + head_size;
+
+	if (strcmp((const char*) p, BAREBOX_CRC_MAGIC_VALUE)) {
+		pr_debug("image does not contain a checksum\n");
+		return true;
+	}
+
+	crc = crc32(crc, buf, head_size + sizeof(uint32_t));
+	crc = crc32(crc, buf + head_size + 2 * sizeof(uint32_t),
+			*psize - (head_size + 2 * sizeof(uint32_t)));
+	crc = cpu_to_be32(crc);
+
+	if (crc != *(p + 1)) {
+		pr_err("embedded checksum is faulty\n");
+		return false;
+	}
+
+	return true;
+}
diff --git a/include/common.h b/include/common.h
index 680a0affb6bc..9200328c3e10 100644
--- a/include/common.h
+++ b/include/common.h
@@ -164,4 +164,13 @@ static inline bool region_overlap(unsigned long starta, unsigned long lena,
 	return 1;
 }
 
+#ifdef CONFIG_VERIFY_EMBEDDED_CRC
+bool barebox_verify_image(void*, size_t);
+#else
+static inline bool barebox_verify_image(void *buf, size_t bufsize)
+{
+	return true;
+}
+#endif
+
 #endif	/* __COMMON_H_ */
-- 
2.8.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 2/2] images: add function to verify checksum in barebox header

[Sascha Hauer]

On Mon, May 23, 2016 at 01:48:42PM +0200, Ulrich Ölmann wrote:
> From: Jan Luebbe <jlu@pengutronix.de>
> 
> Signed-off-by: Jan Luebbe <jlu@pengutronix.de>
> Signed-off-by: Ulrich Ölmann <u.oelmann@pengutronix.de>
> ---
>  common/Kconfig         |  9 ++++++
>  common/Makefile        |  1 +
>  common/barebox-image.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/common.h       |  9 ++++++
>  4 files changed, 104 insertions(+)
>  create mode 100644 common/barebox-image.c
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index 928db0a159e1..bebbb614509d 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -580,6 +580,15 @@ config IMD_TARGET
>  	depends on IMD
>  	depends on !SANDBOX
>  
> +config VERIFY_EMBEDDED_CRC
> +	prompt "Verify the CRC checksum embedded into barebox images"
> +	bool
> +	select CRC32
> +	help
> +	  If you say Y here, the CRC checksum that is embedded into every
> +	  barebox image directly after the header can be verified to guarantee
> +	  the integrity of the image.

When a user reads this he probably expects the option to have an effect.
In fact this option only adds a currently unused function. You should
make it invisible.

> +
>  config KERNEL_INSTALL_TARGET
>  	bool
>  	depends on !SANDBOX
> diff --git a/common/Makefile b/common/Makefile
> index 99681e21215b..0106f0fe7bd9 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -48,6 +48,7 @@ obj-$(CONFIG_STATE)		+= state.o
>  obj-$(CONFIG_RATP)		+= ratp.o
>  obj-$(CONFIG_UIMAGE)		+= image.o uimage.o
>  obj-$(CONFIG_FITIMAGE)		+= image-fit.o
> +obj-$(CONFIG_VERIFY_EMBEDDED_CRC) += barebox-image.o
>  obj-$(CONFIG_MENUTREE)		+= menutree.o
>  obj-$(CONFIG_EFI_GUID)		+= efi-guid.o
>  obj-$(CONFIG_EFI_DEVICEPATH)	+= efi-devicepath.o
> diff --git a/common/barebox-image.c b/common/barebox-image.c
> new file mode 100644
> index 000000000000..733d9ea2ecdf
> --- /dev/null
> +++ b/common/barebox-image.c
> @@ -0,0 +1,85 @@
> +/*
> + * Copyright (c) 2016 Pengutronix, Jan Luebbe <j.luebbe@pengutronix.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +#include <common.h>
> +#include <io.h>
> +#include <filetype.h>
> +
> +static inline size_t get_barebox_head_size(const void *buf)
> +{
> +	if (is_barebox_arm_head(buf))
> +		return ARM_HEAD_SIZE;
> +	else if (is_barebox_mips_head(buf))
> +		return MIPS_HEAD_SIZE;
> +
> +	return 0;
> +}
> +
> +static inline size_t get_barebox_head_size_offset(const void *buf)
> +{
> +	if (is_barebox_arm_head(buf))
> +		return ARM_HEAD_SIZE_OFFSET;
> +	else if (is_barebox_mips_head(buf))
> +		return MIPS_HEAD_SIZE_OFFSET;
> +
> +	return 0;
> +}
> +
> +bool barebox_verify_image(void *buf, size_t bufsize)

Since this code is called from architecture specific code anyway it's
probably nicer to create [arm|mips]_barebox_verify functions. This would
allow you to have only one meaningful size check.

> +{
> +	unsigned int *psize;
> +	size_t head_size;
> +	uint32_t crc = 0;
> +	uint32_t *p;
> +
> +	if (bufsize < ARM_HEAD_SIZE) {
> +		pr_err("buffer is smaller than ARM_HEAD_SIZE\n");
> +		return false;
> +	}

I think the error messaging should be left to the caller. The caller can
provide more context to make the message more useful. For this it might
be useful to return int instead of bool from this function as it allows
to return error codes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox