[PATCH RFC 0/4] efi: CapsuleUpdate: support for dynamic GUIDs
As more boards adopt support for the EFI CapsuleUpdate mechanism, there is a growing issue of being able to target updates to them properly. The current mechanism of hardcoding UUIDs for each board at compile time is unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
In this series, I propose that we adopt v5 GUIDs, these are generated by using a well-known salt GUID as well as board specific information (like the model/revision), these are hashed together and the result is truncated to form a new UUID.
The well-known salt GUID can be specific to the architecture (SoC vendor), or OEM. Exact rules on how these are used (e.g. if vendors should be told to generate their own for their products, and if that can be added upstream, etc) will need to be decided.
Specifically, the following fields are used to generate a GUID for a particular fw_image:
* namespace salt * soc name * board model (usually from dt root model property) * board compatible (usually the first entry in the dt root compatible array). * fw_image name (the string identifying the specific image, especially relevant for board that can update multiple images).
Once generated, the GUIDs can be printed with the "%pUs" format string, these can then be stored externally to U-Boot.
The SoC name field might be controversial, it could be generated from the last entry in the dt root compatible in most cases, or in some board specific way. It might make sense to remove this field if it is unfeasible for some boards.
== Usage ==
Boards can integrate dynamic UUID support as follows:
1. Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if EFI_HAVE_CAPSULE_SUPPORT 2. Skip setting the fw_images image_type_id property. 3. In board_init() (or anywhere before the EFI subsystem is initialised), add a call to efi_capsule_update_info_gen_ids() with the board specific info.
== Limitations ==
* Changing GUIDs
The primary limitation with this approach is that if any of the source fields change, so will the GUID for the board. It is therefore pretty important to ensure that GUID changes are caught during development.
* Supporting multiple boards with a single image
This now requires having an entry with the GUID for every board which might lead to larger UpdateCapsule images.
== Tooling ==
Not part of this RFC is a tool to generate the GUIDs outside of U-Boot. I suspect this might be a requirement, but it makes sense to decide on what fields we use first.
The tool should take in the salt, DTB, and a list of fw_image names. It could also accept values to overwrite the individual fields if they aren't derived from the DTB for some reason. It would then generate the expected GUID.
A potential idea here would be to integrate this into the build system so that it prints a warning if the GUID changes.
== TOOD ==
Missing from this RFC are unit tests for the dynamic UUID feature, these will be implemented for future revisions.
I would appreciate any feedback on the above.
This follows a related discussion started by Ilias: https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72Wk...
--- Caleb Connolly (4): lib: uuid: add UUID v5 support efi: add a helper to generate dynamic UUIDs doc: uefi: document dynamic GUID generation sandbox: switch to dynamic UUIDs
arch/Kconfig | 1 + board/sandbox/sandbox.c | 28 +++++++++++++++------------- doc/develop/uefi/uefi.rst | 35 +++++++++++++++++++++++++++++++++++ include/efi_loader.h | 28 ++++++++++++++++++++++++++++ include/uuid.h | 16 ++++++++++++++++ lib/Kconfig | 8 ++++++++ lib/efi_loader/Kconfig | 14 ++++++++++++++ lib/efi_loader/efi_capsule.c | 33 +++++++++++++++++++++++++++++++++ lib/uuid.c | 33 +++++++++++++++++++++++++++++++++ 9 files changed, 183 insertions(+), 13 deletions(-) --- change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27 base-commit: d097f9e1299a3bdb7de468f0d9bbc63932f461cd
// Caleb (they/them)
Add support for generate version 5 UUIDs, these are determistic and work by hashing a "namespace" UUID together with some unique data. One intended usecase is to allow for dynamically generate payload UUIDs for UEFI capsule updates, so that supported boards can have their own UUIDs without needing to hardcode them.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- include/uuid.h | 16 ++++++++++++++++ lib/Kconfig | 8 ++++++++ lib/uuid.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+)
diff --git a/include/uuid.h b/include/uuid.h index f5a941250f48..229b938d866a 100644 --- a/include/uuid.h +++ b/include/uuid.h @@ -142,8 +142,24 @@ void gen_rand_uuid(unsigned char *uuid_bin); * @param - uuid output type: UUID - 0, GUID - 1 */ void gen_rand_uuid_str(char *uuid_str, int str_format);
+#if CONFIG_IS_ENABLED(UUID_GEN_V5) +/** + * gen_uuid_v5() - generate UUID v5 from namespace and other seed data. + * + * @namespace: pointer to UUID namespace salt + * @uuid: pointer to allocated UUID output + * @...: NULL terminated list of seed data as pairs of pointers + * to data and their lengths + */ +void gen_uuid_v5(struct uuid *namespace, struct uuid *uuid, ...); +#else +static inline void gen_uuid_v5(struct uuid *namespace, struct uuid *uuid, ...) +{ +} +#endif + /** * uuid_str_to_le_bin() - Convert string UUID to little endian binary data. * @uuid_str: pointer to UUID string * @uuid_bin: pointer to allocated array for little endian output [16B] diff --git a/lib/Kconfig b/lib/Kconfig index 189e6eb31aa1..2941532f25cf 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -80,8 +80,16 @@ config RANDOM_UUID help Enable the generation of partitions with random UUIDs if none are provided.
+config UUID_GEN_V5 + bool "Enable UUID version 5 generation" + select LIB_UUID + depends on SHA1 + help + Enable the generation of version 5 UUIDs, these are determistic and + generated from a namespace UUID, and a string (such as a board name). + config SPL_LIB_UUID depends on SPL bool
diff --git a/lib/uuid.c b/lib/uuid.c index 2d7d99535e72..e7fda8dc736d 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -21,8 +21,9 @@ #include <part_efi.h> #include <malloc.h> #include <dm/uclass.h> #include <rng.h> +#include <u-boot/sha1.h>
int uuid_str_valid(const char *uuid) { int i, valid; @@ -368,8 +369,40 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str, } } }
+#if CONFIG_IS_ENABLED(UUID_GEN_V5) +void gen_uuid_v5(struct uuid *namespace, struct uuid *uuid, ...) +{ + sha1_context ctx; + va_list args; + const u8 *data; + u8 hash[SHA1_SUM_LEN]; + + sha1_starts(&ctx); + /* Hash the namespace UUID as salt */ + sha1_update(&ctx, (char *)namespace, UUID_BIN_LEN); + va_start(args, uuid); + + while ((data = va_arg(args, const u8 *))) + sha1_update(&ctx, (char *)data, va_arg(args, int)); + + va_end(args); + sha1_finish(&ctx, hash); + + /* Truncate the hash into output UUID and convert it to big endian */ + cpu_to_be32_array((u32 *)uuid, (u32 *)hash, 4); + + /* Configure variant/version bits */ + clrsetbits_be16(&uuid->time_hi_and_version, + UUID_VERSION_MASK, + 5 << UUID_VERSION_SHIFT); + clrsetbits_8(&uuid->clock_seq_hi_and_reserved, + UUID_VARIANT_MASK, + UUID_VARIANT << UUID_VARIANT_SHIFT); +} +#endif + #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID) void gen_rand_uuid(unsigned char *uuid_bin) { u32 ptr[4];
[...]
#include <dm/uclass.h> #include <rng.h> +#include <u-boot/sha1.h>
int uuid_str_valid(const char *uuid) { int i, valid; @@ -368,8 +369,40 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str, } } }
+#if CONFIG_IS_ENABLED(UUID_GEN_V5) +void gen_uuid_v5(struct uuid *namespace, struct uuid *uuid, ...) +{
sha1_context ctx;va_list args;const u8 *data;u8 hash[SHA1_SUM_LEN];sha1_starts(&ctx);/* Hash the namespace UUID as salt */sha1_update(&ctx, (char *)namespace, UUID_BIN_LEN);va_start(args, uuid);
Should we use sha1 here? Is it described somewhere in UUIDv5 requirements? If not I'd rather have a sha256
while ((data = va_arg(args, const u8 *)))sha1_update(&ctx, (char *)data, va_arg(args, int));
sha1_update second argument is an unsigned int
va_end(args);sha1_finish(&ctx, hash);/* Truncate the hash into output UUID and convert it to big endian */cpu_to_be32_array((u32 *)uuid, (u32 *)hash, 4);/* Configure variant/version bits */clrsetbits_be16(&uuid->time_hi_and_version,UUID_VERSION_MASK,5 << UUID_VERSION_SHIFT);clrsetbits_8(&uuid->clock_seq_hi_and_reserved,UUID_VARIANT_MASK,UUID_VARIANT << UUID_VARIANT_SHIFT);+} +#endif
#if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID) void gen_rand_uuid(unsigned char *uuid_bin) { u32 ptr[4];
-- 2.44.0
Thanks /Ilias
On 24/05/2024 08:01, Ilias Apalodimas wrote:
[...]
#include <dm/uclass.h> #include <rng.h> +#include <u-boot/sha1.h>
int uuid_str_valid(const char *uuid) { int i, valid; @@ -368,8 +369,40 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str, } } }
+#if CONFIG_IS_ENABLED(UUID_GEN_V5) +void gen_uuid_v5(struct uuid *namespace, struct uuid *uuid, ...) +{
sha1_context ctx;va_list args;const u8 *data;u8 hash[SHA1_SUM_LEN];sha1_starts(&ctx);/* Hash the namespace UUID as salt */sha1_update(&ctx, (char *)namespace, UUID_BIN_LEN);va_start(args, uuid);Should we use sha1 here? Is it described somewhere in UUIDv5 requirements? If not I'd rather have a sha256
The spec says sha1 yeah, this doesn't need to be cryptographically secure (the inputs are generally known) but just not have collisions.
That said, we don't need to be spec compliant - just consistent. So I'm fine either way. I'd err on the side of what's fastest to compute (if that even matters here).
while ((data = va_arg(args, const u8 *)))sha1_update(&ctx, (char *)data, va_arg(args, int));sha1_update second argument is an unsigned int
Ah thanks.
va_end(args);sha1_finish(&ctx, hash);/* Truncate the hash into output UUID and convert it to big endian */cpu_to_be32_array((u32 *)uuid, (u32 *)hash, 4);/* Configure variant/version bits */clrsetbits_be16(&uuid->time_hi_and_version,UUID_VERSION_MASK,5 << UUID_VERSION_SHIFT);clrsetbits_8(&uuid->clock_seq_hi_and_reserved,UUID_VARIANT_MASK,UUID_VARIANT << UUID_VARIANT_SHIFT);+} +#endif
- #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID) void gen_rand_uuid(unsigned char *uuid_bin) { u32 ptr[4];
-- 2.44.0
Thanks /Ilias
On Fri, 24 May 2024 at 15:20, Caleb Connolly caleb.connolly@linaro.org wrote:
On 24/05/2024 08:01, Ilias Apalodimas wrote:
[...]
#include <dm/uclass.h> #include <rng.h> +#include <u-boot/sha1.h>
int uuid_str_valid(const char *uuid) { int i, valid; @@ -368,8 +369,40 @@ void uuid_bin_to_str(const unsigned char *uuid_bin, char *uuid_str, } } }
+#if CONFIG_IS_ENABLED(UUID_GEN_V5) +void gen_uuid_v5(struct uuid *namespace, struct uuid *uuid, ...) +{
sha1_context ctx;va_list args;const u8 *data;u8 hash[SHA1_SUM_LEN];sha1_starts(&ctx);/* Hash the namespace UUID as salt */sha1_update(&ctx, (char *)namespace, UUID_BIN_LEN);va_start(args, uuid);Should we use sha1 here? Is it described somewhere in UUIDv5 requirements? If not I'd rather have a sha256
The spec says sha1 yeah, this doesn't need to be cryptographically secure (the inputs are generally known) but just not have collisions.
That said, we don't need to be spec compliant - just consistent. So I'm fine either way. I'd err on the side of what's fastest to compute (if that even matters here).
Ok, that's fine, we can stick to the spec
Cheers /Ilias
while ((data = va_arg(args, const u8 *)))sha1_update(&ctx, (char *)data, va_arg(args, int));sha1_update second argument is an unsigned int
Ah thanks.
va_end(args);sha1_finish(&ctx, hash);/* Truncate the hash into output UUID and convert it to big endian */cpu_to_be32_array((u32 *)uuid, (u32 *)hash, 4);/* Configure variant/version bits */clrsetbits_be16(&uuid->time_hi_and_version,UUID_VERSION_MASK,5 << UUID_VERSION_SHIFT);clrsetbits_8(&uuid->clock_seq_hi_and_reserved,UUID_VARIANT_MASK,UUID_VARIANT << UUID_VARIANT_SHIFT);+} +#endif
- #if defined(CONFIG_RANDOM_UUID) || defined(CONFIG_CMD_UUID) void gen_rand_uuid(unsigned char *uuid_bin) { u32 ptr[4];
-- 2.44.0
Thanks /Ilias
-- // Caleb (they/them)
Introduce a new helper efi_capsule_update_info_gen_ids() which takes several strings to identify the currently running board as well as a platform specific salt UUID and uses this data to populate the capsule update fw images image_type_id field. This allows for determinstic UUIDs to be used that can scale to a large number of different boards and board variants without the need to maintain a big list.
Generating capsule updates can be done using the same namespace, soc, model, compatible, and fw_image name strings.
This is behind an additional config option as it depends on V5 UUIDs and the SHA1 implementation.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- include/efi_loader.h | 28 ++++++++++++++++++++++++++++ lib/efi_loader/Kconfig | 14 ++++++++++++++ lib/efi_loader/efi_capsule.c | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 69442f4e58de..7d6b6ff83229 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -1125,8 +1125,36 @@ struct efi_capsule_update_info { };
extern struct efi_capsule_update_info update_info;
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS) +/** + * efi_capsule_update_info_gen_ids - Generate image_type_id UUIDs + * for all firmware images based on a platform namespace UUID. + * + * @namespace: The arch/platform specific namespace salt. This should be + * hardcoded per platform and replaced by vendors. + * @soc: A string identifying the SoC used on this board. + * @model: The model string for the board. + * @compatible: The most specific (first) root compatible string. + * + * This can be called by board code to populate the image_type_id + * UUID fields deterministically based on the board's model. Allowing + * many boards to be supported without the need for a large hardcoded + * array of fw images. This works using v5 UUIDs. + */ +int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, + const char *model, + const char *compatible); +#else +static inline int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, + const char *model, + const char *compatible) +{ + return -ENOSYS; +} +#endif + /** * Install the ESRT system table. * * Return: status code diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 430bb7f0f7dc..dd8fc1b08812 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -235,8 +235,22 @@ config EFI_CAPSULE_ON_DISK_EARLY If this option is enabled, capsules will be enforced to be executed as part of U-Boot initialisation so that they will surely take place whatever is set to distro_bootcmd.
+config EFI_CAPSULE_DYNAMIC_UUIDS + bool "Dynamic UUIDs for capsules" + depends on EFI_HAVE_CAPSULE_SUPPORT + select UUID_GEN_V5 + help + Select this option if you want to use dynamically generated v5 + UUIDs for your board. To make use of this feature, your board + code should call efi_capsule_update_info_gen_ids() with a seed + UUID to generate the image_type_id field for each fw_image. + + The CapsuleUpdate payloads are expected to generate matching UUIDs + using the same scheme. + + config EFI_CAPSULE_FIRMWARE bool
config EFI_CAPSULE_FIRMWARE_MANAGEMENT diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index de0d49ebebda..9ef67d1b4405 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -19,8 +19,9 @@ #include <mapmem.h> #include <sort.h> #include <sysreset.h> #include <asm/global_data.h> +#include <uuid.h>
#include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h> @@ -403,8 +404,40 @@ out: return status; } #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS) +int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, const char *model, const char *compatible) +{ + int i; + + if (!soc || !model || !compatible) { + log_err("%s: soc, model, or compatible not defined\n", __func__); + return -EINVAL; + } + + if (!update_info.num_images) { + log_err("%s: no fw_images, make sure update_info.num_images is set\n", __func__); + return -ENODATA; + } + + for (i = 0; i < update_info.num_images; i++) { + gen_uuid_v5((struct uuid*)namespace, + (struct uuid *)&update_info.images[i].image_type_id, + soc, strlen(soc), + model, strlen(model), + compatible, strlen(compatible), + update_info.images[i].fw_name, u16_strlen(update_info.images[i].fw_name), + NULL); + + log_debug("Image %ls generated UUID %pUs\n", update_info.images[i].fw_name, + &update_info.images[i].image_type_id); + } + + return 0; +} +#endif + static __maybe_unused bool fwu_empty_capsule(struct efi_capsule_header *capsule) { return !guidcmp(&capsule->capsule_guid, &fwu_guid_os_request_fw_revert) ||
[...]
config EFI_CAPSULE_FIRMWARE_MANAGEMENT diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index de0d49ebebda..9ef67d1b4405 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -19,8 +19,9 @@ #include <mapmem.h> #include <sort.h> #include <sysreset.h> #include <asm/global_data.h> +#include <uuid.h>
#include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h> @@ -403,8 +404,40 @@ out: return status; } #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS) +int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, const char *model, const char *compatible) +{
int i;
Perhaps irrelevant to this patch, but do we need to define the name space in platform code? Can't we just put it on a Kconfig and do the dynamic UUIID generation in efi_capsule.c?
Thanks /Ilias
if (!soc || !model || !compatible) {log_err("%s: soc, model, or compatible not defined\n", __func__);return -EINVAL;}if (!update_info.num_images) {log_err("%s: no fw_images, make sure update_info.num_images is set\n", __func__);return -ENODATA;}for (i = 0; i < update_info.num_images; i++) {gen_uuid_v5((struct uuid*)namespace,(struct uuid *)&update_info.images[i].image_type_id,soc, strlen(soc),model, strlen(model),compatible, strlen(compatible),update_info.images[i].fw_name, u16_strlen(update_info.images[i].fw_name),NULL);log_debug("Image %ls generated UUID %pUs\n", update_info.images[i].fw_name,&update_info.images[i].image_type_id);}return 0;+} +#endif
static __maybe_unused bool fwu_empty_capsule(struct efi_capsule_header *capsule) { return !guidcmp(&capsule->capsule_guid, &fwu_guid_os_request_fw_revert) ||
-- 2.44.0
On 24/05/2024 08:37, Ilias Apalodimas wrote:
[...]
config EFI_CAPSULE_FIRMWARE_MANAGEMENT diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index de0d49ebebda..9ef67d1b4405 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -19,8 +19,9 @@ #include <mapmem.h> #include <sort.h> #include <sysreset.h> #include <asm/global_data.h> +#include <uuid.h>
#include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h> @@ -403,8 +404,40 @@ out: return status; } #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS) +int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, const char *model, const char *compatible) +{
int i;Perhaps irrelevant to this patch, but do we need to define the name space in platform code? Can't we just put it on a Kconfig and do the dynamic UUIID generation in efi_capsule.c?
Having the namespace be a kconfig option probably makes sense, then it could be the thing vendors have to populate for their production boards (and they could just have one for all of their products across many architectures). So by setting one option they'd have totally unique GUIDs for everything.
Thanks /Ilias
if (!soc || !model || !compatible) {log_err("%s: soc, model, or compatible not defined\n", __func__);return -EINVAL;}if (!update_info.num_images) {log_err("%s: no fw_images, make sure update_info.num_images is set\n", __func__);return -ENODATA;}for (i = 0; i < update_info.num_images; i++) {gen_uuid_v5((struct uuid*)namespace,(struct uuid *)&update_info.images[i].image_type_id,soc, strlen(soc),model, strlen(model),compatible, strlen(compatible),update_info.images[i].fw_name, u16_strlen(update_info.images[i].fw_name),NULL);log_debug("Image %ls generated UUID %pUs\n", update_info.images[i].fw_name,&update_info.images[i].image_type_id);}return 0;+} +#endif
- static __maybe_unused bool fwu_empty_capsule(struct efi_capsule_header *capsule) { return !guidcmp(&capsule->capsule_guid, &fwu_guid_os_request_fw_revert) ||
-- 2.44.0
On Fri, 24 May 2024 at 15:17, Caleb Connolly caleb.connolly@linaro.org wrote:
On 24/05/2024 08:37, Ilias Apalodimas wrote:
[...]
config EFI_CAPSULE_FIRMWARE_MANAGEMENT diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c index de0d49ebebda..9ef67d1b4405 100644 --- a/lib/efi_loader/efi_capsule.c +++ b/lib/efi_loader/efi_capsule.c @@ -19,8 +19,9 @@ #include <mapmem.h> #include <sort.h> #include <sysreset.h> #include <asm/global_data.h> +#include <uuid.h>
#include <crypto/pkcs7.h> #include <crypto/pkcs7_parser.h> #include <linux/err.h> @@ -403,8 +404,40 @@ out: return status; } #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
+#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS) +int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, const char *model, const char *compatible) +{
int i;Perhaps irrelevant to this patch, but do we need to define the name space in platform code? Can't we just put it on a Kconfig and do the dynamic UUIID generation in efi_capsule.c?
Having the namespace be a kconfig option probably makes sense, then it could be the thing vendors have to populate for their production boards (and they could just have one for all of their products across many architectures). So by setting one option they'd have totally unique GUIDs for everything.
Exactly and you would be able to reuse the entire machinery without having to add platform code, since the GUID population would live in the the efi firmware parts
Cheers /Ilias
Thanks /Ilias
if (!soc || !model || !compatible) {log_err("%s: soc, model, or compatible not defined\n", __func__);return -EINVAL;}if (!update_info.num_images) {log_err("%s: no fw_images, make sure update_info.num_images is set\n", __func__);return -ENODATA;}for (i = 0; i < update_info.num_images; i++) {gen_uuid_v5((struct uuid*)namespace,(struct uuid *)&update_info.images[i].image_type_id,soc, strlen(soc),model, strlen(model),compatible, strlen(compatible),update_info.images[i].fw_name, u16_strlen(update_info.images[i].fw_name),NULL);log_debug("Image %ls generated UUID %pUs\n", update_info.images[i].fw_name,&update_info.images[i].image_type_id);}return 0;+} +#endif
- static __maybe_unused bool fwu_empty_capsule(struct efi_capsule_header *capsule) { return !guidcmp(&capsule->capsule_guid, &fwu_guid_os_request_fw_revert) ||
-- 2.44.0
-- // Caleb (they/them)
Document how platforms can generate GUIDs at runtime rather than maintaining a list of GUIDs per-board.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- doc/develop/uefi/uefi.rst | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index 0389b269c01b..52076fb4c106 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -448,8 +448,43 @@ the location of the firmware updates is not a very secure practice. Getting this information from the firmware itself is more secure, assuming the firmware has been verified by a previous stage boot loader.
+The image_type_id contains a GUID value which is specific to the image +and board being updated, that is to say it should uniquely identify the +board model (and revision if relevant) and image pair. Traditionally, +these GUIDs are generated manually and hardcoded on a per-board basis, +however this scheme makes it difficult to scale up to support many +boards. + +To address this, v5 GUIDs can be used to generate board-specific GUIDs +at runtime, based on a set of persistent identifiable information: + +.. code-block:: c + + /** + * efi_capsule_update_info_gen_ids - Generate image_type_id UUIDs + * for all firmware images based on a platform namespace UUID. + * + * @namespace: The arch/platform specific namespace salt. This should be + * hardcoded per platform and replaced by vendors. + * @soc: A string identifying the SoC used on this board. + * @model: The model string for the board. + * @compatible: The most specific (first) root compatible string. + * + * This can be called by board code to populate the image_type_id + * UUID fields deterministically based on the board's model. Allowing + * many boards to be supported without the need for a large hardcoded + * array of fw images. This works using v5 UUIDs. + */ + int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc, + const char *model, + const char *compatible); + +These strings are combined with the fw_image name to generate GUIDs for +each image. This function should be called during board init, before the +EFI subsystem is initialised. + The firmware images structure defines the GUID values, image index values and the name of the images that are to be updated through the capsule update feature. These values are to be defined as part of an array. These GUID values would be used by the Firmware Management
On Fri, 26 Apr 2024 at 17:19, Caleb Connolly caleb.connolly@linaro.org wrote:
Document how platforms can generate GUIDs at runtime rather than maintaining a list of GUIDs per-board.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
doc/develop/uefi/uefi.rst | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+)
diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst index 0389b269c01b..52076fb4c106 100644 --- a/doc/develop/uefi/uefi.rst +++ b/doc/develop/uefi/uefi.rst @@ -448,8 +448,43 @@ the location of the firmware updates is not a very secure practice. Getting this information from the firmware itself is more secure, assuming the firmware has been verified by a previous stage boot loader.
+The image_type_id contains a GUID value which is specific to the image
which is/that is
+and board being updated, that is to say it should uniquely identify the +board model (and revision if relevant) and image pair. Traditionally, +these GUIDs are generated manually and hardcoded on a per-board basis, +however this scheme makes it difficult to scale up to support many +boards.
+To address this, v5 GUIDs can be used to generate board-specific GUIDs +at runtime, based on a set of persistent identifiable information:
+.. code-block:: c
/*** efi_capsule_update_info_gen_ids - Generate image_type_id UUIDs* for all firmware images based on a platform namespace UUID.** @namespace: The arch/platform specific namespace salt. This should be* hardcoded per platform and replaced by vendors.* @soc: A string identifying the SoC used on this board.* @model: The model string for the board.* @compatible: The most specific (first) root compatible string.** This can be called by board code to populate the image_type_id* UUID fields deterministically based on the board's model. Allowing
model, allowing
* many boards to be supported without the need for a large hardcoded* array of fw images. This works using v5 UUIDs.*/int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char *soc,const char *model,const char *compatible);+These strings are combined with the fw_image name to generate GUIDs for +each image. This function should be called during board init, before the +EFI subsystem is initialised.
initialized
The firmware images structure defines the GUID values, image index values and the name of the images that are to be updated through the capsule update feature. These values are to be defined as part of an array. These GUID values would be used by the Firmware Management
-- 2.44.0
With these Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Migrate sandbox over to generating it's capsule update image GUIDs dynamically rather than using a set of hardcoded ones.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org --- arch/Kconfig | 1 + board/sandbox/sandbox.c | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig index abd406d48841..0558c90540b6 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -164,8 +164,9 @@ config SANDBOX select SYS_CACHE_SHIFT_4 select IRQ select SUPPORT_EXTENSION_SCAN if CMDLINE select SUPPORT_ACPI + select EFI_CAPSULE_DYNAMIC_UUIDS if EFI_HAVE_CAPSULE_SUPPORT imply BITREVERSE select BLOBLIST imply LTO imply CMD_DM diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c index 802596569c64..68a99ce1fc07 100644 --- a/board/sandbox/sandbox.c +++ b/board/sandbox/sandbox.c @@ -31,36 +31,24 @@ */ gd_t *gd;
#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) -/* GUIDs for capsule updatable firmware images */ -#define SANDBOX_UBOOT_IMAGE_GUID \ +#define SANDBOX_CAPSULE_UPDATE_SALT \ EFI_GUID(0x09d7cf52, 0x0720, 0x4710, 0x91, 0xd1, \ 0x08, 0x46, 0x9b, 0x7f, 0xe9, 0xc8)
-#define SANDBOX_UBOOT_ENV_IMAGE_GUID \ - EFI_GUID(0x5a7021f5, 0xfef2, 0x48b4, 0xaa, 0xba, \ - 0x83, 0x2e, 0x77, 0x74, 0x18, 0xc0) - -#define SANDBOX_FIT_IMAGE_GUID \ - EFI_GUID(0x3673b45d, 0x6a7c, 0x46f3, 0x9e, 0x60, \ - 0xad, 0xab, 0xb0, 0x3f, 0x79, 0x37) - struct efi_fw_image fw_images[] = { #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_RAW) { - .image_type_id = SANDBOX_UBOOT_IMAGE_GUID, .fw_name = u"SANDBOX-UBOOT", .image_index = 1, }, { - .image_type_id = SANDBOX_UBOOT_ENV_IMAGE_GUID, .fw_name = u"SANDBOX-UBOOT-ENV", .image_index = 2, }, #elif defined(CONFIG_EFI_CAPSULE_FIRMWARE_FIT) { - .image_type_id = SANDBOX_FIT_IMAGE_GUID, .fw_name = u"SANDBOX-FIT", .image_index = 1, }, #endif @@ -122,8 +110,22 @@ int dram_init(void) }
int board_init(void) { + int ret; + + if (CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)) { + efi_guid_t salt_guid = SANDBOX_CAPSULE_UPDATE_SALT; + + ret = efi_capsule_update_info_gen_ids(&salt_guid, + "sandbox", + ofnode_read_string(ofnode_root(), "model"), + ofnode_read_string(ofnode_root(), "compatible")); + if (ret) { + printf("Failed to generate GUIDs: %d\n", ret); + return ret; + } + } return 0; }
int ft_board_setup(void *fdt, struct bd_info *bd)
On Fri, 26 Apr 2024 at 17:19, Caleb Connolly caleb.connolly@linaro.org wrote:
Migrate sandbox over to generating it's capsule update image GUIDs dynamically rather than using a set of hardcoded ones.
Signed-off-by: Caleb Connolly caleb.connolly@linaro.org
arch/Kconfig | 1 + board/sandbox/sandbox.c | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig index abd406d48841..0558c90540b6 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -164,8 +164,9 @@ config SANDBOX select SYS_CACHE_SHIFT_4 select IRQ select SUPPORT_EXTENSION_SCAN if CMDLINE select SUPPORT_ACPI
select EFI_CAPSULE_DYNAMIC_UUIDS if EFI_HAVE_CAPSULE_SUPPORT imply BITREVERSE select BLOBLIST imply LTO imply CMD_DMdiff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c index 802596569c64..68a99ce1fc07 100644 --- a/board/sandbox/sandbox.c +++ b/board/sandbox/sandbox.c @@ -31,36 +31,24 @@ */ gd_t *gd;
#if IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) -/* GUIDs for capsule updatable firmware images */ -#define SANDBOX_UBOOT_IMAGE_GUID \ +#define SANDBOX_CAPSULE_UPDATE_SALT \ EFI_GUID(0x09d7cf52, 0x0720, 0x4710, 0x91, 0xd1, \ 0x08, 0x46, 0x9b, 0x7f, 0xe9, 0xc8)
-#define SANDBOX_UBOOT_ENV_IMAGE_GUID \
EFI_GUID(0x5a7021f5, 0xfef2, 0x48b4, 0xaa, 0xba, \0x83, 0x2e, 0x77, 0x74, 0x18, 0xc0)-#define SANDBOX_FIT_IMAGE_GUID \
EFI_GUID(0x3673b45d, 0x6a7c, 0x46f3, 0x9e, 0x60, \0xad, 0xab, 0xb0, 0x3f, 0x79, 0x37)struct efi_fw_image fw_images[] = { #if defined(CONFIG_EFI_CAPSULE_FIRMWARE_RAW) {
.image_type_id = SANDBOX_UBOOT_IMAGE_GUID, .fw_name = u"SANDBOX-UBOOT", .image_index = 1, }, {.image_type_id = SANDBOX_UBOOT_ENV_IMAGE_GUID, .fw_name = u"SANDBOX-UBOOT-ENV", .image_index = 2, },#elif defined(CONFIG_EFI_CAPSULE_FIRMWARE_FIT) {
.image_type_id = SANDBOX_FIT_IMAGE_GUID, .fw_name = u"SANDBOX-FIT", .image_index = 1, },#endif @@ -122,8 +110,22 @@ int dram_init(void) }
int board_init(void) {
int ret;if (CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)) {efi_guid_t salt_guid = SANDBOX_CAPSULE_UPDATE_SALT;ret = efi_capsule_update_info_gen_ids(&salt_guid,"sandbox",ofnode_read_string(ofnode_root(), "model"),ofnode_read_string(ofnode_root(), "compatible"));if (ret) {printf("Failed to generate GUIDs: %d\n", ret);return ret;}} return 0;}
int ft_board_setup(void *fdt, struct bd_info *bd)
-- 2.44.0
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Hi Caleb
On Fri, 26 Apr 2024 at 17:19, Caleb Connolly caleb.connolly@linaro.org wrote:
As more boards adopt support for the EFI CapsuleUpdate mechanism, there is a growing issue of being able to target updates to them properly. The current mechanism of hardcoding UUIDs for each board at compile time is unsustainable, and maintaining lists of GUIDs is similarly cumbersome.
In this series, I propose that we adopt v5 GUIDs, these are generated by using a well-known salt GUID as well as board specific information (like the model/revision), these are hashed together and the result is truncated to form a new UUID.
The well-known salt GUID can be specific to the architecture (SoC vendor), or OEM. Exact rules on how these are used (e.g. if vendors should be told to generate their own for their products, and if that can be added upstream, etc) will need to be decided.
Specifically, the following fields are used to generate a GUID for a particular fw_image:
- namespace salt
- soc name
- board model (usually from dt root model property)
- board compatible (usually the first entry in the dt root compatible array).
- fw_image name (the string identifying the specific image, especially relevant for board that can update multiple images).
Once generated, the GUIDs can be printed with the "%pUs" format string, these can then be stored externally to U-Boot.
The SoC name field might be controversial, it could be generated from the last entry in the dt root compatible in most cases, or in some board specific way. It might make sense to remove this field if it is unfeasible for some boards.
== Usage ==
Boards can integrate dynamic UUID support as follows:
- Adjust Kconfig to depend on EFI_CAPSULE_DYNAMIC_UUIDS if EFI_HAVE_CAPSULE_SUPPORT
- Skip setting the fw_images image_type_id property.
- In board_init() (or anywhere before the EFI subsystem is initialised), add a call to efi_capsule_update_info_gen_ids() with the board specific info.
== Limitations ==
- Changing GUIDs
The primary limitation with this approach is that if any of the source fields change, so will the GUID for the board. It is therefore pretty important to ensure that GUID changes are caught during development.
- Supporting multiple boards with a single image
This now requires having an entry with the GUID for every board which might lead to larger UpdateCapsule images.
== Tooling ==
Not part of this RFC is a tool to generate the GUIDs outside of U-Boot. I suspect this might be a requirement, but it makes sense to decide on what fields we use first.
Yes, tooling would be good.
The tool should take in the salt, DTB, and a list of fw_image names. It could also accept values to overwrite the individual fields if they aren't derived from the DTB for some reason. It would then generate the expected GUID.
A potential idea here would be to integrate this into the build system so that it prints a warning if the GUID changes.
There's work being done in that direction as far as capsules are concerned. Apart from the u-boot binary from your board, the build system should also generate binaries
Thanks /Ilias
== TOOD ==
Missing from this RFC are unit tests for the dynamic UUID feature, these will be implemented for future revisions.
I would appreciate any feedback on the above.
This follows a related discussion started by Ilias: https://lore.kernel.org/u-boot/CAC_iWjJNHa4gMF897MqYZNdbgjFG8K4kwGsTXWuy72Wk...
Caleb Connolly (4): lib: uuid: add UUID v5 support efi: add a helper to generate dynamic UUIDs doc: uefi: document dynamic GUID generation sandbox: switch to dynamic UUIDs
arch/Kconfig | 1 + board/sandbox/sandbox.c | 28 +++++++++++++++------------- doc/develop/uefi/uefi.rst | 35 +++++++++++++++++++++++++++++++++++ include/efi_loader.h | 28 ++++++++++++++++++++++++++++ include/uuid.h | 16 ++++++++++++++++ lib/Kconfig | 8 ++++++++ lib/efi_loader/Kconfig | 14 ++++++++++++++ lib/efi_loader/efi_capsule.c | 33 +++++++++++++++++++++++++++++++++ lib/uuid.c | 33 +++++++++++++++++++++++++++++++++ 9 files changed, 183 insertions(+), 13 deletions(-)
change-id: 20240422-b4-dynamic-uuid-1a5ab1486c27 base-commit: d097f9e1299a3bdb7de468f0d9bbc63932f461cd
// Caleb (they/them)
participants (2)
-
Caleb Connolly -
Ilias Apalodimas