Infrastructure for Mc/Dc test coverage + AES coverage campaign#10845
Open
danielinux wants to merge 21 commits into
Open
Infrastructure for Mc/Dc test coverage + AES coverage campaign#10845danielinux wants to merge 21 commits into
danielinux wants to merge 21 commits into
Conversation
Add MC/DC-targeted unit tests exercising decision and feature coverage across AES (key wrap, GCM, feature), ASN.1, RSA, signature (falcon), and CryptoCb registry surfaces.
Add tests/unit-mcdc/, a standalone white-box program that compiles
wolfcrypt/src/aes.c directly to reach static/WOLFSSL_LOCAL helpers
(GHASH/GHASH_UPDATE ptr guards, _AesNew_common cross-arg checks) that
are structurally unreachable through the public API, closing 19 of the
AES MC/DC residuals. Extend tests/api/test_aes.{c,h} with the
decision/feature coverage cases these build on.
These are for the external ISO 26262 per-module MC/DC campaign; they do
not change library behaviour and are not part of the wolfSSL build.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds test-only infrastructure and new/expanded unit tests to increase structural MC/DC coverage (notably for AES), including a standalone “white-box” harness for exercising API-unreachable decision conditions.
Changes:
- Added
tests/unit-mcdc/white-box harness infrastructure and the first AES-focused harness binary. - Expanded
tests/apicoverage across AES, ASN.1, RSA, PKCS#7, PKCS#12, OCSP wolfIO helpers, Falcon (via liboqs), and OpenSSL-compat X.509 verification paths. - Registered new tests in existing groups so they run in normal CI when features are enabled.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/api.c | Adds additional API-level coverage tests (incl. negative/guard paths) and related helper coverage. |
| tests/api/test_aes.c | Adds extensive AES decision/feature/argument coverage tests (incl. MC/DC-targeted branches). |
| tests/api/test_aes.h | Registers new AES test entry points. |
| tests/api/test_asn.c | Adds ASN.1 decision/feature coverage tests for PKCS#8 and key/cert parsing paths. |
| tests/api/test_asn.h | Registers new ASN.1 coverage test entry points. |
| tests/api/test_certman.c | Adds/extends CertManager/X.509 constraint-related tests (coverage for name constraints/verification paths). |
| tests/api/test_ocsp.c | Adds OCSP-related tests including wolfIO URL/HTTP helper coverage. |
| tests/api/test_ossl_x509.c | Extends OpenSSL-compat X.509 coverage in the primary test module. |
| tests/api/test_ossl_x509_ext.c | Adds coverage for X.509 extension handling paths (incl. name constraints-related logic). |
| tests/api/test_ossl_x509_ext.h | Header updates for the X.509 extension coverage additions. |
| tests/api/test_ossl_x509_vp.c | Adds coverage for X509_VERIFY_PARAM / verification parameter paths. |
| tests/api/test_pkcs12.c | Adds PKCS#12 create/parse guardrail coverage tests. |
| tests/api/test_pkcs12.h | Registers new PKCS#12 guardrail test entry points. |
| tests/api/test_pkcs7.c | Adds PKCS#7 InitWithCert guardrail coverage tests. |
| tests/api/test_pkcs7.h | Registers new PKCS#7 guardrail test entry points. |
| tests/api/test_rsa.c | Adds RSA decision and feature coverage tests (OAEP/PSS/PKCS#1 v1.5 and negative branches). |
| tests/api/test_rsa.h | Registers new RSA coverage test entry points. |
| tests/api/test_signature.c | Adds Falcon sign/verify coverage test (guarded on HAVE_PQC/HAVE_FALCON/HAVE_LIBOQS). |
| tests/api/test_signature.h | Registers the new Falcon signature coverage test. |
| tests/unit-mcdc/README.md | Documents the white-box MC/DC supplement mechanism and build contract. |
| tests/unit-mcdc/test_aes_whitebox.c | Implements AES white-box MC/DC supplement by including wolfcrypt/src/aes.c and calling internal helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Several MC/DC coverage tests called WOLFSSL_LOCAL (hidden-visibility) library functions directly from the in-tree unit.test: - wc_AesCcmCheckTagSize() (test_aes.c) - wc_CryptoCb_Init/Cleanup/GetDevIdAtIndex() (api.c) These only link when the library is built with test-static visibility, so normal (shared) builds failed at link with "undefined reference", breaking essentially every CI build job. Gate the affected assertions on WOLFSSL_TEST_STATIC_BUILD (in addition to the existing feature guards) so they compile out where the symbols are hidden, matching the existing wolfSSL convention for internal-symbol tests. Verified: ./configure --enable-all (no WOLFSSL_TEST_STATIC_BUILD) now builds tests/unit.test cleanly and the full suite passes.
test_wc_AesModesArgMcdc asserted that wc_AesCtrEncrypt() with corrupted aes.rounds returns KEYUSAGE_E, but used sz = 32 (an exact block multiple). When in != out, the full blocks are consumed by a batch path that does not surface the per-block rounds error - the AES-NI batch, or the HAVE_AES_ECB fast path which ignores wc_AesEcbEncrypt()'s return - leaving no trailing partial block, so the function returns 0 and the assertion fails. This was latent (the whole test binary failed to link before the visibility fix) and reproduces in --disable-aesni --enable-aesecb builds. Use a non-block-multiple size (WC_AES_BLOCK_SIZE + 4) so the "(ret == 0) && sz" leftover-handling call runs and fails on the corrupted rounds via wc_AesEncrypt() in every backend. Reported by Copilot review on PR wolfSSL#10845. Verified: test_wc_AesModesArgMcdc now passes under --disable-aesni --enable-aesecb (previously failed) and under --enable-aesni --enable-aesecb.
The new MC/DC coverage tests broke many CI configs under -Werror (which is auto-enabled for in-git-tree builds). Fixes, each verified with a real -Werror build of the relevant config: - test_aes.c: wrap the whole test_wc_AesSivArgMcdc definition in WOLFSSL_AES_SIV && WOLFSSL_AES_128 (was body-only guarded while its prototype is guarded) -> fixes -Wmissing-prototypes when SIV is off. - test_aes.c: mark key/in/out (void) in test_wc_AesModesArgMcdc; they are used only by the per-mode (CTR/CFB/OFB) blocks -> fixes -Wunused-variable when no such mode is enabled. - api.c: guard the test_CryptoCb_* callback helpers with WOLF_CRYPTO_CB && WOLFSSL_TEST_STATIC_BUILD to match their only caller -> fixes -Wunused-function in cryptocb non-static builds. - api.c: register test_wc_CryptoCb_registry under its actual definition condition (WOLF_CRYPTO_CB && HAVE_IO_TESTS_DEPENDENCIES && !ONLY_*) and keep test_wc_CryptoCb registered unconditionally (as on master) -> fixes undeclared / defined-but-unused across cryptocb configs. - api.c: declare session-cache 'mode' under OPENSSL_EXTRA (its only uses) -> fixes -Wunused-variable without opensslextra. - api.c: guard the Enable/DisableOCSPStapling calls in test_wolfSSL_crl_ocsp_object_api with HAVE_CERTIFICATE_STATUS_REQUEST[_V2] -> fixes undefined references with OCSP but no stapling. Verified clean under: --enable-ocsp --enable-ocspstapling, --enable-ocsp (no stapling), and --enable-all; unit.test runs pass.
The check-source-text CI job flags non-ASCII (8-bit) bytes in source. The new MC/DC tests and README used UTF-8 punctuation in comments/prose (em-dash, ellipsis, left-right arrow). Replace with ASCII equivalents (-, ..., <->). Verified: ./.github/scripts/check-source-text.sh on the changed files reports clean.
Several AES ArgMcdc tests corrupt aes.rounds (or cmac.aes.rounds) and expect the subsequent op to fail with KEYUSAGE_E from the in-process software AES path. Under WOLF_CRYPTO_CB_FIND (e.g. --enable-swdev), the "devId != INVALID_DEVID" guard is removed, so CTR/CCM/CMAC ops are offloaded to the registered crypto callback even for INVALID_DEVID; the callback re-derives the key and ignores the corrupted struct, returning 0 instead of KEYUSAGE_E and failing the assertion. Guard those internal-failure checks with #ifndef WOLF_CRYPTO_CB_FIND (wc_AesCtrEncrypt, wc_AesCfb/OfbEncrypt/Decrypt, wc_AesCcmEncrypt/Decrypt, wc_CmacUpdate); the raw-block wc_AesEncryptDirect path in SetKey does not route through cryptocb, so it stays. (void)-cast the locals only used by the guarded checks to keep -Werror clean. MC/DC is unioned across configs, so no union coverage is lost. Verified: --enable-swdev ... (WOLF_CRYPTO_CB_FIND) now passes, and a non-CB_FIND build with all these modes still runs and passes the checks.
Three more config-matrix failures in the AES coverage tests: - Rounds-corruption checks (aes.rounds/cmac.aes.rounds = 0/17 -> expect KEYUSAGE_E) also break under WOLF_CRYPTO_CB_ONLY_AES, which strips the software AES so the op is serviced by the callback and ignores the corrupted struct (same net effect as WOLF_CRYPTO_CB_FIND). Introduce WC_TEST_AES_ROUNDS_OFFLOADED = (WOLF_CRYPTO_CB_FIND || _ONLY_AES), replace the previous WOLF_CRYPTO_CB_FIND-only guards with it, and extend it to the wc_AesEncryptDirect/wc_AesDecryptDirect checks in AesSetKeyArgMcdc (which the ONLY_AES path also offloads). - test_wc_AesFeatureCoverage's GCM-streaming block uses a hardcoded 256-bit key; guard it with WOLFSSL_AES_256 (failed under -DNO_AES_256). - Its AES-KeyWrap block uses a 192-bit key; guard with WOLFSSL_AES_192 (failed under -DNO_AES_192). Verified: --enable-swdev --enable-cryptocb ... -DWOLF_CRYPTO_CB_ONLY_AES and --enable-all -DNO_AES_192 -DNO_AES_256 now both build and pass; a normal --enable-all build still runs the rounds checks.
Three more failures in the branch's added tests, found via the ASAN, C++ and no-client CI configs: - test_wolfSSL_X509V3_EXT leaked 2296 bytes: the added X509_get_ext_d2i(x509, NID, &critical, NULL) calls (used to exercise the critical-flag output) discarded their allocated result. Free each per its actual return type: BASIC_CONSTRAINTS, ASN1_STRING (key usage), AUTHORITY_KEYID, AUTHORITY_INFO_ACCESS, and - for subject_key_identifier - a STACK_OF(ASN1_OBJECT) (wolfSSL_X509_get_ext_d2i wraps a lone obj in a stack). This was the real cause of the sanitize-asan / intelasm / krb-asan job failures (the read_write_ex/ECH/dtls13 asserts printed there are retry-masked and fail identically on master). - C++ build (all-pq-cxx): void* from X509_get_ext_d2i does not implicitly convert; add explicit WOLFSSL_X509_EXTENSION* casts. - no-client link (all-no-client): wolfSSL[_CTX]_UseOCSPStapling[V2] (CSR/CSR2) are client-side APIs; guard those blocks with !NO_WOLFSSL_CLIENT. Verified: full --enable-all + ASAN run is leak-free and passes; --enable-all -DNO_WOLFSSL_CLIENT builds and links.
The aes.rounds-validity check that returns KEYUSAGE_E lives only in the pure-C block encrypt (AesEncryptBlocks_C). On ARMv8 with crypto extensions, --enable-all auto-enables WOLFSSL_ARMASM, whose asm wc_AesEncrypt bypasses that check, so corrupting aes.rounds no longer fails the op (returns 0). This broke the SetKey/CTR/CFB/OFB/CCM/CMAC rounds-corruption assertions on the arm64 CI runners. Extend WC_TEST_AES_ROUNDS_OFFLOADED to also cover WOLFSSL_ARMASM (joining WOLF_CRYPTO_CB_FIND / WOLF_CRYPTO_CB_ONLY_AES). MC/DC of that decision is still obtained from the pure-C configs in the variant union. x86 (incl. AES-NI, which does validate rounds) is unaffected.
…che test The last two red PR jobs, both in the branch's new tests: - intelasm (ASAN): test_wc_AesKeyExportArgMcdc calls wc_AesInit_Id() and wc_AesInit_Label() which succeed (allocating the WC_DEBUG_CIPHER_LIFECYCLE tag) but were never freed -> 8-byte LeakSanitizer leak. Add wc_AesFree() to both blocks. - no-client-no-client-auth (minimal server-only build): test_wolfSSL_session_cache_api_direct's wolfSSL_new() returned NULL because a certless server CTX has no usable cipher suite. Load the test server cert/key (file, with a USE_CERT_BUFFERS_2048 fallback) before wolfSSL_new() in the server-only path; the client path is cert-free as before. Verified: --enable-all + ASAN run of the aes group is leak-free, and CPPFLAGS="-DNO_WOLFSSL_CLIENT -DWOLFSSL_NO_CLIENT_AUTH" now passes.
The apple-m1 "known config A" (FIPS) build broke: the FIPS module's frozen headers declare wc_AesEncryptDirect/wc_AesDecryptDirect as void (not int) and omit wc_CmacFree, so the new tests' ExpectIntEQ(wc_AesEncryptDirect(...)) and wc_CmacFree() usages don't compile. Gate test_wc_AesSetKeyArgMcdc and test_wc_AesCmacArgMcdc on the modern API with the same idiom test_wc_AesEncryptDecryptDirect_WithKey already uses: (!defined(HAVE_FIPS) || !defined(HAVE_FIPS_VERSION) || (HAVE_FIPS_VERSION > 6)) && !defined(HAVE_SELFTEST) The campaign runs non-FIPS, so no coverage is lost where it is measured. Verified: --enable-all still builds and both tests run.
Same FIPS-header mismatch as the earlier AesSetKey/Cmac guard: the FIPS v2/v5 modules lack AES_IV_FIXED_SZ and GCM_NONCE_MIN/MID/MAX_SZ and declare wc_AesEncryptDirect as void, so test_wc_AesModesArgMcdc and test_wc_AesGcmArgMcdc fail to compile there (fatal under -Werror). Gate both on the modern API with the same idiom used by the other AES-DIRECT tests.
clang-tidy (all-c89, async-quic, intelasm) flagged `ccmTag[0] ^= 0x01` as a use of an uninitialized value: the analyzer does not model wc_AesCcmEncrypt writing the tag buffer. Zero-initialize ccmTag; the subsequent encrypt still overwrites it before the tamper, so behavior is unchanged.
The branch's Falcon signature test (tests/api/test_signature.c) guards on HAVE_PQC, which current wolfSSL no longer defines anywhere, so check-source-text flags it as unknown. Register it alongside the other PQC/MLKEM extras.
This branch widened test_wolfSSL_X509V3_EXT's guard from OPENSSL_ALL to (OPENSSL_EXTRA || OPENSSL_ALL). The Authority Info Access sub-test frees its aia stack with wolfSSL_sk_ACCESS_DESCRIPTION_pop_free(aia, NULL), relying on the stack's type-based element free - but wolfssl_sk_get_free_func() only wires up wolfSSL_ACCESS_DESCRIPTION_free for STACK_TYPE_ACCESS_DESCRIPTION under OPENSSL_ALL. In an OPENSSL_EXTRA-only build (now reachable) the NULL callback frees the stack nodes but leaks each ACCESS_DESCRIPTION (struct + method OBJ + location GENERAL_NAME + URI string): 370 bytes, caught by ASAN/valgrind. Pass wolfSSL_ACCESS_DESCRIPTION_free explicitly (available under OPENSSL_EXTRA); correct under OPENSSL_ALL too. Verified leak-free under ASAN with the failing config (--enable-opensslextra --enable-crl ... --disable-fastmath).
Adds a wb_aesni() supplement covering the AES-NI-only file-static helpers that
the aesni build variant instruments: AES_set_{en,de}crypt_key_AESNI's
!userKey/!aes guard and the AesGcm{AadUpdate,EncryptUpdate,DecryptUpdate}_aesni
NULL/size guards, using the same "call the static with both halves of the pair"
idiom as the classic GHASH/GHASH_UPDATE supplements.
Closes 6 of the 8 AES-NI null-guard residuals (aesni_wb 14->20, union
384->390/397). The remaining pair is AES_set_decrypt_key_AESNI's !userKey/!aes:
its valid-argument (false) half needs an AES-NI decrypt-key setup that the
current aesni test set does not exercise; left as a documented residual.
…fault) test_wc_AesCcmArgMcdc segfaulted on the FIPS builds (apple-M1 config A, CAVP self-test), exit 139. The AES *ArgMcdc tests reach the post-key-setup "ret != 0" checkpoints by corrupting aes->rounds = 0 and calling wc_AesCcmEncrypt/Decrypt. That relies on the pure-C AesEncryptBlocks_C guard (if r==0 return KEYUSAGE_E) to turn the corruption into a clean error return. Under the FIPS / self-test module the AES implementation has no such guard, so rounds=0 runs AES with a zero-round key schedule and dereferences past the key schedule -> SIGSEGV. The corruption is already gated by WC_TEST_AES_ROUNDS_OFFLOADED (crypto-cb / asm offload). FIPS and self-test are the same situation - the pure-C guard is not in the compiled path - so add HAVE_FIPS / HAVE_SELFTEST to that macro. The three function-level FIPS-guarded tests (SetKey/Modes/Cmac ArgMcdc) were already skipped; test_wc_AesCcmArgMcdc is not, and its non-corruption CCM coverage now still runs under FIPS while only the rounds-corruption blocks are skipped. Verified: --enable-all (non-FIPS) still runs and passes all rounds-corruption tests; the corruption blocks compile out only under FIPS/self-test.
The CAVP self-test build (--enable-selftest) failed to compile test_aes.c: wc_AesGcmSetExtIV (test_wc_AesGcmDecisionCoverage) and wc_Gmac/wc_GmacVerify (test_wc_AesGmacArgMcdc) are declared only under !WC_NO_RNG in mainline and are absent from the frozen self-test module's headers (they are present under FIPS - the apple-M1 FIPS build compiled and passed both), so -Werror=implicit-function- declaration aborted the build. Gate those specific calls (and the now-otherwise-unused iv buffer) on !defined(WC_NO_RNG) && !defined(HAVE_SELFTEST). wc_GmacSetKey stays available, so its coverage is retained under self-test. FIPS builds are unaffected. Verified: --enable-all still builds and both tests pass; preprocessing test_aes.c with -DHAVE_SELFTEST leaves no wc_AesGcmSetExtIV/wc_Gmac/wc_GmacVerify calls in any always-compiled function (remaining ones are in the WOLFSSL_AESGCM_STREAM stream test, which the self-test config disables).
A second apple-M1 FIPS segfault (--enable-fips=v5): test_wc_AesCcmArgMcdc probes the pure-C CCM inSz-overflow decision by calling wc_AesCcmEncrypt/Decrypt with a 1-byte dummy in/out buffer and a claimed length of 65536, relying on the pure-C path returning AES_CCM_OVERFLOW_E *before* touching the buffer. The FIPS module's CCM does not reject early, so it writes 65536 bytes into the 1-byte buffer and segfaults. (The rounds=0 corruption in the same test was already skipped via WC_TEST_AES_ROUNDS_OFFLOADED, but the two overflow blocks were not.) Every decision this test targets lives in pure-C aes.c CCM code that is not compiled in FIPS/self-test builds, so guard the whole function on !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST) rather than patching each unsafe block. The other *ArgMcdc tests only corrupt rounds (macro-skipped) and have no oversized-buffer calls, so they need no further change. Verified: --enable-all still builds and test_wc_AesCcmArgMcdc passes; the 65536 overflow calls are present non-FIPS and absent when HAVE_FIPS is defined.
The PRB-CAVP-selftest-v2 leg (./configure --enable-selftest=v2, which overlays
frozen wolfCrypt 4.1.0 crypto) failed in test_wc_RsaDecisionCoverage.
Reproduced against that exact build; the failing assertion is:
tests/api/test_rsa.c:1474
ExpectIntEQ(wc_RsaSetRNG(&key, NULL), BAD_FUNC_ARG) /* got 0 */
So the assertion is correct for current wolfCrypt but not part of the frozen
self-test module's contract. This test's whole purpose is MC/DC of the *open*
wolfcrypt/src/rsa.c decision branches, which is not even the rsa.c compiled
under --enable-selftest, so under self-test it measures nothing and only risks
divergent error codes like this one. The sibling key-gen/decision tests in this
file (test_wc_CheckProbablePrime, the RsaKeyGeneration group) exclude
HAVE_SELFTEST for the same reason; do so here too.
Guard the whole function rather than the single assertion: none of it counts
toward the (open-build) MC/DC campaign under self-test, and a whole-function
guard is robust against any other frozen-vs-current divergence in the same
body. HAVE_FIPS is intentionally left running -- that (newer) module honours
these decisions (and excludes the WC_RSA_BLINDING wc_RsaSetRNG block anyway),
and the open MC/DC campaign builds are unaffected.
Verified: with the guard, the test is cleanly skipped under --enable-selftest=v2
(test index 365, matching CI); the open build still runs and passes it.
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.
Description
This PR is test-only, additive infrastructure for structural (MC/DC) test coverage — no library source or behaviour changes.
Two parts:
Reusable MC/DC white-box harness — tests/unit-mcdc/ (new).
A standalone pattern for exercising decision conditions that are structurally unreachable through the public API: static / WOLFSSL_LOCAL helpers whose "impossible" operand combinations every wc_* entry point rejects first (e.g. a size != 0 argument paired with a NULL pointer). The program #includes the module .c directly so those helpers are in scope, and calls them with both halves of each MC/DC independence pair in one binary. It is not part of the wolfSSL build and not registered in tests/api — it's an opt-in supplement for coverage campaigns. The first instance, test_aes_whitebox.c, reaches the AES GHASH / GHASH_UPDATE pointer guards and _AesNew_common cross-argument checks; README.md documents the mechanism and how to add modules.
AES coverage campaign + seed coverage for adjacent modules.
A large batch of decision-, feature-, and argument-check tests, registered in the existing test groups so they run in normal CI:
All new tests/api tests are guarded by the same #if as the code under test, so they auto-report TEST_SKIPPED when the feature is compiled out.
Testing
Checklist