SCEP gap-filling#7
Conversation
The renewal entry points took a new_key argument that was ignored ((void)new_key): the renewed public key is already carried in the CSR and the pkiMessage is signed with current_key. Remove the dead parameter from wolfcert_scep_renewal_req and _renewal_req_ex. Also parse the RFC 8894 section 3.2.1.4 failInfo attribute out of a FAILURE CertRep and populate WolfCertScepResult.fail_info, which was previously always left at -1. wolfcert_scep_parse_pki_message gains an out_fail_info output; all non-consuming callers pass NULL. The SCEP poll roundtrip test now asserts the client surfaces failInfo=4 (badCertId) for an unknown transaction.
Add a public helper to check a GetCACert response against a fingerprint obtained out of band before it is trusted as a CA anchor. It hashes the whole DER certificate with SHA-256 (or SHA-1 / SHA-512) and compares in constant time; WOLFCERT_SCEP_FP_AUTO selects the digest from the expected length. Returns WOLFCERT_ERR_AUTH on mismatch, WOLFCERT_ERR_UNSUPPORTED when the digest is not compiled into wolfSSL. MD5 is intentionally not offered. Unit-tested in test_scep_msg for match, single-bit tamper, AUTO dispatch, and length/argument misuse.
When the passed caps show the CA does not advertise POSTPKIOperation, carry the pkiMessage base64-encoded and percent-escaped in a GET `message` query parameter instead of POSTing it. run_pki_op chooses POST (the default, and whenever caps are unknown) or GET from the caps; a GET URL longer than WOLFCERT_SCEP_MAX_GET_URL (8 KiB) is refused with WOLFCERT_ERR_UNSUPPORTED so the caller falls back to a POST-capable CA. The in-tree test server learns to accept GET PKIOperation: it decodes the message parameter and dispatches it exactly like a POST body. Its query field gets a dedicated WOLFCERT_HTTP_QUERY_SZ (8 KiB) so the encoded message fits, and the client HTTP path ceiling is raised to match. Tests: test_scep_roundtrip enrolls once over the GET path end to end; test_scep_msg unit-tests URL construction and the oversize rejection. docs/ARCHITECTURE.md and the docs/EMBEDDED.md tunables table updated.
wolfSSL now exposes wc_ConstantCompare as a public API (alongside wc_ForceZero). Replace the three hand-rolled XOR-accumulate constant-time comparisons with it: - the CA fingerprint check (scep_client.c, dropping the local ct_diff), - the challenge-password check (scep_server.c), and - the HTTP Basic-auth credential check (est_server.c). Each keeps its existing length-mismatch guard, and wc_ConstantCompare returns 0 iff equal, so behaviour is unchanged.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #7
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.
| uint8_t* ca_der = NULL; | ||
| uint8_t* key_der = NULL; | ||
| size_t ca_len = 0, key_len = 0; | ||
| REQUIRE(make_ca(&ca_der, &ca_len, &key_der, &key_len) == 0); |
There was a problem hiding this comment.
🔵 [Low] Resource leak in test_ca_fingerprint on REQUIRE failure · Resource leaks on error paths
ca_der and key_der allocated by make_ca (via malloc) are only freed at the end of the function; any REQUIRE that fires between line 735 and the free calls exits via return 1, leaking both buffers.
Fix: Add free(ca_der); free(key_der); before every early-return REQUIRE, or use a single goto cleanup label.
| REQUIRE(wolfcert_scep_build_pki_get_url(base, small, sizeof(small), NULL, &url) | ||
| == WOLFCERT_OK); | ||
| REQUIRE(url != NULL); | ||
| REQUIRE(strncmp(url, pfx, strlen(pfx)) == 0); |
There was a problem hiding this comment.
🔵 [Low] Resource leak of url in test_pki_get_url on assertion failure · Resource leaks on error paths
url is heap-allocated by wolfcert_scep_build_pki_get_url; if the strncmp or strlen REQUIRE on lines 801-802 fails, execution exits via return 1 before WOLFCERT_XFREE(url, NULL) on line 803, leaking the allocation.
Fix: Move WOLFCERT_XFREE(url, NULL) before the failing REQUIREs, or free it unconditionally on a cleanup label.
2 participants
SCEP (RFC 8894) gap-filling: GET PKIOperation, CA-fingerprint bootstrap, failInfo, and hardening
Summary
This branch closes several RFC 8894 (SCEP) conformance and robustness gaps in the client and the in-tree test server, and finishes with a small cross-cutting cleanup. It adds the HTTP GET
PKIOperationfallback, an out-of-band CA-fingerprint trust-bootstrap helper, surfaces the CertRep failInfo code, and moves the server's request handling off the stack. No functional behaviour of the existing POST-based flow changes.Four self-contained commits, each green on its own:
scep: surface CertRep failInfo and drop unused renewal new_key paramscep: add wolfcert_scep_verify_ca_fingerprint trust-bootstrap helperscep: add base64 HTTP GET PKIOperation fallback (RFC 8894 4.1)scep,est: use wc_ConstantCompare for constant-time comparisonsWhat's included
1. CertRep
failInfosurfaced; renewal API cleanupfailInfoattribute from a FAILURECertRepand exposes the code (0–4) instead of discarding it, so a caller can tell why an enrollment was rejected.new_keyparameter fromwolfcert_scep_renewal_req[_ex](it was(void)new_key;— the renewed key already travels inside the CSR).2. CA-fingerprint trust-bootstrap helper
wolfcert_scep_verify_ca_fingerprint()verifies a downloaded CA/RA certificate against a fingerprint obtained out of band — the standard SCEP trust-on-first-use bootstrap.WOLFCERT_SCEP_FP_AUTOselects the algorithm from the fingerprint length (20 / 32 / 64).3. HTTP GET
PKIOperationfallback (RFC 8894 §4.1) + server hardeningPOSTPKIOperation, the client carries the base64/percent-encodedpkiMessagein a GET query, capped byWOLFCERT_SCEP_MAX_GET_URL.PKIOperationand dispatches it exactly like a POST.path/querypoint into it, dropping peak request-handling stack from ~27 KB to under 1 KB (the GET message no longer lands on the stack). Buffer sizes are unchanged; only their storage moved. The EST server and client are untouched.Content-Lengthbody no longer leaks the body buffer when the decodedpkiMessageis installed.operation=/message=are matched as whole parameters (start-of-query or after&) rather than as substrings.500, matching its sibling error branches.4.
wc_ConstantCompareadoptionwc_ConstantCompareas a public API (alongsidewc_ForceZero). Replaces the three hand-rolled XOR-accumulate constant-time comparisons (CA fingerprint, challenge password, HTTP Basic-auth) with it. Net −16 lines; behaviour is identical (returns 0 iff equal).Public API changes
wolfcert_scep_verify_ca_fingerprint()+WolfCertScepFpAlgenumfailInfocode surfaced on the response statusnew_keyparameter removed fromwolfcert_scep_renewal_req[_ex]Build requirement
This adds one implicit dependency: wolfCert now calls
wc_ConstantCompare, so the linked wolfSSL must provide it (i.e. not built withWOLFSSL_NO_CONST_CMP). This mirrors the assumption already made forwc_ForceZero, so no new configure/check_config.hgate was added.Testing
ctest), both CMake and autoconf.message=, malformed percent-escape, invalid base64) driven over a raw socket; the GET-with-body leak-prevention path; SHA-512 fingerprint (explicit + AUTO).Notes for reviewers
roundtrip; the allocation uses theheaphint, so static-memory pools are still honoured.docs/EMBEDDED.mdand theinternal.hbuffer comments were updated to reflect the SCEP request buffer moving to the heap.