From 602bb8d9d50dce86ac07f6cf2620dfe076f212b2 Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Wed, 29 Apr 2026 15:58:20 +0200 Subject: [PATCH 1/8] init: rename metadata fields correctly for SHA256 based integrity check --- kernel/src/managers/task/task_init.c | 12 ++++++------ kernel/src/syscalls/sysgate_int_acknowledge.c | 1 + schemas/task/metadata.schema.json.cpp | 8 ++++---- schemas/task/sample.json | 4 ++-- schemas/task/sample.toml | 4 ++-- schemas/task/task_metadata.json | 8 ++++---- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/kernel/src/managers/task/task_init.c b/kernel/src/managers/task/task_init.c index fd240777..2f8cf7d8 100644 --- a/kernel/src/managers/task/task_init.c +++ b/kernel/src/managers/task/task_init.c @@ -34,14 +34,14 @@ typedef enum task_mgr_state { TASK_MANAGER_STATE_BOOT = 0x0UL, /**< at boot time */ /* for each cell of task_meta_table */ TASK_MANAGER_STATE_DISCOVER_SANITATION, /**< magic & version check */ - TASK_MANAGER_STATE_CHECK_META_INTEGRITY,/**< metadata HMAC check */ - TASK_MANAGER_STATE_CHECK_TSK_INTEGRITY, /**< task HMAC check */ + TASK_MANAGER_STATE_CHECK_META_INTEGRITY,/**< metadata SHA-256 check */ + TASK_MANAGER_STATE_CHECK_TSK_INTEGRITY, /**< task SHA-256 check */ TASK_MANAGER_STATE_INIT_LOCALINFO, /**< init dynamic task info into local struct */ TASK_MANAGER_STATE_TSK_MAP, /**< task data copy, bss zeroify, stack init */ TASK_MANAGER_STATE_TSK_SCHEDULE, /**< schedule task (if start at bootup) */ TASK_MANAGER_STATE_FINALIZE, /**< all tasks added, finalize (sort task list) */ TASK_MANAGER_STATE_READY, /**< ready state, everything is clean */ - TASK_MANAGER_STATE_ERROR_SECURITY, /**< hmac or magic error */ + TASK_MANAGER_STATE_ERROR_SECURITY, /**< sha256 or magic error */ TASK_MANAGER_STATE_ERROR_RUNTIME, /**< others (sched...) */ } task_mgr_state_t; @@ -71,7 +71,7 @@ static struct task_mgr_ctx ctx; * the task mapping order is based on the label list (from the smaller to the higher) * so that binary search can be done on the task set below * 3. upgrade each task ELF based on the calculated memory mapping - * 4. forge the task metadata from the new ELF, including HMACs, save it to a dediacted file + * 4. forge the task metadata from the new ELF, including SHA-256 digests, save it to a dediacted file * 5. store the metadata in the first free cell of the .task_list section bellow * * In a different (v2?) mode, it is possible to consider that tasks metadata can be stored @@ -139,7 +139,7 @@ static inline kstatus_t task_init_check_meta_integrity(task_meta_t const * const ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; goto end; } - /* FIXME: call the hmac service in order to validate metadata integrity, + /* FIXME: call the sha256 service in order to validate metadata integrity, and return the result */ pr_info("[task %08x] metadata integrity ok", meta->label); ctx.state = TASK_MANAGER_STATE_CHECK_TSK_INTEGRITY; @@ -164,7 +164,7 @@ static inline kstatus_t task_init_check_tsk_integrity(task_meta_t const * const ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; goto end; } - /* FIXME: call the hmac service in order to validate metadata integrity, + /* FIXME: call the sha256 service in order to validate metadata integrity, and return the result */ pr_info("[task %08x] task code+data integrity ok", meta->label); ctx.state = TASK_MANAGER_STATE_INIT_LOCALINFO; diff --git a/kernel/src/syscalls/sysgate_int_acknowledge.c b/kernel/src/syscalls/sysgate_int_acknowledge.c index c6321b3c..ef8767d5 100644 --- a/kernel/src/syscalls/sysgate_int_acknowledge.c +++ b/kernel/src/syscalls/sysgate_int_acknowledge.c @@ -44,6 +44,7 @@ stack_frame_t *gate_int_acknowledge(stack_frame_t *frame, uint16_t IRQn) mgr_task_set_sysreturn(current, STATUS_DENIED); goto end; } + /* push the inth event into the task input events queue */ if (unlikely(mgr_interrupt_acknowledge_irq(IRQn) != K_STATUS_OKAY)) { /* should not rise while IRQ ownership has been checked! see dts file */ diff --git a/schemas/task/metadata.schema.json.cpp b/schemas/task/metadata.schema.json.cpp index c6e4b375..181b4fe4 100644 --- a/schemas/task/metadata.schema.json.cpp +++ b/schemas/task/metadata.schema.json.cpp @@ -626,7 +626,7 @@ "minItems": 0, "maxItems": CONFIG_MAX_DMA_STREAMS_PER_TASK }, - "task_hmac": { + "task_sha256": { "type": "array", "items": { "type": "object", @@ -648,7 +648,7 @@ ] } }, - "metadata_hmac": { + "metadata_sha256": { "type": "array", "items": { "type": "object", @@ -695,7 +695,7 @@ "devs", "num_dma", "dmas", - "task_hmac", - "metadata_hmac" + "task_sha256", + "metadata_sha256" ] } diff --git a/schemas/task/sample.json b/schemas/task/sample.json index bc8062f1..f76e3d10 100644 --- a/schemas/task/sample.json +++ b/schemas/task/sample.json @@ -163,14 +163,14 @@ "description": "number of task dma stream(s)", "dmas": [] }, - "task_hmac": [ + "task_sha256": [ { "c_type": "uint8_t", "rust_type": "u8", "value": 12 } ], - "metadata_hmac": [ + "metadata_sha256": [ { "c_type": "uint8_t", "rust_type": "u8", diff --git a/schemas/task/sample.toml b/schemas/task/sample.toml index 6dd70b10..9fb6ab22 100644 --- a/schemas/task/sample.toml +++ b/schemas/task/sample.toml @@ -160,12 +160,12 @@ description = "number of task dma stream(s)" dmas = [ ] -[[task_hmac]] +[[task_sha256]] c_type = "uint8_t" rust_type = "u8" value = 12 -[[metadata_hmac]] +[[metadata_sha256]] c_type = "uint8_t" rust_type = "u8" value = 12 diff --git a/schemas/task/task_metadata.json b/schemas/task/task_metadata.json index e415875d..39afcfd6 100644 --- a/schemas/task/task_metadata.json +++ b/schemas/task/task_metadata.json @@ -152,22 +152,22 @@ "description": "dma stream handler identifier" } }, - "task_hmac": { + "task_sha256": { "array": true, "array_size": 32, "element": { "c_type": "uint8_t", "rust_type": "u8", - "description": "task .text+.rodata+.data build time hmac calculation (TBD)" + "description": "task .text+.rodata+.data build time SHA-256 digest" } }, - "metadata_hmac": { + "metadata_sha256": { "array": true, "array_size": 32, "element": { "c_type": "uint8_t", "rust_type": "u8", - "description": "current struct build time hmac calculation" + "description": "current struct build time SHA-256 digest" } } } From f1efdff8fa6f5e44f21188ab992d4b75c71503b6 Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Wed, 29 Apr 2026 16:59:43 +0200 Subject: [PATCH 2/8] genmetadata: adding sha256 calculation on metadata blob generation --- tools/genmetadata/meson.build | 3 +- tools/genmetadata/src/arch/arch.hpp | 8 +++ tools/genmetadata/src/main.cpp | 22 +++++++- tools/genmetadata/src/reflect.hpp | 76 +++++++++++++++++++++++++- tools/genmetadata/src/task_meta.hpp.in | 28 ++++++++++ 5 files changed, 131 insertions(+), 6 deletions(-) diff --git a/tools/genmetadata/meson.build b/tools/genmetadata/meson.build index 87949c23..2bbc4ce4 100644 --- a/tools/genmetadata/meson.build +++ b/tools/genmetadata/meson.build @@ -5,6 +5,7 @@ add_languages('cpp', native: true, required: true) nl_json = dependency('nlohmann_json', native: true, version: '>=3.11') argparse = dependency('argparse', native: true, version: '>=3') +openssl = dependency('openssl', native: true) subdir('src') @@ -15,7 +16,7 @@ genmetadata = executable( include_directories: kernel_inc, link_language: 'cpp', override_options: ['cpp_std=gnu++20'], - dependencies: [ nl_json, argparse ], + dependencies: [ nl_json, argparse, openssl ], build_by_default: true, install_dir: get_option('libexecdir'), native: true, diff --git a/tools/genmetadata/src/arch/arch.hpp b/tools/genmetadata/src/arch/arch.hpp index e4a3376d..52180cce 100644 --- a/tools/genmetadata/src/arch/arch.hpp +++ b/tools/genmetadata/src/arch/arch.hpp @@ -30,3 +30,11 @@ template concept IsAnyOf = (std::same_as || ...); } // namespace arch + +#include "armv8m.hpp" + +namespace arch { + +using memory_spec = armv8m::memory_spec; + +} // namespace arch diff --git a/tools/genmetadata/src/main.cpp b/tools/genmetadata/src/main.cpp index 635ab031..ea82980b 100644 --- a/tools/genmetadata/src/main.cpp +++ b/tools/genmetadata/src/main.cpp @@ -23,13 +23,16 @@ #include #include #include +#include +#include #include #include +#include #include "task_meta.hpp" #include "reflect.hpp" -#include "arch/armv8m.hpp" +#include "arch/arch.hpp" using json = nlohmann::json; @@ -46,7 +49,22 @@ int main(int argc, char *argv[]) std::string out{program.get("output")}; auto data = json::parse(in); auto meta = taskMetadata::from_json(data["task_meta"]); - reflect_to_bin(meta, out); + + std::array metadata_sha256{}; + if (!meta.set_u8_array_field("metadata_sha256", metadata_sha256)) { + throw std::runtime_error("metadata_sha256 field not found or has unexpected type"); + } + + auto blob = reflect_to_bytes(meta); + if (SHA256(blob.data(), blob.size(), metadata_sha256.data()) == nullptr) { + throw std::runtime_error("failed to compute metadata_sha256"); + } + + if (!meta.set_u8_array_field("metadata_sha256", metadata_sha256)) { + throw std::runtime_error("failed to write metadata_sha256 field"); + } + + reflect_to_bin(meta, out); } catch (const std::exception& err) { std::cerr << err.what() << std::endl; diff --git a/tools/genmetadata/src/reflect.hpp b/tools/genmetadata/src/reflect.hpp index 0042bc11..a7028998 100644 --- a/tools/genmetadata/src/reflect.hpp +++ b/tools/genmetadata/src/reflect.hpp @@ -24,6 +24,24 @@ #include #include +#include +#include + +template> +struct scalar_storage { + using type = std::conditional_t>>>; +}; + +template +struct scalar_storage { + using type = std::underlying_type_t; +}; + +template +using scalar_storage_t = typename scalar_storage::type; template struct is_tuple: std::false_type{}; @@ -46,8 +64,10 @@ concept Reflectable = requires(T t) { template, bool>::type = true> static inline void __to_bin(T val, std::ofstream& out) { - constexpr auto size = MemorySpec::template size_of(); - constexpr auto alignment = MemorySpec::template size_of(); + using storage_t = scalar_storage_t; + static_assert(!std::is_void_v, "unsupported scalar size"); + constexpr auto size = MemorySpec::template size_of(); + constexpr auto alignment = MemorySpec::template size_of(); constexpr std::array zero{}; auto current = out.tellp(); auto padding = size - (current % alignment); @@ -60,6 +80,20 @@ static inline void __to_bin(T val, std::ofstream& out) out.write(reinterpret_cast(&val), size); } +template, bool>::type = true> +static inline void __to_bin(T val, std::vector& out) +{ + using storage_t = scalar_storage_t; + static_assert(!std::is_void_v, "unsupported scalar size"); + constexpr auto size = MemorySpec::template size_of(); + constexpr auto alignment = MemorySpec::template size_of(); + auto padding = (size - (out.size() % alignment)) % size; + + out.insert(out.end(), padding, 0); + auto ptr = reinterpret_cast(&val); + out.insert(out.end(), ptr, ptr + size); +} + template static inline void __to_bin(job_flags_t val, std::ofstream& out) { @@ -68,6 +102,14 @@ static inline void __to_bin(job_flags_t val, std::ofstream& out) __to_bin(tmp, out); } +template +static inline void __to_bin(job_flags_t val, std::vector& out) +{ + uint32_t tmp; + std::memcpy(&tmp, &val, sizeof(val)); + __to_bin(tmp, out); +} + template static inline void __to_bin(std::array val, std::ofstream& out) { @@ -76,20 +118,48 @@ static inline void __to_bin(std::array val, std::ofstream& out) } } +template +static inline void __to_bin(std::array val, std::vector& out) +{ + for (T cell: val) { + __to_bin(cell, out); + } +} + template static inline void _to_bin(const Tuple& reflection, std::ofstream& out, std::index_sequence) { ((__to_bin(std::get(reflection), out)), ...); } +template +static inline void _to_bin(const Tuple& reflection, std::vector& out, std::index_sequence) +{ + ((__to_bin(std::get(reflection), out)), ...); +} + template static inline void _to_bin(const std::tuple& reflection, std::ofstream& out) { _to_bin(reflection, out, std::index_sequence_for{}); } +template +static inline void _to_bin(const std::tuple& reflection, std::vector& out) +{ + _to_bin(reflection, out, std::index_sequence_for{}); +} + +template +static inline std::vector reflect_to_bytes(const T& obj) { + std::vector out; + _to_bin(obj.reflect(), out); + return out; +} + template static inline void reflect_to_bin(const T& obj, const std::string& filename) { + auto blob = reflect_to_bytes(obj); std::ofstream out(filename, std::ios::binary); - _to_bin(obj.reflect(), out); + out.write(reinterpret_cast(blob.data()), static_cast(blob.size())); } diff --git a/tools/genmetadata/src/task_meta.hpp.in b/tools/genmetadata/src/task_meta.hpp.in index 17f49e8a..963de0a0 100644 --- a/tools/genmetadata/src/task_meta.hpp.in +++ b/tools/genmetadata/src/task_meta.hpp.in @@ -163,8 +163,36 @@ public: const std::string_view field_name(size_t id) const { return _field_names[id]; } size_t size() const { return std::tuple_size_v; } + template + bool set_u8_array_field(const std::string_view name, const std::array& value) { + bool updated = false; + set_u8_array_field_impl(name, value, updated, std::make_index_sequence<_size>{}); + return updated; + } + const auto& reflect() const { return _fields; } private: + template + void try_set_u8_array_field(const std::string_view name, const std::array& value, bool& updated) { + if (updated || _field_names[Idx] != name) { + return; + } + + using field_t = std::remove_cvref_tfield())>; + if constexpr (std::is_same_v>) { + this->field() = value; + updated = true; + } + } + + template + void set_u8_array_field_impl(const std::string_view name, + const std::array& value, + bool& updated, + std::index_sequence) { + (try_set_u8_array_field(name, value, updated), ...); + } + layout_t _fields{}; }; From e940c06de9b4cf1ac2bfc1d8c5c690644156ff48 Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Wed, 29 Apr 2026 17:25:33 +0200 Subject: [PATCH 3/8] sentry,meta: adding metadata sha256 integrity check support --- kernel/src/managers/task/task_init.c | 33 ++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/kernel/src/managers/task/task_init.c b/kernel/src/managers/task/task_init.c index 2f8cf7d8..33b7b610 100644 --- a/kernel/src/managers/task/task_init.c +++ b/kernel/src/managers/task/task_init.c @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: 2023 Ledger SAS +// SPDX-FileCopyrightText: 2026 H2Lab Development Team // SPDX-License-Identifier: Apache-2.0 /** @@ -15,6 +16,7 @@ #include #include #include +#include #include #include "task_core.h" @@ -123,6 +125,11 @@ static inline kstatus_t task_init_discover_sanitation(task_meta_t const * const return status; } +#if CONFIG_SECU_METADATA_SHA256_CHECK +/* TODO: dev, shm and dma list should be removed from metadata, replaced by dts only info, fixing metadata size definitively */ +static_assert(sizeof(task_meta_t) > 200, "beware that the task_meta_t structure may be too big to be copied on stack!"); +#endif + /** * @brief check_meta_integrity state handling * @@ -139,8 +146,30 @@ static inline kstatus_t task_init_check_meta_integrity(task_meta_t const * const ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; goto end; } - /* FIXME: call the sha256 service in order to validate metadata integrity, - and return the result */ +#if CONFIG_SECU_METADATA_SHA256_CHECK + task_meta_t meta_copy; + uint8_t digest[SHA256_DIGEST_SIZE]; + + if (memcpy(&meta_copy, meta, sizeof(meta_copy)) != &meta_copy) { + pr_err("[task %08x] unable to copy metadata for sha256 verification", meta->label); + ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; + goto end; + } + + (void)memset(meta_copy.metadata_sha256, 0x0, sizeof(meta_copy.metadata_sha256)); + + if (sha256((const uint8_t *)&meta_copy, sizeof(meta_copy), digest) != 0) { + pr_err("[task %08x] metadata sha256 computation failed", meta->label); + ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; + goto end; + } + + if (memcmp(meta->metadata_sha256, digest, sizeof(digest)) != 0) { + pr_err("[task %08x] metadata sha256 mismatch", meta->label); + ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; + goto end; + } +#endif pr_info("[task %08x] metadata integrity ok", meta->label); ctx.state = TASK_MANAGER_STATE_CHECK_TSK_INTEGRITY; status = K_STATUS_OKAY; From 8d1623001f0cd985ad303d583ef4e151140d318f Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Fri, 1 May 2026 08:15:21 +0200 Subject: [PATCH 4/8] fix: fixing invalid static assertion on metadata size --- kernel/src/managers/task/task_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/src/managers/task/task_init.c b/kernel/src/managers/task/task_init.c index 33b7b610..b95697aa 100644 --- a/kernel/src/managers/task/task_init.c +++ b/kernel/src/managers/task/task_init.c @@ -127,7 +127,7 @@ static inline kstatus_t task_init_discover_sanitation(task_meta_t const * const #if CONFIG_SECU_METADATA_SHA256_CHECK /* TODO: dev, shm and dma list should be removed from metadata, replaced by dts only info, fixing metadata size definitively */ -static_assert(sizeof(task_meta_t) > 200, "beware that the task_meta_t structure may be too big to be copied on stack!"); +static_assert(sizeof(task_meta_t) <= 200, "beware that the task_meta_t structure may be too big to be copied on stack!"); #endif /** From b3b0b143b9db5f8e91f0645d75a18304abcc4a32 Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Fri, 1 May 2026 09:04:03 +0200 Subject: [PATCH 5/8] zlib,string: adding required memcmp() implementation for sha256 comparison --- kernel/include/sentry/zlib/string.h | 12 ++++++++ kernel/src/zlib/string.c | 44 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/kernel/include/sentry/zlib/string.h b/kernel/include/sentry/zlib/string.h index 0428f176..3bff5373 100644 --- a/kernel/include/sentry/zlib/string.h +++ b/kernel/include/sentry/zlib/string.h @@ -93,6 +93,18 @@ void *sentry_memset(void *s, int c, unsigned int n); */ void *sentry_memcpy(void * restrict dest, const void* restrict src, size_t n); +/*@ + * requires valid_read(s1+(0..n-1)); + * requires valid_read(s2+(0..n-1)); + * requires \initialized(s1+(0..n-1)); + * requires \initialized(s2+(0..n-1)); + * assigns \result \from indirect:s1, indirect:s2, indirect:n; + * ensures \result == 0 <==> \forall integer k; 0 <= k < n ==> ((uint8_t*)s1)[k] == ((uint8_t*)s2)[k]; + * ensures \result < 0 <==> \exists integer k; 0 <= k < n && ((uint8_t*)s1)[k] < ((uint8_t*)s2)[k] && \forall integer j; 0 <= j < k ==> ((uint8_t*)s1)[j] == ((uint8_t*)s2)[j]; + * ensures \result > 0 <==> \exists integer k; 0 <= k < n && ((uint8_t*)s1)[k] > ((uint8_t*)s2)[k] && \forall integer j; 0 <= j < k ==> ((uint8_t*)s1)[j] == ((uint8_t*)s2)[j]; + */ +int sentry_memcmp(const void *s1, const void *s2, size_t n); + /** * Note: because frama-c as no link notion and uses cpp+internal analysis, we cannot * use the __attribute__((alias("fct"))) trick to alias the sentry_ prefixed diff --git a/kernel/src/zlib/string.c b/kernel/src/zlib/string.c index 6be6316d..c917e3fe 100644 --- a/kernel/src/zlib/string.c +++ b/kernel/src/zlib/string.c @@ -252,6 +252,49 @@ void *sentry_memcpy(void * restrict dest, const void* restrict src, size_t n) return dest; } +/** + * @brief Compare n first bytes of s1 and s2, returning an integer less than, equal to, or greater than zero if s1 is found, respectively, to be less than, to match, or be greater than s2. + * + * INFO: The C standard says that null argument(s) to string functions produce undefined behavior. Here such a case is trapped. + * + * @conforming to: + * POSIX.1-2001, POSIX.1-2008, C89, C99, SVr4, 4.3BSD. + * + * @param s1 first memory area to compare + * @param s2 second memory area to compare + * @param n number of bytes to compare + * + * @return an integer less than, equal to, or greater than zero if s1 is found, respectively, to be less than, to match, or be greater than s2. + */ +int sentry_memcmp(const void *s1, const void *s2, size_t n) +{ + int res = 0; + if (unlikely(s1 == NULL || s2 == NULL)) { + goto err; + } + /*@ assert \valid_read((uint8_t*)s1 + (0 .. (n-1))); */ + /*@ assert \valid_read((uint8_t*)s2 + (0 .. (n-1))); */ + /*@ assert \initialized((uint8_t*)s1+(0..n-1)); */ + /*@ assert \initialized((uint8_t*)s2+(0..n-1)); */ + /*@ assert \separated((uint8_t*)s1 + (0 .. n-1), (uint8_t*)s2 + (0 .. n-1)); */ + + /*@ + * loop invariant 0 <= i <= n; + * loop invariant res == 0; + * loop invariant \forall integer k; 0 <= k < i ==> ((uint8_t*)s1)[k] == ((uint8_t*)s2)[k]; + * loop assigns i, res; + * loop variant n - i; + */ + for (size_t i = 0; i < n; i++) { + if (((uint8_t*)s1)[i] != ((uint8_t*)s2)[i]) { + res = ((uint8_t*)s1)[i] - ((uint8_t*)s2)[i]; + goto err; + } + } +err: + return res; +} + #ifndef TEST_MODE #ifndef __FRAMAC__ /** NOTE: FramaC requires that we use its own compiler builtins (see -eva-builtins-list for more info) */ @@ -259,5 +302,6 @@ void *sentry_memcpy(void * restrict dest, const void* restrict src, size_t n) size_t strnlen(const char *s, size_t maxlen) __attribute__((alias("sentry_strnlen"))); void* memset(void *s, int c, unsigned int n) __attribute__((alias("sentry_memset"))); void* memcpy(void * restrict d, const void * restrict s, size_t) __attribute__((alias("sentry_memcpy"))); +int memcmp(const void *s1, const void *s2, size_t n) __attribute__((alias("sentry_memcmp"))); #endif #endif/*!TEST_MODE*/ From ee3c096bc82c62a8be0cb36d631bae17e0a3906a Mon Sep 17 00:00:00 2001 From: Florent Valette Date: Sat, 2 May 2026 15:47:46 +0200 Subject: [PATCH 6/8] tools,genmetadata: remove code duplication Use C++20 std::span and std::as_bytes to serialize reflactable metadata to std::vector, remove duplicate code for vector and ofstream output. Write to file is out of scope of serialization. --- tools/genmetadata/src/arch/arch.hpp | 1 + tools/genmetadata/src/main.cpp | 19 +++--- tools/genmetadata/src/reflect.hpp | 95 +++++++------------------- tools/genmetadata/src/task_meta.hpp.in | 1 + 4 files changed, 39 insertions(+), 77 deletions(-) diff --git a/tools/genmetadata/src/arch/arch.hpp b/tools/genmetadata/src/arch/arch.hpp index 52180cce..02703a50 100644 --- a/tools/genmetadata/src/arch/arch.hpp +++ b/tools/genmetadata/src/arch/arch.hpp @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: 2024 Ledger SAS +// SPDX-FileCopyrightText: 2026 H2Lab // SPDX-License-Identifier: Apache-2.0 /* diff --git a/tools/genmetadata/src/main.cpp b/tools/genmetadata/src/main.cpp index ea82980b..b6f22f49 100644 --- a/tools/genmetadata/src/main.cpp +++ b/tools/genmetadata/src/main.cpp @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: 2024 Ledger SAS +// SPDX-FileCopyrightText: 2026 H2Lab // SPDX-License-Identifier: Apache-2.0 /* @@ -50,21 +51,23 @@ int main(int argc, char *argv[]) auto data = json::parse(in); auto meta = taskMetadata::from_json(data["task_meta"]); - std::array metadata_sha256{}; - if (!meta.set_u8_array_field("metadata_sha256", metadata_sha256)) { + std::array sha256{}; + if (!meta.set_u8_array_field("metadata_sha256", sha256)) { throw std::runtime_error("metadata_sha256 field not found or has unexpected type"); } - auto blob = reflect_to_bytes(meta); - if (SHA256(blob.data(), blob.size(), metadata_sha256.data()) == nullptr) { + auto blob = reflect::to_bytes(meta); + if (SHA256(reinterpret_cast(blob.data()), blob.size(), sha256.data()) == nullptr) { throw std::runtime_error("failed to compute metadata_sha256"); } - if (!meta.set_u8_array_field("metadata_sha256", metadata_sha256)) { - throw std::runtime_error("failed to write metadata_sha256 field"); - } + // update sha256 in bytearray which is the last field, has to be written at the end of container. + // reverse copy of sha256 (i.e.from end IT to start IT) into reverse begin blob IT. + auto sha256_raw = std::as_bytes(std::span{sha256}); + std::reverse_copy(std::begin(sha256_raw), std::end(sha256_raw), std::rbegin(blob)); - reflect_to_bin(meta, out); + std::ofstream f(out, std::ios::binary); + f.write(reinterpret_cast(blob.data()), static_cast(blob.size())); } catch (const std::exception& err) { std::cerr << err.what() << std::endl; diff --git a/tools/genmetadata/src/reflect.hpp b/tools/genmetadata/src/reflect.hpp index a7028998..b44fea1a 100644 --- a/tools/genmetadata/src/reflect.hpp +++ b/tools/genmetadata/src/reflect.hpp @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: 2024 Ledger SAS +// SPDX-FileCopyrightText: 2026 H2Lab // SPDX-License-Identifier: Apache-2.0 /* @@ -26,6 +27,11 @@ #include #include #include +#include + +namespace reflect { + +using bytearray_t = std::vector; template> struct scalar_storage { @@ -61,105 +67,56 @@ concept Reflectable = requires(T t) { { t.reflect() } -> return_tuple; }; -template, bool>::type = true> -static inline void __to_bin(T val, std::ofstream& out) -{ - using storage_t = scalar_storage_t; - static_assert(!std::is_void_v, "unsupported scalar size"); - constexpr auto size = MemorySpec::template size_of(); - constexpr auto alignment = MemorySpec::template size_of(); - constexpr std::array zero{}; - auto current = out.tellp(); - auto padding = size - (current % alignment); - - /* Insert padding if not aligned */ - if (padding < size) { - out.write(zero.data(), padding); - } - - out.write(reinterpret_cast(&val), size); -} - -template, bool>::type = true> -static inline void __to_bin(T val, std::vector& out) -{ +template +static inline void __to_bytes(std::span bytes, bytearray_t& out ) { using storage_t = scalar_storage_t; static_assert(!std::is_void_v, "unsupported scalar size"); constexpr auto size = MemorySpec::template size_of(); constexpr auto alignment = MemorySpec::template size_of(); auto padding = (size - (out.size() % alignment)) % size; - out.insert(out.end(), padding, 0); - auto ptr = reinterpret_cast(&val); - out.insert(out.end(), ptr, ptr + size); + out.insert(out.end(), padding, std::byte{0}); + out.insert(out.end(), bytes.begin(), bytes.end()); } -template -static inline void __to_bin(job_flags_t val, std::ofstream& out) +template, bool>::type = true> +static inline void __to_bytes(T val, bytearray_t& out) { - uint32_t tmp; - std::memcpy(&tmp, &val, sizeof(val)); - __to_bin(tmp, out); + std::array tmp{val}; + __to_bytes(std::as_bytes(std::span{tmp}), out); } template -static inline void __to_bin(job_flags_t val, std::vector& out) +static inline void __to_bytes(job_flags_t val, bytearray_t& out) { uint32_t tmp; std::memcpy(&tmp, &val, sizeof(val)); - __to_bin(tmp, out); -} - -template -static inline void __to_bin(std::array val, std::ofstream& out) -{ - for (T cell: val) { - __to_bin(cell, out); - } + __to_bytes(tmp, out); } template -static inline void __to_bin(std::array val, std::vector& out) -{ - for (T cell: val) { - __to_bin(cell, out); - } -} - -template -static inline void _to_bin(const Tuple& reflection, std::ofstream& out, std::index_sequence) +static inline void __to_bytes(std::array val, bytearray_t& out) { - ((__to_bin(std::get(reflection), out)), ...); + __to_bytes(std::as_bytes(std::span{val}), out); } template -static inline void _to_bin(const Tuple& reflection, std::vector& out, std::index_sequence) -{ - ((__to_bin(std::get(reflection), out)), ...); -} - -template -static inline void _to_bin(const std::tuple& reflection, std::ofstream& out) +static inline void _to_bytes(const Tuple& reflection, bytearray_t& out, std::index_sequence) { - _to_bin(reflection, out, std::index_sequence_for{}); + ((__to_bytes(std::get(reflection), out)), ...); } template -static inline void _to_bin(const std::tuple& reflection, std::vector& out) +static inline void _to_bytes(const std::tuple& reflection, bytearray_t& out) { - _to_bin(reflection, out, std::index_sequence_for{}); + _to_bytes(reflection, out, std::index_sequence_for{}); } template -static inline std::vector reflect_to_bytes(const T& obj) { - std::vector out; - _to_bin(obj.reflect(), out); +static inline auto to_bytes(const T& obj) { + bytearray_t out; + _to_bytes(obj.reflect(), out); return out; } -template -static inline void reflect_to_bin(const T& obj, const std::string& filename) { - auto blob = reflect_to_bytes(obj); - std::ofstream out(filename, std::ios::binary); - out.write(reinterpret_cast(blob.data()), static_cast(blob.size())); -} +} // namespace reflect diff --git a/tools/genmetadata/src/task_meta.hpp.in b/tools/genmetadata/src/task_meta.hpp.in index 963de0a0..c585044f 100644 --- a/tools/genmetadata/src/task_meta.hpp.in +++ b/tools/genmetadata/src/task_meta.hpp.in @@ -1,4 +1,5 @@ // SPDX-FileCopyrightText: 2024 Ledger SAS +// SPDX-FileCopyrightText: 2026 H2Lab // SPDX-License-Identifier: Apache-2.0 /* From 26ac8b3de3d7f6da463d54d279d13524c8e42c61 Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Sun, 3 May 2026 09:32:34 +0200 Subject: [PATCH 7/8] zlib,proof: fixing WP contract with proper ACSL --- kernel/include/sentry/zlib/string.h | 16 ++++++++-------- kernel/src/zlib/string.c | 10 +++++----- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/kernel/include/sentry/zlib/string.h b/kernel/include/sentry/zlib/string.h index 3bff5373..7300b070 100644 --- a/kernel/include/sentry/zlib/string.h +++ b/kernel/include/sentry/zlib/string.h @@ -94,14 +94,14 @@ void *sentry_memset(void *s, int c, unsigned int n); void *sentry_memcpy(void * restrict dest, const void* restrict src, size_t n); /*@ - * requires valid_read(s1+(0..n-1)); - * requires valid_read(s2+(0..n-1)); - * requires \initialized(s1+(0..n-1)); - * requires \initialized(s2+(0..n-1)); - * assigns \result \from indirect:s1, indirect:s2, indirect:n; - * ensures \result == 0 <==> \forall integer k; 0 <= k < n ==> ((uint8_t*)s1)[k] == ((uint8_t*)s2)[k]; - * ensures \result < 0 <==> \exists integer k; 0 <= k < n && ((uint8_t*)s1)[k] < ((uint8_t*)s2)[k] && \forall integer j; 0 <= j < k ==> ((uint8_t*)s1)[j] == ((uint8_t*)s2)[j]; - * ensures \result > 0 <==> \exists integer k; 0 <= k < n && ((uint8_t*)s1)[k] > ((uint8_t*)s2)[k] && \forall integer j; 0 <= j < k ==> ((uint8_t*)s1)[j] == ((uint8_t*)s2)[j]; + requires \valid_read((uint8_t*)s1+(0..n-1)); + requires \valid_read((uint8_t*)s2+(0..n-1)); + requires \initialized((uint8_t*)s1+(0..n-1)); + requires \initialized((uint8_t*)s2+(0..n-1)); + assigns \result \from indirect:s1, indirect:s2, indirect:n; + ensures \result == 0 <==> \forall integer k; 0 <= k < n ==> ((uint8_t*)s1)[k] == ((uint8_t*)s2)[k]; + ensures \result < 0 <==> \exists integer k; 0 <= k < n && ((uint8_t*)s1)[k] < ((uint8_t*)s2)[k] && \forall integer j; 0 <= j < k ==> ((uint8_t*)s1)[j] == ((uint8_t*)s2)[j]; + ensures \result > 0 <==> \exists integer k; 0 <= k < n && ((uint8_t*)s1)[k] > ((uint8_t*)s2)[k] && \forall integer j; 0 <= j < k ==> ((uint8_t*)s1)[j] == ((uint8_t*)s2)[j]; */ int sentry_memcmp(const void *s1, const void *s2, size_t n); diff --git a/kernel/src/zlib/string.c b/kernel/src/zlib/string.c index c917e3fe..8fb3e9ac 100644 --- a/kernel/src/zlib/string.c +++ b/kernel/src/zlib/string.c @@ -279,11 +279,11 @@ int sentry_memcmp(const void *s1, const void *s2, size_t n) /*@ assert \separated((uint8_t*)s1 + (0 .. n-1), (uint8_t*)s2 + (0 .. n-1)); */ /*@ - * loop invariant 0 <= i <= n; - * loop invariant res == 0; - * loop invariant \forall integer k; 0 <= k < i ==> ((uint8_t*)s1)[k] == ((uint8_t*)s2)[k]; - * loop assigns i, res; - * loop variant n - i; + loop invariant 0 <= i <= n; + loop invariant res == 0; + loop invariant \forall integer k; 0 <= k < i ==> ((uint8_t*)s1)[k] == ((uint8_t*)s2)[k]; + loop assigns i, res; + loop variant n - i; */ for (size_t i = 0; i < n; i++) { if (((uint8_t*)s1)[i] != ((uint8_t*)s2)[i]) { From 444ee9ac730daf6f6a50d5f40d188e31191370d8 Mon Sep 17 00:00:00 2001 From: Philippe Thierry Date: Wed, 3 Jun 2026 07:45:45 +0200 Subject: [PATCH 8/8] sha256: removing metadata_sha256 field zeroification, helping with sha256 runtime calculation --- kernel/src/managers/task/task_init.c | 8 ++------ tools/genmetadata/src/main.cpp | 7 ++----- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/kernel/src/managers/task/task_init.c b/kernel/src/managers/task/task_init.c index b95697aa..49e639ac 100644 --- a/kernel/src/managers/task/task_init.c +++ b/kernel/src/managers/task/task_init.c @@ -125,11 +125,6 @@ static inline kstatus_t task_init_discover_sanitation(task_meta_t const * const return status; } -#if CONFIG_SECU_METADATA_SHA256_CHECK -/* TODO: dev, shm and dma list should be removed from metadata, replaced by dts only info, fixing metadata size definitively */ -static_assert(sizeof(task_meta_t) <= 200, "beware that the task_meta_t structure may be too big to be copied on stack!"); -#endif - /** * @brief check_meta_integrity state handling * @@ -158,7 +153,8 @@ static inline kstatus_t task_init_check_meta_integrity(task_meta_t const * const (void)memset(meta_copy.metadata_sha256, 0x0, sizeof(meta_copy.metadata_sha256)); - if (sha256((const uint8_t *)&meta_copy, sizeof(meta_copy), digest) != 0) { + /* recalculate the sha256 of the metadata. the metadata_sha256 is not a part of the calculation */ + if (sha256((const uint8_t *)meta, offsetof(task_meta_t, metadata_sha256), digest) != 0) { pr_err("[task %08x] metadata sha256 computation failed", meta->label); ctx.state = TASK_MANAGER_STATE_ERROR_SECURITY; goto end; diff --git a/tools/genmetadata/src/main.cpp b/tools/genmetadata/src/main.cpp index b6f22f49..5b3d2f71 100644 --- a/tools/genmetadata/src/main.cpp +++ b/tools/genmetadata/src/main.cpp @@ -52,12 +52,9 @@ int main(int argc, char *argv[]) auto meta = taskMetadata::from_json(data["task_meta"]); std::array sha256{}; - if (!meta.set_u8_array_field("metadata_sha256", sha256)) { - throw std::runtime_error("metadata_sha256 field not found or has unexpected type"); - } - auto blob = reflect::to_bytes(meta); - if (SHA256(reinterpret_cast(blob.data()), blob.size(), sha256.data()) == nullptr) { + const auto hashed_blob_size = blob.size() - sha256.size(); + if (SHA256(reinterpret_cast(blob.data()), hashed_blob_size, sha256.data()) == nullptr) { throw std::runtime_error("failed to compute metadata_sha256"); }