SCEP: add keep-alive / async SCEP session API#12
Conversation
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12
Scan targets checked: wolfcert-bugs, wolfcert-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
Add WolfCertScepSession with blocking (wolfcert_scep_session_open) and non-blocking (wolfcert_scep_session_open_async) opens, plus session PKCSReq, RenewalReq and GetCertInitial in _ex (blocking) and _nb (WANT_READ/WANT_WRITE pumped) forms, with wolfcert_scep_session_fd and wolfcert_scep_session_close. The session reuses the same non-blocking HTTP session layer as EST. Unlike EST it does not require TLS, since SCEP authenticates at the pkiMessage layer, so a plaintext http:// endpoint is accepted. do_scep_round_trip is refactored into reusable scep_prepare (envelope + sign, producing the transactionID and senderNonce) and scep_finish (parse + verify + de-envelop) phases, with the POST/GET transport decision factored into scep_build_transport. The one-shot and session paths share these; existing entry-point behavior is unchanged. An async round trip builds the message once, pumps only the HTTP POST/GET through the session state machine, then parses the CertRep on completion; the crypto stays synchronous. The txid, senderNonce and finish-phase inputs (CA bundle, signer cert/key) are captured in the session so they survive across pumps. scep_prepare now checks the wc_InitRng_ex / wc_RNG_GenerateBlock return codes and fails the round trip on error, rather than building a pkiMessage over an uninitialized transactionID and anti-replay senderNonce. The session records which PKIOperation occupies its single in-flight slot and rejects an async _nb resume that names a different operation or passes a different WolfCertScepResult* out than the one captured at begin; it also records whether it was opened blocking or async so a mismatched _ex/_nb call is rejected with WOLFCERT_ERR_BAD_ARG instead of overwriting (and leaking) the in-flight buffers - including the zeroized private-key copy - or blocking on a non-blocking socket. Like the one-shot path a plaintext http:// session is accepted, but an https:// session now requires verify_server rather than silently running an unauthenticated TLS handshake. The scheme://host[:port] origin builder shared by the EST and SCEP session opens is factored into wolfcert_http_url_origin so the two cannot drift. Tests: test_scep_async_roundtrip drives a direct async enrollment; a blocking-session enrollment followed by a blocking RenewalReq; an async PENDING -> GetCertInitial poll and its blocking mirror (GetCertInitial_ex); an async enrollment followed by an async RenewalReq; and a session forced onto the base64 GET transport. It also exercises the session misuse guards: an _ex call on an async session, an _nb call on a blocking session, and starting a second operation while one is in flight. The async scenarios are pumped through a real poll(2) loop. Registered in both CMake and autoconf. ARCHITECTURE.md, EMBEDDED context and CLAUDE.md notes updated.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #12
Scan targets checked: wolfcert-bugs
Failed targets: wolfcert-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| s->in_signer_len = signer_cert_len; | ||
| s->in_signer_key = dup_buf(signer_key, signer_key_len, heap); | ||
| s->in_signer_key_len = signer_key_len; | ||
| if (s->in_ca_bundle == NULL || s->in_signer == NULL || s->in_signer_key == NULL) { |
There was a problem hiding this comment.
🔵 [Low] scep_session_begin returns WOLFCERT_ERR_MEMORY for zero-length buffer arguments · Incorrect error handling
dup_buf returns NULL for both malloc failure and len == 0. The six public session entry points validate non-NULL pointers but not non-zero lengths for ca_bundle_len, signer_cert_len, and signer_key_len, so a zero-length input reaches the NULL check at line 1226 and returns WOLFCERT_ERR_MEMORY instead of WOLFCERT_ERR_BAD_ARG.
Fix: Add ca_bundle_len > 0, signer_cert_len > 0, and signer_key_len > 0 guards in each public session entry point alongside the existing non-NULL checks.
| WolfCertKeyCfg kcfg2 = { .type = WOLFCERT_KEY_RSA, .param = 2048, | ||
| .dev_id = WOLFCERT_DEVID_SOFTWARE }; | ||
| WolfCertKey* dk2 = NULL; | ||
| REQUIRE(wolfcert_key_generate(&kcfg2, &dk2) == WOLFCERT_OK); |
There was a problem hiding this comment.
🔵 [Low] Resource leak of bsess and related allocations on REQUIRE failure in async_enroll_path · Resource leaks in fixtures and teardown
bsess is opened at line 205, and dk2/csr2/rb/rb_der are allocated in subsequent lines. Any REQUIRE failure in lines 209–229 returns 1 without closing bsess or freeing those resources; ASAN/valgrind will report the open socket and allocations.
Fix: Add a cleanup: label (or explicit close/free before each early return 1) so bsess, dk2, csr2, rb, and rb_der are always released.
| int rc = wolfcert_scep_session_pkcs_req_nb(asess, &caps, | ||
| ca_der->buffer, ca_der->length, ca_der->buffer, ca_der->length, | ||
| dk, csr.data, csr.len, &r1); | ||
| REQUIRE(rc == WOLFCERT_ERR_WANT_READ || rc == WOLFCERT_ERR_WANT_WRITE); |
There was a problem hiding this comment.
🔵 [Low] async_guard_path in-flight guard test is timing-dependent and can become flaky · Race conditions in setup/teardown
The test asserts the first wolfcert_scep_session_pkcs_req_nb call returns WANT_READ/WANT_WRITE. On a fast loopback where the server processes the request before the first non-blocking read, the call can return WOLFCERT_OK; the REQUIRE then fires, making the test fail rather than silently covering the in-flight guard path.
Fix: Introduce a small server-side processing delay (e.g., a configurable scep_response_delay_us in WolfCertServerCfgSrv) to make the timing assumption reliable, or restructure the guard test to not depend on response latency.
2 participants
Summary
Adds a keep-alive / non-blocking SCEP (RFC 8894) client session, the SCEP counterpart to the existing EST session. This is the last piece needed for feature-equivalence with wolfSCEP: SCEP enrollment can now run several PKIOperation round trips over one connection, and drive them non-blocking through a caller-owned event loop (
poll/epoll/kqueue).No public API is changed or removed; this is purely additive plus an internal refactor shared with the EST path.
What's included
WolfCertScepSessionwith blocking and non-blocking opens, plus PKCSReq, RenewalReq and GetCertInitial in_ex(blocking) and_nb(WANT_READ/WANT_WRITEpumped) forms.do_scep_round_tripsplit into reusablescep_prepare(envelope + sign),scep_build_transport(POST vs base64 GET), andscep_finish(parse + verify + de-envelop). The one-shot and session paths share these; one-shot behavior is unchanged.scheme://host[:port]construction duplicated by the EST and SCEP session opens is factored intowolfcert_http_url_originso the two cannot drift.ARCHITECTURE.md,EMBEDDED.mdcontext,CLAUDE.md, plus CMake + autoconf registration of the new test.New public API (
wolfcert/scep.h)wolfcert_scep_session_open/_open_asyncwolfcert_scep_session_closewolfcert_scep_session_fdpoll/epoll/kqueuewolfcert_scep_session_pkcs_req_ex/_nbwolfcert_scep_session_renewal_req_ex/_nbwolfcert_scep_session_get_cert_initial_ex/_nbDesign notes
_ex/_nbsplit, same non-blocking HTTP session layer). The crypto stays synchronous; only the HTTP POST/GET is pumped through the state machine. The transactionID, senderNonce, and finish-phase inputs (CA bundle, signer cert/key) are captured in the session so they survive across pumps.http://endpoint is accepted. Anhttps://endpoint, however, now requiresverify_serverrather than silently running an unauthenticated handshake.Hardening and correctness
scep_prepare: awc_InitRng_ex/wc_RNG_GenerateBlockfailure now fails the round trip instead of building a pkiMessage over an uninitialized transactionID and anti-replay senderNonce. (Pre-existing behavior in the one-shot path; fixed for both, now that the code is shared.)WOLFCERT_ERR_BAD_ARG:_ex/_nbpairing against the session's open mode (this also makes an in-flight-buffer / private-key-copy leak unreachable),WolfCertScepResult*than the one captured at begin (guards against a silently-empty result or a dangling-pointer write).Testing
tests/integration/test_scep_async_roundtrip.cdrives the in-tree test server end to end:_ex-on-async,_nb-on-blocking, in-flight op-type mismatchget_cert_initial_ex)The async scenarios are pumped through a real
poll(2)loop. All 25 project tests pass (including the 12 EST tests, which exercise the shared origin-builder refactor); build is warning-clean.