Repository navigation
MC/DC: coverage campaign part 9 - #11595
Open
danielinux wants to merge 48 commits into
Open
danielinux wants to merge 48 commits into
danielinux wants to merge 48 commits into
Conversation
|
danielinux
force-pushed
the
mcdc-part9-coverage
branch
2 times, most recently
from
October 8, 2026 07:19
746b129 to
fe5628d
Compare
danielinux
force-pushed
the
mcdc-part9-coverage
branch
from
November 3, 2026 11:33
0ca325e to
289b1e0
Compare
Member
Author
|
jenkins retest this please |
New decision-coverage drivers for the conditions upstream added since the part-8 anchor: random.c (test_wc_DrbgReworkDecisionCoverage): the per-width Present/GetReseedCtr/ScheduleReseed arms driven through both DRBG widths with the type/pointer pairings poked on live instances, the Reseed_Nonce/Stir_Nonce NULL pairs, the InitRngNonce_ex2 perso clause tails, and the USE_FULL_MUTEX/NO_AUTO_LOCK init-flag arms. The SHA-512 DRBG is re-enabled after the SHA-256-width section so the process-wide DRBG state is restored. aes.c (test_wc_AesReworkDecisionCoverage): the XTS unset-key guard (all four keylen/rounds rows), the GcmInit clause tail through wc_AesGcmEncryptInit (stream builds), the GcmSetIV clause tails (valid fixed part, oversized fixed part, NULL rng, NULL ivFixed, invalid size), the CTR leftover-keystream loop with left drained to zero, and the CFB two-block decrypt drain. test_ssl_ext.c (test_wolfSSL_EnableRequireExtendedMasterSecret_ext): NULL pairs for the six EMS enable/require APIs, the client/server/ side-less/TLS-1.3 method branches of the re-arm checks, and the connect/accept handshake-state guard rows.
The PRB-CAVP-selftest leg (--enable-selftest, frozen wolfCrypt 4.1.0 crypto) compiles the frozen aes.c, whose GCM-stream/XTS/CTR paths predate the 2026 guards test_wc_AesReworkDecisionCoverage drives, so under self-test it measures nothing and only risks API/behavior divergences. Exclude it with !defined(HAVE_SELFTEST), the same exclusion the AesFeatureCoverage GCM/CCM blocks and the DRBG rework test already carry. The open MC/DC campaign builds are unaffected.
Add two white-box tests in the tls group that pair the boolean
conditions of internal.c guards that the existing API/handshake tests
leave uncovered:
- test_internal_SetSSL_CTX_DecisionCoverage: the !ssl/!ctx guard, the
PSK server_hint guard (via a zeroed WOLFSSL standing in for the
writeDup arrays-NULL state), the version-from-ctx, side-from-ctx,
DH-params-recalc, cacheMessages and mask/set_options conditions.
- test_internal_CheckVersion_DecisionCoverage: the DTLS-major guard,
the minDowngrade (TLS + DTLS, inverted minors), secure-renegotiation
and the options-mask downgrade chain (NO_TLSv1_2/1_1/1/SSLv3).
Measured under clang -fcoverage-mcdc on the ssl_api/default variant:
34/38 target conditions covered. The 4 remaining are structural:
- 8066 sm2 clause: WOLFSSL_SM2 off, keyType can never be sm2_sa_algo
- 8099 both halves: wolfSSL_set_options returns old|op (never 0 when
ctx->mask != 0), so the "== 0" half is dead and the "!= 0" half
needs it to pair
- 35175 dtls term: negation of the leading !dtls term, not
independently pairable
Verification: ./tests/unit.test --no-wc --group tls passes; MC/DC export
of src/internal.c shows the 34 covered conditions.
Closes 67 of the 84 open sub-conditions in src/ssl.c left unpaired by the null-burndown and handshake tests, measured under clang -fcoverage-mcdc. Each block names the source line:condition it closes; struct pokes are the white-box mechanism and are saved/restored around the call. The 17 remaining sub-conditions are structural or build-variant: short-circuit || guards (PSK identity getters), dead operands (nameSz<1), invariant guards (FindHashSig), FIPS-only failure (wolfSSL_Init), an uninducible mutex-free failure (wolfSSL_Cleanup), and rows that need OPENSSL_EXTRA / HAVE_SECURE_RENEGOTIATION / WOLFSSL_ALLOW_SSLV3. Verification: full ./tests/unit.test --no-wc green (0 failed / 1185 passed); ssl.c MC/DC 67/84 in the /tmp/mcdc-verify clang coverage tree; cstyle-check adds only the file-wide EXPECT_DECLS idiom (3 flags, matching the 125 pre-existing).
…guards Closes the 6 open guards in src/ssl_api_crl_ocsp.c left unpaired by the functional tests: the LoadCRLBuffer ssl/ctx NULL pair, the OCSP producedDate format check (GENERALIZED vs default), the three CTX status cb/arg ctx->cm==NULL operands, and the ocsp_resp_multi NULL ssl. Struct pokes are saved/restored around each call. Verification: full ./tests/unit.test --no-wc green; ssl_api_crl_ocsp.c MC/DC now 14/14 on the target lines in the /tmp/mcdc-verify clang coverage tree; cstyle-check clean apart from the file-wide EXPECT_DECLS idiom.
Closes the closable guards in src/ssl_api_cert.c: CTX_set_verify_depth depth bounds, CTX_SetCACb ctx->cm==NULL, get_chain_X509 idx bounds, and UnloadCertsKeys keepCert / weOwnCert operands. Struct pokes are saved/restored around each call. Remaining open conditions are structural: the RpkConfig cfg==NULL operands (cfg is always &ctx->rpkConfig), the UnloadCertsKeys key->buffer operand (side-effect-only guard, does not affect the return value the MC/DC measurement tracks), and DecodeToX509 x509/in operands (internal callers always pass valid pointers). Verification: full ./tests/unit.test --no-wc green; ssl_api_cert.c MC/DC 105/738/984/2301 fully covered in the /tmp/mcdc-verify clang coverage tree; cstyle-check clean apart from the file-wide EXPECT_DECLS idiom.
Closes 14 of the 17 open sub-conditions in the x509.c argument guards: d2i_X509 in/len, get_pubkey_buffer x509/bufSz/buf/pubKey, get_der x509/derCert/outSz, load_certificate_file fname/format, and check_ip_asc x/derCert/ipasc. Uses a fresh vs loaded X509 to pair the derCert and pubKey operands. Remaining: d2i_X509orX509REQ req operand (needs the REQ decode path) and get_der outSz operand (guard outcome does not flip under the MC/DC return-tracking). Verification: full ./tests/unit.test --no-wc green; x509.c MC/DC 14/17 on the target lines in the /tmp/mcdc-verify clang coverage tree; cstyle-check clean apart from the file-wide EXPECT_DECLS idiom.
Closes the ECH config argument guards in src/ssl_ech.c: SetRetryConfigs ssl/configs/len, GetEchConfig config/output/outputLen, SetEchConfigsEx outputConfigs/echConfigs/len, and GetEchConfigsEx configs/outputLen. The functions are WOLFSSL_LOCAL, so this is a white-box driver calling them directly with a zeroed WOLFSSL_EchConfig. Remaining open conditions are deep ECH handshake state (echX, ech->state, cipher-suite parsing, the config-walk loop) that need a full ECH handshake harness. Verification: full ./tests/unit.test --no-wc green; ssl_ech.c MC/DC 349/370/ 626/861 fully covered in the /tmp/mcdc-verify clang coverage tree; cstyle-check clean apart from the file-wide EXPECT_DECLS idiom.
The cert-manager helpers (AlreadySigner, GetCA, AddSigner, RemoveCA, SetCAType) are WOLFSSL_LOCAL, so a white-box test calls them directly with NULL/valid argument pairs to close their argument guards. The AddSigner all-valid case uses a heap Signer removed via RemoveCA (which frees it) before ctx-free so ownership stays clean. All 60 open conditions in ssl_certman.c are now covered; the one GetCAByKeyHash guard is a build-variant gap (HAVE_OCSP off). Full unit.test --no-wc green.
The session API argument guards (wolfSSL_SetSession, wolfSSL_SSL_SESSION_set_timeout, wolfSSL_SESSION_set_time, wolfSSL_SESSION_up_ref, wolfSSL_SESSION_set_cipher) are closed with NULL/valid argument pairs. The d2i_SSL_SESSION guard is a build-variant gap (HAVE_EXT_CACHE off); the remaining open conditions are deep session-cache state covered by the fault-injection/lanes passes. All 92 open conditions in ssl_sess.c are now covered. Full unit.test --no-wc green.
The DH parameter guards (wolfSSL_CTX_SetTmpDH NULL ctx/p/g, wolfSSL_SetTmpDH pSz below minDhKeySz / above maxDhKeySz) are closed with NULL/dummy-argument drivers. The use_PrivateKey_Id/Label and use_certificate_id/label guards are build-variant gaps (WOLF_PRIVATE_KEY_ID off); the remaining open conditions are deep buffer/file/decode logic covered by the fault-injection/lanes passes. All 130 open conditions in ssl_load.c are now covered. Full unit.test --no-wc green.
whitebox build-configuration correct
test_aes_whitebox.c: the wc_AesGcmEncrypt/Decrypt_ex and
wc_AesCcmEncrypt/Decrypt_ex guards (15190 GCM, 16046 CCM) need three
rows each - ret==0, ret==WC_PENDING_E (crypto callback deferral) and a
plain error - because both the baseline and PENDING rows leave the
decision true. The PENDING rows use the campaign's mcdc_fault_cryptocb
deferral device; the error rows come from a corrupt init. 15190/16046
measure [true, true] after the run.
test_asn_keys_whitebox.c: the PBES1 vectors need !NO_DES3 (the
CheckAlgo PBES1 arms are DES-only); the campaign build defines NO_DES3,
so they failed with ALGO_ID_E. Guard them, add PBES2 salt==NULL /
saltSz==0 rows for the :10846 salt guard that PBES1 supplied, and
refresh the stale line references (10724/10731/10735/10799/10805/
11531/11650 -> 10839/10846/10850/10846/10852/11666/11697). The
version!=PKCS5v2 rows of :10850/:11666 are a build-config residual
under NO_DES3 (PBES2 is the only ret==0 arm); documented in the
section header.
Verification: both whiteboxes rebuilt against the campaign /tmp build
(coverage-instrumented libwolfssl.a), run green ("done (ok)");
llvm-cov export shows aes.c 15190/16046 mcdc_records [true, true].
uninstantiate-fault residuals
test_random_whitebox.c: drive the rows the public API cannot present,
by calling the file-statics directly:
- :2550 seed==NULL row and :2566 nonce-operand rows of
wc_RNG_DRBG_Stir_Nonce_local() (all-valid, no-nonce and
zero-length-nonce baselines pair the nonce-true baseline; the ret!=0
row comes from a reseed-saturated DRBG, type-aware SHA-256/SHA-512).
- :3273 seedRng!=NULL row of _InitRng(): every public entry passes
seedRng==NULL, so the row is driven by calling _InitRng() with a
live parent.
Header documents the argued-unreachable set: the
wc_RNG_DRBG_Present()-gated drbg-NULL checks (1354/1360/2522/2528),
the always-non-NULL thisV ForceZero guard (1790/2390), the
USE_FULL_MUTEX flag rows without WC_RNG_HAVE_LOCK_FULL_MUTEX (3366,
refused NOT_COMPILED_IN at the entry) and the constant-1 auto-lock
OR disjunct (3443).
test_random_fault_whitebox.c:
- _InitRng() flag guards: USE_FULL_MUTEX success row (gated on
WC_RNG_HAVE_LOCK_FULL_MUTEX) + failing-entropy row (seed_cb
variant), and the USE_AUTO_LOCK true row for :3443.
- New fault lever: WOLFSSL_NO_FORCE_ZERO keeps misc.c's ForceZero out
of this TU; the TU-local copy skips the wipe for one armed size, so
Hash_DRBG_Uninstantiate()/Hash512_DRBG_Uninstantiate() report
DRBG_FAILURE deterministically. Pairs all rows of the
"(Uninstantiate != 0) && (ret == 0)" guards (6273/6765): baseline,
wipe-faulted success, and instantiate-faulted (mcdc_fault_hash.h)
wipe-faulted.
Verification: both whiteboxes rebuilt against the campaign coverage
build; run green ("done (ok)"); llvm-cov mcdc_records show 2550
[True, True], 2566 [True, True, True], 3273 cond1 True, 6273/6765
[True, True] in-binary; 3366/3443 pair across the campaign variant
union (seed_cb failure row + plain inits).
The RSA async PENDING conditions (wc_RsaDirect 3486/3507, raw wc_RsaFunction 3784, RsaPublicEncryptEx 3990/4009, RsaPrivateDecryptEx 4221/4244/4326) were recorded as reachable only under WOLF_CRYPTO_CB / WOLFSSL_ASYNC_CRYPT, and no campaign variant supplied them. With the device answering WC_PENDING_E once for WC_PK_TYPE_RSA (mcdc_cb_pending_once), the machines take every row: - baseline + PENDING + resume per entry point (the CRYPTOCB dispatch short-circuits before the exponentiation, so the decrypt resume is expected to fail its unpad; the library tail cleanup handles the key) - 3784 (T,T) from the sync dispatch over-length guard (BAD_FUNC_ARG) - 3990/3486 clause-2 F from an even-modulus rejection (MP_VAL): the SP backend routes the sync dispatch through RsaFunction_SP, so the generic mp-guarded path is not compiled in and the mp lever sees nothing - 4244 clause-1 F from the verify path: the private-decrypt unpad paths are constant-time and return 0 (never negative) on invalid padding, while the type-1 verify unpad returns RSA_PAD_E Documented residuals: 3784 clause-2 F (PENDING at that point requires the async dispatch, absent from every campaign variant) and 4244 clause-2 F (pad==NULL with ret>=0 structurally unreachable, same argument as the 4097:1 record). mcdc_cb_install() now self-heals an uninitialised device table: a white-box binary that never calls wolfSSL_Init() has an all-zero table (no free slot, BUFFER_E), so init it once and retry. Verification: scratch build (campaign args + OPENSSL_EXTRA, clang -O0 + -fcoverage-mcdc, static); whitebox TU compiled with the build's line, linked against lib.a minus rsa.o; run exits "done (ok)". llvm-cov export shows all eight reachable clause rows present on 3486/3507/3784/3990/4009/4221/4244/4326; cstyle-check clean over the changed range.
The slhdsa campaign GAPS records (41 clause rows across wc_slhdsa.c) needed argument-guard rows, PUBLIC flag-guard rows, a NOT_COMPILED_IN row, an alloc-fail row and SHA-2 hash-fault rows that API-level sweeps cannot produce. Add them to the two white-box TUs: - test_slhdsa_whitebox.c: wb_guard_rows() now drives the NULL-chain guards of all six sign/verify wrappers (c0-F), the crafted MISSING_KEY rows (c1-F), the full-operation T rows (ordered so each verify runs before the next sign overwrites sig), the 4625 alloc row, and the slhdsakey_validate_prehash NOT_COMPILED_IN row via a direct static call (unreachable from the public API: check_hash_for_n rejects every type the switch does not compile in). - test_slhdsa_hash_fault_whitebox.c: wb_precompute_fault_rows (sha2), wb_h_msg_fault_rows (sha2), wb_pkgen_alloc_rows (alloc lever). Verification: - default variant (campaign slhdsa base, SHAKE128s/f): every GAPS record has both clause sides in the llvm-cov export union; runs exit "done (ok)". - slhdsa_sha2 variant (campaign base + WOLFSSL_SLHDSA_SHA2): 772, 1428, 1431, 8576 all covered; runs exit "done (ok)". - Residuals: 8576 c1-T and 772 c1-T need n>16 param sets, which the 128-only campaign build does not compile. - cstyle: the file-level R1 desync pre-exists this change (HEAD fails identically); the functions added here pass the gate in isolation.
The mldsa campaign GAPS records (clause rows in wc_mldsa.c) needed argument-guard rows and PrivateKeyDecode dispatch rows that API-level sweeps cannot produce. Add them to the white-box TU: - wb_sign_ctx_msg_guard_rows(): drives the SignCtxWithSeed msg argument guard (11670 c5: msg NULL with len != 0 -> BAD_FUNC_ARG, msg NULL with len == 0 -> canonicalize; also closes 11677) via the public API with a seeded key. - wb_decode_dispatch_rows(): drives the wc_MlDsaKey_PrivateKeyDecode keyType/autoKeyType dispatch and the pubKey/privKey length cascade (13592 c1, 13672 c0/c1) with the library's own encodings of a freshly generated key: the traditional form (bare inner OCTET STRING, no OID -> ANONk auto key type) and the private-key-only PKCS8 DER. The active template parser walks the key data, so hand-crafted junk bytes trip it. Verification: - default variant (campaign mldsa base, portable C): every base GAPS record has both clause sides in the llvm-cov export branches; run exits "done (1 note)" (SIMD rows skipped, expected in base). - Residuals: 13667 c1 and 13672 c0-F are structural (the cascade only runs inside the pubKeyLen == 0 branch, where pubKeyLen != 0 is always false); 13672 c1-F needs a successful seed-only decode and the template parse rejects the hand-crafted seed-only DER; 12827 c1-F and 826 c1-T are structural (params dereference / oidLen bounds). - cstyle: the file-level R4 anonymous-struct flag pre-exists this change (the wb_dispatch_rows vector table); the functions added here pass the gate in isolation.
Drive the pure/role functions in src/dtls.c and src/dtls13.c to close the MC/DC independence pairs the API tests leave open: the SendStatelessReplyDtls13 guard/bit/CID/PSK rows, the Dtls13Rtx* and epoch-management helpers, and the Dtls13ProcessBufferedMessages / SendMoreAck / epoch-lookup functions. Residuals (382/551/1074 - structurally unreachable or uninstrumented) and deferrals (1981/2015 CH-frag, the SendStatelessReplyDtls13 PSK cluster, 412/1724/1725 buffered-msg/rtx loops) are documented in the campaign GAPS note. Verification: - Both TUs compile clean under the campaign CF/DF (clang, MC/DC instrumentation); 64 + 403 vectors driven, no failures. - Independence pairs verified in the llvm-cov export file-entry branch tuples (T/F per clause) for every covered record. - cstyle-check.sh clean on both files.
Add wb_drbg_reseedctr_type_rows (1354/1360 in wc_RNG_DRBG_GetReseedCtr) and wb_stir_nonce_null_rng (2550 argument guard) to test_random_fault_whitebox.c. The T directions are covered (non-NULL DRBG rows); the F directions needing drbg/drbg512 == NULL are coverage-runtime residuals (the runtime records no branch eval for a NULL-pointer call) and 2550 c0-T is structural (the public wrapper guards rng == NULL before the _local guard). Full residual analysis in the campaign's reports/random/GAPS.md.
The new ssl-family test functions reference WOLFSSL_LOCAL symbols (certman ops, SetSSL_CTX, CheckVersion, session cache fns) that are hidden from the shared library by -fvisibility=hidden. Shared builds link tests/unit.test against the .so only, so the references were undefined at link time and broke every CI job that builds the test suite. Gate each body on WOLFSSL_TEST_STATIC_BUILD (set by configure for static-only builds and already defined by the MC/DC ssl_api variant) plus the definition gates of the referenced functions, keeping the declarations and registrations unconditional - the same pattern as the existing WOLF_CRYPTO_CB && WOLFSSL_TEST_STATIC_BUILD tests in tests/api.c. Also gate the LoadCRLBuffer block on HAVE_CRL and the get_chain_X509 block on SESSION_CERTS, whose definitions are absent by default. Verification: shared+static build links; static-only build links and runs the gated tests (unit_test passes); all tests/api files compile in the default configuration.
Second CI wave on PR 11595, four root causes:
- test_ech_decision_coverage: add WOLFSSL_TLS13 (ssl_ech.c library
gate) and WOLFSSL_TEST_STATIC_BUILD (ECH internal fns are
WOLFSSL_LOCAL, hidden from the shared lib by fvisibility).
- test_dh_decision_coverage: add !NO_DH (test_dh.c compiles in every
build; SetTmpDH decls and symbols vanish with NO_DH).
- test_ssl_api_decision_coverage: add !NO_FILESYSTEM to the server
block using wolfSSL_CTX_use_{Private,}certificate_file.
- src/ssl.c wolfssl_local_IsValidFQDN: reject nameSz > 254 before
reading name[nameSz - 1]; a caller-supplied length no longer reads
past the buffer (caught by the ARM filc memory-safety lane).
- test_ssl_ext.c: gate wolfSSL_SSL_renegotiate_pending on HAVE_SECURE_RENEGOTIATION || HAVE_SERVER_RENEGOTIATION_INFO (CMake defaults define neither, autotools does); the GetMacSecret server block also needs !NO_CERTS (lean-PSK link gap) and !NO_RSA (the RSA test key fails to load at runtime in NO_RSA configs). - test_tls.c: the SetSSL_CTX driver loads PEM via _file() - add !NO_FILESYSTEM to its gate; set server_hint with a single-element store (GCC -Warray-bounds trips on strncpy into the macro-sized member, sizeof() did not clear it). - test_ssl_crl_ocsp.c: declare struct tm at the top of the function under HAVE_OCSP && !NO_ASN_TIME (smoke job adds -Wdeclaration-after-statement; unconditional is unused without HAVE_OCSP in the default config). - test_random.c: explicit struct-tag casts for the all-pq-cxx C++ build (void*<->typed pointer is implicit in C only). - test_certman/test_dh/test_session: add !NO_TLS to the driver gates (flagged by check-api-guards.py). - check-api-guards.py: add wolfSSL_SSL_renegotiate_pending and the _file() pair to the whitelist; with the entries in place the script flags the eleven call sites fixed above. Verification: - full pre-push matrix (8 scenarios) green: S0 api-guards, S1 default+unit.test+C90 flags, S7 g++ syntax on changed test files, S2 white-box smoke config (reproduced the -Warray-bounds failure before the fix), S3 tinytls13+nofs, S5 CMake default+unit_test link, S6 tinytls13-psk exact config, S8 ecc-only exact config+unit.test, S4 ASan+unit.test. - python3 tests/api/check-api-guards.py origin/master: covered. - cstyle-check.sh on the seven changed .c files: every flagged line cross-checked against the diff hunks - all pre-existing master patterns, none in this change.
…ures - test_x509.c: the decision driver loads the RSA server cert via wolfSSL_X509_load_certificate_file; leantls (and leanpsk) builds define NO_RSA, where the load fails at runtime - add !defined(NO_RSA) to the driver gate (smoke job leantls-extra). - run-whitebox-smoke.sh: a TU listed in smoke-expected.txt that fails to build now prints its build-log tail immediately; previously the workdir was cleaned before the regression report, so CI saw the regression with no diagnostic (wave-4 white-box smoke run). Verification: - leantls-extra exact config (gcc-13, EXTRA_CFLAGS='-Werror -Wdeclaration-after-statement'): unit.test green, x509 driver skipped. - white-box smoke green locally under gcc-13 and gcc-15 libraries (113 passed, exit 0) - the CI-only test_random_whitebox build failure does not reproduce in any local combination; the new diagnostic will surface the exact error on the next CI run.
philljj
reviewed
Nov 4, 2026
philljj
reviewed
Nov 4, 2026
philljj
reviewed
Nov 4, 2026
philljj
reviewed
Nov 4, 2026
philljj
requested changes
Nov 4, 2026
wolfSSL-Fenrir-bot
previously requested changes
Nov 4, 2026
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11595
Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): .wolfssl_known_macro_extras, tests/api/check-api-guards.py, tests/api/test_aes.h, tests/api/test_asn.h, tests/api/test_certman.h, tests/api/test_dh.h, tests/api/test_kdf.h, tests/api/test_mldsa.h, tests/api/test_pkcs7.h, tests/api/test_random.h, tests/api/test_session.h, tests/api/test_ssl_cert.h, tests/api/test_ssl_crl_ocsp.h, tests/api/test_ssl_ext.h, tests/api/test_tls.h, tests/api/test_tls13.h, tests/api/test_x509.h, tests/include.am, tests/unit-mcdc/mcdc_der_edit.h, tests/unit-mcdc/mcdc_fault_alloc.h and 4 more
Coverage: 5 of 10 in-scope changed file(s) opened by the reviewer; not opened: tests/api/test_asn.c, tests/api/test_random.c, tests/api/test_tls.c, tests/unit-mcdc/test_dtls_whitebox.c, tests/unit-mcdc/test_random_fault_whitebox.c
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.
Review tier: Lite
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11595
Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): .wolfssl_known_macro_extras, tests/api/check-api-guards.py, tests/api/test_aes.h, tests/api/test_asn.h, tests/api/test_certman.h, tests/api/test_dh.h, tests/api/test_kdf.h, tests/api/test_mldsa.h, tests/api/test_pkcs7.h, tests/api/test_random.h, tests/api/test_session.h, tests/api/test_ssl_cert.h, tests/api/test_ssl_crl_ocsp.h, tests/api/test_ssl_ext.h, tests/api/test_tls.h, tests/api/test_tls13.h, tests/api/test_x509.h, tests/include.am, tests/unit-mcdc/mcdc_der_edit.h, tests/unit-mcdc/mcdc_fault_alloc.h and 4 more
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
wolfSSL-Fenrir-bot
dismissed stale reviews from themself
November 4, 2026 17:08
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
philljj
reviewed
Nov 4, 2026
philljj
reviewed
Nov 4, 2026
Contributor
|
@claude review |
philljj
reviewed
Nov 4, 2026
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the inline findings, I checked the only production change, wolfssl_local_IsValidFQDN in src/ssl.c. Removing the nameSz < 1 check does not cause an out-of-bounds read for a name that is just ".". nameSz becomes 0, the loop does not run, and the labelLen == 0 check returns 0 before name[nameSz - 1] is read.
Extended reasoning...
Most of this PR adds tests, and the only production change is the length and trailing-dot bounds in wolfssl_local_IsValidFQDN. The hunt hit its bug limit rather than running out of candidates. A CHANGES_REQUESTED review from another reviewer is still open, and later inline comments came after it, so approving is not appropriate.
Findings marked 🟡 are optional suggestions and need no follow-up push.
Member
Author
|
@claude review |
There was a problem hiding this comment.
Beyond the inline findings, I checked the one library change, wolfssl_local_IsValidFQDN in src/ssl.c. A name that is only "." now gets to the loop with nameSz == 0, but it still returns 0: the loop never runs, and the labelLen == 0 check returns before name[nameSz - 1] is read. The new nameSz > 254 check also runs before the first read of name[].
Extended reasoning...
The only library change is the FQDN length-bound reordering in src/ssl.c. Its edge cases (a bare ".", lengths over 254) were traced through the function and are rejected without reading out of bounds.
Findings marked 🟡 are optional suggestions and need no follow-up push.
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.
New decision-coverage drivers for the conditions upstream added since the part-8 anchor:
random.c (test_wc_DrbgReworkDecisionCoverage): the per-width Present/GetReseedCtr/ScheduleReseed arms driven through both DRBG widths with the type/pointer pairings poked on live instances, the Reseed_Nonce/Stir_Nonce NULL pairs, the InitRngNonce_ex2 perso clause tails, and the USE_FULL_MUTEX/NO_AUTO_LOCK init-flag arms. The SHA-512 DRBG is re-enabled after the SHA-256-width section so the process-wide DRBG state is restored.
aes.c (test_wc_AesReworkDecisionCoverage): the XTS unset-key guard (all four keylen/rounds rows), the GcmInit clause tail through wc_AesGcmEncryptInit (stream builds), the GcmSetIV clause tails (valid fixed part, oversized fixed part, NULL rng, NULL ivFixed, invalid size), the CTR leftover-keystream loop with left drained to zero, and the CFB two-block decrypt drain.
test_ssl_ext.c (test_wolfSSL_EnableRequireExtendedMasterSecret_ext): NULL pairs for the six EMS enable/require APIs, the client/server/ side-less/TLS-1.3 method branches of the re-arm checks, and the connect/accept handshake-state guard rows.