C2000 example: HWAES, entropy and probe - #617
Conversation
There was a problem hiding this comment.
Pull request overview
Adds build-time toggles and supporting harness/tools for the TI C2000 F28P55x example to (1) route AES through the on-chip AESA accelerator, (2) switch from the dev-only DRBG seed to a real oscillator-jitter entropy source with on-target checks, and (3) build a measurement-only “entropy probe” image plus host-side analysis to estimate min-entropy.
Changes:
- Introduces
HWAES=1,ENTROPY=1, andENTROPY_PROBE=1toggles and associated build plumbing (including include-path order fixes). - Adds on-target harness routines for hardware-AES vs software cross-checking and entropy-source validation, plus a standalone entropy probe firmware path.
- Adds a numpy-based host tool to analyze probe captures and updates the port README with usage and measurement results.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| embedded/ti-c2000-f28p55x/tools/entropy_analyze.py | New host-side analyzer for ENTROPY_PROBE captures (min-entropy/bias/acf/chi-square). |
| embedded/ti-c2000-f28p55x/Source/wolf_main.c | Adds entropy validation, AESA-vs-SW AES tests, and a probe-only execution path. |
| embedded/ti-c2000-f28p55x/Source/entropy_probe.c | New measurement-only firmware that emits tagged raw/packed entropy samples over SCI. |
| embedded/ti-c2000-f28p55x/README.md | Documents new toggles, hardware AES behavior/perf, and entropy/probe methodology. |
| embedded/ti-c2000-f28p55x/Makefile | Adds toggle wiring, avoids duplicate AES core sources, reorders include paths, and adds probe build. |
| embedded/ti-c2000-f28p55x/Header/user_settings.h | Enables crypto-callback + C2000 AES/entropy configuration under the new toggles. |
Suppressed comments (1)
embedded/ti-c2000-f28p55x/tools/entropy_analyze.py:260
- main() reads capture files via open(...).read() without closing the file handle. Using a context manager avoids leaking descriptors in long-running / repeated invocations (e.g., scripted batch analysis).
path = sys.argv[1] if len(sys.argv) > 1 else "-"
if path == "--selftest":
selftest()
return
text = sys.stdin.read() if path == "-" else open(path, errors="replace").read()
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (5)
embedded/ti-c2000-f28p55x/Source/entropy_probe.c:145
probe_adcSample()also uses a bounded wait but unconditionally reads the ADC result even if the interrupt never arrives. That can return a stale conversion and skew both the raw dump and the packed LSB stream. Consider checking for timeout and handling it explicitly (e.g., return a sentinel, increment an error counter, or print an error tag and stop).
ADC_clearInterruptStatus(ADCC_BASE, ADC_INT_NUMBER1);
ADC_forceSOC(ADCC_BASE, ADC_SOC_NUMBER0);
for (guard = 0; guard < 1000000UL; guard++) {
if (ADC_getInterruptStatus(ADCC_BASE, ADC_INT_NUMBER1)) {
break;
}
}
return ADC_readResult(ADCCRESULT_BASE, ADC_SOC_NUMBER0);
}
embedded/ti-c2000-f28p55x/Source/wolf_main.c:64
random.his already included under other feature toggles in this file; adding another conditional include here increases duplication and the chance of diverging include guards. Consider including<wolfssl/wolfcrypt/random.h>once in a common section (or centralizing it behind a single feature check) so it’s not repeated across multiple#ifdefblocks.
#ifdef WOLF_ENTROPY
#include <wolfssl/wolfcrypt/port/ti/ti-c2000-entropy.h>
#include <wolfssl/wolfcrypt/random.h>
#endif
embedded/ti-c2000-f28p55x/Source/entropy_probe.c:97
probe_dccSample()uses a bounded wait loop, but it doesn’t check whether it actually observed DONE/ERROR before returning a value. If the mux is misconfigured or the peripheral stalls, the function will still return a counter-derived value (often 0) and silently pollute the capture. Consider detecting timeout (guard exhausted) and/or error status and emitting an explicit marker or aborting the probe so the host analysis can trust the samples.
This issue also appears on line 135 of the same file.
/* Bounded wait, scaled to the window, so a bad mux cannot hang. */
for (guard = 0; guard < (window * 256UL) + 100000UL; guard++) {
if (DCC_getSingleShotStatus(base) || DCC_getErrorStatus(base)) {
break;
}
embedded/ti-c2000-f28p55x/Header/user_settings.h:572
- The section header comment says the RNG is “seeded by a DEV-ONLY test seed”, but this block is now conditional and can enable a real entropy source when
WOLF_ENTROPYis set. Update the banner text to reflect the two modes (real entropy vsWOLFSSL_GENSEED_FORTEST) so it doesn’t contradict the code below.
/* ------------------------------------------------------------------------- */
/* RNG - real SHA-256 Hash-DRBG seeded by a DEV-ONLY test seed */
/* ------------------------------------------------------------------------- */
#ifdef WOLF_ENTROPY
embedded/ti-c2000-f28p55x/Source/wolf_main.c:1144
- If
wc_c2000_Entropy_GetRaw()fails, the code still countsonesover whatever happens to be inraw(static buffer), and prints that count alongside the failure. That makes the diagnostic output misleading (e.g., showing 0/2048 ones for a read error). Consider checkingretfirst and printing a distinct “GetRaw failed (ret=…)” line (or zeroingraw) before doing the popcount.
ret = wc_c2000_Entropy_GetRaw(raw, (word32)sizeof(raw), src);
ones = 0;
for (i = 0; i < (word32)sizeof(raw); i++) {
for (b = 0; b < 8; b++) {
if ((raw[i] >> b) & 1) {
…e-hash and packed input
There was a problem hiding this comment.
🟡 Changes recommended
The new on-target entropy validation currently computes and reports the bit-balance result even when the raw-sample read fails, which can mask failures and produce misleading output.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/10 changed files
- Comments generated: 1
- Review effort level: Lite
| for (src = 0; src < WOLFSSL_C2000_ENTROPY_NUM_SRC; src++) { | ||
| ret = wc_c2000_Entropy_GetRaw(raw, (word32)sizeof(raw), src); | ||
| ones = 0; | ||
| for (i = 0; i < (word32)sizeof(raw); i++) { | ||
| for (b = 0; b < 8; b++) { | ||
| if ((raw[i] >> b) & 1) { | ||
| ones++; | ||
| } | ||
| } | ||
| } | ||
| /* 2048 bits; accept 40%..60% ones, i.e. counts 820..1228. */ | ||
| printf("Entropy raw src%d bit balance: %s (%lu/2048 ones)\r\n", | ||
| src, | ||
| (ret == 0 && ones > 819UL && ones < 1229UL) ? "PASS" : "FAIL", | ||
| (unsigned long)ones); | ||
| } |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #617
Scan targets checked: wolfssl-examples-bugs, wolfssl-examples-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
| #endif /* WOLF_SHA1 */ | ||
|
|
||
| #ifndef WOLF_MLDSA_SIGN | ||
| #if !defined(WOLF_MLDSA_SIGN) && !defined(WOLF_MLDSA_OCTETS) |
There was a problem hiding this comment.
wolf_mldsa87_verify_test() is compiled but never called in SECUREBOOT builds · Dead/unreachable code
The compile guard excludes only WOLF_MLDSA_SIGN and WOLF_MLDSA_OCTETS, but main() dispatches WOLF_SECUREBOOT first (line 2480), so with SECUREBOOT=1 this function is defined and never referenced, carrying a second static wc_MlDsaKey (~14 KB) into .bss alongside sb_key.
Fix: Add && !defined(WOLF_SECUREBOOT) to the guard at line 887 and its matching #endif comment at line 973.
…RC, report per-window raw entropy
…age streamed from flash
e99133c to
9046a95
Compare
Adds three toggles to
embedded/ti-c2000-f28p55x, pairing with the wolfSSL PR:HWAES=1routes AES through the accelerator and definesWC_USE_DEVIDsowolfcrypt_testandbenchmarkactually exercise the device rather than silently measuring software,ENTROPY=1swaps the dev-only seed for the real source with on-target validation KATs, andENTROPY_PROBE=1builds a measurement-only image that dumps unconditioned samples over SCI for host min-entropy analysis. Also corrects the include order so-I./Headerprecedes-I$(WOLFROOT), since wolfSSL's documenteduser_settings.hworkflow puts a copy at the wolfSSL tree root that otherwise shadowed the example's own.Requires: wolfSSL/wolfssl#11202