fit_image_verify_required_sigs() looks up the "/signature" node of the
U-Boot control devicetree to work out which keys are marked required.
When that node is absent it sets *no_sigsp and returns 0, so every
caller treats the image as not requiring signatures. Nothing downstream
can fail it after that: a FIT whose signatures are absent, malformed or
invalid verifies successfully, and the only indication is a debug()
call, which is compiled out of a normal build.
Against a control devicetree holding no "/signature" node at all, and a
FIT whose configuration signature carries no "value", an unpatched
build reports both images as verified:
## Checking hash(es) for FIT Image at 10400100 ...
Hash(es) for Image 0 (kernel): sha256+ OK
Hash(es) for Image 1 (fdt-1): sha256+ OK
That '+' is fit_image_verify_with_data() reporting the signature as
verified, on a board holding no keys whatsoever.
Add FIT_REQUIRE_IMAGE_SIGS, which turns a missing "/signature" node
into a verification failure on this gate, and report the missing node
with log_warning() so that the condition is no longer silent. With
CONFIG_FIT_REQUIRE_IMAGE_SIGS=y the same image is refused:
## Checking hash(es) for FIT Image at 10400100 ...
Hash(es) for Image 0 (kernel): fit_image_verify_required_sigs() No
signature node in control FDT: FDT_ERR_NOTFOUND
error!
Unable to verify required signature for '' hash node in 'kernel' image node
Bad hash in FIT image!
The adjacent configuration gate in fit_config_verify_required_keys() reports
the same missing node with debug(), after Jonas Karlman objected that a
verbose error is wrong on a Rockchip board which enables FIT_SIGNATURE for
image hashes alone and has no /signature node by design. This gate keeps
log_warning() for a reason specific to the image side. Under bootm the
refusal surfaces as Bad Data Hash, indistinguishable from a corrupt image,
and nothing in this function can tell an intentional hash-only configuration
from a control FDT that lost its /signature node, so a silent return
withholds the signal from exactly the case this patch exists for. The
message fires once per boot rather than once per image, the level is
LOGL_WARNING rather than an error, and CONFIG_LOGLEVEL suppresses it for
boards which do not want it. On evb-rk3568_defconfig the shipped
CONFIG_SPL_LOGLEVEL is 4, so on that board at its default configuration
this message appears once in SPL.
That same case is why enforcement is a separate option which defaults
to n. Setups which enable FIT_SIGNATURE only for image integrity, and
which legitimately carry no keys, must keep booting exactly as before,
so failing the gate is opt-in.
Measured on sandbox with test/py, running the vboot tests:
option at its default (not set): 32 passed, 0 failed
CONFIG_FIT_REQUIRE_IMAGE_SIGS=y: 19 passed, 13 failed
The 32 are the 31 pre-existing vboot tests, which all pass on an
unpatched build of the same tree, plus the one added by 2/2; the
default configuration is unchanged. The 13 are by design, not a defect:
test_vboot.py boots an unsigned-images FIT against a key-less
devicetree and asserts that it boots, which is precisely the contract
this option withdraws.
boot/image-fit-sig.c is also built into mkimage and fit_check_sign,
where <log.h> is not included. tools/mkimage.h already shims log_debug()
onto debug(); log_warning() is shimmed there the same way, so host tool
output is unchanged.
Not tested:
- SPL: compile coverage only, on evb-rk3568_defconfig with
CONFIG_SPL_FIT_SIGNATURE=y and CONFIG_SPL_FIT_REQUIRE_IMAGE_SIGS=y.
spl/boot/image-fit-sig.o compiles and spl/u-boot-spl links with no
warnings, and the gate's message string is present in the SPL binary
and absent without the patch. No SPL image was booted, so there is no
SPL runtime coverage.
- Sandbox only, on x86-64. No signed-boot hardware.
- Jonas's Rockchip setup was not reproduced.
No Fixes: tag. The early return is original behaviour rather than a
regression. The oldest commit touching it in this path is commit
b983cc2da0ba ("lib: rsa: decouple rsa from FIT image verification"),
which moves the code from common/image-sig.c to common/image-fit-sig.c
rather than introducing it.
Cc: Jonas Karlman <[email protected]>
Signed-off-by: Eva Crystal <[email protected]>
---
boot/Kconfig | 34 ++++++++++++++++++++++++++++++++++
boot/image-fit-sig.c | 15 +++++++++++++--
tools/mkimage.h | 1 +
3 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig
index 0b3399a66d..ff4d609b3f 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -154,6 +154,32 @@ config FIT_REQUIRE_CONFIG_SIGS
need to reuse the same u-boot binary in setups without
keys or rely on image node only signatures.
+config FIT_REQUIRE_IMAGE_SIGS
+ bool "Fail image verification when the control FDT has no /signature
node"
+ depends on FIT_SIGNATURE
+ help
+ fit_image_verify_required_sigs() looks up the "/signature" node of
+ the U-Boot control devicetree to work out which keys are marked
+ required. When that node is absent it returns success, so no image
+ signature can ever be required: a FIT image whose signatures are
+ absent, malformed or invalid verifies successfully.
+
+ Enable this option to treat a missing "/signature" node as an image
+ verification failure instead.
+
+ The missing node is reported once per boot with log_warning(),
+ whether or not this option is enabled, so the condition is visible
+ at the default loglevel and can be suppressed through
+ CONFIG_LOGLEVEL.
+
+ This option covers image signatures only. Signatures on the FIT
+ configuration nodes are checked by a separate gate which this
+ option does not affect.
+
+ Say N if the same U-Boot binary must also run in setups which carry
+ no keys, or which enable FIT_SIGNATURE only for image hash
+ integrity checking rather than for signature enforcement.
+
config FIT_SIGNATURE_MAX_SIZE
hex "Max size of signed FIT structures"
depends on FIT_SIGNATURE
@@ -281,6 +307,14 @@ config SPL_FIT_REQUIRE_CONFIG_SIGS
depends on SPL_FIT_SIGNATURE
select FIT_REQUIRE_CONFIG_SIGS
+config SPL_FIT_REQUIRE_IMAGE_SIGS
+ bool "Fail image verification when the SPL control FDT has no
/signature node"
+ depends on SPL_FIT_SIGNATURE
+ select FIT_REQUIRE_IMAGE_SIGS
+ help
+ Enable the FIT_REQUIRE_IMAGE_SIGS behaviour in SPL. See the help text
+ of FIT_REQUIRE_IMAGE_SIGS for details.
+
config SPL_LOAD_FIT
bool "Enable SPL loading U-Boot as a FIT (basic fitImage features)"
depends on SPL
diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 5a2c6316b1..3826005072 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -188,6 +188,7 @@ int fit_image_verify_required_sigs(const void *fit, int
image_noffset,
const char *data, size_t size,
const void *key_blob, int *no_sigsp)
{
+ static bool reported;
int verify_count = 0;
int noffset;
int key_node;
@@ -201,8 +202,18 @@ int fit_image_verify_required_sigs(const void *fit, int
image_noffset,
*no_sigsp = 1;
key_node = fdt_subnode_offset(key_blob, 0, FIT_SIG_NODENAME);
if (key_node < 0) {
- debug("%s: No signature node found: %s\n", __func__,
- fdt_strerror(key_node));
+ /*
+ * The control FDT is a property of the board rather than of
+ * the image, so report this once per boot instead of once
+ * per image.
+ */
+ if (!reported) {
+ reported = true;
+ log_warning("No signature node in control FDT: %s\n",
+ fdt_strerror(key_node));
+ }
+ if (CONFIG_IS_ENABLED(FIT_REQUIRE_IMAGE_SIGS))
+ return -EPERM;
return 0;
}
diff --git a/tools/mkimage.h b/tools/mkimage.h
index d8d292a809..d65099fe2b 100644
--- a/tools/mkimage.h
+++ b/tools/mkimage.h
@@ -32,6 +32,7 @@
#define log_debug(fmt, args...) debug(fmt, ##args)
#define log_info(fmt, args...) printf(fmt, ##args)
+#define log_warning(fmt, args...) debug(fmt, ##args)
/*
* Use stdout rather than stderr so error messages stay
* interleaved with normal output in the order produced.
--
2.53.0
