Report FIPS module failures through wolfssl_prov_is_running()#428
Report FIPS module failures through wolfssl_prov_is_running()#428yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes FIPS-mode status reporting so that wolfssl_prov_is_running() (and therefore OSSL_PROV_PARAM_STATUS) reflects live wolfCrypt FIPS module failure state instead of always reporting “running”, and adds a standalone regression test plus CI wiring to ensure failures are observable to status-polling callers.
Changes:
- Gate
wolfssl_prov_is_running()onwolfCrypt_GetStatus_fips()underHAVE_FIPSso non-zero FIPS status reportsOSSL_PROV_PARAM_STATUS == 0. - Add new standalone
fips_statustest (plus runner registration) that validates healthy behavior and forced-failure behavior. - Add a test-only build knob (
WOLFPROV_FORCE_FIPS_FAILURE) and CI step to ensure the negative-path assertion actually executes.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/wp_wolfprov.c |
Make provider “running” status reflect live wolfCrypt FIPS status under HAVE_FIPS. |
test/standalone/tests/fips_status/test_fips_status.c |
New standalone regression test for provider status reporting before/after forced FIPS failure. |
test/standalone/tests/fips_status/run.sh |
Runner script for the new standalone test binary. |
test/standalone/runners/run_standalone_tests.sh |
Register/execute the new standalone test in the master standalone runner. |
test/standalone/include.am |
Add build rules for the new fips_status.test standalone program. |
scripts/utils-wolfssl.sh |
Add internal env knob to enable HAVE_FORCE_FIPS_FAILURE in wolfSSL FIPS builds for testing. |
.github/workflows/fips-ready.yml |
Enable the knob in CI and add a step that runs the new test and asserts the forced-failure branch executed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f78f2ba to
a020a00
Compare
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #428
Scan targets checked: wolfprovider-bugs, wolfprovider-src
No new issues found in the changed files. ✅
a020a00 to
3d5eef8
Compare
3d5eef8 to
d65e209
Compare
| ./test/standalone/tests/fips_status/run.sh 2>&1 | tee fips-status.log | ||
| # The forced-failure branch must have actually run in this FIPS build; | ||
| # fail if the test silently degraded to the healthy path only. | ||
| grep -q "correctly rejected after FIPS failure" fips-status.log |
There was a problem hiding this comment.
Is there a more generic and thus more robust check than looking for this specific string? Maybe look for a pass/fail?
| - name: Build wolfProvider with FIPS Ready Bundle | ||
| run: | | ||
| ./scripts/build-wolfprovider.sh --fips-bundle="$FIPS_BUNDLE_PATH" \ | ||
| # WOLFPROV_FORCE_FIPS_FAILURE is an internal, test-only knob (no CLI |
There was a problem hiding this comment.
Can we call this WOLFPROV_FIPS_FORCE_FAIL for consistency with WOLFPROV_FORCE_FAIL?
Summary
Fixes finding 4540: in FIPS builds,
wolfssl_prov_is_running()unconditionallyreturned 1, so a wolfCrypt integrity/POST failure was invisible to callers polling
OSSL_PROV_PARAM_STATUS. This gates the provider's running/status report on the liveFIPS module status and adds a regression test for the new behavior.
Problem
wolfssl_prov_is_running()had only one non-1 path — theWP_CHECK_FORCE_FAILdebuglever — and otherwise always returned 1.
wolfprov_get_params()setsOSSL_PROV_PARAM_STATUSdirectly fromwolfssl_prov_is_running(), so a status-polling caller always saw "running".wp_fipsCbFIPS callback only logged and mutated no state, so a failure itreceived was dropped.
Net effect: a FIPS-aware caller enforcing a fail-stop policy via
OSSL_PROV_PARAM_STATUScould not observe an unhealthy FIPS module. (The wolfCrypt primitives themselves still
return errors in
FIPS_MODE_FAILED, so this is a status-reporting / policy fail-open,not silent fail-open crypto.)
Fix
Gate
wolfssl_prov_is_running()onwolfCrypt_GetStatus_fips()under#ifdef HAVE_FIPS;a non-zero status (integrity/POST or continuous-test failure) returns 0. Querying live
status is a single source of truth and does not depend on the callback having fired.
wp_fipsCbis left as a logger.Regression test
New
test/standalone/tests/fips_status/standalone test. It loadslibwolfprov, then:OSSL_PROV_PARAM_STATUS == 1and that an AES-256-CBC initsucceeds (proves the provider is actually operable, not just reporting healthy).
0and the same operationis now rejected.
The failure is injected by a compile-time mechanism ladder chosen from what the build
provides:
HAVE_FIPS && HAVE_FORCE_FIPS_FAILURE→ realwolfCrypt_SetStatus_fips(IN_CORE_FIPS_E).HAVE_FIPS && __linux__→ ELF symbol interposition ofwolfCrypt_GetStatus_fips(),guarded by a
dlsym(RTLD_DEFAULT, ...)canary that fails loudly if interposition isnot actually in effect (never misreports an ineffective inject as a provider regression).
HAVE_FORCE_FIPS_FAILUREis enabled by an internal, test-only env knob(
WOLFPROV_FORCE_FIPS_FAILURE) — deliberately not a CLI flag, so it is not exposed tocustomers. When set,
-DHAVE_FORCE_FIPS_FAILUREreaches both the wolfSSL FIPS build andthe provider/test builds automatically via wolfSSL's generated
options.h(no relayvariable or source-tree grep).
CI
fips-ready.yml(the source-FIPS job onubuntu-22.04) builds with the knob, runs thetest, and greps for the forced-failure marker so the job fails if the negative branch
did not actually execute (guards against silent degradation).
Files
src/wp_wolfprov.c— the fix; move the FIPS header include to the top.test/standalone/tests/fips_status/{test_fips_status.c,run.sh}— new test + driver.test/standalone/include.am,test/standalone/runners/run_standalone_tests.sh—register the test.
scripts/utils-wolfssl.sh— internalWOLFPROV_FORCE_FIPS_FAILUREknob..github/workflows/fips-ready.yml— build knob + test step + negative-path assertion.Testing
SetStatusand interposition) compile against thereal FIPS headers.