config refactor#766
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors wolfCrypt/wolfSSL configuration into composable include/user_settings/* fragments and simplifies Make-side -D flag emission for hash-based signature parameterization.
Changes:
- Splits the monolithic
include/user_settings.hinto ordered “fragment” headers (cascade/base/sign/hash/features/finalize) and turnsuser_settings.hinto a dispatcher. - Moves SIGN/HASH algorithm-specific configuration into dedicated
sign_*.handhash_*.hfragments with central dispatch headers. - Updates
options.mkso LMS/XMSS Make variables carry only user-provided parameter values, with wolfCrypt-side defines derived in headers.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| options.mk | Drops wolfCrypt-side LMS/XMSS defines from Make flags; keeps only user parameter -Ds. |
| include/user_settings.h | Replaced large inline configuration with ordered includes of fragment headers. |
| include/user_settings/base.h | New: baseline wolfCrypt settings shared by all builds. |
| include/user_settings/cascade.h | New: feature implication cascades + WOLFBOOT_NEEDS_* markers. |
| include/user_settings/sign_dispatch.h | New: includes per-signature fragments based on SIGN flags. |
| include/user_settings/sign_rsa.h | New: RSA verification configuration (and NO_RSA fallback). |
| include/user_settings/sign_ecc.h | New: ECC verification configuration and carve-outs. |
| include/user_settings/sign_ed25519.h | New: ED25519 verification configuration and carve-outs. |
| include/user_settings/sign_ed448.h | New: ED448 verification configuration and carve-outs. |
| include/user_settings/sign_ml_dsa.h | New: ML-DSA (Dilithium) verification configuration and carve-outs. |
| include/user_settings/sign_lms.h | New: LMS verification config; maps Make parameters to wolfCrypt defines. |
| include/user_settings/sign_xmss.h | New: XMSS verification config; maps Make parameters to wolfCrypt defines. |
| include/user_settings/hash_dispatch.h | New: includes hash fragments based on WOLFBOOT_HASH_*. |
| include/user_settings/hash_sha384.h | New: SHA-384 hash selection fragment (+ optional NO_SHA256). |
| include/user_settings/hash_sha3.h | New: SHA3-384 hash selection fragment (+ optional NO_SHA256). |
| include/user_settings/encrypt.h | New: EXT_ENCRYPTED / SECURE_PKCS11 wolfCrypt configuration. |
| include/user_settings/trustzone.h | New: TrustZone secure-mode wolfCrypt configuration. |
| include/user_settings/tpm.h | New: wolfTPM-related config for WOLFBOOT_TPM builds. |
| include/user_settings/wolfhsm.h | New: crypto-callback/key-gen config for wolfHSM client/server builds. |
| include/user_settings/cert_chain.h | New: cert-chain verify mode config for wolfHSM server. |
| include/user_settings/renesas.h | New: Renesas HW crypto offload settings. |
| include/user_settings/platform.h | New: platform-specific SP-math word-size and minor platform knobs. |
| include/user_settings/test_bench.h | New: test/benchmark-specific configuration and RNG selection. |
| include/user_settings/finalize.h | New: reconciles WOLFBOOT_NEEDS_* into NO_* / WC_NO_* and global disables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #ifndef WOLFBOOT_NEEDS_RNG | ||
| # define WC_NO_RNG | ||
| #endif | ||
|
|
||
| #ifndef WOLFBOOT_NEEDS_AES | ||
| # define NO_AES | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_AES_CBC | ||
| # define NO_AES_CBC | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_HMAC | ||
| # define NO_HMAC | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_DEV_RANDOM | ||
| # define NO_DEV_RANDOM | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_ECC_KEY_EXPORT | ||
| # define NO_ECC_KEY_EXPORT | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_ASN | ||
| # define NO_ASN | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_CMAC | ||
| # define NO_CMAC | ||
| #endif | ||
| #ifndef WOLFBOOT_NEEDS_KDF | ||
| # define NO_KDF | ||
| #endif | ||
|
|
||
| /* RSA: skip NO_RSA when NEEDS_RSA is set. */ | ||
| #ifndef WOLFBOOT_NEEDS_RSA | ||
| # define NO_RSA | ||
| #endif |
There was a problem hiding this comment.
Several “negative default” macros are #define’d without checking whether they were already defined earlier by upstream headers or toolchain flags. This can produce noisy macro-redefinition warnings (and makes it harder to intentionally override a default). Recommendation: make these idempotent by adding && !defined(<macro>) to the condition or wrapping each #define with #ifndef <macro> (e.g., #if !defined(WOLFBOOT_NEEDS_AES) && !defined(NO_AES)), similar to how you handled WC_NO_HASHDRBG above.
| #ifdef WOLFCRYPT_SECURE_MODE | ||
| int hal_trng_get_entropy(unsigned char *out, unsigned len); | ||
| # define CUSTOM_RAND_GENERATE_SEED hal_trng_get_entropy | ||
| /* SECURE_MODE outside test/bench used to enable AES CFB explicitly | ||
| * via the finalize.h else-else branch. Keep that behavior here so | ||
| * the TZ-PSA, TZ-FWTPM, and any future SECURE_MODE sub-mode all | ||
| * have AES CFB available. */ | ||
| # ifndef WOLFSSL_AES_CFB | ||
| # define WOLFSSL_AES_CFB | ||
| # endif | ||
| #endif | ||
|
|
||
| #if defined(WOLFCRYPT_TZ_PSA) | ||
| # define WOLFSSL_AES_COUNTER | ||
| # define WOLFSSL_AES_GCM | ||
| # define HAVE_AESGCM | ||
| # define HAVE_AESCCM | ||
| # define HAVE_AES_ECB | ||
| # define WOLFSSL_AES_CFB | ||
| # define WOLFSSL_AES_OFB | ||
| # ifndef NO_DES3 | ||
| # define NO_DES3 | ||
| # endif | ||
| # ifndef NO_DES3_TLS_SUITES | ||
| # define NO_DES3_TLS_SUITES | ||
| # endif | ||
| # define HAVE_CHACHA | ||
| # define HAVE_POLY1305 | ||
| # define WOLFSSL_CMAC | ||
| # define WOLFSSL_ECDSA_DETERMINISTIC_K | ||
| # define WOLFSSL_HAVE_PRF | ||
| # define HAVE_HKDF | ||
| # define HAVE_PBKDF2 | ||
| # define HAVE_PWDBASED | ||
| # define WOLFSSL_KEY_GEN | ||
| # define WC_RSA_PSS | ||
| # define WOLFSSL_PSS_SALT_LEN_DISCOVER | ||
| # define WOLFSSL_RSA_OAEP | ||
| # define HAVE_ECC_KEY_EXPORT | ||
| # define HAVE_ECC_KEY_IMPORT | ||
| #endif | ||
|
|
||
| #if defined(WOLFBOOT_TZ_FWTPM) | ||
| # define WOLFSSL_AES_CFB | ||
| # define WOLFSSL_SHA384 | ||
| #endif |
There was a problem hiding this comment.
WOLFSSL_AES_CFB is already conditionally defined in the WOLFCRYPT_SECURE_MODE block, but is later defined again unconditionally in both WOLFCRYPT_TZ_PSA and WOLFBOOT_TZ_FWTPM. This can trigger macro redefinition warnings on some toolchains. Recommendation: either remove the later #define WOLFSSL_AES_CFB lines (since secure-mode already ensures it) or guard them with #ifndef WOLFSSL_AES_CFB for idempotency, matching the pattern used in the secure-mode block.
| # define WOLFSSL_SP_MATH | ||
| # define WOLFSSL_SP_SMALL | ||
| # define WOLFSSL_HAVE_SP_ECC |
There was a problem hiding this comment.
WOLFSSL_SP_MATH, WOLFSSL_SP_SMALL, and WOLFSSL_HAVE_SP_ECC are defined in the “enabled” branch and then defined again in the later “SP MATH” block, which can cause macro redefinition warnings and makes the intent harder to read. Recommendation: consolidate these into a single place (e.g., keep only the later block), or guard the later block with #ifndef WOLFSSL_SP_MATH / #ifndef WOLFSSL_HAVE_SP_ECC (or broaden the condition so the second block only runs when the first didn’t).
| # define WOLFSSL_SP_MATH | |
| # define WOLFSSL_SP_SMALL | |
| # define WOLFSSL_HAVE_SP_ECC | |
| # if !defined(WOLFSSL_SP_MATH) | |
| # define WOLFSSL_SP_MATH | |
| # endif | |
| # if !defined(WOLFSSL_SP_SMALL) | |
| # define WOLFSSL_SP_SMALL | |
| # endif | |
| # if !defined(WOLFSSL_HAVE_SP_ECC) | |
| # define WOLFSSL_HAVE_SP_ECC | |
| # endif |
| /* Any RSA SIGN flag (or WOLFCRYPT_SECURE_MODE without PKCS11_SMALL) means | ||
| * the build links wolfCrypt's RSA code. sign_rsa.h handles the actual | ||
| * configuration; the marker is set here so finalize.h can see it ahead | ||
| * of finalize-time and skip NO_ASN. */ | ||
| #if defined(WOLFBOOT_SIGN_RSA2048) || \ | ||
| defined(WOLFBOOT_SIGN_RSA3072) || \ | ||
| defined(WOLFBOOT_SIGN_RSA4096) || \ | ||
| defined(WOLFBOOT_SIGN_SECONDARY_RSA2048) || \ | ||
| defined(WOLFBOOT_SIGN_SECONDARY_RSA3072) || \ | ||
| defined(WOLFBOOT_SIGN_SECONDARY_RSA4096) || \ | ||
| defined(WOLFBOOT_SIGN_RSAPSS2048) || \ | ||
| defined(WOLFBOOT_SIGN_RSAPSS3072) || \ | ||
| defined(WOLFBOOT_SIGN_RSAPSS4096) || \ | ||
| defined(WOLFBOOT_SIGN_SECONDARY_RSAPSS2048) || \ | ||
| defined(WOLFBOOT_SIGN_SECONDARY_RSAPSS3072) || \ | ||
| defined(WOLFBOOT_SIGN_SECONDARY_RSAPSS4096) || \ | ||
| (defined(WOLFCRYPT_SECURE_MODE) && !defined(PKCS11_SMALL)) | ||
| # ifndef WOLFBOOT_NEEDS_RSA | ||
| # define WOLFBOOT_NEEDS_RSA | ||
| # endif | ||
| #endif |
There was a problem hiding this comment.
The RSA “trigger condition” is duplicated here and again in include/user_settings/sign_rsa.h (its outer #if). That duplication is easy to let drift over time: the build could end up with RSA enabled but WOLFBOOT_NEEDS_RSA not set (or vice versa), which then impacts finalize.h defaults. Recommendation (preferred): let sign_rsa.h be the single source of truth by defining WOLFBOOT_NEEDS_RSA inside the same outer #if that enables RSA, and remove (or drastically simplify) this duplicated trigger block from cascade.h. An alternative is to factor the condition into a shared macro (e.g., WOLFBOOT_RSA_ENABLED) used by both places.
| /* Any RSA SIGN flag (or WOLFCRYPT_SECURE_MODE without PKCS11_SMALL) means | |
| * the build links wolfCrypt's RSA code. sign_rsa.h handles the actual | |
| * configuration; the marker is set here so finalize.h can see it ahead | |
| * of finalize-time and skip NO_ASN. */ | |
| #if defined(WOLFBOOT_SIGN_RSA2048) || \ | |
| defined(WOLFBOOT_SIGN_RSA3072) || \ | |
| defined(WOLFBOOT_SIGN_RSA4096) || \ | |
| defined(WOLFBOOT_SIGN_SECONDARY_RSA2048) || \ | |
| defined(WOLFBOOT_SIGN_SECONDARY_RSA3072) || \ | |
| defined(WOLFBOOT_SIGN_SECONDARY_RSA4096) || \ | |
| defined(WOLFBOOT_SIGN_RSAPSS2048) || \ | |
| defined(WOLFBOOT_SIGN_RSAPSS3072) || \ | |
| defined(WOLFBOOT_SIGN_RSAPSS4096) || \ | |
| defined(WOLFBOOT_SIGN_SECONDARY_RSAPSS2048) || \ | |
| defined(WOLFBOOT_SIGN_SECONDARY_RSAPSS3072) || \ | |
| defined(WOLFBOOT_SIGN_SECONDARY_RSAPSS4096) || \ | |
| (defined(WOLFCRYPT_SECURE_MODE) && !defined(PKCS11_SMALL)) | |
| # ifndef WOLFBOOT_NEEDS_RSA | |
| # define WOLFBOOT_NEEDS_RSA | |
| # endif | |
| #endif | |
| /* RSA intent is defined by sign_rsa.h, which owns the RSA enablement | |
| * condition and sets WOLFBOOT_NEEDS_RSA as the single source of truth. | |
| * Do not duplicate that trigger logic here. */ |
super secret plz dont look just yet