Re: [PATCH v8 4/8] image-fit-sig: Optionally require signatures

On 8/26/26 18:13, Jonas Karlman wrote:
> Hi Ludwig,
> 
> On 8/26/2026 3:52 PM, Ludwig Nussel wrote:
>> On 8/22/26 00:48, Jonas Karlman wrote:
>>> Hi Ludwig,
>>>
>>> On 8/13/2026 8:09 AM, Ludwig Nussel via U-Boot wrote:
>>>> If U-Boot is built with signature verification but no keys are
>>>> included in the device tree, the boot would still continue.
>>>> Introduce FIT_SIGNATURE_REQUIRED to avoid a fail-open setup.
>>>> Defaults to off so existing setups are not affected; boards wanting
>>>> fail-closed behaviour must enable it explicitly.
>>>>
>>>> Consistently use log_err for errors in fit_config_verify_required_keys()
>>>> while at it
>> [...]
>>>>
>>>>    	/* Work out what we need to verify */
>>>>    	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));
>>>> +		log_err("No signature node found: %s\n", fdt_strerror(key_node));
>>>
>>> This change is causing a verbose error messages on Rockchip where the
>>> FIT_SIGNATURE feature it used to check image integrity during boot. The
>>> config nodes is not expected to contain any signature, only hash(es) of
>>> images.
>>
>> Is the message a problem? The intention was to at least leave some clue
>> about the potential fail-open behavior.
> 
> Yes, printing a verbose error message for something that should not
> always be treated as an error is an issue.
> 
>> Looks like you are also not using signed configurations with is even
>> more suspicious:
>> [...]
> 
> It is not, as mentioned, Rockchip platform only use FIT_SIGNATURE for
> image integrity checks, and not for signed configuration validation.
> 
> This series seem to make an assumption that signed configurations always
> is desired, and for the Rockchip platform that is not the case.
> 
> On Rockchip we enable the FIT_SIGNATURE feature and only ensures image
> hash nodes are populated. At runtime SPL checksum validates the images

Interesting. The documentation of FIT_SIGNATURE clearly says it's about 
signature verification. Nevertheless you are right, common/spl/spl_fit.c 
has this:

        if (CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
                 printf("## Checking hash(es) for Image %s ... ",
                        fit_get_name(fit, node, NULL));
                 if (!fit_image_verify_with_data(...


That looks unnecessary to me as fit_image_verify_with_data() itself 
guards the actual signature processing with if(FIT_IMAGE_ENABLE_VERIFY 
..) which is turned on by CONFIG_IS_ENABLED(FIT_SIGNATURE).

Looks like common/spl/spl_fit.c should not have the FIT_SIGNATURE check, 
right?

Also, fit_image_verify_with_data() looks like another case of fail open. 
If bit rot coincidentally flips a bit in the "hash" node name string and 
therefore no node is found, no check is performed at all and the 
function returns success.

cu
Ludwig

-- 
Ludwig Nussel
Siemens AG
www.siemens.com