More Fenrir fixes

[Frauschi]

Problem

A batch of Fenrir findings against the SCEP client and test server, the server's bind and TLS configuration, and the two CLIs:

• Unenveloped GetCertInitial (F-11063): the test server ignored a de-enveloping failure for messageType 20 and matched the pending entry by transactionID alone, so a signed poll with absent or garbage content could drain a parked request and trigger issuance.
• Issuance failure reply (F-12110): a CSR with a broken self-signature is only caught inside wolfcert_ca_issue, and that branch answered with a plain HTTP 400. The client got no signed CertRep, transactionID, recipientNonce or failInfo, and saw a PKI rejection as a transport error.
• AES-128-CBC not enforced (F-12894): RFC 8894 section 2.9 makes AES128-CBC mandatory-to-implement, but a wolfSSL built with NO_AES_128, NO_AES_CBC or NO_AES_DECRYPT still configured with SCEP on. The client then refused an explicit AES-128 request and in AUTO mode sent 3DES to a CA advertising AES, and the server silently dropped to 3DES.
• transactionID encoding (F-12895): the GetCertInitial entry points put any caller-supplied transactionID on the wire tagged as a PrintableString, including _, control bytes and NUL.
• senderNonce length (F-12896): the server only checked that a senderNonce was present, then echoed it back as the recipientNonce, so an 8- or 17-byte nonce was processed, possibly issued, and answered with a malformed CertRep.
• bind_host fallback (F-9813): wolfcert_server_start fell back to INADDR_ANY whenever inet_pton rejected bind_host, so --listen localhost:8080, an IPv6 literal or a typo bound every IPv4 interface.
• PHA outside EST (F-11068): with tls_post_handshake_auth set, the handshake completes without a client certificate and only the EST enroll handler requests one later. A SCEP server with a client CA and PHA on therefore served clients that never presented a certificate.
• Secrets in argv (F-12911, F-12912): wolfcert-server kept the Basic password and --challenge, and wolfcert-client kept --pass and --challenge, in argv for the life of the process, readable through ps or /proc/<pid>/cmdline.

Fix

• scep: require a pkcsPKIEnvelope on GetCertInitial: every PKIOperation whose envelope does not decrypt to the CA now takes the signed FAILURE CertRep path, polls included.
• scep: answer a CA issuance failure with a CertRep FAILURE: the server replies with pkiStatus FAILURE and failInfo badRequest and closes the connection, for both PKCSReq/RenewalReq and the first poll of a parked request.
• scep: require AES-128-CBC for SCEP builds: check_config.h, configure.ac and CMake all reject such a wolfSSL when SCEP is enabled. The AES-128 guards in the client cipher selection and the server's 3DES branch are gone. The client's 3DES fallback for a CA that does not advertise AES is unchanged.
• scep: reject a non-PrintableString transactionID on GetCertInitial: scep_build_txid, which the one-shot and both session paths share, validates an inherited ID and returns WOLFCERT_ERR_BAD_ARG before any network I/O. Generated IDs are hex and unaffected.
• scep: reject a senderNonce that is not 16 bytes: handle_pki_op answers any other length with HTTP 400, ahead of de-enveloping, issuance and any pending-queue change.
• Reject a bind_host that is not a numeric IPv4 address: bind_host is parsed before any allocation, so a bad address also no longer generates and persists a CA before the start fails. Only a NULL bind_host binds all interfaces. The unused cfg_bind_host copy is gone, and the --listen help says ADDR:PORT.
• Reject post-handshake auth for non-EST server protocols: wolfcert_server_start returns WOLFCERT_ERR_BAD_ARG for PHA with any protocol other than EST.
• Scrub server CLI secrets from argv after parsing: parse_basic zeroes the password half of its argument after copying it, and --challenge is copied before its optarg is zeroed. Both copies are wiped before they are freed, including when a flag is repeated, and a failed copy of --challenge exits instead of starting the server without the challenge check.
• Scrub client CLI credentials from argv after parsing: a new opt_secret() helper copies --pass and --challenge, zeroes the argv original and wipes any earlier copy on a repeat. opts_free() wipes and frees both.

Closes F-9813, F-11063, F-11068, F-12110, F-12894, F-12895, F-12896, F-12911, F-12912.

Behaviour changes for callers

• A wolfSSL built with NO_AES_128, NO_AES_CBC or NO_AES_DECRYPT now fails configure and the check_config.h gate when SCEP is enabled. Build with --disable-scep (CMake -DWOLFCERT_ENABLE_SCEP=OFF) to keep using such a wolfSSL.
• WolfCertServerCfgSrv.bind_host must be NULL or a numeric IPv4 address. A hostname or IPv6 literal now fails with WOLFCERT_ERR_BAD_ARG instead of binding every interface, and so does wolfcert-server --listen localhost:PORT.
• tls_post_handshake_auth is accepted only with WOLFCERT_PROTO_EST.
• wolfcert_scep_get_cert_initial() and the session GetCertInitial calls return WOLFCERT_ERR_BAD_ARG for a transactionID outside the PrintableString repertoire.
• The SCEP test server rejects a senderNonce that is not 16 bytes, and answers a failed issuance with a FAILURE CertRep rather than HTTP 400.

Tests

• scep_poll_roundtrip: an unenveloped and a malformed GetCertInitial for a pending transaction both get FAILURE/badRequest, and the real poll still succeeds afterwards. The long-ID poll case now fills with A instead of 0x11, and new cases refuse a transactionID holding _, 0x11 or NUL on the one-shot API.
• scep_roundtrip: a CSR with one signature bit flipped gets a FAILURE CertRep. check_required_attrs posts an 8-byte and a 17-byte senderNonce and expects both to be rejected.
• scep_async_roundtrip: the long-ID case fills with A instead of 0x11, and a transactionID holding _ is refused on the blocking session API.
• server_ca_store unit: localhost, ::1, an empty string, 127.0.0.300 and an address with a trailing space are each refused with WOLFCERT_ERR_BAD_ARG.
• est_pha_roundtrip: a SCEP server with PHA enabled fails to start.
• cli_proto_scoping.sh: a server started with --basic and --challenge, and a client parked on a --trust FIFO after option parsing, show neither secret in ps output.

[Copilot]

Copilot review overview

🟢 Approval recommended

The security fixes are coherent across implementation, configuration, documentation, and regression tests, with no unresolved correctness issues found.

Review effort: Balanced
Findings: None

What changed in this PR

This PR addresses Fenrir findings across SCEP validation/error handling, server configuration, and CLI credential hygiene.

Changes:

• Enforces valid SCEP envelopes, attributes, transaction IDs, and AES-128-CBC support.
• Rejects invalid bind addresses and non-EST post-handshake authentication.
• Scrubs CLI secrets from process arguments and adds regression coverage.

File	Description
wolfcert/​server.h	Documents bind-address and PHA restrictions.
wolfcert/​scep.h	Documents transaction ID validation.
wolfcert/​check_config.h	Enforces AES-128-CBC for SCEP builds.
tests/​unit/​test_server_ca_store.c	Tests invalid bind hosts.
tests/​integration/​test_scep_roundtrip.c	Tests nonce lengths and issuance failures.
tests/​integration/​test_scep_poll_roundtrip.c	Tests malformed polls and transaction IDs.
tests/​integration/​test_scep_async_roundtrip.c	Tests session transaction ID validation.
tests/​integration/​test_est_pha_roundtrip.c	Tests PHA protocol scoping.
tests/​integration/​cli_proto_scoping.sh	Tests argv secret scrubbing.
tests/​CMakeLists.txt	Adds internal headers to the poll test.
src/​server.c	Validates bind hosts and PHA usage.
src/​scep/​scep_server.c	Tightens request validation and signed failure replies.
src/​scep/​scep_client.c	Validates transaction IDs and requires AES support.
src/​internal.h	Removes the obsolete copied bind host.
Makefile.am	Adds the poll test's internal include path.
docs/​ARCHITECTURE.md	Documents EST-only PHA behavior.
configure.ac	Adds the Autoconf AES capability gate.
CMakeLists.txt	Adds the CMake AES capability gate.
cli/​wolfcert_server.c	Scrubs server secrets and clarifies listen syntax.
cli/​wolfcert_client.c	Copies, scrubs, and securely frees client secrets.
CLAUDE.md	Documents the SCEP AES requirement.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Frauschi]

@wolfSSL-Fenrir-bot review balanced

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #46

Scan targets checked: wolfcert-src, wolfcert-bugs
Coverage: 9 of 12 in-scope changed file(s) opened by the reviewer; not opened: tests/unit/test_server_ca_store.c, wolfcert/scep.h, wolfcert/server.h

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: Balanced

[yosuke-wolfssl]

Most of the fixes look right to me. The branch builds without warnings, and ctest passes 30/30 on macOS.

Main points, inline below:

• GetCertInitial (F-11063): a poll still finds its parked request by transactionID alone. Requiring the envelope stops a poll with absent or garbage content, but not a valid envelope signed by a different key.
• parse_basic(): a failed copy starts the server with Basic auth weakened or off. --challenge already exits on the same failure.
• PrintableString: the new check covers one client path; enc_printable_n() writes the 0x13 tag for every attribute.
• Issuance failure on a poll: the case the commit message names is untested, and SCEP parks a CSR without checking its signature.
• AES-128-CBC requirement: no negative-configure case, the user-facing docs do not mention it, and WOLFCERT_NO_WOLFSSL_FEATURE_CHECK now skips it with no fallback.
• bind_host test: it cannot see the CA-persist ordering the commit message describes.
• Nits: a bare 16, a stale test comment, a long line.

Not on diff lines:

• The HAVE_AES_CBC / WOLFSSL_AES_128 guards in tests/integration/test_scep_roundtrip.c (lines 424, 1067, 1419, 1510, 1586, 1638) and tests/unit/test_scep_msg.c (1954) are now always true in a SCEP build. So is the reason given at test_scep_roundtrip.c:422, "only exist when wolfSSL can supply one".
• wolfcert_server_free() frees cfg_basic_pass and cfg_challenge without wiping them. With the CLIs now wiping their own copies, those are the copies left in the heap.
• wolfcert-server reports a rejected --listen host only as "start failed (Bad argument)"; the bind_host message reaches stderr only with WOLFCERT_LOG set. --listen localhost:PORT used to start, so the error deserves a hint.
• The --tls-post-handshake-auth help text still lists only the TLS requirements, not --proto est.
• Test cases worth adding:
  • a GetCertInitial with a valid envelope, signed by a different key
  • a bad-signature CSR parked on the approval server, then polled
  • a transactionID using the allowed punctuation (-, :, .), and one with @ or a byte above 0x7F
  • a start with bind_host = NULL

[yosuke-wolfssl]

This stops a poll whose content is absent or is not an envelope. handle_get_cert_initial() still finds the entry with pending_find(p, tid, tid_len) and never looks at the poll's signer, and anyone can build a valid envelope because the CA certificate is public.

I checked it on this branch. After the PKCSReq in poll_path(), a wolfcert_scep_get_cert_initial() with r1.transaction_id, a fresh RSA key and that key's own CSR drains the entry and issues. The caller cannot decrypt the reply, which is enveloped to the stored signer, but the real poll at test_scep_poll_roundtrip.c:203 then gets badCertId.

handle_enroll() already has the rule. Passing signer_cert into handle_get_cert_initial() and checking it before pending_remove() would tie the poll to its request:

    if (signer_cert == NULL ||
            signer_matches_csr(signer_cert, signer_cert_len,
                               e->csr_der, e->csr_len, s->heap) != WOLFCERT_OK)
        return send_pki_failure(s, fd, tid, tid_len, snonce, snonce_len,
                                "4" /* badCertId */);

On a mismatch the entry stays queued. That poll would make a good third case next to the two unenveloped ones.

[Frauschi]

Fixed in c0f64f6: the poll's signer must match the parked CSR's key (signer_matches_csr()), else badCertId and the entry stays. Your foreign-key poll is now a test case.

[yosuke-wolfssl]

If either copy fails, the server still starts. A NULL user makes check_basic_auth() admit every request, and a NULL pass makes user: the accepted credential. The --challenge case exits on the same failure, so the two now disagree:

    *user = strndup(arg, (size_t)(colon - arg));
    *pass = strdup(colon + 1);
    wc_ForceZero(colon + 1, (word32)strlen(colon + 1));
    if (*user == NULL || *pass == NULL)
        return -1;

[Frauschi]

Fixed in f11ae82: a failed copy of either half exits, like --challenge.

[yosuke-wolfssl]

Nothing in wolfCert checked the PrintableString characters before this, and wolfSSL has no public helper for it, so the check is needed. But it guards one caller, and enc_printable_n() in src/scep/scep_msg.c is where every 0x13 attribute is written. Checking there would also cover the test server, which copies the request's transactionID into its CertRep unchecked. wolfcert_scep_build_pki_message() runs before the POST, so the rejection would still come before any network I/O, as long as it maps to WOLFCERT_ERR_BAD_ARG.

The CSR subject has the same gap, outside this PR: wolfcert_csr_build() with C=U_,serialNumber=ab_c@1 writes both as PrintableString. wolfSSL defaults countryEnc and serialDevEnc to CTC_PRINTABLE, and parse_subject_dn() checks only length. One helper in src/internal.h could serve both.

[Frauschi]

Moved in d96b045: enc_printable_n() now checks every 0x13 attribute via wolfcert_is_printable_string() in internal.c, returning WOLFCERT_ERR_BAD_ARG before the POST. The test server rejects a bad transactionID with 400. Added the punctuation, @ and 0x80 cases. CSR subject is noted for a follow-up; the helper is ready for it.

[yosuke-wolfssl]

Two things on the poll path:

• Untested. check_bad_csr_sig() runs against the server without approval, so it reaches this branch only through handle_enroll(). A bad-signature CSR parked on the approval server in test_scep_poll_roundtrip.c, then polled, would cover the case the commit message names. The first poll should get FAILURE/badRequest, and a second poll badCertId.
• Parked without checking. That case exists because handle_enroll() parks a CSR without checking its signature, so the client is told PENDING for a request that cannot succeed. Various hardening and fixes #45 makes the EST server verify a CSR before parking it. Doing the same here would refuse it on the PKCSReq instead.

[Frauschi]

Went with verifying before parking (5d8a236): a bad-signature CSR now gets FAILURE/badRequest on the PKCSReq. The poll test checks that, plus badCertId on a later poll since nothing was parked.

[yosuke-wolfssl]

With no store configured, this checks only the return code. If the inet_pton() check moved back below CA generation, this test would still pass. The commit message's claim, that a bad address no longer generates and persists a CA, needs a store here and a check that nothing was written to it.

Every server in the suite sets bind_host, so the NULL branch, now the only way to bind all interfaces, is never run. One start with bind_host = NULL would cover it.

[Frauschi]

Done in 05403a4: bad addresses now run against a memory store and assert nothing was written (verified by moving the check below CA generation), plus a bind_host = NULL start.

[yosuke-wolfssl]

Nit: scep_client.c names this SCEP_NONCE_SZ. Moving that into src/internal.h and using it here would tie the server's check to the size the client generates.

[Frauschi]

Done, SCEP_NONCE_SZ is in internal.h now (3f4f883).

[yosuke-wolfssl]

Rounds 6 and 7 send every attribute, with a senderNonce of the wrong length, so "omits one required attribute" no longer holds. Perhaps:

    /* Each round omits or mis-sizes one required attribute; the last is the
     * control that proves this raw-POST harness reaches the issuance path. */

[Frauschi]

Took your wording.

[yosuke-wolfssl]

• No negative-configure case. scripts/ci/assert-configure-fails.sh has no-rsa-scep for the RSA rule. A matching row for a wolfSSL built with -DNO_AES_128 would show that both build systems keep rejecting it.
• Docs. Only CLAUDE.md records the requirement. docs/EMBEDDED.md:38 still lists just "AES", and neither examples/user_settings.h.example nor the WOLFCERT_SCEP help in zephyr/Kconfig mentions it. Embedded builds are the ones most likely to drop AES-128.
• Opt-out. This #error sits inside the WOLFCERT_NO_WOLFSSL_FEATURE_CHECK block, and the source guards it replaces are gone. An opted-out build without AES-128 now compiles; the client then fails inside the PKCS#7 encoder, and the test server advertises AES and SCEPStandard. Is that intended, or should this check sit outside the opt-out like the RSA rule at line 71?

[Frauschi]

Added the no-aes128-scep negative case (both build systems reject it) and documented the requirement in EMBEDDED.md, user_settings.h.example and the Zephyr Kconfig (b1f26d8). The opt-out is intended: it behaves like the NO_AES check above it, and in Tier 1 it would misfire whenever wolfSSL's options are unresolved.

[yosuke-wolfssl]

Nit: this line runs to about 105 columns, where the rest of the header wraps at 80. docs/ARCHITECTURE.md:169 has the same issue.

[Frauschi]

Wrapped, along with ARCHITECTURE.md (fbc2f24).

[Frauschi]

Thanks! All addressed and folded into the owning commits. Off-diff items: AES test guards removed (b1f26d8), wolfcert_server_free() wipes the secrets (f11ae82), a failed --listen now prints the reason (05403a4), PHA help mentions --proto est (fbc2f24).