Re: [PATCH 09/15] common: tlv: Add TLV-Signature support

[Jonas Rebmann <jre@pengutronix.de>]

Hi,

On 2025-10-14 13:03, Jonas Rebmann wrote:
> Implement TLV signature using the existing placeholders for it. Use the
> existing cryptographic primitives and public key handling used for
> fitimage verification.
> 
> Signature is verified and then must be valid iff CONFIG_TLV_SIGNATURE is
> enabled and a keyring is selected for the decoder. SHA256 hashing is
> hardcoded for now.
> 
> As 16 bit are well sufficient to store the length of the signature
> section in bytes, reduce it to its least significant 16 bit and reserve
> the remaining 16 bit for future use.
> 
> As sig_len where the only reserved bits left, and where zero-reserved,
> this leaves more wiggle room to still expand the format in the future.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>   common/Kconfig       |  4 +++
>   common/tlv/parser.c  | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   include/tlv/format.h | 22 ++++++++++----
>   3 files changed, 105 insertions(+), 5 deletions(-)
> 
> [...]
> 
> diff --git a/common/tlv/parser.c b/common/tlv/parser.c
> index f74ada99d7..0a23beba4e 100644
> --- a/common/tlv/parser.c
> +++ b/common/tlv/parser.c
> 
> [...]
> 
> @@ -17,6 +93,7 @@ int tlv_parse(struct tlv_device *tlvdev,
>   	struct tlv_mapping *map = NULL;
>   	struct tlv_header *header = tlv_device_header(tlvdev);
>   	u32 magic;
> +	u16 reserved;
>   	size_t size;
>   	int ret = 0;
>   	u32 crc = ~0;
> @@ -24,6 +101,7 @@ int tlv_parse(struct tlv_device *tlvdev,
>   	magic = be32_to_cpu(header->magic);
>   
>   	size = tlv_total_len(header);
> +	reserved = get_unaligned_be16(&header->reserved);
>   
>   	if (size == SIZE_MAX) {
>   		pr_warn("Invalid TLV header, overflows\n");
> @@ -36,6 +114,12 @@ int tlv_parse(struct tlv_device *tlvdev,
>   		return -EILSEQ;
>   	}
>   
> +	if (decoder->signature_keyring) {
> +		ret = tlv_verify(header, decoder->signature_keyring);
> +		if (ret)
> +			return ret;
> +	}

At some point I accidentally dropped the special handling of a signed
TLV matched to a decoder that has signature disabled. I believe it is
correct that we keep parsing without signature verification but that v2
should add a warning e.g.:

	} else if (get_unaligned_be16(&header->length_sig)) {
		pr_warn("Skipping verification of TLV signature: "
			"No keyring selected in decoder with magic %08x\n",
			decoder->magic);
	}


> +
>   	for_each_tlv(header, tlv) {
>   		struct tlv_mapping **mappings;
>   		u16 tag = TLV_TAG(tlv);
> [...]

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |