F-7053: sign: propagate make_header() failure to the exit status

main() called make_header()/make_hybrid_header() and discarded their
return value. Both are wrappers around make_header_ex(), which returns
-1 on every "goto failure" path (image file not openable, header malloc
failure, firmware version out of range, certificate chain errors,
signing and output write errors). Since ret is initialized to 0 and is
only reassigned by the optional base_diff() delta step, a signing run
that produced no output image still terminated with status 0, so
Makefile recipes and CI treated the failure as success and moved on
with a missing or stale *_v<ver>_signed.bin. This was also asymmetric
with the key loading path just above, which exits on failure.

Capture the return value of both header helpers, skip the delta step
when header generation failed, and let main() return it.

Add tools/unit-tests/unit-sign-header-failure, covering the exit status
of both the plain and the hybrid signing path when the input image
cannot be opened.
pull/842/head
Daniele Lacamera 2026-08-04 08:16:53 +02:00
parent 675a927fa2
commit dfdcf7eeb5
3 changed files with 143 additions and 5 deletions

View File

@ -3746,8 +3746,8 @@ int main(int argc, char** argv)
exit(1);
}
printf("Creating hybrid signature\n");
make_hybrid_header(pubkey, pubkey_sz, CMD.image_file, CMD.output_image_file,
pubkey2, pubkey_sz2);
ret = make_hybrid_header(pubkey, pubkey_sz, CMD.image_file,
CMD.output_image_file, pubkey2, pubkey_sz2);
DEBUG_PRINT("Signature size: %u\n", CMD.signature_sz);
DEBUG_PRINT("Secondary signature size: %u\n", CMD.secondary_signature_sz);
DEBUG_PRINT("Header size: %u\n", CMD.header_sz);
@ -3756,11 +3756,13 @@ int main(int argc, char** argv)
if (pubkey2)
free(pubkey2);
} else {
make_header(pubkey, pubkey_sz, CMD.image_file, CMD.output_image_file);
ret = make_header(pubkey, pubkey_sz, CMD.image_file,
CMD.output_image_file);
}
if (CMD.delta) {
/* Skip the delta step and propagate the failure to the caller if the
* signed image could not be created. */
if ((ret == 0) && CMD.delta) {
if (CMD.encrypt)
ret = base_diff(CMD.delta_base_file, pubkey, pubkey_sz, 64);
else

View File

@ -62,6 +62,7 @@ TESTS:=unit-parser unit-fdt unit-extflash unit-string unit-spi-flash unit-aes128
unit-tpm-blob unit-policy-create unit-policy-sign unit-rot-auth unit-sdhci-response-bits \
unit-sdhci-disk-unaligned unit-sign-encrypted-output \
unit-sign-hybrid-keyload \
unit-sign-header-failure \
unit-keygen-xmss-params
TESTS+=unit-tpm-check-rot-auth
TESTS+=unit-tpm-api-names
@ -332,6 +333,14 @@ unit-sign-hybrid-keyload: ../../include/target.h unit-sign-hybrid-keyload.c \
-ffunction-sections -fdata-sections \
$(LDFLAGS) -Wl,--gc-sections
unit-sign-header-failure: ../../include/target.h unit-sign-header-failure.c \
$(KEYTOOLS_SIGN_SRCS)
gcc -o $@ $^ -I../keytools $(CFLAGS) -DML_DSA_LEVEL=2 -DDELTA_UPDATES \
-D"LMS_LEVELS=1" -D"LMS_HEIGHT=10" -D"LMS_WINTERNITZ=8" \
-DWOLFBOOT_XMSS_PARAMS=\"XMSS-SHA2_10_256\" \
-ffunction-sections -fdata-sections \
$(LDFLAGS) -Wl,--gc-sections
unit-keygen-xmss-params: ../../include/target.h unit-keygen-xmss-params.c
gcc -o $@ $^ -I../keytools $(CFLAGS) -DML_DSA_LEVEL=2 \
-D"LMS_LEVELS=1" -D"LMS_HEIGHT=10" -D"LMS_WINTERNITZ=8" \

View File

@ -0,0 +1,127 @@
/* unit-sign-header-failure.c
*
* Unit test for sign tool exit status when the manifest cannot be created.
*/
#include <check.h>
#include <limits.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#define WOLFBOOT_HASH_SHA256
#define IMAGE_HEADER_SIZE 512
#define main wolfboot_sign_main
#include "../keytools/sign.c"
#undef main
static const char missing_image[] = "/nonexistent/wolfboot-image.bin";
static int write_file(const char *path, const void *buf, size_t len)
{
FILE *f = fopen(path, "wb");
size_t written;
if (f == NULL) {
return -1;
}
written = fwrite(buf, 1, len, f);
fclose(f);
return written == len ? 0 : -1;
}
/* The sign tool must report a failure to the caller (make, CI) when
* make_header() could not produce the output image. */
START_TEST(test_sign_main_fails_when_image_missing)
{
char tempdir[] = "/tmp/wolfboot-sign-XXXXXX";
char key_path[PATH_MAX];
uint8_t raw_pubkey[64]; /* ECC256 raw Qx + Qy */
char *argv[6];
ck_assert_ptr_nonnull(mkdtemp(tempdir));
snprintf(key_path, sizeof(key_path), "%s/ecc256.raw", tempdir);
memset(raw_pubkey, 0x11, sizeof(raw_pubkey));
ck_assert_int_eq(write_file(key_path, raw_pubkey, sizeof(raw_pubkey)), 0);
argv[0] = "sign";
argv[1] = "--sha-only";
argv[2] = "--ecc256";
argv[3] = (char *)missing_image;
argv[4] = key_path;
argv[5] = "1";
ck_assert_int_ne(wolfboot_sign_main(6, argv), 0);
unlink(key_path);
rmdir(tempdir);
}
END_TEST
/* Same contract for the hybrid path, which uses make_hybrid_header(). */
START_TEST(test_sign_main_fails_when_image_missing_hybrid)
{
char tempdir[] = "/tmp/wolfboot-sign-XXXXXX";
char key_path[PATH_MAX];
char key2_path[PATH_MAX];
uint8_t raw_pubkey[64]; /* ECC256 raw Qx + Qy */
uint8_t raw_pubkey2[32]; /* ED25519 raw public key */
char *argv[8];
ck_assert_ptr_nonnull(mkdtemp(tempdir));
snprintf(key_path, sizeof(key_path), "%s/ecc256.raw", tempdir);
snprintf(key2_path, sizeof(key2_path), "%s/ed25519.raw", tempdir);
memset(raw_pubkey, 0x11, sizeof(raw_pubkey));
memset(raw_pubkey2, 0x22, sizeof(raw_pubkey2));
ck_assert_int_eq(write_file(key_path, raw_pubkey, sizeof(raw_pubkey)), 0);
ck_assert_int_eq(write_file(key2_path, raw_pubkey2, sizeof(raw_pubkey2)),
0);
argv[0] = "sign";
argv[1] = "--sha-only";
argv[2] = "--ecc256";
argv[3] = "--ed25519";
argv[4] = (char *)missing_image;
argv[5] = key_path;
argv[6] = key2_path;
argv[7] = "1";
ck_assert_int_ne(wolfboot_sign_main(8, argv), 0);
unlink(key_path);
unlink(key2_path);
rmdir(tempdir);
}
END_TEST
Suite *wolfboot_suite(void)
{
Suite *s = suite_create("sign-header-failure");
TCase *tcase = tcase_create("make-header");
tcase_add_test(tcase, test_sign_main_fails_when_image_missing);
tcase_add_test(tcase, test_sign_main_fails_when_image_missing_hybrid);
suite_add_tcase(s, tcase);
return s;
}
int main(void)
{
int failed;
Suite *s = wolfboot_suite();
SRunner *runner = srunner_create(s);
srunner_run_all(runner, CK_NORMAL);
failed = srunner_ntests_failed(runner);
srunner_free(runner);
return failed == 0 ? 0 : 1;
}