scep: send a CertRep from every pkiMessage rejection path - #23
scep: send a CertRep from every pkiMessage rejection path#23yosuke-wolfssl wants to merge 1 commit into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Updates the SCEP server to return signed CertRep failure responses for malformed authenticated messages and adds integration tests.
Changes:
- Centralizes failure CertRep generation.
- Covers missing, undecryptable, and unknown message types.
- Adds malformed-message round-trip coverage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Summary |
|---|---|
| src/scep/scep_server.c | Implements shared signed failure responses. Nit (3 votes): clear keep_alive before the fallback response when no transaction ID is available. |
| tests/integration/test_scep_roundtrip.c | Tests malformed SCEP dispatch responses. |
src/scep/scep_server.c:664
- The parser does not require a senderNonce, so a signed request can reach this helper with a valid transactionID but snonce == NULL/zero length. This still emits an HTTP 200 CertRep without recipientNonce (the builder omits it when the pointer is NULL), which violates RFC 8894 §3.2.1 and cannot be matched by a client. Treat a missing senderNonce like a missing transactionID and use the HTTP 400 fallback, or reject it before building the CertRep.
if (tid == NULL || tid_len == 0) {
send_text(s, fd, 400, "Bad Request", "text/plain", "");
return WOLFCERT_ERR_PROTOCOL;
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #23
Scan targets checked: wolfcert-bugs, wolfcert-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Sorry, something went wrong.
- send_pki_failure() answers with a signed CertRep carrying pkiStatus 2 and a failInfo, built from the request's transactionID and senderNonce; a request without a transactionID gets a plain HTTP 400 instead. - handle_pki_op calls it for a missing messageType, an undecryptable pkcsPKIEnvelope and an unrecognized messageType, with failInfo 2, 0 and 2; the messageType branches return WOLFCERT_ERR_PROTOCOL, and a new send_rc keeps the de-envelop code in rc unless the send fails. - handle_enroll's three rejections and handle_get_cert_initial's unknown transactionID call it too; handle_get_cert_initial drops its signer certificate parameters and the env_target locals. - test_scep_roundtrip's check_malformed_dispatch POSTs pkiMessages covering the three branches and requires a CertRep with pkiStatus 2, the failInfo above, the echoed transactionID and senderNonce, and no enveloped messageData. Issue: F-8033
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #23
Scan targets checked: wolfcert-bugs, wolfcert-src
No new issues found in the changed files. ✅
Sorry, something went wrong.
Problem
handle_pki_op() answered three malformed-message cases with a bare
text/plain HTTP 400 instead of a pkiMessage: a missing messageType
attribute, a pkcsPKIEnvelope that fails to decrypt for messageType
19/17, and an unrecognized messageType. All three occur after
wolfcert_scep_parse_pki_message() has verified the CMS signature and
recovered the transactionID and senderNonce, so RFC 8894 section 3.2.1
requires a signed CertRep with pkiStatus FAILURE and a failInfo. A
conforming SCEP client maps any non-200 to a generic transport error and
never surfaces the failInfo. Closes f-8033.
Fix (src/scep/scep_server.c)
New send_pki_failure() builds the CertRep from the transactionID and
senderNonce already in scope, and falls back to an HTTP 400 only when the
request carried no transactionID to echo.
Each clears keep_alive first, so the emitted Connection: header
matches the socket teardown that the non-OK return triggers. The four
pre-existing pkiStatus 2 sites in handle_enroll() and
handle_get_cert_initial() now share the same helper, which lets
handle_get_cert_initial() drop its signer-certificate parameters.
badRequest is used throughout: wolfcert_scep_deenvelop() cannot
distinguish an unsupported cipher from a corrupt or misaddressed
envelope, so the generic code is the honest one.
Tests
check_malformed_dispatch() in tests/integration/test_scep_roundtrip.c
POSTs three hand-built signed pkiMessages — no messageType, 19 over a
payload that is not an EnvelopedData, and 99 over a real envelope — and
requires each reply to parse as a CertRep echoing the transactionID and
senderNonce, with pkiStatus 2, failInfo 2, and no enveloped
messageData.
Verification
fails the new test.