Harden the EST/SCEP server, CA store and HTTP client by Frauschi · Pull Request #25 · wolfSSL/wolfCert

Frauschi · 2026-09-10T13:51:15Z

A round of Fenrir findings against the server, CA-store and HTTP layers — one finding per commit, each independently buildable and testable. No public API breaks; one behavioural change, noted below.

Findings fixed

F-8031 — EST listener without a TLS identity served cleartext. tls_setup() treated missing tls_cert_pem/tls_key_pem as a successful plaintext configuration for every protocol, so an EST server started without TLS material served /cacerts, /csrattrs and /simpleenroll over plain HTTP, putting the HTTP Basic credentials and the CSR on the wire. RFC 7030 has no plaintext mode and the client already refused an http:// EST URL, so the server now refuses the same configuration with WOLFCERT_ERR_TLS before it binds.

F-8009 — IPv6 literals lost their brackets. wolfcert_http_url_parse stores an IP-literal host bracket-stripped, but wolfcert_http_url_origin re-emitted it bare, producing origins like https://::1:8443. The EST and SCEP session opens feed that string back through the parser, where the host scan stops at the leading colon, so every session-based operation against an IPv6 literal broke before the connect; the two request builders had the same defect in the Host header, which RFC 7230 §5.4 requires bracketed.

F-8025 — CSR subject RDNs silently dropped. wolfcert_ca_issue rebuilt the subject field by field and copied only CN, O, OU, C, ST and L, so a CSR carrying e.g. UID or postalCode produced a certificate holding neither, with no error. The remaining components wolfSSL exposes on the decoded CSR are now copied.

F-8026 — A CA-store read error re-rooted the PKI. wolfcert_server_start treated every wolfcert_ca_load failure as an empty store, so a transient read error or a corrupt ca.key.der made the server mint a fresh CA and overwrite the stored one, invalidating every previously issued certificate. A new CA is now generated only on WOLFCERT_ERR_NOT_FOUND, other load errors propagate, and the previously discarded wolfcert_ca_save return now fails the start.

F-8027 — An idle peer pinned the serving thread past stop. wolfcert_server_stop only set the stopping flag, which the accept loop checks while parked at poll() on the listener; once a connection was accepted the flag went unread until the keep-alive loop condition. A peer that connected and then sent nothing left the thread blocked forever in wolfSSL_accept or the handler's first read, hanging any caller that joined it after stop.

F-9771 — A mismatched CA cert/key pair started fine. wolfcert_ca_load only checked that the stored private key decoded; it never parsed the stored certificate or confirmed the certificate carried that key's public half. A store holding an unrelated pair therefore started a server whose issued certificates and PKCS#7 replies were signed by a key the advertised CA certificate did not match, and whose SCEP requests could not be decrypted.

F-11049 — The two HTTP readers disagreed on legal responses. nb_read_some always asked for a full WOLFCERT_HTTP_READ_CHUNK, which the reserve refuses if it would take the accumulator past the body cap plus header budget, so a response fitting inside the allowance but leaving less than one read quantum of headroom was rejected with WOLFCERT_ERR_PROTOCOL. The blocking reader accepted the same response. Both reads are now clamped to the room the allowance still permits.

F-12856 — A query-only URL swallowed the host. The authority scan stopped only at a colon or slash, so http://ca.example?operation=GetCACaps — exactly the shape the SCEP client builds when the configured URL has no path — put the whole query into the host, breaking GetCACaps and PKIOperation against valid servers.

F-8032 — The generated CA had no keyUsage. The bootstrap path emitted basicConstraints CA:TRUE and no keyUsage, leaving a relying party nothing to check the CA against. It now asserts keyCertSign, cRLSign and digitalSignature for every key type, plus keyEncipherment only for RSA — the RSA key also decrypts the pkcsPKIEnvelope, and wolfCert's SCEP is RSA-only. (RFC 8894 section 3.1 does define a CMS PasswordRecipientInfo path for keys that cannot do key transport; wolfCert does not implement it.)

Behavioural change

An EST listener configured without --tls-cert and --tls-key is now rejected up front instead of silently serving cleartext. SCEP authenticates at the pkiMessage layer and keeps its plaintext transport unchanged. README.md and CLAUDE.md quick-starts are updated to match.

Tests

New: test_server_ca_store (mismatched pairs per algorithm, unreadable and unwritable stores, reload for every key type) and test_server_stop_idle (parked in wolfSSL_accept(), parked in the handler read after a completed handshake, the same on a plaintext SCEP listener, and serve_fd() on a caller-supplied non-blocking fd). Extended: test_http for the allowance boundary and the IPv6 parse→origin→parse roundtrip, and test_est_roundtrip for the full RDN set.

test_est_chunked_robustness and test_est_csr_attrs_enforce scripted byte-exact HTTP at a plain socket, so they now drive the server through a small raw TLS client in tls_test_util.h — one write is one TLS record and so one read on the server, preserving the segment boundaries the chunked framing cases depend on.

The ThreadSanitizer job filtered -R 'roundtrip|tls_http', which excluded server_stop_idle, the one test that exercises shutdown from a second thread; the filter now includes it.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #25

Scan targets checked: wolfcert-src, wolfcert-bugs

Findings: 7
7 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #25

Scan targets checked: wolfcert-src, wolfcert-bugs

Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)

💬 3 finding(s) from an earlier review are still open and were not re-posted:

CA mismatch cleanup leaves private-key DER in freed heap — src/ca_issue.c:323
Exact-cap EOF response is rejected before closure is read — src/http.c:1532
Socket-timeout installation failures leave broken serving modes — src/server.c:398

This review was generated automatically by Fenrir. Reported findings require changes before merge.

yosuke-wolfssl

Reviewed the full series and read both Fenrir rounds. This is careful work — nine focused commits, each of which builds standalone (I checked all nine).

What I ran locally (macOS, wolfSSL master): clean build under -Wall -Wextra -Wshadow -Wpedantic, 29/29 ctest pass. Both negative controls fail as they should — reverting src/server.c fails server_stop_idle at "idle before handshake", and stubbing out wc_ed448_import_public() fails server_ca_store at the chain verify. Stale-contract sweep for plaintext/cleartext came back clean; the interop scripts already passed --tls-cert.

Everything below is distinct from the round-2 bot findings. Five blocking.

Blocking

1. The TLS retry loops each resume only one direction

src/server.c:419-423, :68, :99

Arming SO_SNDTIMEO changes what these loops see. TranslateIoReturnCode() maps a send-side EAGAIN/ETIMEDOUT to WOLFSSL_CBIO_ERR_WANT_WRITE, so:

the wolfSSL_accept() loop continues only on WANT_READ — a client whose receive window stalls the server's Certificate flight past 200 ms now has its handshake abandoned, where the blocking socket previously just waited. An ML-DSA chain is tens of KB, so this is reachable.
wolfcert_io_recv() retries only WANT_READ, but wolfSSL_read() returns WANT_WRITE whenever it must send — which is what TLS 1.3 post-handshake auth does, and PHA is supported here (est_pha_roundtrip).
wolfcert_io_send() has the mirror gap for WANT_READ on a key update.

Worth noting this pulls against the round-2 "trickling peer" finding, which asks for a stopping check before every I/O. Both are fixable together — check stopping first, then retry on either direction — but they shouldn't be resolved independently or you'll trade one failure mode for the other.

2. The new tests are gated on the wrong feature

tests/CMakeLists.txt:119, Makefile.am:194

test_server_ca_store sits under WOLFCERT_ENABLE_SCEP but tests the CA store. Under -DWOLFCERT_ENABLE_SCEP=OFF — which the CI matrix builds as cmake-nonrsa-macos — all coverage for F-8026 and F-9771 disappears, including the Ed25519/Ed448 reload fix that nothing else guards. It belongs under WOLFCERT_ENABLE_SERVER. test_server_stop_idle has the mirror problem under WOLFCERT_ENABLE_EST while containing a SCEP case.

3. Three more RDNs are dropped, and the copied ones can still truncate

src/ca_issue.c:744-759, :694

The round-2 givenName finding is the tip of this. I decoded a CSR carrying the full set to separate "decoder never populates it" from "we don't copy it":

Field	Decoder	PR copies
`subjectStreet` → `street`	populated	no
`subjectJC` → `joiC`	populated	no
`subjectJS` → `joiSt`	populated	no
`subjectN`, `subjectI`, `subjectDNQ`	never set	n/a

The last row is the same cause as givenName — wolfSSL's generic subject path is id > ASN_COMMON_NAME (0x03) && id <= ASN_USER_ID (0x12), and those OIDs are 0x29/0x2b/0x2e. The first three are real drops.

Separately, COPY_SUBJ still truncates at CTC_NAME_SIZE - 1 (63) with no error, so a 70-character emailAddress or UID yields a certificate whose subject differs from the CSR silently — the same class of bug F-8025 describes. And the encoding byte isn't carried (dc.subjectCNEnc → nc->subject.commonNameEnc), so a PrintableString RDN comes back as UTF8String.

4. basicConstraints is not marked critical

src/ca_issue.c:93

I decoded a CA generated through the new path:

X509v3 Basic Constraints:
    CA:TRUE
X509v3 Key Usage: critical
    Digital Signature, Key Encipherment, Certificate Sign, CRL Sign

Key Usage criticality and the keyCertSign ⇒ cA requirement are both satisfied. But RFC 5280 §4.2.1.9 is a MUST: "Conforming CAs MUST include this extension in all CA certificates that contain public keys used to validate digital signatures on certificates and MUST mark the extension as critical in such certificates." Pre-existing, but three lines above the new code, in the commit whose whole purpose is making the CA checkable by a relying party.

5. "RFC 8894 limits that envelope to RSA" (PR description)

§3.1 says the opposite: "If the key is not encryption capable (for example, DSA or ECDSA), then the messageData is encrypted using the challengePassword with the CMS PasswordRecipientInfo mechanism." The conditional in the code is right — a non-RSA key can't do key transport, so asserting keyEncipherment on it would violate 5280 — but the reason given is an RFC constraint that doesn't exist. "wolfCert's SCEP is RSA-only" is the accurate framing.

Comments

Three added citations are wrong. I'd fix each once in the file that owns the contract and trim the copies, rather than propagating section numbers — the same contract restated at every site that touches it is a second copy to keep in sync.

Fix in place:

src/http.c:257-259 — §5.3 never mentions the fragment. The synthesized-slash rule is §5.3.1; the fragment rule is §5.1: "The target URI excludes the reference's fragment component, if any, since fragment identifiers are reserved for client-side processing."
src/http.c:104-106 — 3986 §3.2.2 governs the URL host; the Host header field is 7230 §5.4 (Host = uri-host [ ":" port ]). Name both, or drop "Host header" from the sentence.
src/ca_issue.c:98 — 5280 §4.2.1.3 only defines the bits, and the names are already in the string below. The MUST mandating this exact set, RSA conditional included, is RFC 8894 §2.1.2: "the keyUsage extension in the CA certificate MUST indicate that it is valid for digitalSignature and keyEncipherment (if the key is to be used for en/decryption) alongside the usual CA usages of keyCertSign and/or cRLSign."

(Recommendation) Trim the second copies:

src/server.c:222-223 — drop it. Sentence one restates the error string on the next line; sentence two justifies the local line by describing what tls_setup() does.
test_est_tls_roundtrip.c:74-76, test_server_ca_store.c:477-479, test_http.c:100-101 — each restates a contract owned elsewhere. One line saying what the test asserts.
test_http.c:731-733 — keep the local half ("both request builders carry their own copy of the bracketing, so drive each one"), cut the 3986 restatement.

The EST-requires-TLS contract should live once, in wolfcert/server.h:60, which declares both the field and the function — that's the only site that should carry a section number. CLI help text and README prose are user-facing and fine as they are.

New helpers

ca_key_buf_free() — src/store.c:446-447 and :466-467 already contain this function's exact body. As a file-local static in ca_issue.c neither can use it; beside wolfcert_buffer_free() in src/wolfcert.c, both could.
nb_rx_max() is only half adopted — the blocking reader at src/http.c:1123 still open-codes max_body + WOLFCERT_HTTP_HEADER_BUDGET. Two spellings of the quantity whose divergence is exactly what this commit is fixing. It can't call the helper as written since that takes a session rather than a size_t.
The five pub_check hooks — wolfSSL_X509_check_private_key() is exported, available under OPENSSL_EXTRA (which we hard-require), and its backing wc_CheckPrivateKey() covers RSA, ECC, Ed25519, Ed448 and ML-DSA 44/65/87 in one call. I'm not asking you to switch — wc_CheckPrivateKey itself isn't exported so it'd mean going through the compat layer with an X509 and an EVP_PKEY, and the Ed hooks do a load-bearing public-half import that a pure check wouldn't. But was it a considered choice? Worth a line in the commit message either way, since it's ~190 lines duplicating a maintained wolfSSL routine.
Minor: host_is_ip_literal() re-answers with strchr(':') what wolfcert_parse_ip() — used 150 lines away in the same file — answers directly, and the colon heuristic is correct only because of an invariant set in another function. listen_loopback6() is a structural copy of listen_loopback() in the same file. sleep_ms() is a third spelling of a nanosleep already open-coded in test_est_chunked_robustness.c and test_tls_http.c.

ca_check_stored_pair() is clean — it's the only CA-cert validation in the tree and follows the heap-allocated DecodedCert pattern from scep_client.c:591.

Non-blocking

WOLFCERT_SERVER_POLL_MS now means both shutdown cadence and per-connection socket timeout. Lower it for faster shutdown and every peer read becomes a 100 Hz retry spin. Separate macros, or document the coupling.
The WOLFSSL_MLDSA_CHECK_KEY #error in src/key_algs.c:498 belongs in check_config.h with the other resolved-feature validation, and the new requirement should reach CLAUDE.md's hard-fail list and the canonical configure line.
ecc_pub_check puts two 133-byte arrays on the stack unconditionally; house style is WOLFSSL_SMALL_STACK past ~100 bytes. (void)wc_ecc_make_pub(...) at :239 discards a return.
key_algs.h's pub_check note says an implementation "may adopt pub into key (ML-DSA)", but four of five mutate — Ed25519/Ed448 import the verified half (load-bearing) and ECC calls wc_ecc_make_pub(). The name and the note both undersell what the hook does on reload.
dst == &probe in nb_read_some uses pointer identity as a mode flag; a probing int would make the two checks it drives harder to miss.

tls_setup() treated a missing tls_cert_pem or tls_key_pem as a successful plaintext configuration for every protocol, and the accept loop dispatched straight to the handler whenever no TLS context was present. An EST server started without TLS material therefore served /cacerts, /csrattrs, /simpleenroll and /simplereenroll over cleartext HTTP, putting the HTTP Basic credentials and the CSR on the wire in the clear. RFC 7030 has no plaintext mode, and the client side already refused an http:// EST URL, so the server now refuses the same configuration with WOLFCERT_ERR_TLS before it binds. SCEP authenticates at the pkiMessage layer and keeps its plaintext transport. The two integration tests that scripted byte-exact HTTP at a plain socket now drive the server through a small raw TLS client added to tls_test_util.h, pinning the identity the test mints. One write is one TLS record and so one read on the server, which preserves the segment boundaries the chunked framing cases depend on. wolfcert-server rejects --proto est without --tls-cert and --tls-key up front, and the quick start in README.md and CLAUDE.md is updated to match. Fixes F-8031.

wolfcert_http_url_parse stores an IP-literal host with its brackets stripped, but wolfcert_http_url_origin re-emitted it bare, producing an origin such as https://::1:8443. The EST and SCEP session opens feed that string back through wolfcert_http_url_parse to build the session base URL, where the non-bracket host scan stops at the leading colon and yields a bogus host and port, so every session-based operation against an IPv6 literal server URL broke before the connect. The two request builders had the same defect, emitting a Host header of ::1:8443 rather than the bracketed form RFC 7230 section 5.4 requires. Re-add the brackets whenever the stored host carries a colon, which can only happen on the bracket-stripping parse branch, and cover the parse to origin to parse roundtrip in the URL unit test. Fixes F-8009.

wolfcert_ca_issue rebuilt the subject of the issued certificate field by field and only copied CN, O, OU, C, ST and L. Every other name component the CSR builder accepts was discarded without an error, so a CSR carrying for example UID and postalCode produced a certificate holding neither. Copy the remaining components wolfSSL exposes on the decoded CSR. The givenName copy is inert for now because wolfSSL's decoder only stores subject ids up to ASN_USER_ID and never fills subjectGN, but the field is in place for when that changes. The EST round-trip test now enrolls a CSR carrying the full set and requires each component to reappear in the issued certificate. Fixes F-8025.

wolfcert_server_start treated every wolfcert_ca_load failure as an empty store, so a transient read error or a corrupt ca.key.der made the server mint a fresh CA and overwrite the stored one, re-rooting every previously issued certificate. The wolfcert_ca_save return was discarded as well, so a store that could not be written still reported a successful start with a CA that only lived in RAM. Generate a new CA only when the load reports WOLFCERT_ERR_NOT_FOUND, propagate any other load error, and fail the start when saving a freshly generated CA does not succeed. A save that fails halfway is the same trap in miniature: wolfcert_ca_save writes the certificate before the key, so a failing key write left a certificate-only store, which the load path now rejects for good and no later start can repair. Remove the certificate again when the key does not follow, so the next start bootstraps into an empty store instead of a poisoned one. Fixes F-8026.

wolfcert_server_stop only set the stopping flag, which the accept loop checks while parked at poll() on the listener. Once a connection was accepted the flag was not looked at again until the keep-alive loop condition, so a peer that connected and then sent nothing left the serving thread blocked forever in wolfSSL_accept or in the protocol handler's first read, and a caller joining that thread after stop hung. Put a receive timeout on accepted sockets and treat its expiry as resumable: the handshake retries on WANT_READ and wolfcert_io_recv retries on WANT_READ or EAGAIN, both only while shutdown has not been requested. Shutdown latency on an established connection is now bounded by the same cadence as the listener poll, with no cross-thread fd manipulation and so no risk of acting on a recycled descriptor. Those timeouts are the whole of the bound, so a connection whose SO_RCVTIMEO or SO_SNDTIMEO cannot be installed is closed rather than served: without both the handler blocks unbounded again, and with only the send timeout missing a receive expiry would be read as an error and disconnect a healthy idle client. Each idle case in the test waits for the server to reach the blocking site before it stops -- the server's handshake flight, a completed handshake, a served GetCACaps -- so no case can pass with the server still parked at the listener poll. Fixes F-8027.

wolfcert_ca_load only checked that the stored private key could be decoded by one of the registered algorithms. It never parsed the stored certificate and never confirmed the certificate carried the public half of that key, so a store holding an unrelated pair started a server whose issued certificates and PKCS#7 replies were signed by a key the advertised CA certificate did not match, and whose SCEP requests could not be decrypted. The certificate is now parsed before either buffer is adopted, its key algorithm has to agree with the decoded private key, and a new pub_check entry in the key algorithm table compares the two public keys. The comparison lives behind the vtable because DecodedCert.publicKey is framed differently per algorithm: a bare RSAPublicKey for RSA, a full SubjectPublicKeyInfo for ECC, and raw public key bytes for Ed25519, Ed448 and ML-DSA. Every failure exit wipes the stored key DER before releasing it, as wolfcert_ca_free does for the copy it adopts, and the test's key-type list enumerates every enabled ML-DSA parameter set rather than only the first. The five per-algorithm pub_check hooks are not a reimplementation of wolfSSL_X509_check_private_key() by oversight. That call is exported and covers all five algorithms through wc_CheckPrivateKey(), but reaching it means building an X509 and an EVP_PKEY through the compatibility layer, and it is a pure check: the Ed25519, Ed448 and ML-DSA hooks here import the verified public half into the key, which a reloaded PKCS#8 v1 private key has no other way to obtain. Fixes F-9771.

nb_read_some always asked nb_rx_reserve for a full WOLFCERT_HTTP_READ_CHUNK, and nb_rx_reserve refuses any request that would take the accumulator past the caller's body cap plus the header budget. A response that fits inside that allowance but leaves less than one read quantum of headroom was therefore rejected with WOLFCERT_ERR_PROTOCOL before its final bytes were ever read. The blocking reader, which appends only the bytes it actually received, accepted the same response, so the two paths disagreed on which replies were legal. Compute the room the allowance still permits, clamp both the reserve and the read length to it, and raise the protocol error only once no room is left at all. Both the TLS and the transport read paths are clamped. A new unit test drives a non-blocking session against a reply sized to land exactly on the allowance and requires the body to arrive intact. The header budget itself was a bare 8192 repeated in both readers and now in the test, which only pins the boundary while the three agree, so it becomes WOLFCERT_HTTP_HEADER_BUDGET alongside the other overridable HTTP sizes in internal.h and is documented as a RAM knob. It stays separate from WOLFCERT_HTTP_READ_CHUNK: one is a header allowance, the other a read granularity, and tuning the read size must not move the largest response header block the client accepts. A full accumulator is not a protocol error by itself: an EOF-delimited body ending exactly on the allowance is complete, and the blocking reader accepts it. Fall back to a one-byte probe there, which takes the close as the end of the body and rejects only a peer that keeps sending. Fixes F-11049.

A server URL with no path but a query, such as http://ca.example?operation=GetCACaps, put the whole query into the host because the authority scan only stopped at a colon or a slash. The SCEP client builds exactly that shape whenever the configured server URL has no path, so GetCACaps and PKIOperation failed to resolve against valid servers. Terminate the authority at a question mark or a hash as well, and give the request target a leading slash when the remainder does not already start with one. That also covers an explicit port or a bracketed IPv6 literal followed directly by a query, where the port parse stopped at the query and left the target without its slash. Fixes F-12856.

The CA bootstrap path emitted a certificate with basicConstraints CA:TRUE and no keyUsage extension, so a relying party had nothing to check the CA against. The same key signs issued certificates and SCEP CertReps, and on RSA it also decrypts the pkcsPKIEnvelope, so assert keyCertSign, cRLSign and digitalSignature for every key type and keyEncipherment only for RSA. An ECC, Ed25519 or ML-DSA key cannot encipher a key, and RFC 8894 limits the envelope to RSA in any case. The test asserts the full set for every key type, including that keyEncipherment is absent from a non-RSA CA. Fixes F-8032.

wolfcert_ca_load() classified any result pair containing NOT_FOUND as an incomplete store, so a cert read that reported NOT_FOUND beside a key read that failed with I/O or memory error surfaced as WOLFCERT_ERR_PARSE. That hides an actionable store failure behind a parse error, and it reads the same either way round. A read that failed for a reason other than absence is now returned as itself; the store is called incomplete only once the other half actually read back. wolfcert_ca_save() writes the certificate before the key, and rolled the certificate back on a key-write failure only when the caller-supplied vtable happened to provide remove. An absent or failing remove left ca.cert.der committed without its key, which every later start now rejects as incomplete - and the caller was told nothing beyond the write error. The vtable has no rename or commit primitive, so removal is the only rollback available; when it cannot be performed the error now says the store is left incomplete and must be cleared before restart. wolfcert_server_start() re-wrapped that result with a generic "ca_store save failed", overwriting the diagnostic it had just recorded. The load path above it already avoids that for the same reason. test_mixed_read_failure drives both orders of the mixed pair plus the memory case; test_rollback_unavailable drives both a NULL remove and a failing one, and asserts the certificate really is still there and that the next start refuses it.

The accept loop arms SO_RCVTIMEO and SO_SNDTIMEO on every accepted connection, and the three retry loops around wolfSSL_accept(), wolfSSL_read() and wolfSSL_write() each treated one direction's expiry as resumable and the other's as fatal. TranslateIoReturnCode() maps a send-side EAGAIN or ETIMEDOUT to WOLFSSL_CBIO_ERR_WANT_WRITE, so: - wolfSSL_accept() continued on WANT_READ only, and a client whose receive window stalls the server's Certificate flight past the timeout had its handshake abandoned where a blocking socket would have waited. An ML-DSA chain is tens of kilobytes, so this is reachable. - wolfSSL_read() returns WANT_WRITE whenever the record layer must send first, which is what TLS 1.3 post-handshake auth and a key update do. - wolfSSL_write() had the mirror gap on WANT_READ. All three now resume on either direction through tls_want_io(). That alone would let a peer hold the handler indefinitely, because the loops observe the stopping flag only when an I/O call returns. A peer that keeps supplying data never lets a timeout expire, and the keep-alive loop checks the flag only between requests, so one trickled request header held shutdown for as long as the peer kept writing. Both helpers now check the flag before their first I/O. The two have to move together: a direction-agnostic retry without the stopping check is the trickling-peer hang, and the stopping check alone leaves the three one-directional loops. The idle-stop test could not tell a successful shutdown from a server that had already left wolfcert_server_run() on its own: stop_and_wait() polled a flag the serving thread could have set before stop() was called, and the run result was discarded, so a premature handshake or handler failure passed every case. ServerCtx carries run_rc, stop_and_wait() refuses a thread that has already returned, and each listener case asserts WOLFCERT_OK after the join. A fourth case drives a peer trickling an unterminated request header a byte at a time, sized to outlast the shutdown deadline several times over so a server that keeps consuming cannot pass by reaching the end of it.

wolfcert_csr_build() accepts GN= and encodes it into the CSR, but the issued certificate never carries it: wolfSSL's GetRDN() reaches a subject component only through certNameSubject[], which is indexed by id - 3 and runs out at id 22, so ASN_GIVEN_NAME (0x2a) never reaches SetSubject() and dc.subjectGN stays NULL. The COPY_SUBJ in wolfcert_ca_issue() has nothing to read, and the RDN is dropped without an error. Nothing in the library can close that on its own, so the behaviour is pinned at both ends instead: the test asserts the CSR really does carry 2.5.4.42 and that the issued certificate really does lose it. When wolfSSL decodes the component the second assertion fails, which is the signal to drop the workaround comment in src/ca_issue.c.

test_server_ca_store covers the CA store, which no protocol owns, but sat under WOLFCERT_ENABLE_SCEP. The CI matrix builds cmake-nonrsa-macos with SCEP off, where the whole file disappeared - including the only coverage of the Ed25519 and Ed448 reload path. test_server_stop_idle had the mirror problem: gated on EST while carrying a SCEP case. Both now build under WOLFCERT_ENABLE_SERVER alone, in CMake and automake. Neither test cares which protocol the listener speaks, so each picks one that was compiled in; EST has no plaintext mode, so the CA-store test mints a throwaway server identity for that variant. Verified by building with each protocol disabled in turn: 24/24 with SCEP off, 15/15 with EST off, both tests present and passing in each.

Frauschi · 2026-09-11T11:14:56Z

All five blocking items fixed; series is now 13 commits. Two corrections, both in your favour.

1. tls_want_io() resumes either direction on all three loops, folded into the stopping-check commit -- you were right they can't be resolved independently.

2. Both tests on WOLFCERT_ENABLE_SERVER, CMake and automake. Each picks a compiled-in protocol; the CA-store test mints a throwaway identity since EST has no plaintext mode. 24/24 with SCEP off, 15/15 with EST off.

3. street copied and added to the CSR builder's table. Encoding byte carried, tested with a PrintableString CSR built on wolfSSL. Over-long RDNs refused with WOLFCERT_ERR_BAD_ARG, driven through wolfcert_copy_csr_subject() directly since no wolfSSL-built CSR can carry one in the first place.

joiC/joiSt I can't fix -- they're decode-only: CertName carries them and wc_InitCert() sets their *Enc, but nameOid[] and GetOneCertName() have no entry for either. I generated a request with both set and found neither the value nor the JOI OID prefix in the DER, so a copy would never emit. One line records it.

givenName is worse than round 2 said, and my reply there was wrong about the mechanism. The id <= ASN_USER_ID gate in SetSubject() is real but never reached -- GetRDN() calls SetSubject() only when ValidCertNameSubject(id) finds a table entry, and certNameSubject[] is indexed id - 3 with 20 entries covering ids 3 through 22. The four WOLFSSL_CERT_NAME_ALL entries are commented "id 41/42/43/46" but physically sit at ids 19, 20, 21 and 22. So those RDNs never decode, and a certificate carrying 2.5.4.19-22 (physicalDeliveryOfficeName, telephoneNumber, teletexTerminalIdentifier, x121Address) matches the misplaced entries and decodes under the wrong name, into the wrong DecodedCert field and the wrong NID. Fixed on a wolfSSL branch by dropping the four entries and giving the ids their own branches beside ASN_X500_UNIQUE_ID; testwolfcrypt and tests/unit.test pass clean. Separate PR. On this side the drop stays pinned by a canary test that fails the day wolfSSL decodes it.

4. basicConstCrit set; ca_key_usage_set() now also requires the extension present, critical and asserting CA:TRUE.

5. Correct -- section 3.1 defines the PasswordRecipientInfo path. PR description reworded to "wolfCert's SCEP is RSA-only".

Citations and second copies. All three corrected in place; the four restated contracts cut to a line saying what the local code does, with EST-requires-TLS carrying its section number once on wolfcert/server.h.

Helpers. ca_key_buf_free() is now wolfcert_buffer_free_secure() beside wolfcert_buffer_free(), so store.c's two copies use it. nb_rx_max() is rx_max(size_t), which the blocking reader calls instead of open-coding it. host_is_ip_literal() uses wolfcert_parse_ip() directly. sleep_ms() is test_sleep_ms() in tls_test_util.h, replacing all three nanosleep spellings. listen_loopback6() I left alone: it differs in address family, sockaddr type and the setsockopt each needs, so folding costs more branching than the copy.

pub_check vs wolfSSL_X509_check_private_key() -- considered, and the commit message says so now. The blocker is that it's a pure check: the Ed25519, Ed448 and ML-DSA hooks import the verified public half into the key, which a reloaded PKCS#8 v1 private key has no other way to obtain. The compat-layer X509/EVP_PKEY round trip is the secondary cost.

Non-blocking, all five. WOLFCERT_SERVER_IO_TIMEOUT_MS split from the listener cadence. The ML-DSA #error moved to check_config.h -- it has to include dilithium.h, since WOLFSSL_MLDSA_CHECK_KEY is resolved there and not in options.h; CLAUDE.md's hard-fail list records the requirement. ecc_pub_check takes both buffers off the heap in one allocation and checks wc_ecc_make_pub(). The key_algs.h note now says four of five hooks mutate. dst == &probe is a named probing flag.

Rechecked the way you did: -Wall -Wextra -Wshadow -Wpedantic clean apart from the pre-existing -Woverlength-strings in cli/wolfcert_client.c, 29/29 ctest, all 13 commits building standalone, and leaks -atExit clean on the four tests touched.

Force-pushed, so the SHAs in the round-2 replies above are stale.

wolfSSL has no API to move a decoded subject into a Cert, so the CA rebuilds it RDN by RDN and any component without a copy is dropped silently. Three gaps, now split out into wolfcert_copy_csr_subject(): - street (2.5.4.9) is decoded and encodable, and was simply not copied. It is also added to the CSR builder's RDN table, which had no key for it. - The DirectoryString choice was not carried, so wc_InitCert()'s UTF8String default replaced whatever the requester used. A PrintableString RDN came back as UTF8String, which is a different subject to anything comparing encoded names. - An RDN longer than the fixed CertName field was truncated at 63 bytes with no error, issuing a certificate stating a subject the CSR did not ask for. It is now refused with WOLFCERT_ERR_BAD_ARG. Not fixed, because they cannot be: the jurisdiction RDNs are decode-only in wolfSSL - CertName carries joiC and joiSt, but the generator's nameOid[] table and GetOneCertName() have no entry for either, so a copy would never emit. Confirmed by generating a request with both set and finding neither the value nor the JOI OID prefix in the DER. A one-line note records it so the copy is not "restored" later. The over-long case is driven through wolfcert_copy_csr_subject() directly: no wolfSSL-built CSR can carry an RDN past CTC_NAME_SIZE, so there is no CSR to enroll. The encoding case goes end to end through a request built on wolfSSL with PrintableString set explicitly.

RFC 5280 section 4.2.1.9: "Conforming CAs MUST include this extension in all CA certificates that contain public keys used to validate digital signatures on certificates and MUST mark the extension as critical in such certificates." wolfSSL emits the extension whenever Cert.isCA is set but leaves it non-critical unless basicConstCrit is set too, which the generator never did, so a relying party configured to reject a non-critical basicConstraints on a CA would refuse the chain. ca_key_usage_set() now also requires the extension to be present, critical and asserting CA:TRUE, alongside the key usages it already checked. The keyUsage citation in both files pointed at RFC 5280 section 4.2.1.3, which only defines the bit names that the usage string already spells out. The MUST that mandates this exact set, RSA conditional included, is RFC 8894 section 2.1.2.

Three of the section numbers added in this series point at the wrong text: - http.c's fragment note cited RFC 7230 section 5.3, which never mentions the fragment. The synthesized leading slash is section 5.3.1; excluding the fragment from the target is section 5.1. - host_is_ip_literal() cited RFC 3986 section 3.2.2 for both the URL and the Host header. 3986 governs the URL host; the Host field is RFC 7230 section 5.4. - The CA keyUsage set cited RFC 5280 section 4.2.1.3, which only defines the bit names the usage string already spells out. The MUST mandating this set is RFC 8894 section 2.1.2. The same contract restated at every site that touches it is a second copy to keep in sync, so four of those copies are cut to a line saying what the local code does. The EST-requires-TLS contract now carries its section number once, on the field and function that declare it in wolfcert/server.h.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #25

Scan targets checked: wolfcert-src, wolfcert-bugs

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

💬 6 finding(s) from an earlier review are still open and were not re-posted:

CA mismatch cleanup leaves private-key DER in freed heap — src/ca_issue.c:323
Exact-cap EOF response is rejected before closure is read — src/http.c:1532
Socket-timeout installation failures leave broken serving modes — src/server.c:398
givenName is still silently dropped — src/ca_issue.c:753
Mixed CA-store failures return the wrong error — src/ca_issue.c:303
Failed CA rollback can poison the store — src/ca_issue.c:369

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Four quantities or routines had more than one spelling, and the divergence of two of them is what earlier commits in this series were fixing: - The response allowance (body cap plus header budget) was open-coded in the blocking reader and computed by nb_rx_max() in the non-blocking one. Now one rx_max(size_t) that both call. - ca_key_buf_free() in ca_issue.c was the exact body of the wipe-then-free already written twice in store.c, and being file-local neither could use it. Now wolfcert_buffer_free_secure(), beside wolfcert_buffer_free(). - host_is_ip_literal() answered "is this an IPv6 literal" with a colon search, correct only because of an invariant set in another function. wolfcert_parse_ip() answers it directly and is used 150 lines away. - nanosleep() was open-coded three times across the integration tests. Now test_sleep_ms() in tls_test_util.h, which all three include. listen_loopback()/listen_loopback6() are left alone: they differ in address family, sockaddr type and the setsockopt each needs, so folding them costs more branching than the copy.

- WOLFCERT_SERVER_POLL_MS meant both the listener's shutdown cadence and the per-connection socket timeout. Lowering it for faster shutdown also turned every peer read into a retry spin. The timeout is now WOLFCERT_SERVER_IO_TIMEOUT_MS, defaulting to the cadence. - The ML-DSA #error sat in key_algs.c rather than with the other resolved feature validation. Moved to check_config.h, which has to include dilithium.h for it: WOLFSSL_MLDSA_CHECK_KEY is resolved there, not in options.h. CLAUDE.md's hard-fail list and configure notes record it. - ecc_pub_check() put two MAX_ECC_BYTES-derived arrays on the stack unconditionally and discarded wc_ecc_make_pub()'s return. Both buffers come off the heap in one allocation, and a failed derivation is an error. - key_algs.h said pub_check "may adopt pub into key (ML-DSA)". Four of the five mutate: Ed25519 and Ed448 import the verified public half and ECC derives one. The note now says so, since it is what makes freeing a failed key load-bearing. - nb_read_some() used dst == &probe as a mode flag in two places. A named `probing` int says it once.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #25

Scan targets checked: none
Unchanged since last review (not re-run): wolfcert-src, wolfcert-bugs

No new issues found in the changed files.

💬 7 finding(s) from an earlier review are still open and were not re-posted:

CA mismatch cleanup leaves private-key DER in freed heap — src/ca_issue.c:323
Exact-cap EOF response is rejected before closure is read — src/http.c:1532
Socket-timeout installation failures leave broken serving modes — src/server.c:398
givenName is still silently dropped — src/ca_issue.c:753
Mixed CA-store failures return the wrong error — src/ca_issue.c:303
Failed CA rollback can poison the store — src/ca_issue.c:369
Secure buffer cleanup truncates lengths above 4 GiB — src/wolfcert.c:66

This review was generated automatically by Fenrir. Reported findings require changes before merge.

wolfSSL-Fenrir-bot

Fenrir Automated Review — PR #25

Fenrir already completed a review of this PR at commit 54cd99fa0234 (run 2473); its findings are the review threads on the PR. Push new commits to get a re-review of what changed, or comment @wolfSSL-Fenrir-bot review force to run the full review again at this commit.

Frauschi self-assigned this Sep 10, 2026

Frauschi force-pushed the fenrir branch from 1e4ab7d to 45e81b1 Compare September 10, 2026 17:05

Frauschi assigned wolfSSL-Bot Sep 10, 2026

Frauschi requested a review from yosuke-wolfssl September 10, 2026 17:54

Frauschi removed their assignment Sep 10, 2026

Frauschi requested a review from wolfSSL-Fenrir-bot September 10, 2026 17:54

wolfSSL-Fenrir-bot requested changes Sep 10, 2026

View reviewed changes

Frauschi force-pushed the fenrir branch from 45e81b1 to 1637e97 Compare September 11, 2026 06:18

Frauschi requested a review from wolfSSL-Fenrir-bot September 11, 2026 06:21

wolfSSL-Fenrir-bot requested changes Sep 11, 2026

View reviewed changes

Comment thread src/ca_issue.c Outdated

Comment thread src/ca_issue.c Outdated

Comment thread src/ca_issue.c Outdated

Comment thread src/server.c

Comment thread tests/integration/test_server_stop_idle.c Outdated

yosuke-wolfssl requested changes Sep 11, 2026

View reviewed changes

yosuke-wolfssl assigned Frauschi and unassigned wolfSSL-Bot Sep 11, 2026

Frauschi added 13 commits September 11, 2026 10:27

Frauschi force-pushed the fenrir branch from 1637e97 to 6ba2760 Compare September 11, 2026 11:14

Frauschi added 2 commits September 11, 2026 13:48

Frauschi force-pushed the fenrir branch from 6ba2760 to 3bbe595 Compare September 11, 2026 11:51

Frauschi requested a review from wolfSSL-Fenrir-bot September 11, 2026 11:55

wolfSSL-Fenrir-bot requested changes Sep 11, 2026

View reviewed changes

Comment thread src/wolfcert.c Outdated

Frauschi added 2 commits September 11, 2026 15:32

Frauschi force-pushed the fenrir branch from 3bbe595 to 54cd99f Compare September 11, 2026 13:33

Frauschi requested a review from wolfSSL-Fenrir-bot September 11, 2026 13:40

wolfSSL-Fenrir-bot requested changes Sep 11, 2026

View reviewed changes

Frauschi requested a review from wolfSSL-Fenrir-bot September 11, 2026 13:48

wolfSSL-Fenrir-bot reviewed Sep 11, 2026

View reviewed changes

Frauschi requested a review from yosuke-wolfssl September 11, 2026 13:48

Frauschi assigned yosuke-wolfssl Sep 11, 2026

Uh oh!

Conversation

Frauschi commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Findings fixed

Behavioural change

Tests

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #25

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #25

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yosuke-wolfssl left a comment

Choose a reason for hiding this comment

Blocking

1. The TLS retry loops each resume only one direction

2. The new tests are gated on the wrong feature

3. Three more RDNs are dropped, and the copied ones can still truncate

4. basicConstraints is not marked critical

5. "RFC 8894 limits that envelope to RSA" (PR description)

Comments

New helpers

Non-blocking

Uh oh!

Frauschi commented Sep 11, 2026

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #25

Uh oh!

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #25

Uh oh!

wolfSSL-Fenrir-bot left a comment

Choose a reason for hiding this comment

Fenrir Automated Review — PR #25

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Frauschi commented Sep 10, 2026 •

edited

Loading