Add wolfCrypt port for the Nuvoton NuMicro M2354 - #11445
Open
dgarske wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues remain in key handling, concurrency, hardware errors, RNG setup, and header installation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a TrustZone-aware NuMicro M2354 hardware crypto callback port with CRPT, TRNG, Key Store, documentation, and CI support.
Changes:
- Adds hardware-backed hashing, AES/AEAD, ECC, RSA, RNG, and Key Store operations.
- Adds configuration, public APIs, direct RNG seeding, and ASN.1 hex support.
- Adds build integration, documentation, and secure/non-secure cross-compilation CI.
File summaries
| File | Summary | Review findings |
|---|---|---|
wolfssl/wolfcrypt/settings.h |
Enables Nuvoton settings. | No comments. |
wolfssl/wolfcrypt/port/nuvoton/nuvoton_settings.h |
Defines port configuration. | Critical (1): Enable hardware mutexes by default. Moderate (1 each): Gate AES-CCM and AES-GCM on cipher support. |
wolfssl/wolfcrypt/port/nuvoton/nuvoton_key.h |
Declares Key Store APIs. | Nit (3): Remove the nonexistent ECC attachment API from the documentation. |
wolfssl/wolfcrypt/port/nuvoton/nuvoton_cryptocb.h |
Declares callback APIs. | No comments. |
wolfssl/wolfcrypt/include.am |
Installs public headers. | Moderate (2): Update CMake header installation for Nuvoton headers. |
wolfssl/wolfcrypt/asn.h |
Declares hex conversion helper. | Critical (1): Place the declaration inside the existing C-linkage block. |
wolfcrypt/src/random.c |
Integrates direct Nuvoton TRNG seeding. | Moderate (2): Initialize and release the hardware around direct seed calls. |
wolfcrypt/src/port/nuvoton/README.md |
Documents the port. | No comments. |
wolfcrypt/src/port/nuvoton/nuvoton_key.c |
Implements Key Store operations. | Critical (1): Wipe prior software key material and reset state when attaching a Key Store handle. |
wolfcrypt/src/port/nuvoton/nuvoton_hw.h |
Defines hardware interfaces. | Nit (3): Correct the stale AES/GCM/CCM contract comment. |
wolfcrypt/src/port/nuvoton/nuvoton_hw.c |
Implements hardware operations. | Moderate (1): Preserve hardware failures separately from invalid-signature results. |
wolfcrypt/src/port/nuvoton/nuvoton_cryptocb.c |
Registers and dispatches callbacks. | Moderate (1): Make cleanup conditional on successful unregister and idempotent. |
wolfcrypt/src/port/nuvoton/nuvoton_cb_rng.c |
Implements RNG callbacks. | No comments. |
wolfcrypt/src/port/nuvoton/nuvoton_cb_pk.c |
Implements ECC/RSA callbacks. | Moderate (2): Validate ECDH key types, domains, indices, and curve equality before hardware dispatch. |
wolfcrypt/src/port/nuvoton/nuvoton_cb_hash.c |
Implements hash callbacks. | No comments. |
wolfcrypt/src/port/nuvoton/nuvoton_cb_cipher.c |
Implements AES and AEAD callbacks. | Moderate (2): Return a hard error for unsupported Key Store operations instead of declining into MISSING_KEY. |
wolfcrypt/src/include.am |
Registers port sources. | No comments. |
wolfcrypt/src/asn.c |
Implements hex conversion. | No comments. |
CMakeLists.txt |
Adjusts header installation filtering. | Moderate (1): Do not exclude required Nuvoton public headers from CMake installs. |
.github/workflows/nuvoton-m2354-compile.yml |
Builds secure and non-secure variants. | No comments. |
Review details
Suppressed comments (5)
CMakeLists.txt:4746
settings.hincludeswolfssl/wolfcrypt/port/nuvoton/nuvoton_settings.hwheneverWOLFSSL_NUVOTON_M2354is defined, but this exclusion removes the entire directory from CMake installs. A consumer using the installed headers therefore fails to preprocesssettings.hwith the port enabled, even though the Autotools install exposes these public headers. Do not exclude this directory, or install its public headers conditionally.
"wolfssl/wolfcrypt/port/nuvoton"
wolfcrypt/src/port/nuvoton/nuvoton_cryptocb.c:121
- The core
wc_CryptoCb_UnRegisterDevice()is a no-op for invalid or already-unregistered IDs, but this wrapper always callswc_nuvoton_hw_cleanup(). Calling the wrapper twice, or with a different ID, therefore decrements the hardware refcount anyway and can clock-gate/reset CRPT while a valid registration still exists. Tie cleanup to a successful registration removal and make the wrapper idempotent.
wc_CryptoCb_UnRegisterDevice(devId);
wc_nuvoton_hw_cleanup();
wolfcrypt/src/port/nuvoton/nuvoton_hw.c:1488
ECC_VerifySignature()can return nonzero for a timeout or other engine failure as well as for a bad signature, but every nonzero result is converted toSIG_VERIFY_E. The callback then treats that value as a normal result and returns success withres == 0, so a missingCRPT_IRQHandleror wedged accelerator is silently reported as an invalid signature instead ofWC_HW_E; preserve hardware failures separately.
if (rc != 0) {
return SIG_VERIFY_E;
wolfssl/wolfcrypt/port/nuvoton/nuvoton_settings.h:136
- When a selective build enables only
WOLFSSL_NUVOTON_HASH(or another non-cipher engine) while AES-CCM is otherwise configured, this block still definesWOLFSSL_NUVOTON_AESCCM. That retains the CCM staging buffers/helpers even though the dispatcher never routes cipher operations to the port, wasting static SRAM and potentially triggering unused-static warnings under the project's-Werrorwarning set. Gate this macro onWOLFSSL_NUVOTON_CIPHERas well.
#if defined(HAVE_AESCCM) && !defined(NO_AES) && \
!defined(WOLFSSL_NUVOTON_NO_AESCCM)
#undef WOLFSSL_NUVOTON_AESCCM
#define WOLFSSL_NUVOTON_AESCCM
#endif
wolfssl/wolfcrypt/port/nuvoton/nuvoton_settings.h:145
- The same selective-build issue applies to GCM:
WOLFSSL_NUVOTON_AESGCMis enabled withoutWOLFSSL_NUVOTON_CIPHER, so a hash-only configuration still compiles/allocates the GCM path even thoughwc_NuvotonCryptoDevCb()cannot dispatch any cipher operation to it. Gate this macro onWOLFSSL_NUVOTON_CIPHERto keep the documented per-engine selection meaningful.
#if defined(HAVE_AESGCM) && !defined(NO_AES) && \
!defined(WOLFSSL_NUVOTON_NO_AESGCM)
#undef WOLFSSL_NUVOTON_AESGCM
#define WOLFSSL_NUVOTON_AESGCM
#endif
- Files reviewed: 20/20 changed files
- Comments generated: 9
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+120
to
+122
| aes->keylen = (int)(ksKey->bits / 8); | ||
| aes->rounds = (ksKey->bits / 32) + 6; | ||
| aes->devCtx = ksKey; |
Comment on lines
+110
to
+115
| #ifdef WOLFSSL_ASN_HEX_STRING | ||
| /* Convert inSz bytes at input into a NUL terminated lowercase hex string. | ||
| * out needs room for inSz * 2 + 1 bytes. */ | ||
| WOLFSSL_LOCAL void wc_DataToHexString(const byte* input, word32 inSz, | ||
| char* out); | ||
| #endif |
Comment on lines
+49
to
+52
| /* The port works through the wolfSSL crypto callback. */ | ||
| #ifndef WOLF_CRYPTO_CB | ||
| #define WOLF_CRYPTO_CB | ||
| #endif |
Comment on lines
+185
to
+188
| if (ret == WC_NO_ERR_TRACE(BAD_LENGTH_E) || | ||
| ret == WC_NO_ERR_TRACE(BAD_FUNC_ARG)) { | ||
| ret = WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); | ||
| } |
Comment on lines
+527
to
+535
| if (priv == NULL || pub == NULL || priv->dp == NULL || | ||
| info->pk.ecdh.out == NULL || info->pk.ecdh.outlen == NULL) { | ||
| return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); | ||
| } | ||
|
|
||
| curveId = nuvoton_curve_id(priv->dp->id); | ||
| if (curveId == WC_NUVOTON_CURVE_NONE) { | ||
| return WC_NO_ERR_TRACE(CRYPTOCB_UNAVAILABLE); | ||
| } |
Comment on lines
+5671
to
+5680
| int wc_GenerateSeed(OS_Seed* os, byte* output, word32 sz) | ||
| { | ||
| (void)os; | ||
|
|
||
| if (output == NULL || sz == 0) { | ||
| return BAD_FUNC_ARG; | ||
| } | ||
|
|
||
| return wc_nuvoton_hw_trng(output, sz); | ||
| } |
Comment on lines
+152
to
+156
| # Installed unconditionally: settings.h includes nuvoton_settings.h whenever | ||
| # WOLFSSL_NUVOTON_M2354 is set, so an installed tree needs it to preprocess. | ||
| nobase_include_HEADERS+= wolfssl/wolfcrypt/port/nuvoton/nuvoton_settings.h | ||
| nobase_include_HEADERS+= wolfssl/wolfcrypt/port/nuvoton/nuvoton_cryptocb.h | ||
| nobase_include_HEADERS+= wolfssl/wolfcrypt/port/nuvoton/nuvoton_key.h |
Comment on lines
+231
to
+233
| /* Run one AES request. ECB, CBC and CTR only; GCM and CCM are declined by the | ||
| * cipher callback and run in software. */ | ||
| WOLFSSL_LOCAL int wc_nuvoton_hw_aes(wc_NuvotonAesReq* req); |
Comment on lines
+24
to
+28
| * A key written to the store gets a slot; the application then uses the handle | ||
| * rather than the key. Attach it with wc_NuvotonKs_SetAesKey() or | ||
| * wc_NuvotonKs_SetEccKey() and the callback runs the operation through the | ||
| * AES_SetKey_KS and ECC_*_KS driver entry points, so the key material never | ||
| * enters wolfCrypt memory. The handle rides in the object's devCtx. */ |
|
dgarske
force-pushed
the
nuvoton_m2354_cryptocb
branch
from
September 12, 2026 21:14
2cb9b12 to
f81621d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The NuMicro M2354 is an Arm Cortex-M23 (Armv8-M baseline) part with TrustZone, a CRPT cryptographic accelerator, a separate TRNG and a Key Store holding keys in SRAM, Flash and OTP slots. This adds a crypto callback port for it, so software stays compiled in and anything the accelerator cannot do is declined rather than failed.
What it adds
wc_GenerateSeed()chain so aWC_RNGbuilt withINVALID_DEVIDstill gets entropy on bare metal.wc_nuvoton_hw_*interface calls the BSP directly from the secure world, or resolves throughcmse_nonsecure_entryveneers from the non-secure world.Benchmarks. NuMaker-M2354 at 96 MHz. Both columns come from the same build, with SP ECC/RSA and the Thumb-1 assembly enabled, so the software column is wolfSSL configured properly for this core rather than falling back to portable C.
The port README carries the full table.
Hardware / test status
Validated on a NuMaker-M2354 (M2354KJFAE at 96 MHz, Nu-Link2-Me CMSIS-DAP, UART0 on PA6/PA7 at 115200), flashed with
pyocdagainst its builtinm2354kjfaetarget.wolfcrypt_testcompletes with 42 tests passing and none failing, covering every offloaded engine.Two paths the suite does not reach were covered separately on the board: a Key Store round trip, and a sweep of AES-GCM and AES-CCM against software across payload, nonce and tag lengths.
The non-secure TrustZone world was also run on the board, reaching the accelerator through the veneers. The port README records what passed there, and one behaviour in a combined run that is not yet understood.
Host
--enable-all --enable-cryptocbmake checkis unchanged,check-headersandcheck-source-textare clean, and both cross-compile legs build under the project warning set.Related
wolfBoot's M2354 target builds on this port: wolfSSL/wolfBoot#884