From f522047958f99ab7b506ec550f796c0460af1a85 Mon Sep 17 00:00:00 2001
From: =?utf8?q?Rafa=C5=82=20Mi=C5=82ecki?= <rafal@milecki.pl>
Date: Fri, 23 Aug 2019 08:15:23 +0200
Subject: [PATCH] base-files: use JSON for storing firmware validation info
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

So far firmware validation result was binary limited: it was either
successful or not. That meant various limitations, e.g.:
1) Lack of proper feedback on validation problems
2) No way of marking firmware as totally broken (impossible to install)

This change introduces JSON for storing detailed validation info. It
provides a list of performed validation tests and their results. It
allows marking firmware as non-forceable (broken image that can't be
even forced to install).
Example:
{
        "tests": {
                "fwtool_signature": true,
                "fwtool_device_match": true
        },
        "valid": true,
        "forceable": true
}

Implementation is based on *internal* check_image bash script that:
1) Uses existing validation functions
2) Provides helpers for setting extra validation info

This allows e.g. platform_check_image() to call notify_check_broken()
when needed & prevent user from bricking a device.

Right now the new JSON info is used by /sbin/sysupgrade only. It still
doesn't make use of "forceable" as that is planned for later
development.

Further plans for this feature are:
1) Expose firmware validation using some new ubus method
2) Move validation step from /sbin/sysupgrade into "sysupgrade" ubus
   method so:
   a) It's possible to safely sysupgrade using ubus only
   b) /sbin/sysupgrade can be more like just a CLI

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 package/base-files/files/sbin/sysupgrade      | 28 ++++-----
 .../files/usr/libexec/validate_firmware_image | 59 +++++++++++++++++++
 2 files changed, 73 insertions(+), 14 deletions(-)
 create mode 100755 package/base-files/files/usr/libexec/validate_firmware_image

diff --git a/package/base-files/files/sbin/sysupgrade b/package/base-files/files/sbin/sysupgrade
index c27c1fbc477..42f0f6bd22e 100755
--- a/package/base-files/files/sbin/sysupgrade
+++ b/package/base-files/files/sbin/sysupgrade
@@ -2,6 +2,7 @@
 
 . /lib/functions.sh
 . /lib/functions/system.sh
+. /usr/share/libubox/jshn.sh
 
 # initialize defaults
 export MTD_ARGS=""
@@ -191,9 +192,6 @@ add_overlayfiles() {
 	return 0
 }
 
-# hooks
-sysupgrade_image_check="fwtool_check_signature fwtool_check_image platform_check_image"
-
 if [ $SAVE_OVERLAY = 1 ]; then
 	[ ! -d /overlay/upper/etc ] && {
 		echo "Cannot find '/overlay/upper/etc', required for '-c'" >&2
@@ -316,17 +314,19 @@ case "$IMAGE" in
 		;;
 esac
 
-for check in $sysupgrade_image_check; do
-	( $check "$IMAGE" ) || {
-		if [ $FORCE -eq 1 ]; then
-			echo "Image check '$check' failed but --force given - will update anyway!" >&2
-			break
-		else
-			echo "Image check '$check' failed." >&2
-			exit 1
-		fi
-	}
-done
+json_load "$(/usr/libexec/validate_firmware_image "$IMAGE")" || {
+	echo "Failed to check image"
+	exit 1
+}
+json_get_var valid "valid"
+[ "$valid" -eq 0 ] && {
+	if [ $FORCE -eq 1 ]; then
+		echo "Image check failed but --force given - will update anyway!" >&2
+	else
+		echo "Image check failed." >&2
+		exit 1
+	fi
+}
 
 if [ -n "$CONF_IMAGE" ]; then
 	case "$(get_magic_word $CONF_IMAGE cat)" in
diff --git a/package/base-files/files/usr/libexec/validate_firmware_image b/package/base-files/files/usr/libexec/validate_firmware_image
new file mode 100755
index 00000000000..a07796c9dc9
--- /dev/null
+++ b/package/base-files/files/usr/libexec/validate_firmware_image
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+. /lib/functions.sh
+. /lib/functions/system.sh
+. /usr/share/libubox/jshn.sh
+
+include /lib/upgrade
+
+VALID=1
+FORCEABLE=1
+
+# Mark image as invalid but still possible to install
+notify_firmware_invalid() {
+	VALID=0
+}
+
+# Mark image as broken (impossible to install)
+notify_firmware_broken() {
+	VALID=0
+	FORCEABLE=0
+}
+
+# Add result of validation test
+notify_firmware_test_result() {
+	local old_ns
+
+	json_set_namespace validate_firmware_image old_ns
+	json_add_boolean "$1" "$2"
+	json_set_namespace $old_ns
+}
+
+err_to_bool() {
+	[ "$1" -ne 0 ] && echo 0 || echo 1
+}
+
+fwtool_check_signature "$1" >&2
+FWTOOL_SIGNATURE=$?
+[ "$FWTOOL_SIGNATURE" -ne 0 ] && notify_firmware_invalid
+
+fwtool_check_image "$1" >&2
+FWTOOL_DEVICE_MATCH=$?
+[ "$FWTOOL_DEVICE_MATCH" -ne 0 ] && notify_firmware_invalid
+
+json_set_namespace validate_firmware_image old_ns
+json_init
+	json_add_object "tests"
+		json_add_boolean fwtool_signature "$(err_to_bool $FWTOOL_SIGNATURE)"
+		json_add_boolean fwtool_device_match "$(err_to_bool $FWTOOL_DEVICE_MATCH)"
+
+		# Call platform_check_image() here so it can add its test
+		# results and still mark image properly.
+		json_set_namespace $old_ns
+		platform_check_image "$1" >&2 || notify_firmware_invalid
+		json_set_namespace validate_firmware_image old_ns
+	json_close_object
+	json_add_boolean valid "$VALID"
+	json_add_boolean forceable "$FORCEABLE"
+json_dump -i
+json_set_namespace $old_ns
-- 
2.30.2