diff mbox series

Add option to disable cpio CRC check

Message ID 20200605135446.2855531-1-sbabic@denx.de
State Accepted
Headers show
Series Add option to disable cpio CRC check | expand

Commit Message

Stefano Babic June 5, 2020, 1:54 p.m. UTC
Disable CRC check in cpio header if sha256 is enabled. CRC in CPIO is not
a real crc, but it is simply the sum of all bytes belonging to a file as
32 bit value. It is very weak and does not add any further safety if sha256
is activated. CPIO tool is buggy on Linux distros and CRC is not computed
correctly and set to zero when a file is larger than 2GB.
While cpio on OE is patched to fix this, a SWU built outside OE has a wrong
computed CRC and SWUpdate will abort the update.

This option drops the check when the stronger sha256 check is activated.

Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 Kconfig                 | 12 ++++++++++++
 core/cpio_utils.c       |  8 ++------
 core/stream_interface.c | 14 ++++----------
 include/util.h          | 21 +++++++++++++++++++--
 4 files changed, 37 insertions(+), 18 deletions(-)
diff mbox series

Patch

diff --git a/Kconfig b/Kconfig
index 5b9235b..444eb1f 100644
--- a/Kconfig
+++ b/Kconfig
@@ -396,6 +396,18 @@  config HASH_VERIFY
 comment "Hash checking needs an SSL implementation"
 	depends on !SSL_IMPL_OPENSSL && !SSL_IMPL_MBEDTLS
 
+config DISABLE_CPIO_CRC
+	bool "Disable cpio CRC verify if SHA 256 is enabled"
+	depends on HASH_VERIFY
+	default n
+	help
+	  Disable CRC check in cpio header if sha256 is enabled.
+	  CRC in CPIO is not a real crc, but it is simply the sum
+	  of all bytes belonging to a file as 32 bit value. It is
+	  very weak and does not add any further safety if sha256
+	  is activated. CPIO in Linux distros has also a bug and
+	  CRC field is set to 0 when a file is larger as 2GB.
+
 config SIGNED_IMAGES
 	bool "Enable verification of signed images"
 	depends on SSL_IMPL_OPENSSL || SSL_IMPL_MBEDTLS
diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 9afb699..e2e857c 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -793,9 +793,7 @@  off_t extract_next_file(int fd, int fdout, off_t start, int compressed,
 		(unsigned long)checksum,
 		(checksum == fdh.chksum) ? "VERIFIED" : "WRONG");
 
-	if (checksum != fdh.chksum) {
-		ERROR("Checksum WRONG ! Computed 0x%lx, it should be 0x%lx",
-			(unsigned long)checksum, fdh.chksum);
+	if (!swupdate_verify_chksum(checksum, fdh.chksum)) {
 		return -EINVAL;
 	}
 
@@ -839,9 +837,7 @@  int cpio_scan(int fd, struct swupdate_cfg *cfg, off_t start)
 			return -1;
 		}
 
-		if ((uint32_t)(fdh.chksum) != checksum) {
-			ERROR("Checksum verification failed for %s: %x != %x",
-			fdh.filename, (uint32_t)fdh.chksum, checksum);
+		if (!swupdate_verify_chksum(fdh.chksum, checksum)) {
 			return -1;
 		}
 
diff --git a/core/stream_interface.c b/core/stream_interface.c
index e0d6fa4..99e5c62 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -103,11 +103,9 @@  static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs)
 		close(fdout);
 		return -1;
 	}
-	if (checksum != (uint32_t)fdh.chksum) {
+	if (!swupdate_verify_chksum(checksum, fdh.chksum)) {
 		close(fdout);
-		ERROR("Checksum WRONG ! Computed 0x%ux, it should be 0x%ux",
-			(unsigned int)checksum, (unsigned int)fdh.chksum);
-			return -1;
+		return -1;
 	}
 	close(fdout);
 
@@ -218,9 +216,7 @@  static int extract_files(int fd, struct swupdate_cfg *software)
 					close(fdout);
 					return -1;
 				}
-				if (checksum != (unsigned long)fdh.chksum) {
-					ERROR("Checksum WRONG ! Computed 0x%ux, it should be 0x%ux",
-						(unsigned int)checksum, (unsigned int)fdh.chksum);
+				if (!swupdate_verify_chksum(checksum, fdh.chksum)) {
 					close(fdout);
 					return -1;
 				}
@@ -231,9 +227,7 @@  static int extract_files(int fd, struct swupdate_cfg *software)
 				if (copyfile(fd, &fdout, fdh.size, &offset, 0, skip, 0, &checksum, NULL, 0, NULL, NULL) < 0) {
 					return -1;
 				}
-				if (checksum != (unsigned long)fdh.chksum) {
-					ERROR("Checksum WRONG ! Computed 0x%ux, it should be 0x%ux",
-						(unsigned int)checksum, (unsigned int)fdh.chksum);
+				if (!swupdate_verify_chksum(checksum, fdh.chksum)) {
 					return -1;
 				}
 				break;
diff --git a/include/util.h b/include/util.h
index 2f83c8a..55fe70f 100644
--- a/include/util.h
+++ b/include/util.h
@@ -11,6 +11,7 @@ 
 #include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
+#include <stdio.h>
 #if defined(__linux__)
 #include <linux/types.h>
 #endif
@@ -85,6 +86,8 @@  struct installer {
 
 typedef void (*notifier) (RECOVERY_STATUS status, int error, int level, const char *msg);
 
+void notify(RECOVERY_STATUS status, int error, int level, const char *msg);
+void notify_init(void);
 #define swupdate_notify(status, format, level, arg...) do { \
 	if (loglevel >= level) { \
 		char tmpbuf[NOTIFY_BUF_SIZE]; \
@@ -137,6 +140,22 @@  typedef void (*notifier) (RECOVERY_STATUS status, int error, int level, const ch
 
 #define LG_16 4
 #define FROM_HEX(f) from_ascii (f, sizeof f, LG_16)
+#if !defined(CONFIG_DISABLE_CPIO_CRC)
+static inline bool swupdate_verify_chksum(const uint32_t chk1, const uint32_t chk2) {
+	bool ret = (chk1 == chk2);
+	if (!ret) {
+		ERROR("Checksum WRONG ! Computed 0x%ux, it should be 0x%ux",
+			chk1, chk2);
+	}
+	return ret;
+}
+#else
+static inline bool swupdate_verify_chksum(
+		const uint32_t  __attribute__ ((__unused__))chk1,
+		const uint32_t  __attribute__ ((__unused__))chk2) {
+	return true;
+}
+#endif
 uintmax_t
 from_ascii (char const *where, size_t digs, unsigned logbase);
 int ascii_to_hash(unsigned char *hash, const char *s);
@@ -187,8 +206,6 @@  int mkpath(char *dir, mode_t mode);
 int swupdate_file_setnonblock(int fd, bool block);
 
 int register_notifier(notifier client);
-void notify(RECOVERY_STATUS status, int error, int level, const char *msg);
-void notify_init(void);
 int syslog_init(void);
 
 char **splitargs(char *args, int *argc);