Re: [RFC v2 0/2] Prevent alignment issues with "in place" FIT configurations

["Bjørn Mork" <bjorn@mork.no>]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

So if anyone looked at this, then you've noticed that it fails to
consider signing.

The design makes it hard to support the combination.  Algnment must run
last since signing may inject variable sized nodes before the fdt data
properties.  Signing must run last since it hashes the blob as it is,
inluding FDT_NOP tags and property order.

But we can trick this int working by signing before aligning to create
the signature nodes with their proper size and position, and then
sign again as a final step if we had to inject any FDT_NOP tags.

The attached fix works for me, creating valid signatures with aligned
images no matter how many times I re-sign the FIT with different length
signature comments.

Downsides is the obvious double signing, which we already accept for
resizing, and a build-up of FDT_NOP tags.  The latter is only an issue
if you re-sign with signature node size changes. And there's at most one
tag added per fdt node per signature update, so it's hardly a major
problem.


Bjørn


[-- Attachment #2: 0001-fix-re-sign-if-nops-were-added.patch --]
[-- Type: text/x-diff, Size: 1525 bytes --]

From 00f5cf3b08e44856ed826d427f63743180f3ae90 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?= <bjorn@mork.no>
Date: Sun, 6 Nov 2022 22:13:53 +0100
Subject: [RFC v2 3/2] fix: re-sign if nops were added
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 tools/fit_image.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/tools/fit_image.c b/tools/fit_image.c
index 2e215ca2199d..c29e209a8eea 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -66,6 +66,7 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 	struct stat sbuf;
 	void *ptr;
 	int ret = 0;
+	size_t oldsize;
 
 	tfd = mmap_fdt(params->cmdname, tmpfile, size_inc, &ptr, &sbuf, true,
 		       false);
@@ -115,9 +116,22 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc,
 	}
 
 	if (!ret) {
+		oldsize = fdt_size_dt_struct(ptr);
 		ret = fit_align_fdt_images(ptr);
 	}
 
+	/* new FDT_NOP tags must be included in the signed regions */
+	if (!ret && oldsize != fdt_size_dt_struct(ptr)) {
+		ret = fit_add_verification_data(params->keydir,
+						params->keyfile, dest_blob, ptr,
+						params->comment,
+						params->require_keys,
+						params->engine_id,
+						params->cmdname,
+						params->algo_name,
+						&params->summary);
+	}
+
 	if (dest_blob) {
 		munmap(dest_blob, destfd_size);
 		close(destfd);
-- 
2.30.2