Mcdc test coverage: part 5 - #11039
Open
danielinux wants to merge 22 commits into
Open
Conversation
Covers the argument guards of wc_FileLoad, wc_ReadDirFirst/Next, wc_strtok, wc_strsep, wc_strlcpy's truncation arm and wolfSSL_NewThread. wc_port.c had no test group of its own. Builds clean under --enable-all, default and --enable-singlethreaded.
Argument guards of wc_Compress_ex, wc_DeCompress_ex and wc_DeCompressDynamic, plus the inSz overflow cap. Takes compress.c to 8/8 with the existing compress_test() KAT.
The pkcs12 group was almost entirely wc_PKCS12_PBKDF_ex, which lives in pwdbased.c, leaving pkcs12.c's container entry points to the KAT alone. Drives the argument guards of wc_d2i_PKCS12, wc_i2d_PKCS12, wc_PKCS12_parse and wc_PKCS12_free.
wc_SHE_GenerateM1M2M3 and wc_SHE_GenerateM4M5 have a 12-operand and an 8-operand argument OR that account for 20 of wc_she.c's 23 uncovered conditions; the group's other tests only pass valid arguments. One call per operand.
ecc: test_ecc_whitebox.c 336 -> 1591 lines, 4 -> 21 driver classes (curve-id lookup AND-chains, projective add/dbl NULL and range guards, point-at-infinity and collision special cases, FP_ECC cache internals, export_point_der paths). New test_ecc_fault_whitebox.c drives the SMALL_STACK allocation guards via mcdc_fault_alloc.h. rsa: three more drivers (wc_RsaCleanup data/type pairs, wc_CheckProbablePrime_ex qRaw cross-check, wc_RsaFunctionNonBlock).
First white-box coverage for the ASN.1/PKCS layer. asn: 24 drivers over the template engine, the ASN.1 primitives and the OID/signature matching. pkcs12: 14 drivers over the container statics the API group never reaches.
The 7 parse/verify tests become TEST_DECL_GROUP("ocsp") so the MC/DC
campaign can drive asn.c's OCSP paths without --api. The TLS-coupled
ocsp tests stay bare: they need live connections.
wolfSSL_strnstr is not in any public wolfcrypt header, so the port group cannot reach it. Drives both operands of its "n >= s2_len && s1[0]" loop guard.
Four asn white-boxes, one per functional area, so they can be worked in parallel: keys/PKCS8, extensions and name constraints, OCSP/CRL, cert generation and aux. Plus a pkcs7 white-box covering the streaming state machine, signer-info parsing and the recipient helpers. Takes asn.c to 745/1510 and pkcs7.c to 329/1058.
Both were left mid-edit when their authoring sessions were cut off. ext: a comment reading "takes only DNS_entry*/Base_entry*" closed itself early on the embedded */, so the rest became code. revocation: DecodedCert.serial is an array not a pointer; SlhDsaKey is an incomplete type unless SLH-DSA is on, so the opaque key argument is now a plain buffer; and InitOcspResponse needs real OcspEntry and CertStatus storage or the decoder dereferences a NULL single.
asn: the DecodedCert/names/time area -- AltNameDup, SetCurve, SetEccPublicKey, the DNS-entry builders, the GetRDN/GetCertName OID dispatch, and the date/time helpers driven with hand-built UTCTime and GeneralizedTime strings. pkcs7: the ASN.1-walk chains in PKCS7_VerifySignedData and the Enveloped/AuthEnveloped/EncryptedData decoders, reached by sweeping truncated and single-byte-corrupted copies of real messages. Both verified to run, not just compile.
tsp: first white-box for the RFC 3161 timestamp protocol. pkcs12: the deep DER-walk conditions in the parse path, reached with truncated and byte-corrupted copies of the in-tree .p12 blobs.
The public wc_ed25519_verify_msg_init/update/final reject a NULL key before delegating, so the identical checks inside the static ed25519_verify_msg_*_with_sha helpers are unreachable through the API. Drive the three statics directly, issuing both the key==NULL vector and its all-false partner in this binary.
It contributed nothing in all six variants. WOLFSSL_WC_LMS_SMALL reshapes LmsParams and LmsState and WOLFSSL_LMS_VERIFY_ONLY drops keygen and signing, so every driver built on wb_make_params() failed to compile there; guard them on WB_GAP_SIGN. The rest overran the 600s test timeout in a 3-level height-5 keygen that could only fail anyway, since the module config pins WOLFSSL_LMS_MAX_LEVELS to 2 and HssPrivKey holds state[LMS_MAX_LEVELS]. Guard that driver on LMS_MAX_LEVELS >= 3 and shrink the full-cycle tree to height 2, which still crosses one subtree boundary. Unbuffer stdout so a timeout kill keeps the log that says where it stopped.
SHA3_USE_AVX2(f) gained an IS_CPU_INTEL(f) operand upstream. Real cpuid on a non-Intel host reports [AVX2, !Intel], the same row the existing AVX2-only call produces, so neither operand had an independence pair. Force the vendor bit to supply the missing [T,T] row. Sha3Update and Sha3Final also gained a block-count range guard. Every public wrapper passes one of the WC_SHA3_*_COUNT constants, so neither operand is reachable through the API; call both statics directly with a count either side of the range.
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Test-only PR extending the ISO 26262 (ASIL-D) MC/DC campaign to cover remaining large decision clusters in wolfCrypt’s ASN.1/PKCS layer and related modules, primarily via new tests/api groups and unit-mcdc white-box supplements.
Changes:
- Added multiple
tests/unit-mcdc/white-box supplements that#includesource files to drive static/internal decision paths. - Added new
tests/apigroups (port,compress, plus new SHE/PKCS#12 decision-coverage cases) and registered them in the test harness/build. - Adjusted test registration (including grouping OCSP tests) and build files to compile the new test sources.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit-mcdc/test_wc_port_whitebox.c | White-box driver to exercise internal wolfSSL_strnstr loop-guard operand pairs. |
| tests/unit-mcdc/test_wc_lms_impl_whitebox_gap.c | Reduces LMS keygen runtime for instrumented runs; adds conditional compilation stubs and unbuffered stdout. |
| tests/unit-mcdc/test_tsp_whitebox.c | White-box supplement for TSP / ASN TSP failure-half and static-helper coverage. |
| tests/unit-mcdc/test_rsa_whitebox.c | Adds coverage drivers for additional RSA guards (cleanup, probable-prime cross-check, nonblock guard). |
| tests/unit-mcdc/test_pkcs7_decode_whitebox.c | Adds decode-path ASN.1 walk truncation/corruption sweeps for PKCS#7 decoders. |
| tests/unit-mcdc/test_pkcs12_whitebox.c | Adds PKCS#12 white-box drivers for guards, alloc-failures, DER walks, and LENGTH_ONLY_E pass-throughs. |
| tests/unit-mcdc/test_pkcs12_parse_whitebox.c | Adds PKCS#12 deep-parse DER/BER-walk coverage and synthetic indefinite/EOC cases. |
| tests/unit-mcdc/test_ed25519_whitebox.c | Adds coverage for file-static streaming-verify helper key == NULL operand. |
| tests/unit-mcdc/test_ecc_whitebox.c | Expands ECC white-box coverage across many internal guards and special cases. |
| tests/unit-mcdc/test_ecc_fault_whitebox.c | Adds fault-injection sweep for ECC SMALL_STACK allocation-failure guards. |
| tests/unit-mcdc/test_asn_whitebox.c | New ASN.1 template-engine and primitive white-box driver covering many internal decisions. |
| tests/api/test_she.h / tests/api/test_she.c | Registers and implements a new SHE decision-coverage test. |
| tests/api/test_port.h / tests/api/test_port.c | New portability-layer decision-coverage tests and group registration. |
| tests/api/test_pkcs12.h / tests/api/test_pkcs12.c | Registers and implements a new PKCS#12 container API decision-coverage test. |
| tests/api/test_compress.h / tests/api/test_compress.c | New zlib-wrapper decision-coverage tests and group registration. |
| tests/api/include.am | Adds new api test sources and headers to build and distribution lists. |
| tests/api.c | Includes and registers new port/compress groups; groups OCSP tests under ocsp. |
| CMakeLists.txt | Adds the two new tests/api sources to the CMake test build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wc_HpkeCopyPrivateKey's
if (ret != 0 && *copy != NULL)
is only reached from Encap/Decap on the blinding path, where the copy
succeeds, so only the all-false row was ever seen and neither operand had
an independence pair. Both need the row where the copy is already
allocated when the failure happens, which no allocation fault can produce
here: mcdc_fault_alloc.h redefines XMALLOC for this translation unit only,
and the ECC branch allocates inside wc_ecc_key_new() in ecc.c.
Craft the two source keys instead. A key with no curve set leaves the
switch before allocating, and a key whose dp->id names no curve in the
table still exports and still allocates a copy, then fails the import.
hpke.c 80/82 -> 82/82.
Include <limits.h> in test_compress.c rather than relying on another header pulling it in for INT_MAX. Free wc_PKCS12_parse's outputs with the types it allocates them with: DYNAMIC_TYPE_PUBLIC_KEY for the key and DYNAMIC_TYPE_PKCS for the cert buffer, which comes from a WC_DerCertList node. The white-box was using PRIVATE_KEY/CERT, which misaccounts under memory tracking. Run the SHE argument guards only on a context wc_SHE_Init() accepted, since `she` is an uninitialised stack object otherwise. Note in test_port.c that wc_strlcpy returns the number of bytes copied rather than XSTRLEN(src) as BSD strlcpy does, so the truncating call is expected to report 2.
puf_test() drives enroll/reconstruct/derive/identity with well-formed arguments, so each entry guard of the form (ctx == NULL || x == NULL) is only ever seen all-false and neither operand gets an independence pair. The lane has no per-module main() and cannot #include-and-trim, so this rides as a lane_extra_source constructor and reaches puf.c's counters through its public entry points, the same arrangement test_sp_cortexm_whitebox.c uses. puf.c 4/12 -> 11/12 on the lane.
These three tests fail against master today and pass once the wc_port and pkcs7 fixes land; they are the executable statement of what those fixes must do. wc_strlcpy must return the length of src, not the bytes it copied, which is what makes the documented truncation check (ret >= dstSize) work and what its own doxygen comment already specifies. wc_strlcat must return the total length attempted and must not measure dst past dstSize: an unbounded scan of a dst holding no NUL is the out-of-bounds read strlcat(3) bounds this at to prevent. The dst in that case keeps a NUL at the very end of the array so an unbounded implementation reads a wrong length and fails the assertion rather than running off the buffer under a sanitizer. Tearing down a PKCS7 whose AuthEnvelopedData decode stopped part-way must not leak the encryptedContent buffer that stream->bufferPt holds across WANT_READ re-entries. Counting allocators wrap the whole cycle; an untouched blob runs first as a control so an imbalance is attributable to the aborted decode rather than to ambient allocation. Expected values verified against the OpenBSD strlcpy/strlcat sources and against a build carrying the fixes.
danielinux
force-pushed
the
mcdc-test-coverage
branch
from
August 4, 2026 08:17
3fde38e to
4ffdb83
Compare
danielinux
marked this pull request as ready for review
August 4, 2026 08:41
Member
Author
|
Some tests expected to fail (e.g. Smoke test) until #11047 is merged |
|
retest this please |
wc_mlkem_poly.c selects an implementation 28 times with
if (IS_INTEL_AVX512(cpuid_flags) && (SAVE_VECTOR_REGISTERS2() == 0))
and on an AVX512-capable host only the all-true row is ever seen, so
neither operand had an independence pair.
The save operand needs the library's own hook. In a userspace build
types.h resolves SAVE_VECTOR_REGISTERS2() to the literal 0, so "(0 == 0)"
is structurally true and has no false side at all; it is real where the
save can be refused. WC_CHECK_FOR_INTR_SIGNALS is the #ifndef extension
point types.h offers for that, so defining it before the included .c
routes all 58 sites through a variable this file controls.
The feature operand needs more than on/off. The dispatches are chains --
VBMI, then plain AVX512, then AVX2, then portable C -- so an arm's true
row only happens when every richer feature above it is absent; with all
bits set the first arm always wins and the rest are never evaluated. Each
row clears a different suffix of that ladder.
Every compiled parameter set is swept: the compress/decompress dispatches
are du/dv specific, so ML-KEM-768 alone never reaches the du=11/dv=5 code.
wc_mlkem_poly.c 27/190 -> 122/190.
Same shape as the mlkem dispatches, so the same two levers apply. SAVE_VECTOR_REGISTERS2() resolves to the literal 0 in a userspace build, making "(0 == 0)" structurally true with no false side; both files now route it through types.h's WC_CHECK_FOR_INTR_SIGNALS extension point. wc_mldsa.c additionally needed the feature ladder and a sweep of every compiled parameter set, since many of its dispatches are guarded by (k == ..) && (l == ..) first. It uses the ctx-based sign/verify entry points, which are the ones this config compiles. sp_x86_64.c already cleared BMI2, ADX and MOVBE one at a time; its richest chains are four operands, so the two that were missing are AVX2 and the save. Added a pass for each. wc_mldsa.c 268/443 -> 352/435 sp_x86_64.c 245/598 -> 348/598
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.
MC/DC coverage: ASN.1/PKCS layer, wolfCrypt completeness (Part 5)
Test-only. Extends the ISO 26262 (ASIL-D) MC/DC campaign to the last large
untouched block of wolfCrypt — the ASN.1/PKCS layer — and closes the
accounting for every top-level
wolfcrypt/src/*.c.No library behaviour is changed. The only non-test file touched is
CMakeLists.txt, which registers the two new test sources.Coverage
Aggregate 6872 / 10486 MC/DC conditions (65.5%) across 62 modules.
asn.cpkcs7.cpkcs12.ctsp.casn_tsp.cwc_port.cwc_she.ccompress.cecc.c(394/603) andrsa.c(182/283) were advanced and are frozen here;remaining gaps are deferred to Part 6.
What's added
tests/apigroups:port,compress, plus SHE and PKCS#12 decisioncases, and a deterministic
ocspgroup (its tests previously registered asbare
TEST_DECL, reachable only via--api).tests/unit-mcdc/white-box supplements forasn.c,pkcs7.c,pkcs12.c,tsp.candwc_port.c. These#includethe source directly toreach file-statics that no public entry point can drive — the densest
decision clusters in the layer are static.
Notes for review
EXTRA_DISTonly. Each defines its ownmain()andincludes a
.c, so they cannot joinunit.test; the out-of-tree coveragecampaign compiles them standalone.
certs/rather than newly authored.HAVE_FIPS/HAVE_SELFTESTsites in this layer are structurallyuncoverable in an open build and are recorded as justified residuals, not
gaps.