scep: name the issuing CA in the GetCertInitial IssuerAndSubject - #21
scep: name the issuing CA in the GetCertInitial IssuerAndSubject#21yosuke-wolfssl wants to merge 1 commit into
Conversation
- wolfcert_scep_issuer_and_subject selects the issuer Name from the envelope-target certificate: its own subject when it sets basic constraints CA, otherwise the name of its issuer. The SEQUENCE length, allocation and copy all use the selected Name, and the parameter is renamed ra_cert_der/ra_cert_len. - The function and its internal.h declaration are WOLFCERT_TEST_VIS, and both doc comments state the selection rule. - tests: make_signed_cert issues a certificate under the make_ca CA with its own subject and an optional CA flag; test_issuer_and_subject_issuer_name checks the built structure for an RA certificate, the CA itself and a sub-CA, comparing both Names byte for byte. Issue: F-8022
There was a problem hiding this comment.
Pull request overview
This PR fixes SCEP GetCertInitial polling in split RA/CA deployments by ensuring the RFC 8894 IssuerAndSubject “issuer” Name identifies the issuing CA (not the RA end-entity), and adds unit coverage for issuer≠subject scenarios.
Changes:
- Update
wolfcert_scep_issuer_and_subject()to select the issuer Name from eithersubjectRaw(CA) orissuerRaw(RA end-entity). - Expose
wolfcert_scep_issuer_and_subject()to in-tree tests viaWOLFCERT_TEST_VISand clarify its contract in comments. - Add new unit-test helpers to create CA-signed certificates (issuer ≠ subject) and validate issuer/subject Name bytes for split-RA, self-signed CA, and sub-CA cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/scep/scep_msg.c |
Corrects IssuerAndSubject issuer Name selection (CA vs RA certificate). |
src/internal.h |
Updates internal/test-visible prototype and documentation for IssuerAndSubject builder. |
tests/unit/test_scep_msg.c |
Adds certificate-issuing helper and new unit test covering issuer-name selection across deployment shapes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #21
Scan targets checked: wolfcert-bugs, wolfcert-src
No new issues found in the changed files. ✅
Frauschi
left a comment
There was a problem hiding this comment.
Read through the IssuerAndSubject change. The issuer selection is the right call and the split RA/CA case it fixes is real - two comments inline.
The bigger one is the encoding: both Names are emitted without their SEQUENCE headers, so the structure is not decodable as SEQUENCE { issuer Name, subject Name } whichever DN goes into it. That is pre-existing, but it sits on the lines this PR rewrites and the new test locks the layout in, so it seems worth taking here. The second is a question about deployments where the RA's own issuer is not the enrolling CA.
| if (ic.subjectRaw == NULL || ic.subjectRawLen <= 0 || | ||
| /* A CA certificate issues under its own name. An RA certificate is an | ||
| * end entity, so the CA that will issue is the one that issued it. */ | ||
| if (ic.extBasicConstSet && ic.isCA) { |
There was a problem hiding this comment.
The two Names go in without their SEQUENCE headers, so what a peer receives is not IssuerAndSubject ::= SEQUENCE { issuer Name, subject Name }. wolfSSL's subjectRaw / issuerRaw point past the header - GetCertName() sets them only after GetASN_Sequence() has consumed it - so concatenating the two raw blobs inside one outer SEQUENCE loses both wrappers. I built this branch and called the function with examples/certs/ecc/ca-cert.pem: the output starts 30 3c 31 26 30 24 ..., i.e. after the outer header the next tag is 31 (SET), not 30. A decoder takes the first RDN for the whole issuer Name and then hits a parse error.
The encoding predates this PR, but the point of the change is a DN the server can match a pending request on, and it never gets that far. check_issuer_and_subject() also asserts the flat layout byte for byte, which pins the deviation into a regression test. Nothing else in the tree catches it either - the test server matches GetCertInitial on transactionID and never decodes the messageData, so scep_poll_roundtrip passes either way.
Could you wrap each Name in its own 0x30 <len> before concatenating, size the outer length from the two TLV lengths, and have the test skip the inner header (or decode the Names) so it validates the conformant form?
|
|
||
| if (ic.subjectRaw == NULL || ic.subjectRawLen <= 0 || | ||
| /* A CA certificate issues under its own name. An RA certificate is an | ||
| * end entity, so the CA that will issue is the one that issued it. */ |
There was a problem hiding this comment.
This derives the issuing CA from the envelope-target certificate alone. Right for the usual NDES shape, but wrong whenever the RA's own certificate came from somewhere other than the enrolling CA - an RA credential issued by a policy CA while enrolment is served by a separate issuing CA, or an RA cert still carrying a pre-rollover issuer DN. Both call sites (scep_client.c:1229 and :1749) already hold ca_bundle, so preferring the bundle entry whose subject matches the derived issuer, and falling back to this heuristic when nothing matches, would close that cheaply. Otherwise I would state the assumption in the internal.h contract: the RA certificate must be issued by the CA that will issue the requested cert.
Two small things on the same condition. ic.extBasicConstSet && is redundant - wolfSSL assigns isCA in exactly one place (DecodeBasicCaConstraintInternal), and only after VERIFY_AND_SET_OID has already set extBasicConstSet, so isCA is never 1 without it. And make_signed_cert(is_ca = 0) emits no basicConstraints at all, while a real RA certificate carries one - worth shaping the fixture like what the split-RA path will actually meet.
Problem
wolfcert_scep_issuer_and_subjectcopied the subject Name out of the certificate it was handed into theissuerfield of the RFC 8894 section 3.3.2IssuerAndSubject, and both call sites passra_cert. In a split RA/CA deployment — Microsoft NDES, or any RA-fronted CA —ra_certis an end-entity RA certificate, so a GetCertInitial poll named the RA instead of the issuing CA. A server that locates a pending request by DN cannot find it, and one that cross-checks the pair rejects the poll outright.That is the deployment where it matters most: an RA with manual approval is what returns
PENDINGand drives a client into GetCertInitial in the first place. Nothing caught it because every test certificate in the tree is self-signed, which makes subject and issuer indistinguishable.Fix (
src/scep/scep_msg.c)The issuer Name is selected from the certificate itself:
ra_certThe third row is why a blind
subjectRaw→issuerRawswap is not the fix: it would name the root instead of the actual issuer.ca_bundleis not consulted —issuerRawalready is the CA's subject DN, byte for byte, so no signature or public API change is needed. The parameter is renamedra_cert_der/ra_cert_lento match what callers pass, and the function isWOLFCERT_TEST_VISso tests can link it.Closes
f-8022.Tests (
tests/unit/test_scep_msg.c)make_signed_cert()issues a certificate under the existingmake_ca()CA with its own subject and a settable CA flag — the first helper in the tree producing issuer ≠ subject.test_issuer_and_subject_issuer_name()builds the structure for all three targets above and compares both Names byte for byte.Verification
ctestpass, includingscep_poll_roundtrip.