[PATCH 00/15] TLV-Signature and keyrings

[PATCH 00/15] TLV-Signature and keyrings

[Jonas Rebmann]

This series introduces everything needed for the use of signed TLVs in
barebox. This allows for signed TLVs to be part of a secure boot chain,
if CONFIG_TLV_SIGNATURE is enabled, keys are configured and the decoder
is configured to require signature.

As TLV signature verification uses the public_keys list, propagated by
keytoc.c with the public keys selected in CONFIG_CRYPTO_PUBLIC_KEYS, the
keyring feature was introduced to allow separate keys for separate
concerns.

The existing fitimage verification now only verifies against keys in the
"fit" keyring. To require a valid signature of TLVs, specify a
tlv_decoder::signature_keyring in the decoder. No signature verification
is performed if signature_keyring is NULL for a decoder matched to the
TLV magic.

A new builtin decoder was added to common/tlv/barebox.c with the magic
0x61bb95f3 and .signature_keyring = "tlv". Consequently
CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS now adds the insecure development
keys to both the "tlv" and the "fit" keyring. This allows for quick
testing and debugging of decoders requiring signature.

For the creation of signed TLVs, bareboxtlv-generator.py was updated
with --sign and --verify options for TLV binary encoding and decoding
respectively.

Changes to the TLV format and -tool usage as well as the breaking
changes to the keyspec syntax are documented in Documentation/.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Jonas Rebmann (15):
      common: clean up TLV code
      crypto: Add support for keyrings
      fit: only accept keys from "fit"-keyring
      crypto: keytoc: Rename "hint" to "fit-hint" and do not use it in identifiers
      commands: keys: update output format to include keyring
      commands: tlv: Error out on invalid TLVs
      scripts: bareboxtlv-generator: Implement signature
      scripts: bareboxtlv-generator: Increase max_size in example schema
      common: tlv: Add TLV-Signature support
      common: tlv: default decoder for signed TLV
      crypto: Use "development" keys for "fit" and "tlv" keyring
      test: py: add signature to TLV integration tests
      ci: pytest: Add kconfig fragment for TLV signature integration tests
      doc/barebox-tlv: Update documentation regarding TLV-Signature
      Documentation: migration-2025.11.0: List changes to CONFIG_CRYPTO_PUBLIC_KEYS

 .github/workflows/test-labgrid-pytest.yml          |   1 +
 .../devicetree/bindings/nvmem/barebox,tlv.yaml     |   1 +
 .../migration-guides/migration-2025.11.0.rst       |  17 ++
 Documentation/user/barebox-tlv.rst                 |  49 +++-
 commands/keys.c                                    |   8 +-
 commands/tlv.c                                     |   2 +-
 common/Kconfig                                     |   4 +
 .../boards/configs/enable_tlv_sig_testing.config   |  13 +
 common/image-fit.c                                 |  13 +-
 common/tlv/barebox.c                               |  25 +-
 common/tlv/parser.c                                | 102 ++++++-
 crypto/Makefile                                    |   6 +-
 crypto/fit-4096-development.key                    |  51 ++++
 crypto/fit-ecdsa-development.key                   |   5 +
 crypto/public-keys.c                               |  15 +-
 crypto/rsa.c                                       |   1 +
 include/crypto/public_key.h                        |  22 +-
 include/tlv/format.h                               |  29 +-
 include/tlv/tlv.h                                  |   1 +
 .../bareboxtlv-generator/bareboxtlv-generator.py   | 242 ++++++++++++++--
 scripts/bareboxtlv-generator/requirements.txt      |   1 +
 scripts/bareboxtlv-generator/schema-example.yaml   |   2 +-
 scripts/include/linux/overflow.h                   | 312 +++++++++++++++++++++
 scripts/keytoc.c                                   | 259 +++++++++++------
 test/py/test_tlv.py                                | 205 +++++++++++---
 25 files changed, 1202 insertions(+), 184 deletions(-)
---
base-commit: 39309dcb356714fc3f345f52ff30b0281d65e27b
change-id: 20251014-tlv-signature-2673b1a24445

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

[PATCH 01/15] common: clean up TLV code

[Jonas Rebmann]

This is a preparatory commit for TLV signature verification:

 - Include overflow checks for TLV sizes
 - Be careful with narrowing conversions
 - Add and improve comments

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 common/tlv/parser.c              |  18 ++-
 include/tlv/format.h             |  12 +-
 scripts/include/linux/overflow.h | 312 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 337 insertions(+), 5 deletions(-)

diff --git a/common/tlv/parser.c b/common/tlv/parser.c
index fc46092650..f74ada99d7 100644
--- a/common/tlv/parser.c
+++ b/common/tlv/parser.c
@@ -16,15 +16,20 @@ int tlv_parse(struct tlv_device *tlvdev,
 	const struct tlv *tlv = NULL;
 	struct tlv_mapping *map = NULL;
 	struct tlv_header *header = tlv_device_header(tlvdev);
-	u32 magic, size;
+	u32 magic;
+	size_t size;
 	int ret = 0;
 	u32 crc = ~0;
 
-
 	magic = be32_to_cpu(header->magic);
 
 	size = tlv_total_len(header);
 
+	if (size == SIZE_MAX) {
+		pr_warn("Invalid TLV header, overflows\n");
+		return -EOVERFLOW;
+	}
+
 	crc = crc32_be(crc, header, size - 4);
 	if (crc != tlv_crc(header)) {
 		pr_warn("Invalid CRC32. Should be %08x\n", crc);
@@ -133,7 +138,8 @@ void tlv_of_unregister_fixup(struct tlv_device *tlvdev)
 struct tlv_header *tlv_read(const char *filename, size_t *nread)
 {
 	struct tlv_header *header = NULL, *tmpheader;
-	int size, fd, ret;
+	size_t size;
+	int fd, ret;
 
 	fd = open(filename, O_RDONLY);
 	if (fd < 0)
@@ -153,6 +159,12 @@ struct tlv_header *tlv_read(const char *filename, size_t *nread)
 
 	size = tlv_total_len(header);
 
+	if (size == SIZE_MAX) {
+		pr_warn("Invalid TLV header, overflows\n");
+		ret = -EOVERFLOW;
+		goto err;
+	}
+
 	tmpheader = realloc(header, size);
 	if (!tmpheader) {
 		struct stat st;
diff --git a/include/tlv/format.h b/include/tlv/format.h
index 29c8a6d42d..c4521c3131 100644
--- a/include/tlv/format.h
+++ b/include/tlv/format.h
@@ -18,6 +18,7 @@
 
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/overflow.h>
 #include <asm/unaligned.h>
 #include <linux/build_bug.h>
 
@@ -48,6 +49,8 @@ struct tlv_header {
 	__be32 length_tlv; /* in bytes */
 	__be32 length_sig; /* in bytes */
 	struct tlv tlvs[];
+	/* __be32 crc; */
+	/* u8 sig[]; */
 };
 static_assert(sizeof(struct tlv_header) == 3 * 4);
 
@@ -56,8 +59,13 @@ static_assert(sizeof(struct tlv_header) == 3 * 4);
 
 static inline size_t tlv_total_len(const struct tlv_header *header)
 {
-	return sizeof(struct tlv_header) + get_unaligned_be32(&header->length_tlv)
-		+ get_unaligned_be32(&header->length_sig) + 4;
+	size_t ret;
+
+	ret = size_add(sizeof(struct tlv_header), get_unaligned_be32(&header->length_tlv));
+	ret = size_add(ret, sizeof(__be32)); /* CRC appended after TLVs */
+	ret = size_add(ret, get_unaligned_be32(&header->length_sig)); /* optional signature appended after CRC */
+
+	return ret; /* SIZE_MAX on overflow */
 }
 
 /*
diff --git a/scripts/include/linux/overflow.h b/scripts/include/linux/overflow.h
new file mode 100644
index 0000000000..f9b60313ea
--- /dev/null
+++ b/scripts/include/linux/overflow.h
@@ -0,0 +1,312 @@
+/* SPDX-License-Identifier: GPL-2.0 OR MIT */
+#ifndef __LINUX_OVERFLOW_H
+#define __LINUX_OVERFLOW_H
+
+#include <linux/compiler.h>
+#include <linux/limits.h>
+#include <linux/const.h>
+
+/*
+ * We need to compute the minimum and maximum values representable in a given
+ * type. These macros may also be useful elsewhere. It would seem more obvious
+ * to do something like:
+ *
+ * #define type_min(T) (T)(is_signed_type(T) ? (T)1 << (8*sizeof(T)-1) : 0)
+ * #define type_max(T) (T)(is_signed_type(T) ? ((T)1 << (8*sizeof(T)-1)) - 1 : ~(T)0)
+ *
+ * Unfortunately, the middle expressions, strictly speaking, have
+ * undefined behaviour, and at least some versions of gcc warn about
+ * the type_max expression (but not if -fsanitize=undefined is in
+ * effect; in that case, the warning is deferred to runtime...).
+ *
+ * The slightly excessive casting in type_min is to make sure the
+ * macros also produce sensible values for the exotic type _Bool. [The
+ * overflow checkers only almost work for _Bool, but that's
+ * a-feature-not-a-bug, since people shouldn't be doing arithmetic on
+ * _Bools. Besides, the gcc builtins don't allow _Bool* as third
+ * argument.]
+ *
+ * Idea stolen from
+ * https://mail-index.netbsd.org/tech-misc/2007/02/05/0000.html -
+ * credit to Christian Biere.
+ */
+#define __type_half_max(type) ((type)1 << (8*sizeof(type) - 1 - is_signed_type(type)))
+#define type_max(T) ((T)((__type_half_max(T) - 1) + __type_half_max(T)))
+#define type_min(T) ((T)((T)-type_max(T)-(T)1))
+
+/*
+ * Avoids triggering -Wtype-limits compilation warning,
+ * while using unsigned data types to check a < 0.
+ */
+#define is_non_negative(a) ((a) > 0 || (a) == 0)
+#define is_negative(a) (!(is_non_negative(a)))
+
+/*
+ * Allows for effectively applying __must_check to a macro so we can have
+ * both the type-agnostic benefits of the macros while also being able to
+ * enforce that the return value is, in fact, checked.
+ */
+static inline bool __must_check __must_check_overflow(bool overflow)
+{
+	return unlikely(overflow);
+}
+
+/**
+ * check_add_overflow() - Calculate addition with overflow checking
+ * @a: first addend
+ * @b: second addend
+ * @d: pointer to store sum
+ *
+ * Returns 0 on success.
+ *
+ * *@d holds the results of the attempted addition, but is not considered
+ * "safe for use" on a non-zero return value, which indicates that the
+ * sum has overflowed or been truncated.
+ */
+#define check_add_overflow(a, b, d)	\
+	__must_check_overflow(__builtin_add_overflow(a, b, d))
+
+/**
+ * check_sub_overflow() - Calculate subtraction with overflow checking
+ * @a: minuend; value to subtract from
+ * @b: subtrahend; value to subtract from @a
+ * @d: pointer to store difference
+ *
+ * Returns 0 on success.
+ *
+ * *@d holds the results of the attempted subtraction, but is not considered
+ * "safe for use" on a non-zero return value, which indicates that the
+ * difference has underflowed or been truncated.
+ */
+#define check_sub_overflow(a, b, d)	\
+	__must_check_overflow(__builtin_sub_overflow(a, b, d))
+
+/**
+ * check_mul_overflow() - Calculate multiplication with overflow checking
+ * @a: first factor
+ * @b: second factor
+ * @d: pointer to store product
+ *
+ * Returns 0 on success.
+ *
+ * *@d holds the results of the attempted multiplication, but is not
+ * considered "safe for use" on a non-zero return value, which indicates
+ * that the product has overflowed or been truncated.
+ */
+#define check_mul_overflow(a, b, d)	\
+	__must_check_overflow(__builtin_mul_overflow(a, b, d))
+
+/**
+ * check_shl_overflow() - Calculate a left-shifted value and check overflow
+ * @a: Value to be shifted
+ * @s: How many bits left to shift
+ * @d: Pointer to where to store the result
+ *
+ * Computes *@d = (@a << @s)
+ *
+ * Returns true if '*@d' cannot hold the result or when '@a << @s' doesn't
+ * make sense. Example conditions:
+ *
+ * - '@a << @s' causes bits to be lost when stored in *@d.
+ * - '@s' is garbage (e.g. negative) or so large that the result of
+ *   '@a << @s' is guaranteed to be 0.
+ * - '@a' is negative.
+ * - '@a << @s' sets the sign bit, if any, in '*@d'.
+ *
+ * '*@d' will hold the results of the attempted shift, but is not
+ * considered "safe for use" if true is returned.
+ */
+#define check_shl_overflow(a, s, d) __must_check_overflow(({		\
+	typeof(a) _a = a;						\
+	typeof(s) _s = s;						\
+	typeof(d) _d = d;						\
+	u64 _a_full = _a;						\
+	unsigned int _to_shift =					\
+		is_non_negative(_s) && _s < 8 * sizeof(*d) ? _s : 0;	\
+	*_d = (_a_full << _to_shift);					\
+	(_to_shift != _s || is_negative(*_d) || is_negative(_a) ||	\
+	(*_d >> _to_shift) != _a);					\
+}))
+
+#define __overflows_type_constexpr(x, T) (			\
+	is_unsigned_type(typeof(x)) ?				\
+		(x) > type_max(typeof(T)) :			\
+	is_unsigned_type(typeof(T)) ?				\
+		(x) < 0 || (x) > type_max(typeof(T)) :		\
+	(x) < type_min(typeof(T)) || (x) > type_max(typeof(T)))
+
+#define __overflows_type(x, T)		({	\
+	typeof(T) v = 0;			\
+	check_add_overflow((x), v, &v);		\
+})
+
+/**
+ * overflows_type - helper for checking the overflows between value, variables,
+ *		    or data type
+ *
+ * @n: source constant value or variable to be checked
+ * @T: destination variable or data type proposed to store @x
+ *
+ * Compares the @x expression for whether or not it can safely fit in
+ * the storage of the type in @T. @x and @T can have different types.
+ * If @x is a constant expression, this will also resolve to a constant
+ * expression.
+ *
+ * Returns: true if overflow can occur, false otherwise.
+ */
+#define overflows_type(n, T)					\
+	__builtin_choose_expr(__is_constexpr(n),		\
+			      __overflows_type_constexpr(n, T),	\
+			      __overflows_type(n, T))
+
+/**
+ * castable_to_type - like __same_type(), but also allows for casted literals
+ *
+ * @n: variable or constant value
+ * @T: variable or data type
+ *
+ * Unlike the __same_type() macro, this allows a constant value as the
+ * first argument. If this value would not overflow into an assignment
+ * of the second argument's type, it returns true. Otherwise, this falls
+ * back to __same_type().
+ */
+#define castable_to_type(n, T)						\
+	__builtin_choose_expr(__is_constexpr(n),			\
+			      !__overflows_type_constexpr(n, T),	\
+			      __same_type(n, T))
+
+/**
+ * size_mul() - Calculate size_t multiplication with saturation at SIZE_MAX
+ * @factor1: first factor
+ * @factor2: second factor
+ *
+ * Returns: calculate @factor1 * @factor2, both promoted to size_t,
+ * with any overflow causing the return value to be SIZE_MAX. The
+ * lvalue must be size_t to avoid implicit type conversion.
+ */
+static inline size_t __must_check size_mul(size_t factor1, size_t factor2)
+{
+	size_t bytes;
+
+	if (check_mul_overflow(factor1, factor2, &bytes))
+		return SIZE_MAX;
+
+	return bytes;
+}
+
+/**
+ * size_add() - Calculate size_t addition with saturation at SIZE_MAX
+ * @addend1: first addend
+ * @addend2: second addend
+ *
+ * Returns: calculate @addend1 + @addend2, both promoted to size_t,
+ * with any overflow causing the return value to be SIZE_MAX. The
+ * lvalue must be size_t to avoid implicit type conversion.
+ */
+static inline size_t __must_check size_add(size_t addend1, size_t addend2)
+{
+	size_t bytes;
+
+	if (check_add_overflow(addend1, addend2, &bytes))
+		return SIZE_MAX;
+
+	return bytes;
+}
+
+/**
+ * size_sub() - Calculate size_t subtraction with saturation at SIZE_MAX
+ * @minuend: value to subtract from
+ * @subtrahend: value to subtract from @minuend
+ *
+ * Returns: calculate @minuend - @subtrahend, both promoted to size_t,
+ * with any overflow causing the return value to be SIZE_MAX. For
+ * composition with the size_add() and size_mul() helpers, neither
+ * argument may be SIZE_MAX (or the result with be forced to SIZE_MAX).
+ * The lvalue must be size_t to avoid implicit type conversion.
+ */
+static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
+{
+	size_t bytes;
+
+	if (minuend == SIZE_MAX || subtrahend == SIZE_MAX ||
+	    check_sub_overflow(minuend, subtrahend, &bytes))
+		return SIZE_MAX;
+
+	return bytes;
+}
+
+/**
+ * array_size() - Calculate size of 2-dimensional array.
+ * @a: dimension one
+ * @b: dimension two
+ *
+ * Calculates size of 2-dimensional array: @a * @b.
+ *
+ * Returns: number of bytes needed to represent the array or SIZE_MAX on
+ * overflow.
+ */
+#define array_size(a, b)	size_mul(a, b)
+
+/**
+ * array3_size() - Calculate size of 3-dimensional array.
+ * @a: dimension one
+ * @b: dimension two
+ * @c: dimension three
+ *
+ * Calculates size of 3-dimensional array: @a * @b * @c.
+ *
+ * Returns: number of bytes needed to represent the array or SIZE_MAX on
+ * overflow.
+ */
+#define array3_size(a, b, c)	size_mul(size_mul(a, b), c)
+
+/**
+ * flex_array_size() - Calculate size of a flexible array member
+ *                     within an enclosing structure.
+ * @p: Pointer to the structure.
+ * @member: Name of the flexible array member.
+ * @count: Number of elements in the array.
+ *
+ * Calculates size of a flexible array of @count number of @member
+ * elements, at the end of structure @p.
+ *
+ * Return: number of bytes needed or SIZE_MAX on overflow.
+ */
+#define flex_array_size(p, member, count)				\
+	__builtin_choose_expr(__is_constexpr(count),			\
+		(count) * sizeof(*(p)->member) + __must_be_array((p)->member),	\
+		size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))
+
+/**
+ * struct_size() - Calculate size of structure with trailing flexible array.
+ * @p: Pointer to the structure.
+ * @member: Name of the array member.
+ * @count: Number of elements in the array.
+ *
+ * Calculates size of memory needed for structure of @p followed by an
+ * array of @count number of @member elements.
+ *
+ * Return: number of bytes needed or SIZE_MAX on overflow.
+ */
+#define struct_size(p, member, count)					\
+	__builtin_choose_expr(__is_constexpr(count),			\
+		sizeof(*(p)) + flex_array_size(p, member, count),	\
+		size_add(sizeof(*(p)), flex_array_size(p, member, count)))
+
+/**
+ * struct_size_t() - Calculate size of structure with trailing flexible array
+ * @type: structure type name.
+ * @member: Name of the array member.
+ * @count: Number of elements in the array.
+ *
+ * Calculates size of memory needed for structure @type followed by an
+ * array of @count number of @member elements. Prefer using struct_size()
+ * when possible instead, to keep calculations associated with a specific
+ * instance variable of type @type.
+ *
+ * Return: number of bytes needed or SIZE_MAX on overflow.
+ */
+#define struct_size_t(type, member, count)					\
+	struct_size((type *)NULL, member, count)
+
+#endif /* __LINUX_OVERFLOW_H */

-- 
2.51.0.297.gca2559c1d6

[PATCH 02/15] crypto: Add support for keyrings

[Jonas Rebmann]

Public keys for TLV verification will be handled with the mechanism used
in the existing fitimage verification where scripts/keytoc.c generates
public_keys.h based on CONFIG_CRYPTO_PUBLIC_KEYS.

Expand the existing keyspec parser to support a new syntax of
  keyring=<keyring>[,fit-hint=<fit_hint>]:<crt>
in addition to the legacy syntax of
  [<fit_hint>:]<crt>

With the new keyspec parsing, keyring name and fit hint need to confirm
to the regular expression
  [a-zA-Z][a-zA-Z0-9_-]*

For backwards compatibility, the old syntax stays supported with the
keyring defaulting to "fit" and a warning emitted.

Note however that the restriction of fit name hints to above regex
applies even when supplied in the legacy syntax.

To be able to error out when encountering an invalid keyspec, some
refactoring of keytoc.c was necessary.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 crypto/public-keys.c        |   2 +
 include/crypto/public_key.h |   1 +
 scripts/keytoc.c            | 186 +++++++++++++++++++++++++++++++++-----------
 3 files changed, 145 insertions(+), 44 deletions(-)

diff --git a/crypto/public-keys.c b/crypto/public-keys.c
index 8884e263b3..a870ec5438 100644
--- a/crypto/public-keys.c
+++ b/crypto/public-keys.c
@@ -46,6 +46,8 @@ static struct public_key *public_key_dup(const struct public_key *key)
 	k->type = key->type;
 	if (key->key_name_hint)
 		k->key_name_hint = xstrdup(key->key_name_hint);
+	if (key->keyring)
+		k->keyring = xstrdup(key->keyring);
 	k->hash = xmemdup(key->hash, key->hashlen);
 	k->hashlen = key->hashlen;
 
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 7edea2d691..c9dd38cc33 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -15,6 +15,7 @@ struct public_key {
 	enum public_key_type type;
 	struct list_head list;
 	char *key_name_hint;
+	char *keyring;
 	unsigned char *hash;
 	unsigned int hashlen;
 
diff --git a/scripts/keytoc.c b/scripts/keytoc.c
index 074af6f0b4..a2b508bf49 100644
--- a/scripts/keytoc.c
+++ b/scripts/keytoc.c
@@ -25,6 +25,15 @@
 #include <openssl/param_build.h>
 #include <openssl/store.h>
 
+#include <stdbool.h>
+#include <ctype.h>
+
+struct keyinfo {
+	char *keyname;
+	char *keyring;
+	char *path;
+};
+
 static int dts, standalone;
 
 static void enomem_exit(const char *func)
@@ -491,7 +500,7 @@ static int print_hash(EVP_PKEY *key)
 	return ret ? -EINVAL : 0;
 }
 
-static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_name_c)
+static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *keyring, const char *key_name_c)
 {
 	char group[128];
 	size_t outlen;
@@ -553,6 +562,7 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_na
 			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", key_name_c);
 			fprintf(outfilep, "\t.type = PUBLIC_KEY_TYPE_ECDSA,\n");
 			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
+			fprintf(outfilep, "\t.keyring = \"%s\",\n", keyring);
 			fprintf(outfilep, "\t.hash = %s_hash,\n", key_name_c);
 			fprintf(outfilep, "\t.hashlen = %u,\n", SHA256_DIGEST_LENGTH);
 			fprintf(outfilep, "\t.ecdsa = &%s,\n", key_name_c);
@@ -565,9 +575,9 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *key_na
 	return 0;
 }
 
-static const char *try_resolve_env(const char *input)
+static char *try_resolve_env(char *input)
 {
-	const char *var;
+	char *var;
 
 	if (strncmp(input, "__ENV__", 7))
 		return input;
@@ -583,7 +593,7 @@ static const char *try_resolve_env(const char *input)
 	return var;
 }
 
-static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name_c)
+static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *keyring, const char *key_name_c)
 {
 	BIGNUM *modulus, *r_squared;
 	uint64_t exponent = 0;
@@ -659,6 +669,7 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name
 			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", key_name_c);
 			fprintf(outfilep, "\t.type = PUBLIC_KEY_TYPE_RSA,\n");
 			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
+			fprintf(outfilep, "\t.keyring = \"%s\",\n", keyring);
 			fprintf(outfilep, "\t.hash = %s_hash,\n", key_name_c);
 			fprintf(outfilep, "\t.hashlen = %u,\n", SHA256_DIGEST_LENGTH);
 			fprintf(outfilep, "\t.rsa = &%s,\n", key_name_c);
@@ -671,18 +682,18 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *key_name
 	return 0;
 }
 
-static int gen_key(const char *keyname, const char *path)
+static int gen_key(struct keyinfo *info)
 {
 	int ret;
 	EVP_PKEY *key;
 	char *tmp, *key_name_c;
 
 	/* key name handling */
-	keyname = try_resolve_env(keyname);
-	if (!keyname)
+	info->keyname = try_resolve_env(info->keyname);
+	if (!info->keyname)
 		exit(1);
 
-	tmp = key_name_c = strdup(keyname);
+	tmp = key_name_c = strdup(info->keyname);
 
 	while (*tmp) {
 		if (*tmp == '-')
@@ -691,32 +702,104 @@ static int gen_key(const char *keyname, const char *path)
 	}
 
 	/* path/URI handling */
-	path = try_resolve_env(path);
-	if (!path)
+	info->path = try_resolve_env(info->path);
+	if (!info->path)
 		exit(1);
 
-	if (!strncmp(path, "pkcs11:", 7)) {
-		ret = engine_get_pub_key(path, &key);
+	if (!strncmp(info->path, "pkcs11:", 7)) {
+		ret = engine_get_pub_key(info->path, &key);
 		if (ret)
 			exit(1);
 	} else {
-		ret = pem_get_pub_key(path, &key);
+		ret = pem_get_pub_key(info->path, &key);
 		if (ret)
 			exit(1);
 	}
 
 	/* generate built-in keys */
-	ret = gen_key_ecdsa(key, keyname, key_name_c);
+	ret = gen_key_ecdsa(key, info->keyname, info->keyring, key_name_c);
 	if (ret == -EOPNOTSUPP)
 		return ret;
 
 	if (ret)
-		ret = gen_key_rsa(key, keyname, key_name_c);
+		ret = gen_key_rsa(key, info->keyname, info->keyring, key_name_c);
 
 	return ret;
 }
 
-static void get_name_path(const char *keyspec, char **keyname, char **path)
+static bool is_identifier(char **s)
+{
+	char *p = *s;
+
+	/* [a-zA-Z] */
+	if (!isalpha(*p))
+		return false;
+	p++;
+
+	/* [a-zA-Z0-9_-]* */
+	while (isalnum(*p) || *p == '_' || *p == '-')
+		p++;
+
+	*s = p;
+	return true;
+}
+
+static bool parse_info(char *p, struct keyinfo *out)
+{
+	char *k = p;
+	char *v;
+
+	if (*p == '\0') /* spec starts with colon. This is valid. */
+		return true;
+
+	if (!is_identifier(&p))
+		return false;
+
+	if (*p == '\0') {
+		out->keyname = strdup(k);
+		if (!k)
+			enomem_exit(__func__);
+		return true; /* legacy syntax */
+	}
+
+	/* new syntax: `<k>=<v>[,k=v[...]]` */
+	for (;;) {
+		if (*p != '=')
+			return false;
+		*p = '\0';
+
+		p++;
+		/* read `<k>=`, excepting <v> */
+		v = p;
+		if (!is_identifier(&p))
+			return false;
+
+		if (*p == '\0' || *p == ',') {
+			char d = *p;
+			*p = '\0';
+			v = strdup(v);
+			if (!v)
+				enomem_exit(__func__);
+			if (strcmp(k, "keyring") == 0)
+				out->keyring = strdup(v);
+			else if (strcmp(k, "hint") == 0)
+				out->keyname = strdup(v);
+			else
+				return false;
+
+			if (d == '\0')
+				return true;
+		} else {
+			return false;
+		}
+		p++;
+		k = p;
+		if (!is_identifier(&p))
+			return true;
+	}
+}
+
+static bool get_name_path(const char *keyspec, struct keyinfo *out)
 {
 	char *sep, *spec;
 
@@ -724,24 +807,24 @@ static void get_name_path(const char *keyspec, char **keyname, char **path)
 	if (!spec)
 		enomem_exit(__func__);
 
-	/* Split <key-hint>:<key-path> pair, <key-hint> is optional */
 	sep = strchr(spec, ':');
 	if (!sep) {
-		*path = spec;
-		return;
+		out->path = spec;
+		return true;
 	}
 
 	*sep = 0;
-	*keyname = strdup(spec);
-	if (!*keyname)
-		enomem_exit(__func__);
-
+	if (!parse_info(spec, out)) {
+		free(spec);
+		return false;
+	}
 	sep++;
-	*path = strdup(sep);
-	if (!*path)
+	out->path = strdup(sep);
+	if (!out->path)
 		enomem_exit(__func__);
 
 	free(spec);
+	return true;
 }
 
 int main(int argc, char *argv[])
@@ -749,6 +832,8 @@ int main(int argc, char *argv[])
 	int i, opt, ret;
 	char *outfile = NULL;
 	int keynum = 1;
+	int keycount;
+	struct keyinfo *keylist;
 
 	outfilep = stdout;
 
@@ -776,13 +861,33 @@ int main(int argc, char *argv[])
 	}
 
 	if (optind == argc) {
-		fprintf(stderr, "Usage: %s [-ods] <key_name_hint>:<crt> ...\n", argv[0]);
+		fprintf(stderr, "Usage: %s [-ods] keyring=<keyring>,hint=<hint>:<crt> ...\n", argv[0]);
 		fprintf(stderr, "\t-o FILE\twrite output into FILE instead of stdout\n");
 		fprintf(stderr, "\t-d\tgenerate device tree snippet instead of C code\n");
 		fprintf(stderr, "\t-s\tgenerate standalone key outside FIT image keyring\n");
 		exit(1);
 	}
 
+	keycount = argc - optind;
+	keylist = calloc(sizeof(struct keyinfo), keycount);
+
+	for (i = 0; i < keycount; i++) {
+		const char *keyspec = argv[optind + i];
+		struct keyinfo *info = &keylist[i];
+
+		if (!keyspec)
+			exit(1);
+
+		if (!strncmp(keyspec, "pkcs11:", 7)) { // legacy format of pkcs11 URI
+			info->path = strdup(keyspec);
+		} else {
+			if (!get_name_path(keyspec, info)) {
+				fprintf(stderr, "invalid keyspec %i: %s\n", optind, keyspec);
+				exit(1);
+			}
+		}
+	}
+
 	if (dts) {
 		fprintf(outfilep, "/dts-v1/;\n");
 		fprintf(outfilep, "/ {\n");
@@ -795,32 +900,24 @@ int main(int argc, char *argv[])
 		fprintf(outfilep, "#include <crypto/rsa.h>\n");
 	}
 
-	for (i = optind; i < argc; i++) {
-		const char *keyspec = argv[i];
-		char *keyname = NULL;
-		char *path = NULL;
-
-		keyspec = try_resolve_env(keyspec);
-		if (!keyspec)
-			exit(1);
 
-		if (!strncmp(keyspec, "pkcs11:", 7))
-			path = strdup(keyspec);
-		else
-			get_name_path(keyspec, &keyname, &path);
+	for (i = 0; i < keycount; i++) {
+		struct keyinfo *info = &keylist[i];
 
-		if (!keyname) {
-			ret = asprintf(&keyname, "key_%d", keynum++);
+		if (!info->keyname) {
+			ret = asprintf(&info->keyname, "key_%d", keynum++);
 			if (ret < 0)
 				enomem_exit("asprintf");
 		}
 
-		ret = gen_key(keyname, path);
+		if (!info->keyring) {
+			info->keyring = strdup("fit");
+			fprintf(stderr, "Warning: No keyring provided in keyspec, defaulting to keyring=fit for %s\n", argv[optind + i]);
+		}
+
+		ret = gen_key(info);
 		if (ret)
 			exit(1);
-
-		free(keyname);
-		free(path);
 	}
 
 	if (dts) {
@@ -828,5 +925,6 @@ int main(int argc, char *argv[])
 		fprintf(outfilep, "};\n");
 	}
 
+	/* all keyinfo fields freed on exit */
 	exit(0);
 }

-- 
2.51.0.297.gca2559c1d6

[PATCH 03/15] fit: only accept keys from "fit"-keyring

[Jonas Rebmann]

Separate keys shall be used for fitimage verification and the upcoming
TLV verification.

Based on the newly introduced keyring feature, limit fitimage
verification to the keys in the keyring literally named "fit", which is
also the current default keyring name in keytoc for backwards
compatibility.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 common/image-fit.c          | 13 ++++++++-----
 crypto/public-keys.c        | 13 ++++++++++---
 crypto/rsa.c                |  1 +
 include/crypto/public_key.h |  9 ++++++++-
 include/tlv/tlv.h           |  1 +
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 3017ccb504..0cbe8baf6f 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -261,7 +261,7 @@ static struct digest *fit_alloc_digest(struct device_node *sig_node,
 static int fit_check_signature(struct fit_handle *handle, struct device_node *sig_node,
 			       enum hash_algo algo, void *hash)
 {
-	const char *fail_reason = "no built-in keys";
+	const char *fail_reason;
 	const struct public_key *key;
 	const char *key_name = NULL;
 	int sig_len;
@@ -274,10 +274,13 @@ static int fit_check_signature(struct fit_handle *handle, struct device_node *si
 		return -EINVAL;
 	}
 
+	fail_reason = "no matching keys";
+
 	of_property_read_string(sig_node, "key-name-hint", &key_name);
 	if (key_name) {
-		key = public_key_get(key_name);
+		key = public_key_get(key_name, "fit");
 		if (key) {
+			fail_reason = "verification failed";
 			ret = public_key_verify(key, sig_value, sig_len, hash, algo);
 			if (handle->verbose)
 				pr_info("Key %*phN (%s) -> signature %s\n", key->hashlen,
@@ -287,13 +290,13 @@ static int fit_check_signature(struct fit_handle *handle, struct device_node *si
 		}
 	}
 
-	for_each_public_key(key) {
-		fail_reason = "verification failed";
+	for_each_public_key_keyring(key, "fit") {
 
 		/* Don't recheck with same key as before */
-		if (streq_ptr(key->key_name_hint, key_name))
+		if (key_name && streq_ptr(key->key_name_hint, key_name))
 			continue;
 
+		fail_reason = "verification failed";
 		ret = public_key_verify(key, sig_value, sig_len, hash, algo);
 
 		if (handle->verbose)
diff --git a/crypto/public-keys.c b/crypto/public-keys.c
index a870ec5438..adc1f51d17 100644
--- a/crypto/public-keys.c
+++ b/crypto/public-keys.c
@@ -17,11 +17,11 @@ const struct public_key *public_key_next(const struct public_key *prev)
 	return NULL;
 }
 
-const struct public_key *public_key_get(const char *name)
+const struct public_key *public_key_get(const char *name, const char *keyring)
 {
 	const struct public_key *key;
 
-	list_for_each_entry(key, &public_keys, list) {
+	for_each_public_key_keyring(key, keyring) {
 		if (!strcmp(key->key_name_hint, name))
 			return key;
 	}
@@ -31,8 +31,15 @@ const struct public_key *public_key_get(const char *name)
 
 int public_key_add(struct public_key *key)
 {
-	if (public_key_get(key->key_name_hint))
+	if (!key->keyring || *key->keyring == '\0') {
+		pr_warn("Aborting addition of public key: No keyring specified\n");
+		return -EINVAL;
+	}
+
+	if (public_key_get(key->key_name_hint, key->keyring)) {
+		pr_warn("Aborting addition of public key: Duplicate fit name hint\n");
 		return -EEXIST;
+	}
 
 	list_add_tail(&key->list, &public_keys);
 
diff --git a/crypto/rsa.c b/crypto/rsa.c
index 13b6553c95..24cec70d31 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -481,6 +481,7 @@ static void rsa_init_keys_of(void)
 			continue;
 		}
 
+		key->keyring = "fit";
 		ret = public_key_add(key);
 		if (ret)
 			pr_err("Cannot add rsa key %s: %pe\n",
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index c9dd38cc33..44ae09e4d0 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -2,6 +2,7 @@
 #define __CRYPTO_PUBLIC_KEY_H
 
 #include <digest.h>
+#include <string.h>
 
 struct rsa_public_key;
 struct ecdsa_public_key;
@@ -26,12 +27,18 @@ struct public_key {
 };
 
 int public_key_add(struct public_key *key);
-const struct public_key *public_key_get(const char *name);
+const struct public_key *public_key_get(const char *name, const char *keyring);
 const struct public_key *public_key_next(const struct public_key *prev);
 
 #define for_each_public_key(key) \
 		for (key = public_key_next(NULL); key; key = public_key_next(key))
 
+#define for_each_public_key_keyring(key, _keyring)                        \
+	for_each_public_key(key)                                          \
+		if (!key->keyring || strcmp(key->keyring, _keyring) != 0) \
+			continue;                                         \
+		else
+
 int public_key_verify(const struct public_key *key, const uint8_t *sig,
 		      const uint32_t sig_len, const uint8_t *hash,
 		      enum hash_algo algo);
diff --git a/include/tlv/tlv.h b/include/tlv/tlv.h
index 2a3fb14392..536f61646c 100644
--- a/include/tlv/tlv.h
+++ b/include/tlv/tlv.h
@@ -42,6 +42,7 @@ extern int tlv_handle_eth_address_seq(struct tlv_device *dev, struct tlv_mapping
 
 struct tlv_decoder {
 	u32 magic;
+	const char *signature_keyring;
 	void *driverata;
 	struct tlv_mapping **mappings;
 	struct driver driver;

-- 
2.51.0.297.gca2559c1d6

[PATCH 04/15] crypto: keytoc: Rename "hint" to "fit-hint" and do not use it in identifiers

[Jonas Rebmann]

The notion of name hints being appropriate as key identifiers is a
relict from when public key infrastructure was only for fit images.

Instead of mixing autogenerated key_<counter> and <hint> prefixes in key
identifiers, simply use key_<counter> for all keys.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/keytoc.c | 109 +++++++++++++++++++++++++------------------------------
 1 file changed, 49 insertions(+), 60 deletions(-)

diff --git a/scripts/keytoc.c b/scripts/keytoc.c
index a2b508bf49..a663cd20ae 100644
--- a/scripts/keytoc.c
+++ b/scripts/keytoc.c
@@ -29,9 +29,10 @@
 #include <ctype.h>
 
 struct keyinfo {
-	char *keyname;
+	char *name_hint;
 	char *keyring;
 	char *path;
+	char *name_c;
 };
 
 static int dts, standalone;
@@ -366,7 +367,7 @@ static int print_bignum(BIGNUM *num, int num_bits, int width)
 
 	arr = malloc(num_bits / width * sizeof(*arr));
 	if (!arr)
-		enomem_exit("malloc");
+		enomem_exit(__func__);
 
 	for (i = 0; i < num_bits / width; i++) {
 		BN_mod(tmp, num, big2_32, ctx); /* n = N mod B */
@@ -500,7 +501,7 @@ static int print_hash(EVP_PKEY *key)
 	return ret ? -EINVAL : 0;
 }
 
-static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *keyring, const char *key_name_c)
+static int gen_key_ecdsa(EVP_PKEY *key, struct keyinfo *info)
 {
 	char group[128];
 	size_t outlen;
@@ -530,7 +531,7 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *keyrin
 		fprintf(stderr, "ERROR: generating a dts snippet for ECDSA keys is not yet supported\n");
 		return -EOPNOTSUPP;
 	} else {
-		fprintf(outfilep, "\nstatic unsigned char %s_hash[] = {\n\t", key_name_c);
+		fprintf(outfilep, "\nstatic unsigned char %s_hash[] = {\n\t", info->name_c);
 
 		ret = print_hash(key);
 		if (ret)
@@ -538,37 +539,37 @@ static int gen_key_ecdsa(EVP_PKEY *key, const char *key_name, const char *keyrin
 
 		fprintf(outfilep, "\n};\n\n");
 
-		fprintf(outfilep, "\nstatic uint64_t %s_x[] = {", key_name_c);
+		fprintf(outfilep, "\nstatic uint64_t %s_x[] = {", info->name_c);
 		ret = print_bignum(key_x, bits, 64);
 		if (ret)
 			return ret;
 
 		fprintf(outfilep, "\n};\n\n");
 
-		fprintf(outfilep, "static uint64_t %s_y[] = {", key_name_c);
+		fprintf(outfilep, "static uint64_t %s_y[] = {", info->name_c);
 		ret = print_bignum(key_y, bits, 64);
 		if (ret)
 			return ret;
 
 		fprintf(outfilep, "\n};\n\n");
 
-		fprintf(outfilep, "static struct ecdsa_public_key %s = {\n", key_name_c);
+		fprintf(outfilep, "static struct ecdsa_public_key %s = {\n", info->name_c);
 
 		fprintf(outfilep, "\t.curve_name = \"%s\",\n", group);
-		fprintf(outfilep, "\t.x = %s_x,\n", key_name_c);
-		fprintf(outfilep, "\t.y = %s_y,\n", key_name_c);
+		fprintf(outfilep, "\t.x = %s_x,\n", info->name_c);
+		fprintf(outfilep, "\t.y = %s_y,\n", info->name_c);
 		fprintf(outfilep, "};\n");
 		if (!standalone) {
-			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", key_name_c);
+			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", info->name_c);
 			fprintf(outfilep, "\t.type = PUBLIC_KEY_TYPE_ECDSA,\n");
-			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
-			fprintf(outfilep, "\t.keyring = \"%s\",\n", keyring);
-			fprintf(outfilep, "\t.hash = %s_hash,\n", key_name_c);
+			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", info->name_hint);
+			fprintf(outfilep, "\t.keyring = \"%s\",\n", info->keyring);
+			fprintf(outfilep, "\t.hash = %s_hash,\n", info->name_c);
 			fprintf(outfilep, "\t.hashlen = %u,\n", SHA256_DIGEST_LENGTH);
-			fprintf(outfilep, "\t.ecdsa = &%s,\n", key_name_c);
+			fprintf(outfilep, "\t.ecdsa = &%s,\n", info->name_c);
 			fprintf(outfilep, "};\n");
 			fprintf(outfilep, "\n");
-			fprintf(outfilep, "const struct public_key *__%s_public_key __ll_elem(.public_keys.rodata.%s) = &%s_public_key;\n", key_name_c, key_name_c, key_name_c);
+			fprintf(outfilep, "const struct public_key *__%s_public_key __ll_elem(.public_keys.rodata.%s) = &%s_public_key;\n", info->name_c, info->name_c, info->name_c);
 		}
 	}
 
@@ -579,7 +580,7 @@ static char *try_resolve_env(char *input)
 {
 	char *var;
 
-	if (strncmp(input, "__ENV__", 7))
+	if (!input || strncmp(input, "__ENV__", 7))
 		return input;
 
 	var = getenv(input + 7);
@@ -593,7 +594,7 @@ static char *try_resolve_env(char *input)
 	return var;
 }
 
-static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *keyring, const char *key_name_c)
+static int gen_key_rsa(EVP_PKEY *key, struct keyinfo *info)
 {
 	BIGNUM *modulus, *r_squared;
 	uint64_t exponent = 0;
@@ -608,7 +609,7 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *keyring,
 	bits = BN_num_bits(modulus);
 
 	if (dts) {
-		fprintf(outfilep, "\t\tkey-%s {\n", key_name_c);
+		fprintf(outfilep, "\t\tkey-%s {\n", info->name_c);
 		fprintf(outfilep, "\t\t\trsa,r-squared = <");
 		ret = print_bignum(r_squared, bits, 32);
 		if (ret)
@@ -626,10 +627,10 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *keyring,
 			exponent & 0xffffffff);
 		fprintf(outfilep, "\t\t\trsa,n0-inverse = <0x%0x>;\n", n0_inv);
 		fprintf(outfilep, "\t\t\trsa,num-bits = <0x%0x>;\n", bits);
-		fprintf(outfilep, "\t\t\tkey-name-hint = \"%s\";\n", key_name_c);
+		fprintf(outfilep, "\t\t\tkey-name-hint = \"%s\";\n", info->name_c);
 		fprintf(outfilep, "\t\t};\n");
 	} else {
-		fprintf(outfilep, "\nstatic unsigned char %s_hash[] = {\n\t", key_name_c);
+		fprintf(outfilep, "\nstatic unsigned char %s_hash[] = {\n\t", info->name_c);
 
 		ret = print_hash(key);
 		if (ret)
@@ -637,14 +638,14 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *keyring,
 
 		fprintf(outfilep, "\n};\n\n");
 
-		fprintf(outfilep, "\nstatic uint32_t %s_modulus[] = {", key_name_c);
+		fprintf(outfilep, "\nstatic uint32_t %s_modulus[] = {", info->name_c);
 		ret = print_bignum(modulus, bits, 32);
 		if (ret)
 			return ret;
 
 		fprintf(outfilep, "\n};\n\n");
 
-		fprintf(outfilep, "static uint32_t %s_rr[] = {", key_name_c);
+		fprintf(outfilep, "static uint32_t %s_rr[] = {", info->name_c);
 		ret = print_bignum(r_squared, bits, 32);
 		if (ret)
 			return ret;
@@ -652,30 +653,30 @@ static int gen_key_rsa(EVP_PKEY *key, const char *key_name, const char *keyring,
 		fprintf(outfilep, "\n};\n\n");
 
 		if (standalone) {
-			fprintf(outfilep, "struct rsa_public_key __key_%s;\n", key_name_c);
-			fprintf(outfilep, "struct rsa_public_key __key_%s = {\n", key_name_c);
+			fprintf(outfilep, "struct rsa_public_key __key_%s;\n", info->name_c);
+			fprintf(outfilep, "struct rsa_public_key __key_%s = {\n", info->name_c);
 		} else {
-			fprintf(outfilep, "static struct rsa_public_key %s = {\n", key_name_c);
+			fprintf(outfilep, "static struct rsa_public_key %s = {\n", info->name_c);
 		}
 
 		fprintf(outfilep, "\t.len = %d,\n", bits / 32);
 		fprintf(outfilep, "\t.n0inv = 0x%0x,\n", n0_inv);
-		fprintf(outfilep, "\t.modulus = %s_modulus,\n", key_name_c);
-		fprintf(outfilep, "\t.rr = %s_rr,\n", key_name_c);
+		fprintf(outfilep, "\t.modulus = %s_modulus,\n", info->name_c);
+		fprintf(outfilep, "\t.rr = %s_rr,\n", info->name_c);
 		fprintf(outfilep, "\t.exponent = 0x%0lx,\n", exponent);
 		fprintf(outfilep, "};\n");
 
 		if (!standalone) {
-			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", key_name_c);
+			fprintf(outfilep, "\nstatic struct public_key %s_public_key = {\n", info->name_c);
 			fprintf(outfilep, "\t.type = PUBLIC_KEY_TYPE_RSA,\n");
-			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", key_name);
-			fprintf(outfilep, "\t.keyring = \"%s\",\n", keyring);
-			fprintf(outfilep, "\t.hash = %s_hash,\n", key_name_c);
+			fprintf(outfilep, "\t.key_name_hint = \"%s\",\n", info->name_hint);
+			fprintf(outfilep, "\t.keyring = \"%s\",\n", info->keyring);
+			fprintf(outfilep, "\t.hash = %s_hash,\n", info->name_c);
 			fprintf(outfilep, "\t.hashlen = %u,\n", SHA256_DIGEST_LENGTH);
-			fprintf(outfilep, "\t.rsa = &%s,\n", key_name_c);
+			fprintf(outfilep, "\t.rsa = &%s,\n", info->name_c);
 			fprintf(outfilep, "};\n");
 			fprintf(outfilep, "\n");
-			fprintf(outfilep, "const struct public_key *__%s_public_key __ll_elem(.public_keys.rodata.%s) = &%s_public_key;\n", key_name_c, key_name_c, key_name_c);
+			fprintf(outfilep, "const struct public_key *__%s_public_key __ll_elem(.public_keys.rodata.%s) = &%s_public_key;\n", info->name_c, info->name_c, info->name_c);
 		}
 	}
 
@@ -686,23 +687,7 @@ static int gen_key(struct keyinfo *info)
 {
 	int ret;
 	EVP_PKEY *key;
-	char *tmp, *key_name_c;
-
-	/* key name handling */
-	info->keyname = try_resolve_env(info->keyname);
-	if (!info->keyname)
-		exit(1);
 
-	tmp = key_name_c = strdup(info->keyname);
-
-	while (*tmp) {
-		if (*tmp == '-')
-			*tmp = '_';
-		tmp++;
-	}
-
-	/* path/URI handling */
-	info->path = try_resolve_env(info->path);
 	if (!info->path)
 		exit(1);
 
@@ -717,12 +702,12 @@ static int gen_key(struct keyinfo *info)
 	}
 
 	/* generate built-in keys */
-	ret = gen_key_ecdsa(key, info->keyname, info->keyring, key_name_c);
+	ret = gen_key_ecdsa(key, info);
 	if (ret == -EOPNOTSUPP)
 		return ret;
 
 	if (ret)
-		ret = gen_key_rsa(key, info->keyname, info->keyring, key_name_c);
+		ret = gen_key_rsa(key, info);
 
 	return ret;
 }
@@ -756,7 +741,7 @@ static bool parse_info(char *p, struct keyinfo *out)
 		return false;
 
 	if (*p == '\0') {
-		out->keyname = strdup(k);
+		out->name_hint = strdup(k);
 		if (!k)
 			enomem_exit(__func__);
 		return true; /* legacy syntax */
@@ -782,8 +767,8 @@ static bool parse_info(char *p, struct keyinfo *out)
 				enomem_exit(__func__);
 			if (strcmp(k, "keyring") == 0)
 				out->keyring = strdup(v);
-			else if (strcmp(k, "hint") == 0)
-				out->keyname = strdup(v);
+			else if (strcmp(k, "fit-hint") == 0)
+				out->name_hint = strdup(v);
 			else
 				return false;
 
@@ -831,7 +816,6 @@ int main(int argc, char *argv[])
 {
 	int i, opt, ret;
 	char *outfile = NULL;
-	int keynum = 1;
 	int keycount;
 	struct keyinfo *keylist;
 
@@ -904,11 +888,16 @@ int main(int argc, char *argv[])
 	for (i = 0; i < keycount; i++) {
 		struct keyinfo *info = &keylist[i];
 
-		if (!info->keyname) {
-			ret = asprintf(&info->keyname, "key_%d", keynum++);
-			if (ret < 0)
-				enomem_exit("asprintf");
-		}
+		/* resolve __ENV__ for name_hint and path */
+		info->name_hint = try_resolve_env(info->name_hint);
+		info->path = try_resolve_env(info->path);
+
+		if (asprintf(&info->name_c, "key_%i", i + 1) < 0)
+			enomem_exit("asprintf");
+
+		/* unfortunately, the fit name hint is mandatory in the barebox codebase */
+		if (!info->name_hint)
+			info->name_hint = info->name_c;
 
 		if (!info->keyring) {
 			info->keyring = strdup("fit");

-- 
2.51.0.297.gca2559c1d6

[PATCH 05/15] commands: keys: update output format to include keyring

[Jonas Rebmann]

Also show the algorithm used.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 commands/keys.c             |  8 ++------
 include/crypto/public_key.h | 12 ++++++++++++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/commands/keys.c b/commands/keys.c
index 2d85e8124f..e1e87f3e9e 100644
--- a/commands/keys.c
+++ b/commands/keys.c
@@ -7,12 +7,8 @@ static int do_keys(int argc, char *argv[])
 	const struct public_key *key;
 
 	for_each_public_key(key) {
-		printf("KEY: %*phN", key->hashlen, key->hash);
-
-		if (key->key_name_hint)
-			printf(" (%s)\n", key->key_name_hint);
-		else
-			printf("\n");
+		printf("KEY: %*phN\tTYPE: %s\tKEYRING: %s\tHINT: %s\n", key->hashlen,
+		       key->hash, public_key_type_string(key->type), key->keyring, key->key_name_hint);
 	}
 
 	return 0;
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 44ae09e4d0..269b878a04 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -12,6 +12,18 @@ enum public_key_type {
 	PUBLIC_KEY_TYPE_ECDSA,
 };
 
+static inline const char *public_key_type_string(enum public_key_type type)
+{
+	switch (type) {
+	case PUBLIC_KEY_TYPE_RSA:
+		return "RSA";
+	case PUBLIC_KEY_TYPE_ECDSA:
+		return "ECDSA";
+	default:
+		return "unknown";
+	}
+}
+
 struct public_key {
 	enum public_key_type type;
 	struct list_head list;

-- 
2.51.0.297.gca2559c1d6

[PATCH 06/15] commands: tlv: Error out on invalid TLVs

[Jonas Rebmann]

TLV parsing happens in the TLV drivers probe function
(tlv_probe_from_magic()) but register_device() does not propagate
errors from match(), always returning zero.

match() however ensures that dev->driver is always NULL if an error
occurred on probe and since we're only probing the TLV driver, rely on
that as indicator of an error during TLV parsing (e.g. CRC mismatch).

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 commands/tlv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/tlv.c b/commands/tlv.c
index 6118a2947f..c01a7b0399 100644
--- a/commands/tlv.c
+++ b/commands/tlv.c
@@ -29,7 +29,7 @@ static int do_tlv(int argc, char *argv[])
 		return COMMAND_ERROR_USAGE;
 
 	tlvdev = tlv_register_device_by_path(filename, NULL);
-	if (IS_ERR(tlvdev)) {
+	if (IS_ERR(tlvdev) || tlvdev->dev.driver == NULL) {
 		printf("Could not open \"%s\": %m\n", filename);
 		return COMMAND_ERROR;
 	}

-- 
2.51.0.297.gca2559c1d6

[PATCH 07/15] scripts: bareboxtlv-generator: Implement signature

[Jonas Rebmann]

Implement signature and verification for RSA and ECDSA with PKCS1 v1.5
padding and SHA256 hashing based on previous stubs.

Sign the header (consisting of magic, length of the TLV list in bytes,
and a 0 as placeholder for the length of the signature in bytes), and
the TLVs. Store the signature between TLVs and CRC. Include the
signature in the CRC.

The pyca cryptography module and the OpenSSL CLI is required if the
signature features are to be used.

pyca/cryptography does not support PKCS#11 tokens, and no other python
module currently supports this transparently which is why the private
key is accessed via the OpenSSL CLI. This way, any OpenSSL supported
provider can be used e.g. to pass pkcs11-URI's (once pkcs11-provider is
correctly configured for OpenSSL).

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 .../bareboxtlv-generator/bareboxtlv-generator.py   | 242 +++++++++++++++++++--
 scripts/bareboxtlv-generator/requirements.txt      |   1 +
 2 files changed, 225 insertions(+), 18 deletions(-)

diff --git a/scripts/bareboxtlv-generator/bareboxtlv-generator.py b/scripts/bareboxtlv-generator/bareboxtlv-generator.py
index 6174fe91cd..5613f5f0ed 100755
--- a/scripts/bareboxtlv-generator/bareboxtlv-generator.py
+++ b/scripts/bareboxtlv-generator/bareboxtlv-generator.py
@@ -1,7 +1,11 @@
 #!/usr/bin/env python3
 
 import argparse
+import os
+import hashlib
+import shutil
 import struct
+import subprocess
 import sys
 
 import yaml
@@ -14,11 +18,178 @@ except ModuleNotFoundError:
 
 _crc32_mpeg = mkPredefinedCrcFun("crc-32-mpeg")
 
+MAGIC_LENGTH = 12
+SPKI_LENGTH = 4
+
+def openssl(args, stdin: bytes = None) -> bytes:
+    """
+    Invoke the OpenSSL CLI with the given arguments
+
+    Parameters:
+        args: List of arguments for the openssl command (excluding 'openssl' itself)
+        stdin: Input bytes to pass to the command's stdin
+
+    Returns:
+        bytes: stdout of the command
+    """
+    cmd = ["openssl"] + args
+
+    result = subprocess.run(
+        cmd,
+        input=stdin,
+        stdout=subprocess.PIPE,
+        check=True
+    )
+
+    return result.stdout
 
 class MaxSizeReachedException(Exception):
     pass
 
 
+class PrivateKey:
+    """
+    A private key for signing TLVs, requires the cryptography module
+    """
+
+    def __init__(self, path: str | None = None):
+        """
+        Load a private key from:
+        - PKCS#12 (.p12/.pfx)
+        - PEM/DER private key file
+        """
+
+        try:
+            from cryptography.hazmat.primitives import serialization
+        except ModuleNotFoundError:
+            print("Error: missing pyca/cryptography dependency", file=sys.stderr)
+            sys.exit(127)
+
+        if shutil.which("openssl") is None:
+            print("The `openssl` binary is required for cryptographic operations but wasn't found in PATH!")
+            sys.exit(127)
+
+        self.inkey = path
+        self.public_key = serialization.load_pem_public_key(openssl(["pkey", "-pubout", "-in", self.inkey]));
+
+    def sign(self, message: bytes) -> bytes:
+        """
+        Sign message with RSA, or ECDSA automatically based on key type.
+        """
+
+        from cryptography.hazmat.primitives.asymmetric import rsa, ec
+        from cryptography.hazmat.primitives.asymmetric.utils import decode_dss_signature
+
+        # Access private keys only via the openssl cli so that any configured provider, such as pkcs11, can be used.
+        sig = openssl(["pkeyutl", "-sign", "-rawin", "-digest", "sha256", "-inkey", self.inkey], stdin = message)
+
+        if isinstance(self.public_key, rsa.RSAPublicKey):
+            return sig
+        elif isinstance(self.public_key, ec.EllipticCurvePublicKey):
+            r, s = decode_dss_signature(sig)
+            key_bits = self.public_key.curve.key_size
+            assert key_bits % 8 == 0
+            key_bytes = key_bits // 8
+            sig  = r.to_bytes(key_bytes, byteorder="big")
+            sig += s.to_bytes(key_bytes, byteorder="big")
+            return sig
+        else:
+            raise TypeError("Unsupported key type")
+
+    def spki_hash(self) -> bytes:
+        """
+        Four bytes of SHA256 digest of the derived public key's SubjectPublicKeyInfo.
+        Used for faster identification of the key to be used for decryption.
+        """
+        pub = PublicKey(pubkey = self.public_key)
+        return pub.spki_hash()
+
+
+class PublicKey:
+    """
+    A public key for validating TLVs signatures, requires the cryptography module
+    """
+
+    def __init__(self, path: str | None = None, pubkey: bytes | None = None):
+        """
+        Load a private key from:
+        - PKCS#12 (.p12/.pfx)
+        - PEM/DER public key file
+        - existing object
+        """
+        try:
+            from cryptography.hazmat.primitives import serialization
+            from cryptography import x509
+        except ModuleNotFoundError:
+            print("Error: missing pyca/cryptography dependency", file=sys.stderr)
+            sys.exit(127)
+
+        if pubkey is not None:
+            assert path is None
+            self.pubkey = pubkey
+        else:
+            with open(path, "rb") as f:
+                data = f.read()
+
+            if path.endswith((".p12", ".pfx")):
+                privatekey, cert, _ = serialization.pkcs12.load_key_and_certificates(data, password=None)
+                self.pubkey = cert.public_key()
+
+            else:
+                try:
+                    self.pubkey = serialization.load_pem_public_key(data)
+                except ValueError:
+                    try:
+                        self.pubkey = serialization.load_der_public_key(data)
+                    except ValueError:
+                        try:
+                            self.pubkey = x509.load_pem_x509_certificate(data).public_key()
+                        except ValueError:
+                            self.pubkey = serialization.load_pem_public_key(openssl(["pkey", "-pubout", "-in", path]))
+
+    def verify(self, message: bytes, signature: bytes) -> bool:
+        """
+        Verify signature
+        """
+
+        from cryptography.hazmat.primitives import hashes
+        from cryptography.hazmat.primitives.asymmetric import rsa, ec, padding
+        from cryptography.hazmat.primitives.asymmetric.utils import encode_dss_signature
+        from cryptography import exceptions
+
+        try:
+            if isinstance(self.pubkey, rsa.RSAPublicKey):
+                self.pubkey.verify(signature, message, padding.PKCS1v15(), hashes.SHA256())
+            elif isinstance(self.pubkey, ec.EllipticCurvePublicKey):
+                key_bits = self.pubkey.curve.key_size
+                assert key_bits % 8 == 0
+                key_bytes = key_bits // 8
+                r = int.from_bytes(signature[:key_bytes], byteorder="big")
+                s = int.from_bytes(signature[key_bytes:], byteorder="big")
+
+                der_sig = encode_dss_signature(r, s)
+                self.pubkey.verify(der_sig, message, ec.ECDSA(hashes.SHA256()))
+            else:
+                raise TypeError("Unsupported key type")
+            return True
+        except exceptions.InvalidSignature:
+            return False
+
+    def spki_hash(self) -> bytes:
+        """
+        Four bytes of SHA256 digest of the public key's SubjectPublicKeyInfo.
+        Used for faster identification of the key to be used for decryption.
+        """
+
+        from cryptography.hazmat.primitives import serialization
+
+        spki = self.pubkey.public_bytes(
+            encoding=serialization.Encoding.DER,
+            format=serialization.PublicFormat.SubjectPublicKeyInfo,
+        )
+        return hashlib.sha256(spki).digest()[:SPKI_LENGTH]
+
+
 class FactoryDataset:
     """
     Generates TLV-encoded datasets that can be used with Barebox's
@@ -41,8 +212,9 @@ class FactoryDataset:
     # };                                                        #
     #############################################################
 
-    Limitations:
-    * Signing is currently not supported and length_sig is always 0x0.
+    Note:
+    * Signature is preceded with four bytes of pubkey checksum which is included in the length_sig field
+    * The length_sig field is set to zero for signage and must be zeroed before verification
     """
 
     def __init__(self, schema):
@@ -130,7 +302,7 @@ class FactoryDataset:
         """
         self.schema = schema
 
-    def encode(self, data):
+    def encode(self, data, sign: PrivateKey | None = None) -> bytes:
         """
         Generate an EEPROM image for the given data.
 
@@ -138,6 +310,7 @@ class FactoryDataset:
         """
         # build payload of TLVs
         tlvs = b""
+        signature = b""
 
         for name, value in data.items():
             if name not in self.schema["tags"]:
@@ -192,12 +365,20 @@ class FactoryDataset:
 
             tlvs += struct.pack(f">HH{fmt}", tag["tag"], struct.calcsize(fmt), bin)
 
+        sig_len = 0
+
         # Convert the framing data.
-        len_sig = 0x0  # Not implemented.
-        header = struct.pack(">3I", self.schema["magic"], len(tlvs), len_sig)
-        crc_raw = _crc32_mpeg(header + tlvs)
+        header = struct.pack(">3I", self.schema["magic"], len(tlvs), sig_len)
+
+        if sign:
+            # Sign with sig_len = 0, the actual signature length will not be signed!
+            signature = sign.spki_hash() + sign.sign(header + tlvs)
+            # Actual header now with length of the signature
+            header = struct.pack(">3I", self.schema["magic"], len(tlvs), len(signature))
+
+        crc_raw = _crc32_mpeg(header + tlvs + signature)
         crc = struct.pack(">I", crc_raw)
-        bin = header + tlvs + crc
+        bin = header + tlvs + signature + crc
 
         # Check length
         if "max_size" in self.schema and len(bin) > self.schema["max_size"]:
@@ -206,7 +387,7 @@ class FactoryDataset:
             )
         return bin
 
-    def decode(self, bin):
+    def decode(self, bin, pubkey: PublicKey | None = None):
         """
         Decode a TLV-encoded binary image.
 
@@ -218,17 +399,13 @@ class FactoryDataset:
         if len(bin) < 16:
             raise ValueError("Supplied binary is too small to be TLV-encoded data.")
 
-        bin_magic, bin_tlv_len, bin_sig_len = struct.unpack(">3I", bin[:12])
+        bin_magic, bin_tlv_len, bin_sig_len = struct.unpack(">3I", bin[:MAGIC_LENGTH])
         # check magic
         if bin_magic != self.schema["magic"]:
             raise ValueError(f'Magic missmatch. Is {hex(bin_magic)} but expected {hex(self.schema["magic"])}')
 
-        # check signature length
-        if bin_sig_len != 0:
-            raise ValueError("Signature check is not supported!")
-
         # check crc
-        crc_offset = 12 + bin_tlv_len + bin_sig_len
+        crc_offset = MAGIC_LENGTH + bin_tlv_len + bin_sig_len
         if crc_offset > len(bin) - 4:
             raise ValueError("crc location calculated behind binary.")
         bin_crc = struct.unpack(">I", bin[crc_offset : crc_offset + 4])[0]  # noqa E203
@@ -236,8 +413,26 @@ class FactoryDataset:
         if bin_crc != calc_crc:
             raise ValueError(f"CRC missmatch. Is {hex(bin_crc)} but expected {hex(calc_crc)}.")
 
-        ptr = 12
-        while ptr < crc_offset:
+        # check signature length
+        if bin_sig_len != 0 and pubkey is None:
+            print("WARNING: TLV contains a signature but signature verification not enabled via --verify!", file=sys.stderr)
+        elif bin_sig_len == 0 and pubkey is not None:
+            raise ValueError("TLV signature validation was requested but TLV is unsigned.")
+        elif pubkey is not None:
+            sig_offset = MAGIC_LENGTH + bin_tlv_len
+            bin_sig = bin[sig_offset + SPKI_LENGTH : sig_offset + bin_sig_len]
+            spki = bin[sig_offset : sig_offset + SPKI_LENGTH]
+            if spki != pubkey.spki_hash():
+                raise ValueError("TLV signature SPKI mismatch.")
+
+            # verify file excluding signature itself, and excluding signature length field
+            bin_verify = bytearray(bin[:sig_offset])
+            bin_verify[8:12] = struct.pack(">I", 0)
+            if not pubkey.verify(bin_verify, bin_sig):
+                raise ValueError("TLV signature validation failed.")
+
+        ptr = MAGIC_LENGTH
+        while ptr < MAGIC_LENGTH + bin_tlv_len:
             tag_id, tag_len = struct.unpack_from(">HH", bin, ptr)
             data_ptr = ptr + 4
             ptr += tag_len + 4
@@ -299,7 +494,9 @@ def _main():
     parser = argparse.ArgumentParser(description="Generate a TLV dataset for the Barebox TLV parser")
     parser.add_argument("schema", help="YAML file describing the data.")
     parser.add_argument("--input-data", help="YAML file containing data to write to the binary.")
+    parser.add_argument("--sign", help=" When using --input-data: Private key to sign the TLV with.")
     parser.add_argument("--output-data", help="YAML file where the contents of the binary will be written to.")
+    parser.add_argument("--verify", help="When using --output-data: Public key to verify the signature against")
     parser.add_argument("binary", help="Path to where export data to be copied into DUT's EEPROM.")
     args = parser.parse_args()
 
@@ -311,14 +508,23 @@ def _main():
     if args.input_data:
         with open(args.input_data) as d_fh:
             data = yaml.load(d_fh, Loader=yaml.SafeLoader)
-        bin = eeprom.encode(data)
+
+        if args.sign:
+            privkey = PrivateKey(path=args.sign)
+        else:
+            privkey = None
+        bin = eeprom.encode(data, sign=privkey)
         with open(args.binary, "wb+") as out_fh:
             out_fh.write(bin)
 
     if args.output_data:
         with open(args.binary, "rb") as in_fh:
             bin = in_fh.read()
-        data = eeprom.decode(bin)
+        if args.verify:
+            pubkey = PublicKey(path=args.verify)
+        else:
+            pubkey = None
+        data = eeprom.decode(bin, pubkey=pubkey)
         with open(args.output_data, "w+") as out_fh:
             yaml.dump(data, out_fh)
 
diff --git a/scripts/bareboxtlv-generator/requirements.txt b/scripts/bareboxtlv-generator/requirements.txt
index a1f7e3b3f2..9125d46b55 100644
--- a/scripts/bareboxtlv-generator/requirements.txt
+++ b/scripts/bareboxtlv-generator/requirements.txt
@@ -1,2 +1,3 @@
 crcmod
 pyaml
+cryptography[signature]

-- 
2.51.0.297.gca2559c1d6

[PATCH 08/15] scripts: bareboxtlv-generator: Increase max_size in example schema

[Jonas Rebmann]

This is to accommodate the rather large RSA signatures when using the
example schema to test the signature feature.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 scripts/bareboxtlv-generator/schema-example.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/bareboxtlv-generator/schema-example.yaml b/scripts/bareboxtlv-generator/schema-example.yaml
index b5362ddf58..34ee8b1def 100644
--- a/scripts/bareboxtlv-generator/schema-example.yaml
+++ b/scripts/bareboxtlv-generator/schema-example.yaml
@@ -1,5 +1,5 @@
 magic: 0x61bb95f2
-max_size: 0x100
+max_size: 0x400
 
 tags:
   factory-timestamp:

-- 
2.51.0.297.gca2559c1d6

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

[Jonas Rebmann]

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/Kconfig b/common/Kconfig
index d923d4c4b6..b7ac7094a6 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -1122,6 +1122,10 @@ config TLV
 	  barebox TLV is a scheme for storing factory data on non-volatile
 	  storage. Unlike state, it's meant to be read-only.
 
+config TLV_SIGNATURE
+	bool "barebox TLV signature support"
+	select CRYPTO_BUILTIN_KEYS
+
 config TLV_DRV
 	bool "barebox TLV generic driver"
 	depends on TLV
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
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include "tlv/format.h"
 #define pr_fmt(fmt) "barebox-tlv: " fmt
 
 #include <common.h>
@@ -9,6 +10,81 @@
 #include <linux/stat.h>
 #include <crc.h>
 #include <net.h>
+#include <crypto/public_key.h>
+
+static int tlv_verify_try_key(const struct public_key *key, const uint8_t *sig,
+			      const uint32_t sig_len, const void *data,
+			      unsigned long data_len)
+{
+	enum hash_algo algo = HASH_ALGO_SHA256;
+	int ret;
+	struct digest *digest = digest_alloc_by_algo(algo);
+	void *hash;
+
+	if (!digest)
+		return -ENOMEM;
+
+	digest_init(digest);
+	if (IS_ERR(digest)) {
+		digest_free(digest);
+		return -EINVAL;
+	}
+	digest_update(digest, data, data_len);
+	hash = xzalloc(digest_length(digest));
+	digest_final(digest, hash);
+
+	ret = public_key_verify(key, sig, sig_len, hash, algo);
+
+	digest_free(digest);
+	free(hash);
+
+	if (ret)
+		return -ENOKEY;
+	return 0;
+}
+
+static int tlv_verify(struct tlv_header *header, const char *keyring)
+{
+	const struct public_key *key;
+	size_t payload_sz = tlv_spki_hash_offset(header);
+	void *spki_tlv_ptr = (void *)header + payload_sz;
+	u32 spki_tlv = get_unaligned_le32(spki_tlv_ptr);
+	const int SPKI_LEN = 4;
+	u32 sig_len = get_unaligned_be16(&header->length_sig);
+	int ret;
+	int count_spki_matches = 0;
+
+	if (!IS_ENABLED(CONFIG_TLV_SIGNATURE)) {
+		pr_err("TLV signature selected in decoder but not enabled!\n");
+		return -ENOSYS;
+	} else if (sig_len == 0) {
+		pr_err("TLV signature selected in decoder but an unsigned TLV matched by magic %08x!\n", be32_to_cpu(header->magic));
+		return -EPROTO;
+	}
+	/* signature length field must always be zeroed during signage and verification */
+	header->length_sig = 0;
+
+	for_each_public_key_keyring(key, keyring) {
+		u32 spki_key = get_unaligned_le32(key->hash);
+
+		if (spki_key == spki_tlv) {
+			count_spki_matches++;
+			ret = tlv_verify_try_key(key, spki_tlv_ptr + SPKI_LEN, sig_len - SPKI_LEN, header, payload_sz);
+			if (!ret)
+				return 0;
+			pr_warn("TLV spki %08x matched available key but signature verification failed: %s!\n", spki_tlv, strerror(-ret));
+		}
+	}
+
+	/* reset signature length field after verification to avoid later confusion */
+	put_unaligned_be16(sig_len, &header->length_sig);
+
+	if (!count_spki_matches) {
+		pr_warn("TLV spki %08x matched no key!\n", spki_tlv);
+		return -ENOKEY;
+	}
+	return -EINVAL;
+}
 
 int tlv_parse(struct tlv_device *tlvdev,
 	      const struct tlv_decoder *decoder)
@@ -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;
+	}
+
 	for_each_tlv(header, tlv) {
 		struct tlv_mapping **mappings;
 		u16 tag = TLV_TAG(tlv);
diff --git a/include/tlv/format.h b/include/tlv/format.h
index c4521c3131..cbe0a132b1 100644
--- a/include/tlv/format.h
+++ b/include/tlv/format.h
@@ -47,11 +47,12 @@ struct tlv {
 struct tlv_header {
 	__be32 magic;
 	__be32 length_tlv; /* in bytes */
-	__be32 length_sig; /* in bytes */
+	__be16 reserved;
+	__be16 length_sig; /* in bytes */
 	struct tlv tlvs[];
-	/* __be32 crc; */
 	/* u8 sig[]; */
-};
+	/* __be32 crc; */
+} __packed;
 static_assert(sizeof(struct tlv_header) == 3 * 4);
 
 #define for_each_tlv(tlv_head, tlv) \
@@ -62,8 +63,19 @@ static inline size_t tlv_total_len(const struct tlv_header *header)
 	size_t ret;
 
 	ret = size_add(sizeof(struct tlv_header), get_unaligned_be32(&header->length_tlv));
-	ret = size_add(ret, sizeof(__be32)); /* CRC appended after TLVs */
-	ret = size_add(ret, get_unaligned_be32(&header->length_sig)); /* optional signature appended after CRC */
+	ret = size_add(ret, get_unaligned_be16(&header->length_sig)); /* optional signature appended after TLVs */
+	ret = size_add(ret, sizeof(__be32)); /* CRC at end of file */
+
+	return ret; /* SIZE_MAX on overflow */
+}
+
+/*
+ * Retrieve length of header+TLVs (offset of spki hash part of signature if available)
+ */
+
+static inline size_t tlv_spki_hash_offset(const struct tlv_header *header)
+{
+	size_t ret = size_add(sizeof(struct tlv_header), get_unaligned_be32(&header->length_tlv));
 
 	return ret; /* SIZE_MAX on overflow */
 }

-- 
2.51.0.297.gca2559c1d6

[PATCH 10/15] common: tlv: default decoder for signed TLV

[Jonas Rebmann]

Introduce a second default encoder that behaves just like barebox_tlv_v1
but uses the "tlv" keyring.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 .../devicetree/bindings/nvmem/barebox,tlv.yaml     |  1 +
 common/tlv/barebox.c                               | 25 +++++++++++++++++++++-
 include/tlv/format.h                               |  1 +
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml b/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml
index 7a77b9a0b4..7d678ee04a 100644
--- a/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml
+++ b/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml
@@ -22,6 +22,7 @@ properties:
     items:
       - enum:
         - barebox,tlv-v1        # magic: 0x61bb95f2
+        - barebox,tlv-v1-signed # magic: 0x61bb95f3
         - lxa,tlv-baseboard-v1  # magic: 0xbc288dfe
         - lxa,tlv-powerboard-v1 # magic: 0xdca5a870
       - const: barebox,tlv
diff --git a/common/tlv/barebox.c b/common/tlv/barebox.c
index b941463b31..d4970e6ebf 100644
--- a/common/tlv/barebox.c
+++ b/common/tlv/barebox.c
@@ -177,16 +177,39 @@ static struct of_device_id of_matches[] = {
 	{ .compatible = "barebox,tlv-v1" },
 	{ /* sentinel */}
 };
+static struct of_device_id of_matches_signed[] = {
+	{ .compatible = "barebox,tlv-v1-signed" },
+	{ /* sentinel */ }
+};
 
 static struct tlv_decoder barebox_tlv_v1 = {
 	.magic = TLV_MAGIC_BAREBOX_V1,
 	.driver.name = "barebox-tlv-v1",
 	.driver.of_compatible = of_matches,
 	.mappings = mappings,
+	.signature_keyring = NULL,
+};
+
+static struct tlv_decoder barebox_tlv_v1_signed = {
+	.magic = TLV_MAGIC_BAREBOX_V1_SIGNED,
+	.driver.name = "barebox-tlv-v1-signed",
+	.driver.of_compatible = of_matches_signed,
+	.mappings = mappings,
+	.signature_keyring = "tlv",
 };
 
 static int tlv_register_default(void)
 {
-	return tlv_register_decoder(&barebox_tlv_v1);
+	int err;
+
+	err = tlv_register_decoder(&barebox_tlv_v1);
+	if (err)
+		return err;
+	if (IS_ENABLED(CONFIG_TLV_SIGNATURE)) {
+		err = tlv_register_decoder(&barebox_tlv_v1_signed);
+		if (err)
+			return err;
+	}
+	return 0;
 }
 device_initcall(tlv_register_default);
diff --git a/include/tlv/format.h b/include/tlv/format.h
index cbe0a132b1..ab12f0c9bb 100644
--- a/include/tlv/format.h
+++ b/include/tlv/format.h
@@ -23,6 +23,7 @@
 #include <linux/build_bug.h>
 
 #define TLV_MAGIC_BAREBOX_V1		0x61bb95f2
+#define TLV_MAGIC_BAREBOX_V1_SIGNED	0x61bb95f3
 #define TLV_MAGIC_LXA_BASEBOARD_V1	0xbc288dfe
 #define TLV_MAGIC_LXA_POWERBOARD_V1	0xc6895c21
 #define TLV_MAGIC_LXA_IOBOARD_V1	0xdca5a870

-- 
2.51.0.297.gca2559c1d6

[PATCH 11/15] crypto: Use "development" keys for "fit" and "tlv" keyring

[Jonas Rebmann]

All users of the CONFIG_CRYPTO_PUBLIC_KEYS feature should update to the
new syntax making keyring selection mandatory.

Instead of just making the addition of the builtin snakeoil keys
explicit for the "fit" key, also add them to the "tlv" key to use them
as a testing set for TLV keys too.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 crypto/Makefile | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/crypto/Makefile b/crypto/Makefile
index 08b9a46e4c..076ba4f686 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -33,10 +33,12 @@ CONFIG_CRYPTO_PUBLIC_KEYS := $(foreach d,$(CONFIG_CRYPTO_PUBLIC_KEYS),"$(d)")
 
 ifdef CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS
 ifdef CONFIG_CRYPTO_RSA
-CONFIG_CRYPTO_PUBLIC_KEYS += rsa-devel:$(srctree)/crypto/fit-4096-development.crt
+CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=rsa-devel:$(srctree)/crypto/fit-4096-development.crt
+CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-4096-development.crt
 endif
 ifdef CONFIG_CRYPTO_ECDSA
-CONFIG_CRYPTO_PUBLIC_KEYS += ecdsa-devel:$(srctree)/crypto/fit-ecdsa-development.crt
+CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=ecdsa-devel:$(srctree)/crypto/fit-ecdsa-development.crt
+CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-ecdsa-development.crt
 endif
 endif
 

-- 
2.51.0.297.gca2559c1d6

[PATCH 12/15] test: py: add signature to TLV integration tests

[Jonas Rebmann]

Add TLV signature to TLV integration tests:
 - Signed TLV using development RSA key
 - Modify payload and fix CRC for a "tampered" tlv
 - Include both cases in generator and tlv-command tests.

Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
testing. Consequentially add the matching private keys from the public
repository at [1].

[1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 crypto/fit-4096-development.key  |  51 ++++++++++
 crypto/fit-ecdsa-development.key |   5 +
 test/py/test_tlv.py              | 205 +++++++++++++++++++++++++++++++--------
 3 files changed, 219 insertions(+), 42 deletions(-)

diff --git a/crypto/fit-4096-development.key b/crypto/fit-4096-development.key
new file mode 100644
index 0000000000..526cdfc2b5
--- /dev/null
+++ b/crypto/fit-4096-development.key
@@ -0,0 +1,51 @@
+-----BEGIN RSA PRIVATE KEY-----
+MIIJKgIBAAKCAgEAyZHkijUfqoAvEELaxSLnjyqhTprEilnf7JqvSCMDUUXv2dEl
+k1r4RwiBowJp/4W3sOx4gASEHM2xlDWyPYZUR/1btZeVJvOIRPWfw8JoLT3tbbST
+OIw04Bk6MUh3LbgBtxbbKGGkFewq3Ob1XQOWcY3ZAzfLFuooPWJQ6X+IiczkrA0r
+GnwpuhHlb8tOdQZRjDevIVVvEkRjRiqrAw5pKTy/Mt/SJJ/yC7qJptIQskQ42y3R
+qHeCVmP6ZF6VV1scNmHr8+kRD19DhCos6DLWq2pCFPwnSmgM4T0FWcJsMiNta+rt
+Rq+kG7RjOOYbbqvuk3vkMrQTRQeAYfdnuOGimGQYxiVh9quOMVG6NJ2hylTa0S/E
+PKQUQvK9A8bnDul522XPmMVHOtLXVGtwKx9xQUx/2D7aoqlmVJSGQeAMNi0NEFhq
+buGdXuJ/2cKwtobClkz0WbbMlI4UBM7V4qP3JSkxiojRKhtNHtdE3KtF3ronRJaU
++yDggNobWLiJ4TtQ0OAC1REEJGq7s9k8ASi+6s+VX7yVtlGWMIjbBGAl8lqgXXsA
+prRrmtofaQgEPVvcSAIbuch/JqpHhs8vHBiWb/KZdOe/vMiYQE/d9/KY8rybZa3D
+hKvzN7X59ymOYLILHB73Xxi5bQA61DaeYE4KnPiJaEDrUccrlFjMBIwGrosCAwEA
+AQKCAgEApVkIIFdzommEMdKloxD+4nIV4GUU1GjlRzGcl5AhKIo2NndaW4ZEJADW
+VuGkEfeet4NDVcBen0IcaXeivtVyTZuHn2646zrajbbvV6Yhzvr9yQBXxAs/VJVd
+JxBKszY+MfKN1JJEB7ezcYIDxEktH/k8C2e5MRLj73a26NO1LVTmQDyNHyy7Deeg
+ThR4R4bnXh5PiwiKFHIE/YoCvn8TxMAQF6uCtoh+BSD/ydiH2bQc766mTYu7XyKk
+Q7FS0FXszq+E3pBRbkq3F7OBIviRIAwKKSyvDlpMNnfX68mQ95AYMm6ENXffJtrS
+ido4ppBjJJh8mRses4FzzukkLITq2qBoZQn+3XfR12+YbWKbCFIAXSu1b4tJetLC
+wYUp6EuGKCS2XK8OJXbY4M2D1t7bCVlRpptZBBiD7romkLYQ+jRwPbWhjUY6Ktw3
+ceUf9XJtNVKjHMp7n6C3gdxe2ivK02RYscT3brRq7TUjSzGjH1z/HUQSwr31+tXD
+dw2fkb2qQn2KUB5hjKcqU/Dxrqjorvf+kjGwXTtAD01Y4r8xRD3HK31zKAgrheN6
+15shoKRM4imKCD+fTBQjBBVZpTT8xNbo1m+y0joFEeDW0U5Lc3A/DhyTUsfHj4Im
+H02Cg4wiXXGyJ57fSyyNFKaa0EuLSdOl0zT1bXiy2TxiyTrsgAECggEBAPz+DcTO
+OvWtU/f+SyAfP86xd3bSnQzBXtoK2iI49uGAAkIXLU881V5sFz41UWJ6g1G0PjKy
+FWjxTCJytgso9TkISC42TOTE9VUqL4Y4KHhY93nnMAzKNLJC04onqmimDjn1I5Di
+DD3k3yCYPQEPInz84tdZyB+mRSncdsOL0Mpzhl5fjgGy+pi78K3/B4PUepR3n+Wl
+4JqKP4SeIL189+ChnSrgVsLzdvpOWJ2cu8DO9qGfz7F0ufJlehUILWPfhYlWUZ/c
+AUja5QWJEuSQHsKcOJSD4fpjuBWy3ASKlDy7BQSSoASibsl8FfQ/WmQBd3H8Y5/U
+20psjFuOa/02TusCggEBAMv3V4ccYieiUNkzi6WyyzlR9sULtAG4Cvi1HRw+o6mV
+VeNFNo8hw+g8f26URvuB06xXyuZepk+oMtEPiFfGYFFg4s22QJRGzqBfC5+8//Mf
+rcIsU88S75JCZjPDSxOFgzSDAG1gPfX4i8BZHgqaT749TewbeLc0ehvVrcLnXMwB
+3JpDNmiuNzA2pJAWbLezKazhW6XbpkTtHDqZTswDK+3AFBm7j/cqqeXojd93EbrV
+0ggyiNMx/O42DHVwZ+51HdJ+C7KDHR0wzgFMu24zyoymzZOaiKjm2rQi/B4mJ9Er
+oCaCfhVGo/Kq7Y5V7G+x4gag8oVQNCJh/lERrvgBduECggEAT5tpnbn/F3tY5rof
+zZXHsDRrkPoo7PCT9ixgA1DFbqOnEkDUwxAzW6jLj4mbeE9wru72e2FKF2GGQXiz
+C8PxleajP9daTsojII9LsQJOyb/E75jtp7ig6E7a3agpmRBXfalDbb2TeI5iH5GH
+8KNgiM/SWU0pCbx6GvgCbvm501qSt3N97c7xx8mrrDSJmtPrVnhl2g9eI4LJBeP0
+DWwbW5W/LNS2uFV/5Ldubvn4omz9clIlOoOuVzXTOnb+QWT+Uf7VZGYICXLHifxd
+84neBALAUwtEulNSg5FqZgttJcb7hzrUG2E5VzEyf07IFJvZiAaRGqQR9NM/PzgL
+hvvlzQKCAQEAi/wmy2kUiKUjHd79oexzA9UYKyacFW3twcHzx7XJ95Kxjriq+FMx
+NIuI3ijQCr+QukDK1Y7yT8tdjRQ+/Bb/dfqrzomeCuYJ3BE/VhOOCpucUp6/qmgR
+mm0N3crUFQLWCM08FtUt0UoTCCFht98uiZ9jgn9cO0i94aqmhhTqIG3KrOkiR3gC
+Eon+KZHqba1+FdPZZZy5oaamcCVV6jjnBlaEtSCAbx+N2WfhLxR2S6eCbfPY6jHt
+qMPZiyRpgERLAnNVrd/EtIsRZ9z06m6LPjsg7oPp9Rnz0hwMsth3DV0GnkeDJzED
+RoI/ZifcjNAmE2yU5iAkl9Bvjc44Kqg+oQKCAQEAvSi4W2kVUoIwlmBHGLge/Rng
+YyScmAoG4Cavy0Ie6AHPtHayHFdI/rAyiVFnKU5Xuj4qgB54dLa94bQrIu451wls
+3Jyy/J8WkcW9r/dZFMN6gMoZ0u+xt6KdYe2tQnyW/CG4svDyfckcW2VHdh2A3vqH
+xlGNmo/HaOeovxWNQkQGQeuXnIcrUvwaFTmGIxLdEO5TAQzLeWSrXldMtVBUAMaJ
+LClOqNIGRxMRYhZOPVnkedEQmJqgxvcrn8F/91mXQHVnQBOvsgyQDgtS3V0EIAOD
+rWePjgB8twJknHuab8qH/1z3cQ5QRxQ6lffcIoWgXS59QBBT+jIqMT2oKyGkPw==
+-----END RSA PRIVATE KEY-----
diff --git a/crypto/fit-ecdsa-development.key b/crypto/fit-ecdsa-development.key
new file mode 100644
index 0000000000..2b13c877a3
--- /dev/null
+++ b/crypto/fit-ecdsa-development.key
@@ -0,0 +1,5 @@
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEIEsUW5DEOhD1CYHCnPfDULwbRQO9Yjt2/xM5SoY2GUQtoAoGCCqGSM49
+AwEHoUQDQgAEowCa2OYfPdGRr1JpSYONOA3N2jwJjGbPbfG6uBzKg1VqOOk0a/Vf
+BfEbQev6X96HCd6zvvC2tjBgvICW8UB0TQ==
+-----END EC PRIVATE KEY-----
diff --git a/test/py/test_tlv.py b/test/py/test_tlv.py
index 79f9f9d01b..1200281dbc 100644
--- a/test/py/test_tlv.py
+++ b/test/py/test_tlv.py
@@ -1,6 +1,7 @@
 import os
 import re
 import subprocess
+import struct
 from pathlib import Path
 from .helper import skip_disabled
 
@@ -8,71 +9,191 @@ import pytest
 
 
 class _TLV_Testdata:
-    def generator(self, args, check=True):
+    def generator(self, args, expect_failure=False, input=None):
         cmd = [os.sys.executable, str(self.generator_py)] + args
-        res = subprocess.run(cmd, text=True)
+        res = subprocess.run(cmd, text=True, input=input, encoding="utf-8", capture_output=True)
         if res.returncode == 127:
             pytest.skip("test skipped due to missing host dependencies")
-
-        if check and res.returncode != 0:
-            raise RuntimeError(f"generator failed ({res.returncode}): {res.stdout}\n{res.stderr}")
+        if res.returncode == 0 and expect_failure:
+            raise RuntimeError(
+                f"`{' '.join(cmd)}` succeded unexpectedly:\n{res.stderr}\n{res.stdout}"
+            )
+        elif res.returncode != 0 and not expect_failure:
+            raise RuntimeError(
+                f"`{' '.join(cmd)}` failed unexpectedly with {res.returncode}:\n{res.stderr}\n{res.stdout}"
+            )
         return res
 
+    def overwrite_magic(self, new_magic):
+        with open(self.schema, "r", encoding="utf-8") as f:
+            patched_schema = "".join(
+                re.sub(r"^magic:\s*0x[a-fA-F0-9]{8}\s*$", f"magic: {new_magic}\n", line)
+                for line in f
+            )
+        return patched_schema
+
+    def tlv_gen(self, outfile, magic=None, sign=None):
+        param = ["--input-data", str(self.data)]
+        if sign:
+            param += ["--sign", str(sign)]
+        if magic:
+            param += ["/dev/stdin"]
+        else:
+            param += [str(self.schema)]
+        param += [str(outfile)]
+        ret = self.generator(param, input=self.overwrite_magic(magic) if magic else None)
+        assert outfile.exists(), f"TLV {outfile} not created from {' '.join(param)}"
+        return ret
+
+    def tlv_read(self, binfile, magic=None, verify=None, expect_failure=False):
+        param = ["--output-data", "/dev/null"]
+        if verify:
+            param += ["--verify", str(verify)]
+        if magic:
+            param += ["/dev/stdin"]
+        else:
+            param += [str(self.schema)]
+        param += [str(binfile)]
+        ret = self.generator(
+            param,
+            input=self.overwrite_magic(magic) if magic else None,
+            expect_failure=expect_failure,
+        )
+        return ret
+
+    def corrupt(self, fnin, fnout, fix_crc=False):
+        try:
+            from crcmod.predefined import mkPredefinedCrcFun
+        except ModuleNotFoundError:
+            pytest.skip("test skipped due to missing dependency python-crcmod")
+            return
+
+        _crc32_mpeg = mkPredefinedCrcFun("crc-32-mpeg")
+
+        with open(fnin, "r+b") as f:
+            data = bytearray(f.read())
+        data[0x20] ^= 1
+        if fix_crc:
+            crc_raw = _crc32_mpeg(data[:-4])
+            crc = struct.pack(">I", crc_raw)
+            data[-4:] = crc
+        with open(fnout, "wb") as f:
+            f.write(data)
+
     def __init__(self, testfs):
         self.dir = Path(testfs)
         self.scripts_dir = Path("scripts/bareboxtlv-generator")
         self.data = self.scripts_dir / "data-example.yaml"
         self.schema = self.scripts_dir / "schema-example.yaml"
         self.generator_py = self.scripts_dir / "bareboxtlv-generator.py"
-        self.unsigned_bin = self.dir / 'unsigned.tlv'
-        self.corrupted_bin = self.dir / 'unsigned_corrupted.tlv'
+        self.privkey_rsa = Path("crypto/fit-4096-development.key")
+        self.pubkey_rsa = Path("crypto/fit-4096-development.crt")
+        self.privkey_ecdsa = Path("crypto/fit-ecdsa-development.key")
+        self.pubkey_ecdsa = Path("crypto/fit-ecdsa-development.crt")
+        self.unsigned_bin = self.dir / "unsigned.tlv"
+        self.corrupted_bin = self.dir / "unsigned_corrupted.tlv"
+        self.signed_bin = self.dir / "signed.tlv"
+        self.ecdsa_signed_bin = self.dir / "ecdsa-signed.tlv"
+        self.tampered_bin = self.dir / "signed-tampered.tlv"
+        self.tampered_ecdsa_bin = self.dir / "ecdsa-signed-tampered.tlv"
+
 
 @pytest.fixture(scope="module")
 def tlv_testdata(testfs):
     t = _TLV_Testdata(testfs)
-    t.generator(["--input-data", str(t.data), str(t.schema), str(t.unsigned_bin)])
-    assert t.unsigned_bin.exists(), "unsigned TLV not created"
 
-    with open(t.unsigned_bin, 'r+b') as f:
-        data = bytearray(f.read())
-    data[0x20] ^= 1
-    with open(t.corrupted_bin, "wb") as f:
-        f.write(data)
+    t.tlv_gen(t.unsigned_bin)
+    t.tlv_gen(t.signed_bin, sign=t.privkey_rsa, magic="0x61bb95f3")
+    t.tlv_gen(t.ecdsa_signed_bin, sign=t.privkey_ecdsa, magic="0x61bb95f3")
+
+    t.corrupt(t.unsigned_bin, t.corrupted_bin)
+    t.corrupt(t.signed_bin, t.tampered_bin, fix_crc=True)
+    t.corrupt(t.ecdsa_signed_bin, t.tampered_ecdsa_bin, fix_crc=True)
 
     return t
 
+
 def test_tlv_generator(tlv_testdata):
     t = tlv_testdata
-    out_yaml = t.dir / 'out.yaml'
+    out_yaml = t.dir / "out.yaml"
 
+    t.tlv_read(t.unsigned_bin)
+    t.tlv_read(t.signed_bin, verify=t.pubkey_rsa, magic="0x61bb95f3")
+    t.tlv_read(t.ecdsa_signed_bin, verify=t.pubkey_ecdsa, magic="0x61bb95f3")
 
-    good = t.generator(["--output-data", str(out_yaml), str(t.schema), str(t.unsigned_bin)], check=False)
-    assert good.returncode == 0, f"valid unsigned TLV failed to decode: {good.stderr}\n{good.stdout}"
+    t.tlv_read(t.corrupted_bin, expect_failure=True)
+    t.tlv_read(t.tampered_bin, verify=t.pubkey_rsa, magic="0x61bb95f3", expect_failure=True)
+    t.tlv_read(t.tampered_ecdsa_bin, verify=t.pubkey_ecdsa, magic="0x61bb95f3", expect_failure=True)
 
-    bad = t.generator(["--output-data", str(t.dir / 'bad.yaml'), str(t.schema), str(t.corrupted_bin)], check=False)
-    assert bad.returncode != 0, "unsigned TLV with invalid CRC unexpectedly decoded successfully"
 
-def test_tlv_command(barebox, barebox_config, tlv_testdata):
+@pytest.fixture(scope="module")
+def tlv_cmdtest(barebox_config, tlv_testdata):
     skip_disabled(barebox_config, "CONFIG_CMD_TLV")
-    t = tlv_testdata
-    with open(t.data, 'r', encoding='utf-8') as f:
-        yaml_lines = [l.strip() for l in f if l.strip() and not l.strip().startswith('#')]
-
-    stdout = barebox.run_check(f"tlv /mnt/9p/testfs/{t.unsigned_bin.name}")
-
-    # work around 9pfs printing here after a failed network test
-    tlv_offset = next((i for i, line in enumerate(stdout) if line.startswith("tlv")), None)
-    tlv_lines = stdout[tlv_offset + 1:-1]
-
-    assert len(yaml_lines) == len(tlv_lines), \
-        f"YAML and TLV output line count mismatch for {t.unsigned_bin.name}"
-
-    for yline, tline in zip(yaml_lines, tlv_lines):
-        m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
-        assert m, f"malformed tlv line: {tline}"
-        tkey, tval = m.group(1), m.group(2)
-        m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
-        assert m, f"malformed yaml line: {yline}"
-        ykey, yval = m.group(1), m.group(2) or m.group(3)
-        assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
-        assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS")
+
+    class _TLV_Cmdtest:
+        def __init__(self, tlv_testdata):
+            self.t = tlv_testdata
+            with open(tlv_testdata.data, "r", encoding="utf-8") as f:
+                self.yaml_lines = [
+                    l.strip() for l in f if l.strip() and not l.strip().startswith("#")
+                ]
+
+        def test(self, barebox, fn, fail=False):
+            cmd = f"tlv /mnt/9p/testfs/{fn}"
+            stdout, stderr, exitcode = barebox.run(cmd, timeout=2)
+            if fail:
+                assert exitcode != 0
+                return
+            elif exitcode != 0:
+                raise RuntimeError(f"`{cmd}` failed with exitcode {exitcode}:\n{stderr}\n{stdout}")
+
+            # work around a corner case of 9pfs printing here (after a failed network test?)
+            tlv_offset = next((i for i, line in enumerate(stdout) if line.startswith("tlv")), None)
+            tlv_lines = stdout[tlv_offset + 1 : -1]
+
+            assert len(self.yaml_lines) == len(tlv_lines), (
+                f"YAML and TLV output line count mismatch for {fn}"
+            )
+
+            for yline, tline in zip(self.yaml_lines, tlv_lines):
+                m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
+                assert m, f"malformed tlv line: {tline}"
+                tkey, tval = m.group(1), m.group(2)
+                m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
+                assert m, f"malformed yaml line: {yline}"
+                ykey, yval = m.group(1), m.group(2) or m.group(3)
+                assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
+                assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"
+
+    return _TLV_Cmdtest(tlv_testdata)
+
+
+def test_tlv_cmd_unsigned(barebox, barebox_config, tlv_cmdtest):
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
+    tlv_cmdtest.test(barebox, tlv_cmdtest.t.unsigned_bin.name)
+
+
+def test_tlv_cmd_signed(barebox, barebox_config, tlv_cmdtest):
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
+    tlv_cmdtest.test(barebox, tlv_cmdtest.t.signed_bin.name)
+
+
+def test_tlv_cmd_ecdsa_signed(barebox, barebox_config, tlv_cmdtest):
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_ECDSA")
+    tlv_cmdtest.test(barebox, tlv_cmdtest.t.ecdsa_signed_bin.name)
+
+
+def test_tlv_cmd_corrupted(barebox, barebox_config, tlv_cmdtest):
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
+    tlv_cmdtest.test(barebox, tlv_cmdtest.t.corrupted_bin.name, fail=True)
+
+
+def test_tlv_cmd_tampered(barebox, barebox_config, tlv_cmdtest):
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
+    tlv_cmdtest.test(barebox, tlv_cmdtest.t.tampered_bin.name, fail=True)
+
+
+def test_tlv_cmd_ecdsa_tampered(barebox, barebox_config, tlv_cmdtest):
+    skip_disabled(barebox_config, "CONFIG_CRYPTO_ECDSA")
+    tlv_cmdtest.test(barebox, tlv_cmdtest.t.tampered_ecdsa_bin.name, fail=True)

-- 
2.51.0.297.gca2559c1d6

[PATCH 13/15] ci: pytest: Add kconfig fragment for TLV signature integration tests

[Jonas Rebmann]

For targets that support inclusion of the testfs, make sure to inject
all config options needed to run the signature verification tests from
the test_tlv.py which invoke the TLV command and verify some of the
generated test TLVs against the builtin developer keys.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 .github/workflows/test-labgrid-pytest.yml           |  1 +
 common/boards/configs/enable_tlv_sig_testing.config | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/.github/workflows/test-labgrid-pytest.yml b/.github/workflows/test-labgrid-pytest.yml
index 4569e475bc..d9822fc206 100644
--- a/.github/workflows/test-labgrid-pytest.yml
+++ b/.github/workflows/test-labgrid-pytest.yml
@@ -89,6 +89,7 @@ jobs:
 
         if [ "${{ steps.used-features.outputs.testfs }}" = "true" ]; then
           KCONFIG_ADD="${KCONFIG_ADD} common/boards/configs/enable_dm_testing.config"
+          KCONFIG_ADD="${KCONFIG_ADD} common/boards/configs/enable_tlv_sig_testing.config"
         fi
 
         ./MAKEALL -O ${KBUILD_OUTPUT} -l "" -v 0 ${{matrix.defconfig}}
diff --git a/common/boards/configs/enable_tlv_sig_testing.config b/common/boards/configs/enable_tlv_sig_testing.config
new file mode 100644
index 0000000000..f1729fece5
--- /dev/null
+++ b/common/boards/configs/enable_tlv_sig_testing.config
@@ -0,0 +1,13 @@
+CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS=y
+CONFIG_CRYPTO_ECC=y
+CONFIG_CRYPTO_RSA=y
+CONFIG_CRYPTO_ECDSA=y
+CONFIG_CRYPTO_BUILTIN_KEYS=y
+CONFIG_CRYPTO_PUBLIC_KEYS=""
+CONFIG_KEYTOC=y
+CONFIG_TLV=y
+CONFIG_TLV_DRV=y
+CONFIG_TLV_BAREBOX=y
+CONFIG_TLV_SIGNATURE=y
+CONFIG_CMD_KEYS=y
+CONFIG_CMD_TLV=y

-- 
2.51.0.297.gca2559c1d6

[PATCH 14/15] doc/barebox-tlv: Update documentation regarding TLV-Signature

[Jonas Rebmann]

 - Update the data structure documentation
 - Explain the signature scheme
 - Add signature to bareboxtlv-generator usage description

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 Documentation/user/barebox-tlv.rst | 49 +++++++++++++++++++++++++++++---------
 1 file changed, 38 insertions(+), 11 deletions(-)

diff --git a/Documentation/user/barebox-tlv.rst b/Documentation/user/barebox-tlv.rst
index 4e83cdf47f..46d1f7a006 100644
--- a/Documentation/user/barebox-tlv.rst
+++ b/Documentation/user/barebox-tlv.rst
@@ -25,22 +25,25 @@ The TLV binary has the following format:
 .. code-block:: C
 
      struct tlv {
-         be16 tag; /* 2 bytes */
-         be16 len; /* 2 bytes */
+         be16 tag;
+         be16 len;
          u8 payload[];
      };
 
-     struct binfile {
-         be32 magic_version;
-         be32 length_tlv; /* 4 bytes */
-         be32 length_sig; /* 4 bytes */
-         struct tlv tlvs[];
-         be32 crc32;
+     struct sig {
+       u8 spki_hash_prefix[4];
+       u8 signature[];
      };
 
-.. note::
-  Even though the header has a ``length_sig`` field,
-  there is currently no support for cryptographic signatures.
+     struct tlv_format {
+         be32 magic;
+         be32 length_tlv;   /* in bytes */
+         be16 reserved;     /* must be 0 */
+         be16 length_sig;   /* in bytes */
+         struct tlv tlvs[];
+         struct sig signature; /* omitted if length_sig == 0 */
+         be32 crc;
+     };
 
 Tags
 ----
@@ -61,6 +64,24 @@ These common tags are defined in ``common/tlv/barebox.c``.
 The tag range ``0x8000`` to ``0xFFFF`` is intended for custom extensions.
 Parsing must be handled by board-specific extensions.
 
+Signature
+---------
+
+If ``length_sig`` is zero, signature is disabled.
+The ``signature`` section of the file is exactly ``length_sig`` bytes long.
+This includes the ``spki_hash_prefix``, followed by the signature itself.
+``spki_hash_prefix`` is the first bytes of the sha256 hash of the *Subject
+Public Key Info* and its purpose is to allow for quicker matching of a signed
+TLV with its corresponding public key during signature verification.
+
+The ``signature`` shall be verified on all fields before the ``signature`` field,
+but with ``length_sig`` overwritten with 0,
+hashed with sha256
+and signed using any of the supported algorithms (currently RSA and some ECDSA curves).
+
+Note that ECDSA signatures are not DER encoded but rather plain concatenation
+of r and s (each extended to the size of the ECDSA-key).
+
 Data Types
 ----------
 
@@ -88,6 +109,12 @@ With these information in place a TLV binary can be created:
    ./bareboxtlv-generator.py --input-data data-example.yaml \
                              schema-example.yaml tlv.bin
 
+To additionally sign the TLV, supply a private key using the ``--sign KEY`` option.
+
+As ``bareboxtlv-generator.py`` internally uses ``openssl pkeyutl`` for
+accessing the private key, when using OpenSSL 3, any provider, such as pkcs11,
+that is correctly configured, can be used as KEY.
+
 .. note::
   The ``FactoryDataset`` class in ``bareboxtlv-generator.py``
    is intended to be used as a library.

-- 
2.51.0.297.gca2559c1d6

[PATCH 15/15] Documentation: migration-2025.11.0: List changes to CONFIG_CRYPTO_PUBLIC_KEYS

[Jonas Rebmann]

As part of the introduction of TLV signature in public key keyrings,
there is a new interpreter for CONFIG_CRYPTO_PUBLIC_KEYS in
scripts/keytoc.c. All users should update to the new syntax and are
presented with a warning. Users with invalid characters in the legacy
syntax must change the fit-hint and will be presented with an error.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 Documentation/migration-guides/migration-2025.11.0.rst | 17 +++++++++++++++++
 Documentation/user/barebox-tlv.rst                     | 16 ++++++++--------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/Documentation/migration-guides/migration-2025.11.0.rst b/Documentation/migration-guides/migration-2025.11.0.rst
new file mode 100644
index 0000000000..87b1486fda
--- /dev/null
+++ b/Documentation/migration-guides/migration-2025.11.0.rst
@@ -0,0 +1,17 @@
+Release v2025.11.0
+==================
+
+Configuration options
+---------------------
+
+* The syntax of ``CONFIG_CRYPTO_PUBLIC_KEYS`` was updated with the introduction
+  of the keyring feature. Previously, keys selected via
+  ``CONFIG_CRYPTO_PUBLIC_KEYS`` were only used for fitimage verification. Now,
+  fitimage verification uses the "fit" keyring. "fit" will be selected as the
+  default keyring for transition but a warning will be emitted when no keyring
+  is explicitly provided. Existing users should update their keyspec for
+  fitimage public keys to ``keyring=fit[,fit-hint=<fit_hint>]:<crt>``
+* The fit-hints in ``CONFIG_CRYPTO_PUBLIC_KEYS`` are now limited to identifiers
+  matching the regular expression ``[a-zA-Z][a-zA-Z0-9_-]*``. Public keys with
+  a ``fit-hint`` not conforming to this results in an error, affected key-hints
+  must be changed. Please reach out to the mailing list if this causes issues.
diff --git a/Documentation/user/barebox-tlv.rst b/Documentation/user/barebox-tlv.rst
index 46d1f7a006..1c1abfb43a 100644
--- a/Documentation/user/barebox-tlv.rst
+++ b/Documentation/user/barebox-tlv.rst
@@ -31,8 +31,8 @@ The TLV binary has the following format:
      };
 
      struct sig {
-       u8 spki_hash_prefix[4];
-       u8 signature[];
+         u8 spki_hash_prefix[4];
+         u8 signature[];
      };
 
      struct tlv_format {
@@ -67,12 +67,12 @@ Parsing must be handled by board-specific extensions.
 Signature
 ---------
 
-If ``length_sig`` is zero, signature is disabled.
-The ``signature`` section of the file is exactly ``length_sig`` bytes long.
-This includes the ``spki_hash_prefix``, followed by the signature itself.
-``spki_hash_prefix`` is the first bytes of the sha256 hash of the *Subject
-Public Key Info* and its purpose is to allow for quicker matching of a signed
-TLV with its corresponding public key during signature verification.
+If ``length_sig`` is zero, signature is disabled.  The ``signature`` section of
+the file is exactly ``length_sig`` bytes long.  This includes the
+``spki_hash_prefix``, followed by the signature itself.  ``spki_hash_prefix``
+is the first four bytes of the sha256 hash of the *Subject Public Key Info* and
+its purpose is to allow for quicker matching of a signed TLV with its
+corresponding public key during signature verification.
 
 The ``signature`` shall be verified on all fields before the ``signature`` field,
 but with ``length_sig`` overwritten with 0,

-- 
2.51.0.297.gca2559c1d6

Re: [PATCH 04/15] crypto: keytoc: Rename "hint" to "fit-hint" and do not use it in identifiers

[Jonas Rebmann]

Hi,

On 2025-10-14 13:02, Jonas Rebmann wrote:
> The notion of name hints being appropriate as key identifiers is a
> relict from when public key infrastructure was only for fit images.
> 
> Instead of mixing autogenerated key_<counter> and <hint> prefixes in key
> identifiers, simply use key_<counter> for all keys.

The usage message in keytoc still says keyring=<keyring>,hint=<hint>:<crt>
with hint instead of fit-hint. Will correct that in v2.

  - 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    |

Re: [PATCH 14/15] doc/barebox-tlv: Update documentation regarding TLV-Signature

[Jonas Rebmann]

Hi,

On 2025-10-14 13:03, Jonas Rebmann wrote:
>   - Update the data structure documentation
>   - Explain the signature scheme
>   - Add signature to bareboxtlv-generator usage description

I forgot to update the kconfig help for CRYPTO_BUILTIN_KEYS. In v2 I
would include in this documentation patch the change to:

   This option should be a space separated list of filenames of public
   key specifications. While other syntaxes are supported for legacy
   reasons, each entry should have the format
   keyring=<keyring>[,fit-hint=<hint>]:<crt>
   
   <keyring> specifies the keyring for the key to be included in (e.g.
   "fit" if this key is to be used for fitimage verification).
   
   <fit-hint> is used as a key name hint for fitimage verificaiton to
   find a key without iterating over all keys.
   
   <crt> specifies the path of a public key to be included in barebox.
   It can be a PEM-formatted file containing an X.509 certificate. If a
   certificate path starts with "pkcs11:" it is interpreted as a PKCS#11
   URI rather than a file.
   
   Placeholders such as __ENV__VAR_NAME can be used to look up the
   corresponding value in the environment variable VAR_NAME for public
   key paths/URIs as well as key name hints.
   
   Examples specified directly:
   
   - CONFIG_CRYPTO_PUBLIC_KEYS="keyring=fit:pkcs11:object=foo"
   - CONFIG_CRYPTO_PUBLIC_KEYS="keyring=fit,fit-hint=myhint:pkcs11:object=foo"
   - CONFIG_CRYPTO_PUBLIC_KEYS="keyring=fit,fit-hint=myhint:pkcs11:object=foo keyring=fit:/foobar/baz.der"
   - CONFIG_CRYPTO_PUBLIC_KEYS="keyring=fit,fit-hint=myhint:pkcs11:object=foo keyring=fit,fit-hint=myotherhint:/foobar/baz.der"
   
   Example specified indirectly by two environment variables:
   
   - myhint="myhint"
   - myname="pkcs11:object=foo" (.der could be used too)
   - CONFIG_CRYPTO_PUBLIC_KEYS="keyring=fit,fit-hint=__ENV__myhint:__ENV__myname"
   
   Example specified indirectly by a single environment variable:
   
   - mykey="keyring=fit,fit-hint=myhint:pkcs11:object=foo" (.der could be used too)
   - CONFIG_CRYPTO_PUBLIC_KEYS="__ENV__mykey"


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    |

Re: [PATCH 15/15] Documentation: migration-2025.11.0: List changes to CONFIG_CRYPTO_PUBLIC_KEYS

[Jonas Rebmann]

Hi,

On 2025-10-14 13:03, Jonas Rebmann wrote:
> As part of the introduction of TLV signature in public key keyrings,
> there is a new interpreter for CONFIG_CRYPTO_PUBLIC_KEYS in
> scripts/keytoc.c. All users should update to the new syntax and are
> presented with a warning. Users with invalid characters in the legacy
> syntax must change the fit-hint and will be presented with an error.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>   Documentation/migration-guides/migration-2025.11.0.rst | 17 +++++++++++++++++
>   Documentation/user/barebox-tlv.rst                     | 16 ++++++++--------
>   2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/migration-guides/migration-2025.11.0.rst b/Documentation/migration-guides/migration-2025.11.0.rst
> new file mode 100644
> index 0000000000..87b1486fda
> --- /dev/null
> +++ b/Documentation/migration-guides/migration-2025.11.0.rst
> @@ -0,0 +1,17 @@
> +Release v2025.11.0
> +==================
> +
> +Configuration options
> +---------------------
> +
> +* The syntax of ``CONFIG_CRYPTO_PUBLIC_KEYS`` was updated with the introduction
> +  of the keyring feature. Previously, keys selected via
> +  ``CONFIG_CRYPTO_PUBLIC_KEYS`` were only used for fitimage verification. Now,
> +  fitimage verification uses the "fit" keyring. "fit" will be selected as the
> +  default keyring for transition but a warning will be emitted when no keyring
> +  is explicitly provided. Existing users should update their keyspec for
> +  fitimage public keys to ``keyring=fit[,fit-hint=<fit_hint>]:<crt>``
> +* The fit-hints in ``CONFIG_CRYPTO_PUBLIC_KEYS`` are now limited to identifiers
> +  matching the regular expression ``[a-zA-Z][a-zA-Z0-9_-]*``. Public keys with
> +  a ``fit-hint`` not conforming to this results in an error, affected key-hints
> +  must be changed. Please reach out to the mailing list if this causes issues.
> diff --git a/Documentation/user/barebox-tlv.rst b/Documentation/user/barebox-tlv.rst
> index 46d1f7a006..1c1abfb43a 100644
> --- a/Documentation/user/barebox-tlv.rst
> +++ b/Documentation/user/barebox-tlv.rst
> @@ -31,8 +31,8 @@ The TLV binary has the following format:
>        };
>   
>        struct sig {
> -       u8 spki_hash_prefix[4];
> -       u8 signature[];
> +         u8 spki_hash_prefix[4];
> +         u8 signature[];
>        };
>   
>        struct tlv_format {
> @@ -67,12 +67,12 @@ Parsing must be handled by board-specific extensions.
>   Signature
>   ---------
>   
> -If ``length_sig`` is zero, signature is disabled.
> -The ``signature`` section of the file is exactly ``length_sig`` bytes long.
> -This includes the ``spki_hash_prefix``, followed by the signature itself.
> -``spki_hash_prefix`` is the first bytes of the sha256 hash of the *Subject
> -Public Key Info* and its purpose is to allow for quicker matching of a signed
> -TLV with its corresponding public key during signature verification.
> +If ``length_sig`` is zero, signature is disabled.  The ``signature`` section of
> +the file is exactly ``length_sig`` bytes long.  This includes the
> +``spki_hash_prefix``, followed by the signature itself.  ``spki_hash_prefix``
> +is the first four bytes of the sha256 hash of the *Subject Public Key Info* and
> +its purpose is to allow for quicker matching of a signed TLV with its
> +corresponding public key during signature verification.
>   
>   The ``signature`` shall be verified on all fields before the ``signature`` field,
>   but with ``length_sig`` overwritten with 0,
> 

The changes to Documentation/user/barebox-tlv.rst are supposed to be
part of the previous patch. Will correct that in v2.

  - 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    |

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

[Jonas Rebmann]

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    |

Re: [PATCH 03/15] fit: only accept keys from "fit"-keyring

[Ahmad Fatoum]

Hi,

On 10/14/25 1:02 PM, Jonas Rebmann wrote:
> Separate keys shall be used for fitimage verification and the upcoming
> TLV verification.
> 
> Based on the newly introduced keyring feature, limit fitimage
> verification to the keys in the keyring literally named "fit", which is
> also the current default keyring name in keytoc for backwards
> compatibility.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  common/image-fit.c          | 13 ++++++++-----
>  crypto/public-keys.c        | 13 ++++++++++---
>  crypto/rsa.c                |  1 +
>  include/crypto/public_key.h |  9 ++++++++-
>  include/tlv/tlv.h           |  1 +
>  5 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 3017ccb504..0cbe8baf6f 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -261,7 +261,7 @@ static struct digest *fit_alloc_digest(struct device_node *sig_node,
>  static int fit_check_signature(struct fit_handle *handle, struct device_node *sig_node,
>  			       enum hash_algo algo, void *hash)
>  {
> -	const char *fail_reason = "no built-in keys";
> +	const char *fail_reason;
>  	const struct public_key *key;
>  	const char *key_name = NULL;
>  	int sig_len;
> @@ -274,10 +274,13 @@ static int fit_check_signature(struct fit_handle *handle, struct device_node *si
>  		return -EINVAL;
>  	}
>  
> +	fail_reason = "no matching keys";
> +
>  	of_property_read_string(sig_node, "key-name-hint", &key_name);
>  	if (key_name) {
> -		key = public_key_get(key_name);
> +		key = public_key_get(key_name, "fit");
>  		if (key) {
> +			fail_reason = "verification failed";
>  			ret = public_key_verify(key, sig_value, sig_len, hash, algo);
>  			if (handle->verbose)
>  				pr_info("Key %*phN (%s) -> signature %s\n", key->hashlen,
> @@ -287,13 +290,13 @@ static int fit_check_signature(struct fit_handle *handle, struct device_node *si
>  		}
>  	}
>  
> -	for_each_public_key(key) {
> -		fail_reason = "verification failed";
> +	for_each_public_key_keyring(key, "fit") {
>  
>  		/* Don't recheck with same key as before */
> -		if (streq_ptr(key->key_name_hint, key_name))
> +		if (key_name && streq_ptr(key->key_name_hint, key_name))

Ouch. This should be fixed up into the commit introducing streq_ptr as
that one breaks handling of multiple keys where there are no hints..

I sent a fixup.


Cheers,
Ahmad

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

Re: [PATCH 05/15] commands: keys: update output format to include keyring

[Ahmad Fatoum]

Hi,

On 10/14/25 1:02 PM, Jonas Rebmann wrote:
> Also show the algorithm used.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  commands/keys.c             |  8 ++------
>  include/crypto/public_key.h | 12 ++++++++++++
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/commands/keys.c b/commands/keys.c
> index 2d85e8124f..e1e87f3e9e 100644
> --- a/commands/keys.c
> +++ b/commands/keys.c
> @@ -7,12 +7,8 @@ static int do_keys(int argc, char *argv[])
>  	const struct public_key *key;
>  
>  	for_each_public_key(key) {
> -		printf("KEY: %*phN", key->hashlen, key->hash);
> -
> -		if (key->key_name_hint)
> -			printf(" (%s)\n", key->key_name_hint);
> -		else
> -			printf("\n");
> +		printf("KEY: %*phN\tTYPE: %s\tKEYRING: %s\tHINT: %s\n", key->hashlen,
> +		       key->hash, public_key_type_string(key->type), key->keyring, key->key_name_hint);

Given that hint is only relevant to FIT, I think it would be better to
hide it when it's empty.

Cheers,
Ahmad

>  	}
>  
>  	return 0;
> diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> index 44ae09e4d0..269b878a04 100644
> --- a/include/crypto/public_key.h
> +++ b/include/crypto/public_key.h
> @@ -12,6 +12,18 @@ enum public_key_type {
>  	PUBLIC_KEY_TYPE_ECDSA,
>  };
>  
> +static inline const char *public_key_type_string(enum public_key_type type)
> +{
> +	switch (type) {
> +	case PUBLIC_KEY_TYPE_RSA:
> +		return "RSA";
> +	case PUBLIC_KEY_TYPE_ECDSA:
> +		return "ECDSA";
> +	default:
> +		return "unknown";
> +	}
> +}
> +
>  struct public_key {
>  	enum public_key_type type;
>  	struct list_head list;
> 

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

Re: [PATCH 06/15] commands: tlv: Error out on invalid TLVs

[Ahmad Fatoum]

On 10/14/25 1:02 PM, Jonas Rebmann wrote:
> TLV parsing happens in the TLV drivers probe function
> (tlv_probe_from_magic()) but register_device() does not propagate
> errors from match(), always returning zero.
> 
> match() however ensures that dev->driver is always NULL if an error
> occurred on probe and since we're only probing the TLV driver, rely on
> that as indicator of an error during TLV parsing (e.g. CRC mismatch).
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  commands/tlv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/commands/tlv.c b/commands/tlv.c
> index 6118a2947f..c01a7b0399 100644
> --- a/commands/tlv.c
> +++ b/commands/tlv.c
> @@ -29,7 +29,7 @@ static int do_tlv(int argc, char *argv[])
>  		return COMMAND_ERROR_USAGE;
>  
>  	tlvdev = tlv_register_device_by_path(filename, NULL);
> -	if (IS_ERR(tlvdev)) {
> +	if (IS_ERR(tlvdev) || tlvdev->dev.driver == NULL) {
>  		printf("Could not open \"%s\": %m\n", filename);
>  		return COMMAND_ERROR;
>  	}
> 

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

Re: [PATCH 05/15] commands: keys: update output format to include keyring

[Jonas Rebmann]

Hi Ahmad,

On 2025-10-22 11:43, Ahmad Fatoum wrote:
>> +		printf("KEY: %*phN\tTYPE: %s\tKEYRING: %s\tHINT: %s\n", key->hashlen,
>> +		       key->hash, public_key_type_string(key->type), key->keyring, key->key_name_hint);
> 
> Given that hint is only relevant to FIT, I think it would be better to
> hide it when it's empty.

The key name hints are still defined, and unique amongst all keys. Just
like before my series, if no hint is provided in keyspec, keytoc
generates one: key_<num>.

Therefore it should always be shown.

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    |

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

[Ahmad Fatoum]

On 10/14/25 1:03 PM, Jonas Rebmann wrote:
> diff --git a/common/Kconfig b/common/Kconfig
> index d923d4c4b6..b7ac7094a6 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -1122,6 +1122,10 @@ config TLV
>  	  barebox TLV is a scheme for storing factory data on non-volatile
>  	  storage. Unlike state, it's meant to be read-only.
>  
> +config TLV_SIGNATURE
> +	bool "barebox TLV signature support"

depends on TLV or open an if TLV to span this as well as TLV_DRV and
TLV_BAREBOX.

> +	select CRYPTO_BUILTIN_KEYS
> +
>  config TLV_DRV
>  	bool "barebox TLV generic driver"
>  	depends on TLV
> 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
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  
> +#include "tlv/format.h"

pr_fmt definition must be before any headers to ensure that it's seen
when pr_info is defined, even with future changes to headers.

>  #define pr_fmt(fmt) "barebox-tlv: " fmt
>  
>  #include <common.h>
> @@ -9,6 +10,81 @@
>  #include <linux/stat.h>
>  #include <crc.h>
>  #include <net.h>
> +#include <crypto/public_key.h>
> +
> +static int tlv_verify_try_key(const struct public_key *key, const uint8_t *sig,
> +			      const uint32_t sig_len, const void *data,
> +			      unsigned long data_len)
> +{
> +	enum hash_algo algo = HASH_ALGO_SHA256;
> +	int ret;
> +	struct digest *digest = digest_alloc_by_algo(algo);

Make it either the last definition or move the assignment before the if.

> +	void *hash;
> +
> +	if (!digest)
> +		return -ENOMEM;
> +
> +	digest_init(digest);
> +	if (IS_ERR(digest)) {
> +		digest_free(digest);
> +		return -EINVAL;
> +	}
> +	digest_update(digest, data, data_len);
> +	hash = xzalloc(digest_length(digest));
> +	digest_final(digest, hash);
> +
> +	ret = public_key_verify(key, sig, sig_len, hash, algo);
> +
> +	digest_free(digest);
> +	free(hash);
> +
> +	if (ret)
> +		return -ENOKEY;

We have no strerror text for ENOKEY, but I believe it's a bit
out-of-place here anyway. -ENOKEY could be an error code from
tlv_verify, but when trying a specific key, -ENOKEY appears to be
inaccurate.

> +	return 0;
> +}
> +
> +static int tlv_verify(struct tlv_header *header, const char *keyring)
> +{
> +	const struct public_key *key;
> +	size_t payload_sz = tlv_spki_hash_offset(header);
> +	void *spki_tlv_ptr = (void *)header + payload_sz;

Can this be made const void *.

> +	u32 spki_tlv = get_unaligned_le32(spki_tlv_ptr);
> +	const int SPKI_LEN = 4;

Drop the const without a pointer (this isn't C++).

> +	u32 sig_len = get_unaligned_be16(&header->length_sig);
> +	int ret;
> +	int count_spki_matches = 0;
> +
> +	if (!IS_ENABLED(CONFIG_TLV_SIGNATURE)) {
> +		pr_err("TLV signature selected in decoder but not enabled!\n");

Remove TLV prefix here and below, there's already a barebox-tlv: prefix.

> +		return -ENOSYS;
> +	} else if (sig_len == 0) {
> +		pr_err("TLV signature selected in decoder but an unsigned TLV matched by magic %08x!\n", be32_to_cpu(header->magic));
> +		return -EPROTO;
> +	}
> +	/* signature length field must always be zeroed during signage and verification */

s/signage/signing/

> +	header->length_sig = 0;
> +
> +	for_each_public_key_keyring(key, keyring) {
> +		u32 spki_key = get_unaligned_le32(key->hash);
> +
> +		if (spki_key == spki_tlv) {
> +			count_spki_matches++;
> +			ret = tlv_verify_try_key(key, spki_tlv_ptr + SPKI_LEN, sig_len - SPKI_LEN, header, payload_sz);
> +			if (!ret)
> +				return 0;
> +			pr_warn("TLV spki %08x matched available key but signature verification failed: %s!\n", spki_tlv, strerror(-ret));

"%s", strerror(-ret) -> "%pE", PTR_ERR(ret)

> +		}
> +	}
> +
> +	/* reset signature length field after verification to avoid later confusion */
> +	put_unaligned_be16(sig_len, &header->length_sig);
> +
> +	if (!count_spki_matches) {
> +		pr_warn("TLV spki %08x matched no key!\n", spki_tlv);
> +		return -ENOKEY;
> +	}
> +	return -EINVAL;
> +}
>  
>  int tlv_parse(struct tlv_device *tlvdev,
>  	      const struct tlv_decoder *decoder)
> @@ -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;
> +	}
> +
>  	for_each_tlv(header, tlv) {
>  		struct tlv_mapping **mappings;
>  		u16 tag = TLV_TAG(tlv);
> diff --git a/include/tlv/format.h b/include/tlv/format.h
> index c4521c3131..cbe0a132b1 100644
> --- a/include/tlv/format.h
> +++ b/include/tlv/format.h
> @@ -47,11 +47,12 @@ struct tlv {
>  struct tlv_header {
>  	__be32 magic;
>  	__be32 length_tlv; /* in bytes */
> -	__be32 length_sig; /* in bytes */
> +	__be16 reserved;
> +	__be16 length_sig; /* in bytes */
>  	struct tlv tlvs[];
> -	/* __be32 crc; */
>  	/* u8 sig[]; */
> -};
> +	/* __be32 crc; */
> +} __packed;
>  static_assert(sizeof(struct tlv_header) == 3 * 4);
>  
>  #define for_each_tlv(tlv_head, tlv) \
> @@ -62,8 +63,19 @@ static inline size_t tlv_total_len(const struct tlv_header *header)
>  	size_t ret;
>  
>  	ret = size_add(sizeof(struct tlv_header), get_unaligned_be32(&header->length_tlv));
> -	ret = size_add(ret, sizeof(__be32)); /* CRC appended after TLVs */
> -	ret = size_add(ret, get_unaligned_be32(&header->length_sig)); /* optional signature appended after CRC */
> +	ret = size_add(ret, get_unaligned_be16(&header->length_sig)); /* optional signature appended after TLVs */
> +	ret = size_add(ret, sizeof(__be32)); /* CRC at end of file */
> +
> +	return ret; /* SIZE_MAX on overflow */
> +}
> +
> +/*
> + * Retrieve length of header+TLVs (offset of spki hash part of signature if available)
> + */
> +
> +static inline size_t tlv_spki_hash_offset(const struct tlv_header *header)
> +{
> +	size_t ret = size_add(sizeof(struct tlv_header), get_unaligned_be32(&header->length_tlv));
>  
>  	return ret; /* SIZE_MAX on overflow */

Shouldn't you then check for SIZE_MAX at callsites?

Cheers,
Ahmad

>  }
> 

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

Re: [PATCH 10/15] common: tlv: default decoder for signed TLV

[Ahmad Fatoum]

Hi,

On 10/14/25 1:03 PM, Jonas Rebmann wrote:
> Introduce a second default encoder that behaves just like barebox_tlv_v1
> but uses the "tlv" keyring.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  .../devicetree/bindings/nvmem/barebox,tlv.yaml     |  1 +
>  common/tlv/barebox.c                               | 25 +++++++++++++++++++++-
>  include/tlv/format.h                               |  1 +
>  3 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml b/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml
> index 7a77b9a0b4..7d678ee04a 100644
> --- a/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/barebox,tlv.yaml
> @@ -22,6 +22,7 @@ properties:
>      items:
>        - enum:
>          - barebox,tlv-v1        # magic: 0x61bb95f2
> +        - barebox,tlv-v1-signed # magic: 0x61bb95f3
>          - lxa,tlv-baseboard-v1  # magic: 0xbc288dfe
>          - lxa,tlv-powerboard-v1 # magic: 0xdca5a870
>        - const: barebox,tlv
> diff --git a/common/tlv/barebox.c b/common/tlv/barebox.c
> index b941463b31..d4970e6ebf 100644
> --- a/common/tlv/barebox.c
> +++ b/common/tlv/barebox.c
> @@ -177,16 +177,39 @@ static struct of_device_id of_matches[] = {
>  	{ .compatible = "barebox,tlv-v1" },
>  	{ /* sentinel */}
>  };
> +static struct of_device_id of_matches_signed[] = {
> +	{ .compatible = "barebox,tlv-v1-signed" },
> +	{ /* sentinel */ }
> +};
>  
>  static struct tlv_decoder barebox_tlv_v1 = {
>  	.magic = TLV_MAGIC_BAREBOX_V1,
>  	.driver.name = "barebox-tlv-v1",
>  	.driver.of_compatible = of_matches,
>  	.mappings = mappings,
> +	.signature_keyring = NULL,

I'd drop it given it's explicitly meant as optional parameter.

> +};
> +
> +static struct tlv_decoder barebox_tlv_v1_signed = {
> +	.magic = TLV_MAGIC_BAREBOX_V1_SIGNED,
> +	.driver.name = "barebox-tlv-v1-signed",
> +	.driver.of_compatible = of_matches_signed,
> +	.mappings = mappings,
> +	.signature_keyring = "tlv",
>  };
>  
>  static int tlv_register_default(void)
>  {
> -	return tlv_register_decoder(&barebox_tlv_v1);
> +	int err;
> +
> +	err = tlv_register_decoder(&barebox_tlv_v1);
> +	if (err)
> +		return err;
> +	if (IS_ENABLED(CONFIG_TLV_SIGNATURE)) {
> +		err = tlv_register_decoder(&barebox_tlv_v1_signed);
> +		if (err)
> +			return err;
> +	}
> +	return 0;
>  }
>  device_initcall(tlv_register_default);
> diff --git a/include/tlv/format.h b/include/tlv/format.h
> index cbe0a132b1..ab12f0c9bb 100644
> --- a/include/tlv/format.h
> +++ b/include/tlv/format.h
> @@ -23,6 +23,7 @@
>  #include <linux/build_bug.h>
>  
>  #define TLV_MAGIC_BAREBOX_V1		0x61bb95f2
> +#define TLV_MAGIC_BAREBOX_V1_SIGNED	0x61bb95f3
>  #define TLV_MAGIC_LXA_BASEBOARD_V1	0xbc288dfe
>  #define TLV_MAGIC_LXA_POWERBOARD_V1	0xc6895c21
>  #define TLV_MAGIC_LXA_IOBOARD_V1	0xdca5a870
> 

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

Re: [PATCH 11/15] crypto: Use "development" keys for "fit" and "tlv" keyring

[Ahmad Fatoum]

On 10/14/25 1:03 PM, Jonas Rebmann wrote:
> All users of the CONFIG_CRYPTO_PUBLIC_KEYS feature should update to the
> new syntax making keyring selection mandatory.
> 
> Instead of just making the addition of the builtin snakeoil keys
> explicit for the "fit" key, also add them to the "tlv" key to use them
> as a testing set for TLV keys too.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  crypto/Makefile | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 08b9a46e4c..076ba4f686 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -33,10 +33,12 @@ CONFIG_CRYPTO_PUBLIC_KEYS := $(foreach d,$(CONFIG_CRYPTO_PUBLIC_KEYS),"$(d)")
>  
>  ifdef CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS
>  ifdef CONFIG_CRYPTO_RSA
> -CONFIG_CRYPTO_PUBLIC_KEYS += rsa-devel:$(srctree)/crypto/fit-4096-development.crt
> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=rsa-devel:$(srctree)/crypto/fit-4096-development.crt
> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-4096-development.crt
>  endif
>  ifdef CONFIG_CRYPTO_ECDSA
> -CONFIG_CRYPTO_PUBLIC_KEYS += ecdsa-devel:$(srctree)/crypto/fit-ecdsa-development.crt
> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=ecdsa-devel:$(srctree)/crypto/fit-ecdsa-development.crt
> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-ecdsa-development.crt

We don't want people to overload tlv and instead use their own keyring
names. This is already too late for fit, but for this, let's call it
tlv-example?

Cheers,
Ahmad

>  endif
>  endif
>  
> 

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

Re: [PATCH 12/15] test: py: add signature to TLV integration tests

[Ahmad Fatoum]

Hi,

On 10/14/25 1:03 PM, Jonas Rebmann wrote:
> Add TLV signature to TLV integration tests:
>  - Signed TLV using development RSA key
>  - Modify payload and fix CRC for a "tampered" tlv
>  - Include both cases in generator and tlv-command tests.
> 
> Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
> testing. Consequentially add the matching private keys from the public
> repository at [1].
> 
> [1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>  crypto/fit-4096-development.key  |  51 ++++++++++
>  crypto/fit-ecdsa-development.key |   5 +

Move this into test/?

>  test/py/test_tlv.py              | 205 +++++++++++++++++++++++++++++++--------
>  3 files changed, 219 insertions(+), 42 deletions(-)
> 
> diff --git a/crypto/fit-4096-development.key b/crypto/fit-4096-development.key
> new file mode 100644
> index 0000000000..526cdfc2b5
> --- /dev/null
> +++ b/crypto/fit-4096-development.key
> @@ -0,0 +1,51 @@
> +-----BEGIN RSA PRIVATE KEY-----
> +MIIJKgIBAAKCAgEAyZHkijUfqoAvEELaxSLnjyqhTprEilnf7JqvSCMDUUXv2dEl
> +k1r4RwiBowJp/4W3sOx4gASEHM2xlDWyPYZUR/1btZeVJvOIRPWfw8JoLT3tbbST
> +OIw04Bk6MUh3LbgBtxbbKGGkFewq3Ob1XQOWcY3ZAzfLFuooPWJQ6X+IiczkrA0r
> +GnwpuhHlb8tOdQZRjDevIVVvEkRjRiqrAw5pKTy/Mt/SJJ/yC7qJptIQskQ42y3R
> +qHeCVmP6ZF6VV1scNmHr8+kRD19DhCos6DLWq2pCFPwnSmgM4T0FWcJsMiNta+rt
> +Rq+kG7RjOOYbbqvuk3vkMrQTRQeAYfdnuOGimGQYxiVh9quOMVG6NJ2hylTa0S/E
> +PKQUQvK9A8bnDul522XPmMVHOtLXVGtwKx9xQUx/2D7aoqlmVJSGQeAMNi0NEFhq
> +buGdXuJ/2cKwtobClkz0WbbMlI4UBM7V4qP3JSkxiojRKhtNHtdE3KtF3ronRJaU
> ++yDggNobWLiJ4TtQ0OAC1REEJGq7s9k8ASi+6s+VX7yVtlGWMIjbBGAl8lqgXXsA
> +prRrmtofaQgEPVvcSAIbuch/JqpHhs8vHBiWb/KZdOe/vMiYQE/d9/KY8rybZa3D
> +hKvzN7X59ymOYLILHB73Xxi5bQA61DaeYE4KnPiJaEDrUccrlFjMBIwGrosCAwEA
> +AQKCAgEApVkIIFdzommEMdKloxD+4nIV4GUU1GjlRzGcl5AhKIo2NndaW4ZEJADW
> +VuGkEfeet4NDVcBen0IcaXeivtVyTZuHn2646zrajbbvV6Yhzvr9yQBXxAs/VJVd
> +JxBKszY+MfKN1JJEB7ezcYIDxEktH/k8C2e5MRLj73a26NO1LVTmQDyNHyy7Deeg
> +ThR4R4bnXh5PiwiKFHIE/YoCvn8TxMAQF6uCtoh+BSD/ydiH2bQc766mTYu7XyKk
> +Q7FS0FXszq+E3pBRbkq3F7OBIviRIAwKKSyvDlpMNnfX68mQ95AYMm6ENXffJtrS
> +ido4ppBjJJh8mRses4FzzukkLITq2qBoZQn+3XfR12+YbWKbCFIAXSu1b4tJetLC
> +wYUp6EuGKCS2XK8OJXbY4M2D1t7bCVlRpptZBBiD7romkLYQ+jRwPbWhjUY6Ktw3
> +ceUf9XJtNVKjHMp7n6C3gdxe2ivK02RYscT3brRq7TUjSzGjH1z/HUQSwr31+tXD
> +dw2fkb2qQn2KUB5hjKcqU/Dxrqjorvf+kjGwXTtAD01Y4r8xRD3HK31zKAgrheN6
> +15shoKRM4imKCD+fTBQjBBVZpTT8xNbo1m+y0joFEeDW0U5Lc3A/DhyTUsfHj4Im
> +H02Cg4wiXXGyJ57fSyyNFKaa0EuLSdOl0zT1bXiy2TxiyTrsgAECggEBAPz+DcTO
> +OvWtU/f+SyAfP86xd3bSnQzBXtoK2iI49uGAAkIXLU881V5sFz41UWJ6g1G0PjKy
> +FWjxTCJytgso9TkISC42TOTE9VUqL4Y4KHhY93nnMAzKNLJC04onqmimDjn1I5Di
> +DD3k3yCYPQEPInz84tdZyB+mRSncdsOL0Mpzhl5fjgGy+pi78K3/B4PUepR3n+Wl
> +4JqKP4SeIL189+ChnSrgVsLzdvpOWJ2cu8DO9qGfz7F0ufJlehUILWPfhYlWUZ/c
> +AUja5QWJEuSQHsKcOJSD4fpjuBWy3ASKlDy7BQSSoASibsl8FfQ/WmQBd3H8Y5/U
> +20psjFuOa/02TusCggEBAMv3V4ccYieiUNkzi6WyyzlR9sULtAG4Cvi1HRw+o6mV
> +VeNFNo8hw+g8f26URvuB06xXyuZepk+oMtEPiFfGYFFg4s22QJRGzqBfC5+8//Mf
> +rcIsU88S75JCZjPDSxOFgzSDAG1gPfX4i8BZHgqaT749TewbeLc0ehvVrcLnXMwB
> +3JpDNmiuNzA2pJAWbLezKazhW6XbpkTtHDqZTswDK+3AFBm7j/cqqeXojd93EbrV
> +0ggyiNMx/O42DHVwZ+51HdJ+C7KDHR0wzgFMu24zyoymzZOaiKjm2rQi/B4mJ9Er
> +oCaCfhVGo/Kq7Y5V7G+x4gag8oVQNCJh/lERrvgBduECggEAT5tpnbn/F3tY5rof
> +zZXHsDRrkPoo7PCT9ixgA1DFbqOnEkDUwxAzW6jLj4mbeE9wru72e2FKF2GGQXiz
> +C8PxleajP9daTsojII9LsQJOyb/E75jtp7ig6E7a3agpmRBXfalDbb2TeI5iH5GH
> +8KNgiM/SWU0pCbx6GvgCbvm501qSt3N97c7xx8mrrDSJmtPrVnhl2g9eI4LJBeP0
> +DWwbW5W/LNS2uFV/5Ldubvn4omz9clIlOoOuVzXTOnb+QWT+Uf7VZGYICXLHifxd
> +84neBALAUwtEulNSg5FqZgttJcb7hzrUG2E5VzEyf07IFJvZiAaRGqQR9NM/PzgL
> +hvvlzQKCAQEAi/wmy2kUiKUjHd79oexzA9UYKyacFW3twcHzx7XJ95Kxjriq+FMx
> +NIuI3ijQCr+QukDK1Y7yT8tdjRQ+/Bb/dfqrzomeCuYJ3BE/VhOOCpucUp6/qmgR
> +mm0N3crUFQLWCM08FtUt0UoTCCFht98uiZ9jgn9cO0i94aqmhhTqIG3KrOkiR3gC
> +Eon+KZHqba1+FdPZZZy5oaamcCVV6jjnBlaEtSCAbx+N2WfhLxR2S6eCbfPY6jHt
> +qMPZiyRpgERLAnNVrd/EtIsRZ9z06m6LPjsg7oPp9Rnz0hwMsth3DV0GnkeDJzED
> +RoI/ZifcjNAmE2yU5iAkl9Bvjc44Kqg+oQKCAQEAvSi4W2kVUoIwlmBHGLge/Rng
> +YyScmAoG4Cavy0Ie6AHPtHayHFdI/rAyiVFnKU5Xuj4qgB54dLa94bQrIu451wls
> +3Jyy/J8WkcW9r/dZFMN6gMoZ0u+xt6KdYe2tQnyW/CG4svDyfckcW2VHdh2A3vqH
> +xlGNmo/HaOeovxWNQkQGQeuXnIcrUvwaFTmGIxLdEO5TAQzLeWSrXldMtVBUAMaJ
> +LClOqNIGRxMRYhZOPVnkedEQmJqgxvcrn8F/91mXQHVnQBOvsgyQDgtS3V0EIAOD
> +rWePjgB8twJknHuab8qH/1z3cQ5QRxQ6lffcIoWgXS59QBBT+jIqMT2oKyGkPw==
> +-----END RSA PRIVATE KEY-----
> diff --git a/crypto/fit-ecdsa-development.key b/crypto/fit-ecdsa-development.key
> new file mode 100644
> index 0000000000..2b13c877a3
> --- /dev/null
> +++ b/crypto/fit-ecdsa-development.key
> @@ -0,0 +1,5 @@
> +-----BEGIN EC PRIVATE KEY-----
> +MHcCAQEEIEsUW5DEOhD1CYHCnPfDULwbRQO9Yjt2/xM5SoY2GUQtoAoGCCqGSM49
> +AwEHoUQDQgAEowCa2OYfPdGRr1JpSYONOA3N2jwJjGbPbfG6uBzKg1VqOOk0a/Vf
> +BfEbQev6X96HCd6zvvC2tjBgvICW8UB0TQ==
> +-----END EC PRIVATE KEY-----
> diff --git a/test/py/test_tlv.py b/test/py/test_tlv.py
> index 79f9f9d01b..1200281dbc 100644
> --- a/test/py/test_tlv.py
> +++ b/test/py/test_tlv.py
> @@ -1,6 +1,7 @@
>  import os
>  import re
>  import subprocess
> +import struct
>  from pathlib import Path
>  from .helper import skip_disabled
>  
> @@ -8,71 +9,191 @@ import pytest
>  
>  
>  class _TLV_Testdata:
> -    def generator(self, args, check=True):
> +    def generator(self, args, expect_failure=False, input=None):
>          cmd = [os.sys.executable, str(self.generator_py)] + args
> -        res = subprocess.run(cmd, text=True)
> +        res = subprocess.run(cmd, text=True, input=input, encoding="utf-8", capture_output=True)
>          if res.returncode == 127:
>              pytest.skip("test skipped due to missing host dependencies")
> -
> -        if check and res.returncode != 0:
> -            raise RuntimeError(f"generator failed ({res.returncode}): {res.stdout}\n{res.stderr}")
> +        if res.returncode == 0 and expect_failure:
> +            raise RuntimeError(
> +                f"`{' '.join(cmd)}` succeded unexpectedly:\n{res.stderr}\n{res.stdout}"
> +            )
> +        elif res.returncode != 0 and not expect_failure:
> +            raise RuntimeError(
> +                f"`{' '.join(cmd)}` failed unexpectedly with {res.returncode}:\n{res.stderr}\n{res.stdout}"
> +            )
>          return res
>  
> +    def overwrite_magic(self, new_magic):
> +        with open(self.schema, "r", encoding="utf-8") as f:
> +            patched_schema = "".join(
> +                re.sub(r"^magic:\s*0x[a-fA-F0-9]{8}\s*$", f"magic: {new_magic}\n", line)
> +                for line in f
> +            )
> +        return patched_schema
> +
> +    def tlv_gen(self, outfile, magic=None, sign=None):
> +        param = ["--input-data", str(self.data)]
> +        if sign:
> +            param += ["--sign", str(sign)]
> +        if magic:
> +            param += ["/dev/stdin"]
> +        else:
> +            param += [str(self.schema)]
> +        param += [str(outfile)]
> +        ret = self.generator(param, input=self.overwrite_magic(magic) if magic else None)
> +        assert outfile.exists(), f"TLV {outfile} not created from {' '.join(param)}"
> +        return ret
> +
> +    def tlv_read(self, binfile, magic=None, verify=None, expect_failure=False):
> +        param = ["--output-data", "/dev/null"]
> +        if verify:
> +            param += ["--verify", str(verify)]
> +        if magic:
> +            param += ["/dev/stdin"]
> +        else:
> +            param += [str(self.schema)]
> +        param += [str(binfile)]
> +        ret = self.generator(
> +            param,
> +            input=self.overwrite_magic(magic) if magic else None,
> +            expect_failure=expect_failure,
> +        )
> +        return ret
> +
> +    def corrupt(self, fnin, fnout, fix_crc=False):
> +        try:
> +            from crcmod.predefined import mkPredefinedCrcFun
> +        except ModuleNotFoundError:
> +            pytest.skip("test skipped due to missing dependency python-crcmod")
> +            return
> +
> +        _crc32_mpeg = mkPredefinedCrcFun("crc-32-mpeg")
> +
> +        with open(fnin, "r+b") as f:
> +            data = bytearray(f.read())
> +        data[0x20] ^= 1
> +        if fix_crc:
> +            crc_raw = _crc32_mpeg(data[:-4])
> +            crc = struct.pack(">I", crc_raw)
> +            data[-4:] = crc
> +        with open(fnout, "wb") as f:
> +            f.write(data)
> +
>      def __init__(self, testfs):
>          self.dir = Path(testfs)
>          self.scripts_dir = Path("scripts/bareboxtlv-generator")
>          self.data = self.scripts_dir / "data-example.yaml"
>          self.schema = self.scripts_dir / "schema-example.yaml"
>          self.generator_py = self.scripts_dir / "bareboxtlv-generator.py"
> -        self.unsigned_bin = self.dir / 'unsigned.tlv'
> -        self.corrupted_bin = self.dir / 'unsigned_corrupted.tlv'
> +        self.privkey_rsa = Path("crypto/fit-4096-development.key")
> +        self.pubkey_rsa = Path("crypto/fit-4096-development.crt")
> +        self.privkey_ecdsa = Path("crypto/fit-ecdsa-development.key")
> +        self.pubkey_ecdsa = Path("crypto/fit-ecdsa-development.crt")
> +        self.unsigned_bin = self.dir / "unsigned.tlv"
> +        self.corrupted_bin = self.dir / "unsigned_corrupted.tlv"
> +        self.signed_bin = self.dir / "signed.tlv"
> +        self.ecdsa_signed_bin = self.dir / "ecdsa-signed.tlv"
> +        self.tampered_bin = self.dir / "signed-tampered.tlv"
> +        self.tampered_ecdsa_bin = self.dir / "ecdsa-signed-tampered.tlv"
> +
>  
>  @pytest.fixture(scope="module")
>  def tlv_testdata(testfs):
>      t = _TLV_Testdata(testfs)
> -    t.generator(["--input-data", str(t.data), str(t.schema), str(t.unsigned_bin)])
> -    assert t.unsigned_bin.exists(), "unsigned TLV not created"
>  
> -    with open(t.unsigned_bin, 'r+b') as f:
> -        data = bytearray(f.read())
> -    data[0x20] ^= 1
> -    with open(t.corrupted_bin, "wb") as f:
> -        f.write(data)
> +    t.tlv_gen(t.unsigned_bin)
> +    t.tlv_gen(t.signed_bin, sign=t.privkey_rsa, magic="0x61bb95f3")
> +    t.tlv_gen(t.ecdsa_signed_bin, sign=t.privkey_ecdsa, magic="0x61bb95f3")
> +
> +    t.corrupt(t.unsigned_bin, t.corrupted_bin)
> +    t.corrupt(t.signed_bin, t.tampered_bin, fix_crc=True)
> +    t.corrupt(t.ecdsa_signed_bin, t.tampered_ecdsa_bin, fix_crc=True)
>  
>      return t
>  
> +
>  def test_tlv_generator(tlv_testdata):
>      t = tlv_testdata
> -    out_yaml = t.dir / 'out.yaml'
> +    out_yaml = t.dir / "out.yaml"
>  
> +    t.tlv_read(t.unsigned_bin)
> +    t.tlv_read(t.signed_bin, verify=t.pubkey_rsa, magic="0x61bb95f3")
> +    t.tlv_read(t.ecdsa_signed_bin, verify=t.pubkey_ecdsa, magic="0x61bb95f3")
>  
> -    good = t.generator(["--output-data", str(out_yaml), str(t.schema), str(t.unsigned_bin)], check=False)
> -    assert good.returncode == 0, f"valid unsigned TLV failed to decode: {good.stderr}\n{good.stdout}"
> +    t.tlv_read(t.corrupted_bin, expect_failure=True)
> +    t.tlv_read(t.tampered_bin, verify=t.pubkey_rsa, magic="0x61bb95f3", expect_failure=True)
> +    t.tlv_read(t.tampered_ecdsa_bin, verify=t.pubkey_ecdsa, magic="0x61bb95f3", expect_failure=True)
>  
> -    bad = t.generator(["--output-data", str(t.dir / 'bad.yaml'), str(t.schema), str(t.corrupted_bin)], check=False)
> -    assert bad.returncode != 0, "unsigned TLV with invalid CRC unexpectedly decoded successfully"
>  
> -def test_tlv_command(barebox, barebox_config, tlv_testdata):
> +@pytest.fixture(scope="module")
> +def tlv_cmdtest(barebox_config, tlv_testdata):
>      skip_disabled(barebox_config, "CONFIG_CMD_TLV")
> -    t = tlv_testdata
> -    with open(t.data, 'r', encoding='utf-8') as f:
> -        yaml_lines = [l.strip() for l in f if l.strip() and not l.strip().startswith('#')]
> -
> -    stdout = barebox.run_check(f"tlv /mnt/9p/testfs/{t.unsigned_bin.name}")
> -
> -    # work around 9pfs printing here after a failed network test
> -    tlv_offset = next((i for i, line in enumerate(stdout) if line.startswith("tlv")), None)
> -    tlv_lines = stdout[tlv_offset + 1:-1]
> -
> -    assert len(yaml_lines) == len(tlv_lines), \
> -        f"YAML and TLV output line count mismatch for {t.unsigned_bin.name}"
> -
> -    for yline, tline in zip(yaml_lines, tlv_lines):
> -        m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
> -        assert m, f"malformed tlv line: {tline}"
> -        tkey, tval = m.group(1), m.group(2)
> -        m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
> -        assert m, f"malformed yaml line: {yline}"
> -        ykey, yval = m.group(1), m.group(2) or m.group(3)
> -        assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
> -        assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS")
> +
> +    class _TLV_Cmdtest:
> +        def __init__(self, tlv_testdata):
> +            self.t = tlv_testdata
> +            with open(tlv_testdata.data, "r", encoding="utf-8") as f:
> +                self.yaml_lines = [
> +                    l.strip() for l in f if l.strip() and not l.strip().startswith("#")
> +                ]
> +
> +        def test(self, barebox, fn, fail=False):
> +            cmd = f"tlv /mnt/9p/testfs/{fn}"
> +            stdout, stderr, exitcode = barebox.run(cmd, timeout=2)
> +            if fail:
> +                assert exitcode != 0
> +                return
> +            elif exitcode != 0:
> +                raise RuntimeError(f"`{cmd}` failed with exitcode {exitcode}:\n{stderr}\n{stdout}")
> +
> +            # work around a corner case of 9pfs printing here (after a failed network test?)
> +            tlv_offset = next((i for i, line in enumerate(stdout) if line.startswith("tlv")), None)
> +            tlv_lines = stdout[tlv_offset + 1 : -1]
> +
> +            assert len(self.yaml_lines) == len(tlv_lines), (
> +                f"YAML and TLV output line count mismatch for {fn}"
> +            )
> +
> +            for yline, tline in zip(self.yaml_lines, tlv_lines):
> +                m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
> +                assert m, f"malformed tlv line: {tline}"
> +                tkey, tval = m.group(1), m.group(2)
> +                m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
> +                assert m, f"malformed yaml line: {yline}"
> +                ykey, yval = m.group(1), m.group(2) or m.group(3)
> +                assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
> +                assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"
> +
> +    return _TLV_Cmdtest(tlv_testdata)
> +
> +
> +def test_tlv_cmd_unsigned(barebox, barebox_config, tlv_cmdtest):
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.unsigned_bin.name)
> +
> +
> +def test_tlv_cmd_signed(barebox, barebox_config, tlv_cmdtest):
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.signed_bin.name)
> +
> +
> +def test_tlv_cmd_ecdsa_signed(barebox, barebox_config, tlv_cmdtest):
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_ECDSA")
> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.ecdsa_signed_bin.name)
> +
> +
> +def test_tlv_cmd_corrupted(barebox, barebox_config, tlv_cmdtest):
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.corrupted_bin.name, fail=True)
> +
> +
> +def test_tlv_cmd_tampered(barebox, barebox_config, tlv_cmdtest):
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.tampered_bin.name, fail=True)
> +
> +
> +def test_tlv_cmd_ecdsa_tampered(barebox, barebox_config, tlv_cmdtest):
> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_ECDSA")
> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.tampered_ecdsa_bin.name, fail=True)
> 

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

Re: [PATCH 12/15] test: py: add signature to TLV integration tests

[Ahmad Fatoum]

On 10/22/25 12:04 PM, Ahmad Fatoum wrote:
> Hi,
> 
> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>> Add TLV signature to TLV integration tests:
>>  - Signed TLV using development RSA key
>>  - Modify payload and fix CRC for a "tampered" tlv
>>  - Include both cases in generator and tlv-command tests.
>>
>> Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
>> testing. Consequentially add the matching private keys from the public
>> repository at [1].
>>
>> [1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/
>>
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>>  crypto/fit-4096-development.key  |  51 ++++++++++
>>  crypto/fit-ecdsa-development.key |   5 +
> 
> Move this into test/?

Ah, I see the *.crt files are already in crypto...
Can't you concatenate the *.key and *.crt files into a single pem file?

That's what we do for test/self/development_rsa2048.pem and it works
there. Removes clutter a bit.

Cheers,
Ahmad

> 
>>  test/py/test_tlv.py              | 205 +++++++++++++++++++++++++++++++--------
>>  3 files changed, 219 insertions(+), 42 deletions(-)
>>
>> diff --git a/crypto/fit-4096-development.key b/crypto/fit-4096-development.key
>> new file mode 100644
>> index 0000000000..526cdfc2b5
>> --- /dev/null
>> +++ b/crypto/fit-4096-development.key
>> @@ -0,0 +1,51 @@
>> +-----BEGIN RSA PRIVATE KEY-----
>> +MIIJKgIBAAKCAgEAyZHkijUfqoAvEELaxSLnjyqhTprEilnf7JqvSCMDUUXv2dEl
>> +k1r4RwiBowJp/4W3sOx4gASEHM2xlDWyPYZUR/1btZeVJvOIRPWfw8JoLT3tbbST
>> +OIw04Bk6MUh3LbgBtxbbKGGkFewq3Ob1XQOWcY3ZAzfLFuooPWJQ6X+IiczkrA0r
>> +GnwpuhHlb8tOdQZRjDevIVVvEkRjRiqrAw5pKTy/Mt/SJJ/yC7qJptIQskQ42y3R
>> +qHeCVmP6ZF6VV1scNmHr8+kRD19DhCos6DLWq2pCFPwnSmgM4T0FWcJsMiNta+rt
>> +Rq+kG7RjOOYbbqvuk3vkMrQTRQeAYfdnuOGimGQYxiVh9quOMVG6NJ2hylTa0S/E
>> +PKQUQvK9A8bnDul522XPmMVHOtLXVGtwKx9xQUx/2D7aoqlmVJSGQeAMNi0NEFhq
>> +buGdXuJ/2cKwtobClkz0WbbMlI4UBM7V4qP3JSkxiojRKhtNHtdE3KtF3ronRJaU
>> ++yDggNobWLiJ4TtQ0OAC1REEJGq7s9k8ASi+6s+VX7yVtlGWMIjbBGAl8lqgXXsA
>> +prRrmtofaQgEPVvcSAIbuch/JqpHhs8vHBiWb/KZdOe/vMiYQE/d9/KY8rybZa3D
>> +hKvzN7X59ymOYLILHB73Xxi5bQA61DaeYE4KnPiJaEDrUccrlFjMBIwGrosCAwEA
>> +AQKCAgEApVkIIFdzommEMdKloxD+4nIV4GUU1GjlRzGcl5AhKIo2NndaW4ZEJADW
>> +VuGkEfeet4NDVcBen0IcaXeivtVyTZuHn2646zrajbbvV6Yhzvr9yQBXxAs/VJVd
>> +JxBKszY+MfKN1JJEB7ezcYIDxEktH/k8C2e5MRLj73a26NO1LVTmQDyNHyy7Deeg
>> +ThR4R4bnXh5PiwiKFHIE/YoCvn8TxMAQF6uCtoh+BSD/ydiH2bQc766mTYu7XyKk
>> +Q7FS0FXszq+E3pBRbkq3F7OBIviRIAwKKSyvDlpMNnfX68mQ95AYMm6ENXffJtrS
>> +ido4ppBjJJh8mRses4FzzukkLITq2qBoZQn+3XfR12+YbWKbCFIAXSu1b4tJetLC
>> +wYUp6EuGKCS2XK8OJXbY4M2D1t7bCVlRpptZBBiD7romkLYQ+jRwPbWhjUY6Ktw3
>> +ceUf9XJtNVKjHMp7n6C3gdxe2ivK02RYscT3brRq7TUjSzGjH1z/HUQSwr31+tXD
>> +dw2fkb2qQn2KUB5hjKcqU/Dxrqjorvf+kjGwXTtAD01Y4r8xRD3HK31zKAgrheN6
>> +15shoKRM4imKCD+fTBQjBBVZpTT8xNbo1m+y0joFEeDW0U5Lc3A/DhyTUsfHj4Im
>> +H02Cg4wiXXGyJ57fSyyNFKaa0EuLSdOl0zT1bXiy2TxiyTrsgAECggEBAPz+DcTO
>> +OvWtU/f+SyAfP86xd3bSnQzBXtoK2iI49uGAAkIXLU881V5sFz41UWJ6g1G0PjKy
>> +FWjxTCJytgso9TkISC42TOTE9VUqL4Y4KHhY93nnMAzKNLJC04onqmimDjn1I5Di
>> +DD3k3yCYPQEPInz84tdZyB+mRSncdsOL0Mpzhl5fjgGy+pi78K3/B4PUepR3n+Wl
>> +4JqKP4SeIL189+ChnSrgVsLzdvpOWJ2cu8DO9qGfz7F0ufJlehUILWPfhYlWUZ/c
>> +AUja5QWJEuSQHsKcOJSD4fpjuBWy3ASKlDy7BQSSoASibsl8FfQ/WmQBd3H8Y5/U
>> +20psjFuOa/02TusCggEBAMv3V4ccYieiUNkzi6WyyzlR9sULtAG4Cvi1HRw+o6mV
>> +VeNFNo8hw+g8f26URvuB06xXyuZepk+oMtEPiFfGYFFg4s22QJRGzqBfC5+8//Mf
>> +rcIsU88S75JCZjPDSxOFgzSDAG1gPfX4i8BZHgqaT749TewbeLc0ehvVrcLnXMwB
>> +3JpDNmiuNzA2pJAWbLezKazhW6XbpkTtHDqZTswDK+3AFBm7j/cqqeXojd93EbrV
>> +0ggyiNMx/O42DHVwZ+51HdJ+C7KDHR0wzgFMu24zyoymzZOaiKjm2rQi/B4mJ9Er
>> +oCaCfhVGo/Kq7Y5V7G+x4gag8oVQNCJh/lERrvgBduECggEAT5tpnbn/F3tY5rof
>> +zZXHsDRrkPoo7PCT9ixgA1DFbqOnEkDUwxAzW6jLj4mbeE9wru72e2FKF2GGQXiz
>> +C8PxleajP9daTsojII9LsQJOyb/E75jtp7ig6E7a3agpmRBXfalDbb2TeI5iH5GH
>> +8KNgiM/SWU0pCbx6GvgCbvm501qSt3N97c7xx8mrrDSJmtPrVnhl2g9eI4LJBeP0
>> +DWwbW5W/LNS2uFV/5Ldubvn4omz9clIlOoOuVzXTOnb+QWT+Uf7VZGYICXLHifxd
>> +84neBALAUwtEulNSg5FqZgttJcb7hzrUG2E5VzEyf07IFJvZiAaRGqQR9NM/PzgL
>> +hvvlzQKCAQEAi/wmy2kUiKUjHd79oexzA9UYKyacFW3twcHzx7XJ95Kxjriq+FMx
>> +NIuI3ijQCr+QukDK1Y7yT8tdjRQ+/Bb/dfqrzomeCuYJ3BE/VhOOCpucUp6/qmgR
>> +mm0N3crUFQLWCM08FtUt0UoTCCFht98uiZ9jgn9cO0i94aqmhhTqIG3KrOkiR3gC
>> +Eon+KZHqba1+FdPZZZy5oaamcCVV6jjnBlaEtSCAbx+N2WfhLxR2S6eCbfPY6jHt
>> +qMPZiyRpgERLAnNVrd/EtIsRZ9z06m6LPjsg7oPp9Rnz0hwMsth3DV0GnkeDJzED
>> +RoI/ZifcjNAmE2yU5iAkl9Bvjc44Kqg+oQKCAQEAvSi4W2kVUoIwlmBHGLge/Rng
>> +YyScmAoG4Cavy0Ie6AHPtHayHFdI/rAyiVFnKU5Xuj4qgB54dLa94bQrIu451wls
>> +3Jyy/J8WkcW9r/dZFMN6gMoZ0u+xt6KdYe2tQnyW/CG4svDyfckcW2VHdh2A3vqH
>> +xlGNmo/HaOeovxWNQkQGQeuXnIcrUvwaFTmGIxLdEO5TAQzLeWSrXldMtVBUAMaJ
>> +LClOqNIGRxMRYhZOPVnkedEQmJqgxvcrn8F/91mXQHVnQBOvsgyQDgtS3V0EIAOD
>> +rWePjgB8twJknHuab8qH/1z3cQ5QRxQ6lffcIoWgXS59QBBT+jIqMT2oKyGkPw==
>> +-----END RSA PRIVATE KEY-----
>> diff --git a/crypto/fit-ecdsa-development.key b/crypto/fit-ecdsa-development.key
>> new file mode 100644
>> index 0000000000..2b13c877a3
>> --- /dev/null
>> +++ b/crypto/fit-ecdsa-development.key
>> @@ -0,0 +1,5 @@
>> +-----BEGIN EC PRIVATE KEY-----
>> +MHcCAQEEIEsUW5DEOhD1CYHCnPfDULwbRQO9Yjt2/xM5SoY2GUQtoAoGCCqGSM49
>> +AwEHoUQDQgAEowCa2OYfPdGRr1JpSYONOA3N2jwJjGbPbfG6uBzKg1VqOOk0a/Vf
>> +BfEbQev6X96HCd6zvvC2tjBgvICW8UB0TQ==
>> +-----END EC PRIVATE KEY-----
>> diff --git a/test/py/test_tlv.py b/test/py/test_tlv.py
>> index 79f9f9d01b..1200281dbc 100644
>> --- a/test/py/test_tlv.py
>> +++ b/test/py/test_tlv.py
>> @@ -1,6 +1,7 @@
>>  import os
>>  import re
>>  import subprocess
>> +import struct
>>  from pathlib import Path
>>  from .helper import skip_disabled
>>  
>> @@ -8,71 +9,191 @@ import pytest
>>  
>>  
>>  class _TLV_Testdata:
>> -    def generator(self, args, check=True):
>> +    def generator(self, args, expect_failure=False, input=None):
>>          cmd = [os.sys.executable, str(self.generator_py)] + args
>> -        res = subprocess.run(cmd, text=True)
>> +        res = subprocess.run(cmd, text=True, input=input, encoding="utf-8", capture_output=True)
>>          if res.returncode == 127:
>>              pytest.skip("test skipped due to missing host dependencies")
>> -
>> -        if check and res.returncode != 0:
>> -            raise RuntimeError(f"generator failed ({res.returncode}): {res.stdout}\n{res.stderr}")
>> +        if res.returncode == 0 and expect_failure:
>> +            raise RuntimeError(
>> +                f"`{' '.join(cmd)}` succeded unexpectedly:\n{res.stderr}\n{res.stdout}"
>> +            )
>> +        elif res.returncode != 0 and not expect_failure:
>> +            raise RuntimeError(
>> +                f"`{' '.join(cmd)}` failed unexpectedly with {res.returncode}:\n{res.stderr}\n{res.stdout}"
>> +            )
>>          return res
>>  
>> +    def overwrite_magic(self, new_magic):
>> +        with open(self.schema, "r", encoding="utf-8") as f:
>> +            patched_schema = "".join(
>> +                re.sub(r"^magic:\s*0x[a-fA-F0-9]{8}\s*$", f"magic: {new_magic}\n", line)
>> +                for line in f
>> +            )
>> +        return patched_schema
>> +
>> +    def tlv_gen(self, outfile, magic=None, sign=None):
>> +        param = ["--input-data", str(self.data)]
>> +        if sign:
>> +            param += ["--sign", str(sign)]
>> +        if magic:
>> +            param += ["/dev/stdin"]
>> +        else:
>> +            param += [str(self.schema)]
>> +        param += [str(outfile)]
>> +        ret = self.generator(param, input=self.overwrite_magic(magic) if magic else None)
>> +        assert outfile.exists(), f"TLV {outfile} not created from {' '.join(param)}"
>> +        return ret
>> +
>> +    def tlv_read(self, binfile, magic=None, verify=None, expect_failure=False):
>> +        param = ["--output-data", "/dev/null"]
>> +        if verify:
>> +            param += ["--verify", str(verify)]
>> +        if magic:
>> +            param += ["/dev/stdin"]
>> +        else:
>> +            param += [str(self.schema)]
>> +        param += [str(binfile)]
>> +        ret = self.generator(
>> +            param,
>> +            input=self.overwrite_magic(magic) if magic else None,
>> +            expect_failure=expect_failure,
>> +        )
>> +        return ret
>> +
>> +    def corrupt(self, fnin, fnout, fix_crc=False):
>> +        try:
>> +            from crcmod.predefined import mkPredefinedCrcFun
>> +        except ModuleNotFoundError:
>> +            pytest.skip("test skipped due to missing dependency python-crcmod")
>> +            return
>> +
>> +        _crc32_mpeg = mkPredefinedCrcFun("crc-32-mpeg")
>> +
>> +        with open(fnin, "r+b") as f:
>> +            data = bytearray(f.read())
>> +        data[0x20] ^= 1
>> +        if fix_crc:
>> +            crc_raw = _crc32_mpeg(data[:-4])
>> +            crc = struct.pack(">I", crc_raw)
>> +            data[-4:] = crc
>> +        with open(fnout, "wb") as f:
>> +            f.write(data)
>> +
>>      def __init__(self, testfs):
>>          self.dir = Path(testfs)
>>          self.scripts_dir = Path("scripts/bareboxtlv-generator")
>>          self.data = self.scripts_dir / "data-example.yaml"
>>          self.schema = self.scripts_dir / "schema-example.yaml"
>>          self.generator_py = self.scripts_dir / "bareboxtlv-generator.py"
>> -        self.unsigned_bin = self.dir / 'unsigned.tlv'
>> -        self.corrupted_bin = self.dir / 'unsigned_corrupted.tlv'
>> +        self.privkey_rsa = Path("crypto/fit-4096-development.key")
>> +        self.pubkey_rsa = Path("crypto/fit-4096-development.crt")
>> +        self.privkey_ecdsa = Path("crypto/fit-ecdsa-development.key")
>> +        self.pubkey_ecdsa = Path("crypto/fit-ecdsa-development.crt")
>> +        self.unsigned_bin = self.dir / "unsigned.tlv"
>> +        self.corrupted_bin = self.dir / "unsigned_corrupted.tlv"
>> +        self.signed_bin = self.dir / "signed.tlv"
>> +        self.ecdsa_signed_bin = self.dir / "ecdsa-signed.tlv"
>> +        self.tampered_bin = self.dir / "signed-tampered.tlv"
>> +        self.tampered_ecdsa_bin = self.dir / "ecdsa-signed-tampered.tlv"
>> +
>>  
>>  @pytest.fixture(scope="module")
>>  def tlv_testdata(testfs):
>>      t = _TLV_Testdata(testfs)
>> -    t.generator(["--input-data", str(t.data), str(t.schema), str(t.unsigned_bin)])
>> -    assert t.unsigned_bin.exists(), "unsigned TLV not created"
>>  
>> -    with open(t.unsigned_bin, 'r+b') as f:
>> -        data = bytearray(f.read())
>> -    data[0x20] ^= 1
>> -    with open(t.corrupted_bin, "wb") as f:
>> -        f.write(data)
>> +    t.tlv_gen(t.unsigned_bin)
>> +    t.tlv_gen(t.signed_bin, sign=t.privkey_rsa, magic="0x61bb95f3")
>> +    t.tlv_gen(t.ecdsa_signed_bin, sign=t.privkey_ecdsa, magic="0x61bb95f3")
>> +
>> +    t.corrupt(t.unsigned_bin, t.corrupted_bin)
>> +    t.corrupt(t.signed_bin, t.tampered_bin, fix_crc=True)
>> +    t.corrupt(t.ecdsa_signed_bin, t.tampered_ecdsa_bin, fix_crc=True)
>>  
>>      return t
>>  
>> +
>>  def test_tlv_generator(tlv_testdata):
>>      t = tlv_testdata
>> -    out_yaml = t.dir / 'out.yaml'
>> +    out_yaml = t.dir / "out.yaml"
>>  
>> +    t.tlv_read(t.unsigned_bin)
>> +    t.tlv_read(t.signed_bin, verify=t.pubkey_rsa, magic="0x61bb95f3")
>> +    t.tlv_read(t.ecdsa_signed_bin, verify=t.pubkey_ecdsa, magic="0x61bb95f3")
>>  
>> -    good = t.generator(["--output-data", str(out_yaml), str(t.schema), str(t.unsigned_bin)], check=False)
>> -    assert good.returncode == 0, f"valid unsigned TLV failed to decode: {good.stderr}\n{good.stdout}"
>> +    t.tlv_read(t.corrupted_bin, expect_failure=True)
>> +    t.tlv_read(t.tampered_bin, verify=t.pubkey_rsa, magic="0x61bb95f3", expect_failure=True)
>> +    t.tlv_read(t.tampered_ecdsa_bin, verify=t.pubkey_ecdsa, magic="0x61bb95f3", expect_failure=True)
>>  
>> -    bad = t.generator(["--output-data", str(t.dir / 'bad.yaml'), str(t.schema), str(t.corrupted_bin)], check=False)
>> -    assert bad.returncode != 0, "unsigned TLV with invalid CRC unexpectedly decoded successfully"
>>  
>> -def test_tlv_command(barebox, barebox_config, tlv_testdata):
>> +@pytest.fixture(scope="module")
>> +def tlv_cmdtest(barebox_config, tlv_testdata):
>>      skip_disabled(barebox_config, "CONFIG_CMD_TLV")
>> -    t = tlv_testdata
>> -    with open(t.data, 'r', encoding='utf-8') as f:
>> -        yaml_lines = [l.strip() for l in f if l.strip() and not l.strip().startswith('#')]
>> -
>> -    stdout = barebox.run_check(f"tlv /mnt/9p/testfs/{t.unsigned_bin.name}")
>> -
>> -    # work around 9pfs printing here after a failed network test
>> -    tlv_offset = next((i for i, line in enumerate(stdout) if line.startswith("tlv")), None)
>> -    tlv_lines = stdout[tlv_offset + 1:-1]
>> -
>> -    assert len(yaml_lines) == len(tlv_lines), \
>> -        f"YAML and TLV output line count mismatch for {t.unsigned_bin.name}"
>> -
>> -    for yline, tline in zip(yaml_lines, tlv_lines):
>> -        m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
>> -        assert m, f"malformed tlv line: {tline}"
>> -        tkey, tval = m.group(1), m.group(2)
>> -        m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
>> -        assert m, f"malformed yaml line: {yline}"
>> -        ykey, yval = m.group(1), m.group(2) or m.group(3)
>> -        assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
>> -        assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS")
>> +
>> +    class _TLV_Cmdtest:
>> +        def __init__(self, tlv_testdata):
>> +            self.t = tlv_testdata
>> +            with open(tlv_testdata.data, "r", encoding="utf-8") as f:
>> +                self.yaml_lines = [
>> +                    l.strip() for l in f if l.strip() and not l.strip().startswith("#")
>> +                ]
>> +
>> +        def test(self, barebox, fn, fail=False):
>> +            cmd = f"tlv /mnt/9p/testfs/{fn}"
>> +            stdout, stderr, exitcode = barebox.run(cmd, timeout=2)
>> +            if fail:
>> +                assert exitcode != 0
>> +                return
>> +            elif exitcode != 0:
>> +                raise RuntimeError(f"`{cmd}` failed with exitcode {exitcode}:\n{stderr}\n{stdout}")
>> +
>> +            # work around a corner case of 9pfs printing here (after a failed network test?)
>> +            tlv_offset = next((i for i, line in enumerate(stdout) if line.startswith("tlv")), None)
>> +            tlv_lines = stdout[tlv_offset + 1 : -1]
>> +
>> +            assert len(self.yaml_lines) == len(tlv_lines), (
>> +                f"YAML and TLV output line count mismatch for {fn}"
>> +            )
>> +
>> +            for yline, tline in zip(self.yaml_lines, tlv_lines):
>> +                m = re.match(r'^\s*([^=]+) = "(.*)";$', tline)
>> +                assert m, f"malformed tlv line: {tline}"
>> +                tkey, tval = m.group(1), m.group(2)
>> +                m = re.match(r'^([^:]+):\s*(?:"([^"]*)"\s*|(.*))$', yline)
>> +                assert m, f"malformed yaml line: {yline}"
>> +                ykey, yval = m.group(1), m.group(2) or m.group(3)
>> +                assert ykey == tkey, f"key mismatch: {ykey} != {tkey}"
>> +                assert str(yval) == str(tval), f"value mismatch for {ykey}: {yval} != {tval}"
>> +
>> +    return _TLV_Cmdtest(tlv_testdata)
>> +
>> +
>> +def test_tlv_cmd_unsigned(barebox, barebox_config, tlv_cmdtest):
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
>> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.unsigned_bin.name)
>> +
>> +
>> +def test_tlv_cmd_signed(barebox, barebox_config, tlv_cmdtest):
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
>> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.signed_bin.name)
>> +
>> +
>> +def test_tlv_cmd_ecdsa_signed(barebox, barebox_config, tlv_cmdtest):
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_ECDSA")
>> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.ecdsa_signed_bin.name)
>> +
>> +
>> +def test_tlv_cmd_corrupted(barebox, barebox_config, tlv_cmdtest):
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
>> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.corrupted_bin.name, fail=True)
>> +
>> +
>> +def test_tlv_cmd_tampered(barebox, barebox_config, tlv_cmdtest):
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_RSA")
>> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.tampered_bin.name, fail=True)
>> +
>> +
>> +def test_tlv_cmd_ecdsa_tampered(barebox, barebox_config, tlv_cmdtest):
>> +    skip_disabled(barebox_config, "CONFIG_CRYPTO_ECDSA")
>> +    tlv_cmdtest.test(barebox, tlv_cmdtest.t.tampered_ecdsa_bin.name, fail=True)
>>
> 

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

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

[Jonas Rebmann]

Hi Ahmad,

Just answering to what I don't immediately apply for v2:

On 2025-10-22 12:00, Ahmad Fatoum wrote:
>> +/*
>> + * Retrieve length of header+TLVs (offset of spki hash part of signature if available)
>> + */
>> +
>> +static inline size_t tlv_spki_hash_offset(const struct tlv_header *header)
>> +{
>> +	size_t ret = size_add(sizeof(struct tlv_header), get_unaligned_be32(&header->length_tlv));
>>   
>>   	return ret; /* SIZE_MAX on overflow */
> 
> Shouldn't you then check for SIZE_MAX at callsites?

I chose not to implement this check in all places using those
size/offset helpers.

In "[PATCH 01/15] common: clean up TLV code", I check early in the TLV
handling, that tlv_total_len() doesn't overflow. Later on, it is
guaranteed that calls to tlv_total_len(), tlv_spki_hash_offset() and the
such cannot overflow.

If I where to check at callsites of tlv_spki_hash_offset() I'd need to
check at all callsites of all TLV size/offset helperss, which seemed
unnecessary.

What do you think?

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    |

Re: [PATCH 10/15] common: tlv: default decoder for signed TLV

[Jonas Rebmann]

Hi,

On 2025-10-22 12:01, Ahmad Fatoum wrote:
> Hi,
> 
> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>> Introduce a second default encoder that behaves just like barebox_tlv_v1
>> but uses the "tlv" keyring.
>
> [...]
> 
>>>>   static struct tlv_decoder barebox_tlv_v1 = {
>>   	.magic = TLV_MAGIC_BAREBOX_V1,
>>   	.driver.name = "barebox-tlv-v1",
>>   	.driver.of_compatible = of_matches,
>>   	.mappings = mappings,
>> +	.signature_keyring = NULL,
> 
> I'd drop it given it's explicitly meant as optional parameter.

It is, but I'd like this to be a bit more explicit here. Something has
changed in barebox_tlv_v1 with my patch: It is now to be considered /the
unsinged decoder/ as opposed to barebox_tlv_v1_signed.

>> +};
>> +
>> +static struct tlv_decoder barebox_tlv_v1_signed = {
>> +	.magic = TLV_MAGIC_BAREBOX_V1_SIGNED,
>> +	.driver.name = "barebox-tlv-v1-signed",
>> +	.driver.of_compatible = of_matches_signed,
>> +	.mappings = mappings,
>> +	.signature_keyring = "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    |

Re: [PATCH 12/15] test: py: add signature to TLV integration tests

[Jonas Rebmann]

Hi,

On 2025-10-22 12:04, Ahmad Fatoum wrote:
> Hi,
> 
> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>> Add TLV signature to TLV integration tests:
>>   - Signed TLV using development RSA key
>>   - Modify payload and fix CRC for a "tampered" tlv
>>   - Include both cases in generator and tlv-command tests.
>>
>> Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
>> testing. Consequentially add the matching private keys from the public
>> repository at [1].
>>
>> [1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/
>>
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>>   crypto/fit-4096-development.key  |  51 ++++++++++
>>   crypto/fit-ecdsa-development.key |   5 +
> 
> Move this into test/?

I chose to move the keys in with their certificates:
crypto/fit-4096-development.crt
crypto/fit-ecdsa-development.crt

Should I separate them, or move the certificates too (and adjust all
references such as for CONFIG_CRYPTO_PUBLIC_KEYS)?

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    |

Re: [PATCH 12/15] test: py: add signature to TLV integration tests

[Ahmad Fatoum]

On 10/22/25 1:08 PM, Jonas Rebmann wrote:
> Hi,
> 
> On 2025-10-22 12:04, Ahmad Fatoum wrote:
>> Hi,
>>
>> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>>> Add TLV signature to TLV integration tests:
>>>   - Signed TLV using development RSA key
>>>   - Modify payload and fix CRC for a "tampered" tlv
>>>   - Include both cases in generator and tlv-command tests.
>>>
>>> Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
>>> testing. Consequentially add the matching private keys from the public
>>> repository at [1].
>>>
>>> [1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/
>>>
>>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>>> ---
>>>   crypto/fit-4096-development.key  |  51 ++++++++++
>>>   crypto/fit-ecdsa-development.key |   5 +
>>
>> Move this into test/?
> 
> I chose to move the keys in with their certificates:
> crypto/fit-4096-development.crt
> crypto/fit-ecdsa-development.crt
> 
> Should I separate them, or move the certificates too (and adjust all
> references such as for CONFIG_CRYPTO_PUBLIC_KEYS)?

I am not sure crypto/ is the best place for this, but given that you
aren't introducing them there, just leave it in crypto/ for now.

I think it would be best to combine crt and key into a single pem file
and adjust the Makefile, so we don't have loose files.

Cheers,
Ahmad

> 
> Regards,
> Jonas
> 

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

Re: [PATCH 11/15] crypto: Use "development" keys for "fit" and "tlv" keyring

[Jonas Rebmann]

Hi,

On 2025-10-22 12:02, Ahmad Fatoum wrote:
> 
> 
> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>> All users of the CONFIG_CRYPTO_PUBLIC_KEYS feature should update to the
>> new syntax making keyring selection mandatory.
>>
>> Instead of just making the addition of the builtin snakeoil keys
>> explicit for the "fit" key, also add them to the "tlv" key to use them
>> as a testing set for TLV keys too.
>>
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>>   crypto/Makefile | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/crypto/Makefile b/crypto/Makefile
>> index 08b9a46e4c..076ba4f686 100644
>> --- a/crypto/Makefile
>> +++ b/crypto/Makefile
>> @@ -33,10 +33,12 @@ CONFIG_CRYPTO_PUBLIC_KEYS := $(foreach d,$(CONFIG_CRYPTO_PUBLIC_KEYS),"$(d)")
>>   
>>   ifdef CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS
>>   ifdef CONFIG_CRYPTO_RSA
>> -CONFIG_CRYPTO_PUBLIC_KEYS += rsa-devel:$(srctree)/crypto/fit-4096-development.crt
>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=rsa-devel:$(srctree)/crypto/fit-4096-development.crt
>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-4096-development.crt
>>   endif
>>   ifdef CONFIG_CRYPTO_ECDSA
>> -CONFIG_CRYPTO_PUBLIC_KEYS += ecdsa-devel:$(srctree)/crypto/fit-ecdsa-development.crt
>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=ecdsa-devel:$(srctree)/crypto/fit-ecdsa-development.crt
>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-ecdsa-development.crt
> 
> We don't want people to overload tlv and instead use their own keyring
> names. This is already too late for fit, but for this, let's call it
> tlv-example?

This would mean in the previous patch for `struct tlv_decoder
barebox_tlv_v1_signed` to also set the keyring to "tlv-example". My
understanding of the decoders in common/tlv/barebox.c is that they are
more than an example, rather an offer that for somewhat generic use,
somewhat generic decoders are provided. Hence the very generic keyring
name "tlv".

I need to use the barebox_tlv_v1_signed in the tests and I want to make
use of CONFIG_CRYPTO_PUBLIC_KEYS for that. I'm not sure if I understand
the role of CONFIG_CRYPTO_PUBLIC_KEYS and the decoders in
common/tlv/barebox.c correctly though.

Maybe we rename this keyring to "tlv-generic"?

If it helps, I could update the commit message to discourage use of a
catchall keyring in more sophisticated setups.

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    |

Re: [PATCH 11/15] crypto: Use "development" keys for "fit" and "tlv" keyring

[Ahmad Fatoum]

Hi,

On 10/22/25 1:17 PM, Jonas Rebmann wrote:
> Hi,
> 
> On 2025-10-22 12:02, Ahmad Fatoum wrote:
>>
>>
>> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>>> All users of the CONFIG_CRYPTO_PUBLIC_KEYS feature should update to the
>>> new syntax making keyring selection mandatory.
>>>
>>> Instead of just making the addition of the builtin snakeoil keys
>>> explicit for the "fit" key, also add them to the "tlv" key to use them
>>> as a testing set for TLV keys too.
>>>
>>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>>> ---
>>>   crypto/Makefile | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/crypto/Makefile b/crypto/Makefile
>>> index 08b9a46e4c..076ba4f686 100644
>>> --- a/crypto/Makefile
>>> +++ b/crypto/Makefile
>>> @@ -33,10 +33,12 @@ CONFIG_CRYPTO_PUBLIC_KEYS := $(foreach d,
>>> $(CONFIG_CRYPTO_PUBLIC_KEYS),"$(d)")
>>>     ifdef CONFIG_CRYPTO_BUILTIN_DEVELOPMENT_KEYS
>>>   ifdef CONFIG_CRYPTO_RSA
>>> -CONFIG_CRYPTO_PUBLIC_KEYS += rsa-devel:$(srctree)/crypto/fit-4096-
>>> development.crt
>>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=rsa-devel:
>>> $(srctree)/crypto/fit-4096-development.crt
>>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-4096-
>>> development.crt
>>>   endif
>>>   ifdef CONFIG_CRYPTO_ECDSA
>>> -CONFIG_CRYPTO_PUBLIC_KEYS += ecdsa-devel:$(srctree)/crypto/fit-
>>> ecdsa-development.crt
>>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=fit,fit-hint=ecdsa-devel:
>>> $(srctree)/crypto/fit-ecdsa-development.crt
>>> +CONFIG_CRYPTO_PUBLIC_KEYS += keyring=tlv:$(srctree)/crypto/fit-
>>> ecdsa-development.crt
>>
>> We don't want people to overload tlv and instead use their own keyring
>> names. This is already too late for fit, but for this, let's call it
>> tlv-example?
> 
> This would mean in the previous patch for `struct tlv_decoder
> barebox_tlv_v1_signed` to also set the keyring to "tlv-example". My
> understanding of the decoders in common/tlv/barebox.c is that they are
> more than an example, rather an offer that for somewhat generic use,
> somewhat generic decoders are provided. Hence the very generic keyring
> name "tlv".
> 
> I need to use the barebox_tlv_v1_signed in the tests and I want to make
> use of CONFIG_CRYPTO_PUBLIC_KEYS for that. I'm not sure if I understand
> the role of CONFIG_CRYPTO_PUBLIC_KEYS and the decoders in
> common/tlv/barebox.c correctly though.
> 
> Maybe we rename this keyring to "tlv-generic"?

Ok.

> If it helps, I could update the commit message to discourage use of a
> catchall keyring in more sophisticated setups.

You can do that, but it probably should be in the docs or in the Kconfig
help text to have a chance of being read.

Cheers,
Ahmad

> 
> Regards,
> Jonas
> 

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

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

[Ahmad Fatoum]

Hi Jonas,

On 10/22/25 12:43 PM, Jonas Rebmann wrote:
> Hi Ahmad,
> 
> Just answering to what I don't immediately apply for v2:
> 
> On 2025-10-22 12:00, Ahmad Fatoum wrote:
>>> +/*
>>> + * Retrieve length of header+TLVs (offset of spki hash part of
>>> signature if available)
>>> + */
>>> +
>>> +static inline size_t tlv_spki_hash_offset(const struct tlv_header
>>> *header)
>>> +{
>>> +    size_t ret = size_add(sizeof(struct tlv_header),
>>> get_unaligned_be32(&header->length_tlv));
>>>         return ret; /* SIZE_MAX on overflow */
>>
>> Shouldn't you then check for SIZE_MAX at callsites?
> 
> I chose not to implement this check in all places using those
> size/offset helpers.
> 
> In "[PATCH 01/15] common: clean up TLV code", I check early in the TLV
> handling, that tlv_total_len() doesn't overflow. Later on, it is
> guaranteed that calls to tlv_total_len(), tlv_spki_hash_offset() and the
> such cannot overflow.
> 
> If I where to check at callsites of tlv_spki_hash_offset() I'd need to
> check at all callsites of all TLV size/offset helperss, which seemed
> unnecessary.
> 
> What do you think?

If it's already checked, you don't need to repeat the check.

Thanks,
Ahmad

> 
> Regards,
> Jonas
> 

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

Re: [PATCH 12/15] test: py: add signature to TLV integration tests

[Jonas Rebmann]

Hi,

On 2025-10-22 12:11, Ahmad Fatoum wrote:
> 
> 
> On 10/22/25 12:04 PM, Ahmad Fatoum wrote:
>> Hi,
>>
>> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>>> Add TLV signature to TLV integration tests:
>>>   - Signed TLV using development RSA key
>>>   - Modify payload and fix CRC for a "tampered" tlv
>>>   - Include both cases in generator and tlv-command tests.
>>>
>>> Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
>>> testing. Consequentially add the matching private keys from the public
>>> repository at [1].
>>>
>>> [1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/
>>>
>>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>>> ---
>>>   crypto/fit-4096-development.key  |  51 ++++++++++
>>>   crypto/fit-ecdsa-development.key |   5 +
>>
>> Move this into test/?
> 
> Ah, I see the *.crt files are already in crypto...
> Can't you concatenate the *.key and *.crt files into a single pem file?
> 
> That's what we do for test/self/development_rsa2048.pem and it works
> there. Removes clutter a bit.

I'd prefer not to. I suppose our tooling supports this, users that
utilize CRYPTO_BUILTIN_DEVELOPMENT_KEYS for testing may not; and they
should not have to pick apart private and public key again.

I'd consider concatenating them most of the time not the best practice.
You'll have a file of which `file` tells you it's an "OpenSSH public
key", but if you open it and then scroll down, you realize it's a
private key.

Yes this particular private key is all but private but lets not endorse
this practice.

Keeping them separates also makes it visible where we use the private
key: We need it when creating the signed TLVs in test/py/test_tlv.py and
only there.

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    |

Re: [PATCH 12/15] test: py: add signature to TLV integration tests

[Ahmad Fatoum]

Hi,

On 10/22/25 2:28 PM, Jonas Rebmann wrote:
> Hi,
> 
> On 2025-10-22 12:11, Ahmad Fatoum wrote:
>>
>>
>> On 10/22/25 12:04 PM, Ahmad Fatoum wrote:
>>> Hi,
>>>
>>> On 10/14/25 1:03 PM, Jonas Rebmann wrote:
>>>> Add TLV signature to TLV integration tests:
>>>>   - Signed TLV using development RSA key
>>>>   - Modify payload and fix CRC for a "tampered" tlv
>>>>   - Include both cases in generator and tlv-command tests.
>>>>
>>>> Use the keys selected by CRYPTO_BUILTIN_DEVELOPMENT_KEYS for all TLV
>>>> testing. Consequentially add the matching private keys from the public
>>>> repository at [1].
>>>>
>>>> [1]: https://git.pengutronix.de/cgit/ptx-code-signing-dev/
>>>>
>>>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>>>> ---
>>>>   crypto/fit-4096-development.key  |  51 ++++++++++
>>>>   crypto/fit-ecdsa-development.key |   5 +
>>>
>>> Move this into test/?
>>
>> Ah, I see the *.crt files are already in crypto...
>> Can't you concatenate the *.key and *.crt files into a single pem file?
>>
>> That's what we do for test/self/development_rsa2048.pem and it works
>> there. Removes clutter a bit.
> 
> I'd prefer not to. I suppose our tooling supports this, users that
> utilize CRYPTO_BUILTIN_DEVELOPMENT_KEYS for testing may not; and they
> should not have to pick apart private and public key again.

Which users? These keys are for barebox-internal consumption.

> I'd consider concatenating them most of the time not the best practice.
> You'll have a file of which `file` tells you it's an "OpenSSH public
> key", but if you open it and then scroll down, you realize it's a
> private key.
> 
> Yes this particular private key is all but private but lets not endorse
> this practice.

I don't buy this argument.

> Keeping them separates also makes it visible where we use the private
> key: We need it when creating the signed TLVs in test/py/test_tlv.py and
> only there.

The private key we already have in tree are piggy backing on the public
key. I think we should do the same here as well.

Cheers,
Ahmad

> 
> Regards,
> Jonas
> 

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