On Tue, Sep 15, 2026 at 08:09:03AM +1200, Eva Crystal wrote: > This follows up my report of 2026-09-08, "[SECURITY] FIT signature > enforcement fails open when the control DTB has no /signature node": > > > https://lore.kernel.org/u-boot/CA+pz=k+hAU5dqbO4GOLJ0fRMRdyrq=ktsnwuc0ocr3r9pot...@mail.gmail.com/ > > Tom asked for a fix and a test following the sending_patches process, so > here they are. Everything below was measured on sandbox at > commit a44f46af0aa1 ("MAINTAINERS: tee: update Jens Wiklander's email > address"). > > > The fail-open > ------------- > > fit_image_verify_required_sigs() at boot/image-fit-sig.c:187 looks up the > "/signature" node of the U-Boot control devicetree and returns success > when it is absent, at boot/image-fit-sig.c:203-207: > > /* Work out what we need to verify */ > *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)); > return 0; > } > > With no /signature node there are no keys marked 'required', so nothing > downstream can fail the image. The only indication is a debug() call, > compiled out of a normal build. > > 'iminfo' at this HEAD verifies images but not configurations, so it > reaches this gate directly. Against a control devicetree with no > /signature node at all, and a FIT whose configuration signature carries > no 'value': > > ## Checking hash(es) for FIT Image at 10400100 ... > Hash(es) for Image 0 (kernel): sha256+ > Hash(es) for Image 1 (fdt-1): sha256+ > > That '+' is fit_image_verify_with_data() reporting the signature > verified, on a board holding no keys whatsoever. The comment beside it in > boot/image-fit.c is correct that only 'required' keys can fail an image, > and that is precisely what makes a missing /signature node fatal to the > security property rather than to the boot. > > With CONFIG_FIT_REQUIRE_CONFIG_SIGS=y the same command gives: > > ## Checking hash(es) for FIT Image at 10400100 ... > Hash(es) for Image 0 (kernel): 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! > > > Relationship to Ludwig Nussel's series > -------------------------------------- > > The same early return exists in the configuration path, at > boot/image-fit-sig.c:686-690, and that half is already being addressed. > Ludwig Nussel's "Improve FIT signature handling" series has been fixing > it since April and is at v8: > > v7 4/8, direct link: > > https://lore.kernel.org/u-boot/20260618144742.v7.4.6d1d5d14140677a76a7cd7bb99a9088d8c8f480c@changeid/ > > v8 4/8 review thread: > > https://lore.kernel.org/u-boot/[email protected]/ > > That series introduces CONFIG_FIT_REQUIRE_CONFIG_SIGS, closes the > configuration gate, and also restructures the closing check at > boot/image-fit-sig.c:733 to cover the case where /signature exists but > holds no required="conf" keys, where reqd_sigs stays 0 and the check > cannot fire. The v7 form was applied to u-boot/next on 2026-06-30 and > then dropped after CI failed. > > This series is complementary, not competing. v2 through v8 change only > fit_config_verify_required_keys(); fit_image_verify_required_sigs() is > untouched throughout. That image gate is what these patches close. The > Kconfig symbol here is deliberately Ludwig's FIT_REQUIRE_CONFIG_SIGS > rather than a new name, so that whichever lands first the other reduces > to a smaller change rather than a conflicting one. If 4/8 lands first, > all that remains of patch 1/2 is the image-gate hunk. > > > Kconfig name and default > ------------------------ > > CONFIG_FIT_REQUIRE_CONFIG_SIGS, depends on FIT_SIGNATURE, default n, > plus SPL_FIT_REQUIRE_CONFIG_SIGS which selects it. > > Default n is a measured choice, not caution. Full test_vboot suite on > sandbox with these patches applied: > > option at its default (not set): 32 passed, 0 failed > CONFIG_FIT_REQUIRE_CONFIG_SIGS=y: 19 passed, 13 failed > > The 13 are not a defect in the patch. test_vboot's first step is > > test/py/tests/test_vboot.py:312 > run_bootm(sha_algo, 'unsigned images', ' - OK' if algo_arg else 'dev-', > True) > > which boots an unsigned-images FIT against a devicetree that has no keys > yet and asserts that it boots. That assertion encodes the old contract, > so enforcing correctly breaks it: > > E assert 'dev-' in "... No signature node in control FDT: FDT_ERR_NOTFOUND > Bad Data Hash ... ERROR -2: ... can't get kernel image!" > > This is the same class of CI breakage that got v7 dropped from next. > Making the option default y would require test_vboot to be restructured > first, which is what Ludwig's 5/8 does. Until that lands, default n keeps > the shipped configuration byte-for-byte unchanged in behaviour. > > > The console message is conditional > ---------------------------------- > > The message is printed only when the option is enabled; the debug() call > is kept otherwise. Jonas Karlman objected to making it unconditional: > > > https://lore.kernel.org/u-boot/[email protected]/ > > "Yes, printing a verbose error message for something that should not > always be treated as an error is an issue." > > Rockchip enables FIT_SIGNATURE for image hash integrity alone and has no > /signature node by design, so an unconditional message is noise on every > boot there. Gating it means a board that has not asked for enforcement > sees no change at all. Measured with the option off: the string > "No signature node in control FDT" appears 0 times in the boot output, > and 'iminfo' prints the same 'sha256+' lines as an unpatched build. > > printf() is used rather than log_err() so that this applies to main > standalone; log_err() in this file requires Ludwig's 3/8. If these are > ever squashed together, log_err() is the better choice. > > > NOT TESTED > ---------- > > Being explicit, because some of this matters for review: > > - SPL: SPL_FIT_REQUIRE_CONFIG_SIGS is added and compiles through > CONFIG_IS_ENABLED, but I have not built or run any SPL configuration. > Compile coverage only. It needs someone with SPL verified boot. > - Host tools: mkimage and fit_check_sign build with the patch, but I have > not exercised any fit_check_sign behaviour change. CONFIG_IS_ENABLED > is false in the host build since tools/Makefile only defines > CONFIG_FIT_SIGNATURE, so host behaviour should be unchanged -- argued, > not measured. > - Sandbox only. I have no signed-boot hardware. Everything above is > sandbox_defconfig on x86-64. > - Jonas's Rockchip setup: not reproduced. I reasoned that the off path > stays quiet and then measured that it does on sandbox, which is not the > same as booting an RK3588. > > > 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"), and that one > moves the code from common/image-sig.c to common/image-fit-sig.c rather > than introducing it. I could not identify an introducing commit with > enough confidence to tag one, so I have not.
This is a very long cover letter. It also doesn't fit our normal style (which you can see many examples of with "git log --merges"), can you please rewrite it to be more concise and fit the normal style? Thanks. -- Tom
signature.asc
Description: PGP signature
