Skip to content

scep: name the issuing CA in the GetCertInitial IssuerAndSubject #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: main
Choose a base branch
from
Open

scep: name the issuing CA in the GetCertInitial IssuerAndSubject #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions src/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,11 +352,12 @@ typedef struct {
const char* fail_info;
} WolfCertScepAttrs;

/* Build a DER-encoded IssuerAndSubject SEQUENCE (RFC 8894 section 3.3.2) from
* the raw Name bytes of the RA/CA cert (-> issuer) and a CSR (-> subject).
* Used as the enveloped content of a GetCertInitial pkiMessage. */
int wolfcert_scep_issuer_and_subject(const uint8_t* issuer_cert_der, size_t issuer_cert_len,
const uint8_t* csr_der, size_t csr_len,
/* Build the GetCertInitial IssuerAndSubject (RFC 8894 section 3.3.2). The
* issuer Name is the envelope-target cert's own subject when it is a CA,
* otherwise its issuer; the subject Name comes from the CSR. */
WOLFCERT_TEST_VIS int wolfcert_scep_issuer_and_subject(
const uint8_t* ra_cert_der, size_t ra_cert_len,
const uint8_t* csr_der, size_t csr_len,
WolfCertBuffer* out_der, void* heap);

WOLFCERT_TEST_VIS int wolfcert_scep_envelop(const uint8_t* ra_cert_der,
Expand Down
43 changes: 27 additions & 16 deletions src/scep/scep_msg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,23 +787,23 @@ WOLFCERT_TEST_VIS int wolfcert_scep_parse_pki_message(const uint8_t* pki_der,
return WOLFCERT_OK;
}

/* Build RFC 8894 section 3.3.2 IssuerAndSubject:
* IssuerAndSubject ::= SEQUENCE { issuer Name, subject Name }
* where the issuer Name is copied from the RA/CA cert and the subject
* Name is copied from the CSR. This is the enveloped content of a
* GetCertInitial (messageType 20) pkiMessage - it lets the server
* locate the pending request by DN when transactionID matching is
* ambiguous. */
int wolfcert_scep_issuer_and_subject(const uint8_t* issuer_cert_der, size_t issuer_cert_len,
const uint8_t* csr_der, size_t csr_len,
/* Build the GetCertInitial (messageType 20) enveloped content, RFC 8894
* section 3.3.2 IssuerAndSubject ::= SEQUENCE { issuer Name, subject Name }:
* the Name of the issuing CA, then the subject Name from the CSR. */
WOLFCERT_TEST_VIS int wolfcert_scep_issuer_and_subject(
const uint8_t* ra_cert_der, size_t ra_cert_len,
const uint8_t* csr_der, size_t csr_len,
WolfCertBuffer* out_der, void* heap)
Comment thread
yosuke-wolfssl marked this conversation as resolved.
{
if (issuer_cert_der == NULL || csr_der == NULL || out_der == NULL)
const uint8_t* issuer_name;
int issuer_name_len;

if (ra_cert_der == NULL || csr_der == NULL || out_der == NULL)
return WOLFCERT_ERR_BAD_ARG;

DecodedCert ic;
wc_InitDecodedCert(&ic, (byte*)issuer_cert_der,
(word32)issuer_cert_len, heap);
wc_InitDecodedCert(&ic, (byte*)ra_cert_der,
(word32)ra_cert_len, heap);

int rc = wc_ParseCert(&ic, CERT_TYPE, NO_VERIFY, NULL);
if (rc != 0) {
Expand All @@ -821,14 +821,25 @@ int wolfcert_scep_issuer_and_subject(const uint8_t* issuer_cert_der, size_t issu
return WOLFCERT_ERR_PARSE;
}

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. */

@Frauschi Frauschi Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (ic.extBasicConstSet && ic.isCA) {

@Frauschi Frauschi Sep 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

issuer_name = ic.subjectRaw;
issuer_name_len = ic.subjectRawLen;
}
else {
issuer_name = ic.issuerRaw;
issuer_name_len = ic.issuerRawLen;
}

if (issuer_name == NULL || issuer_name_len <= 0 ||
sc.subjectRaw == NULL || sc.subjectRawLen <= 0) {
wc_FreeDecodedCert(&ic);
wc_FreeDecodedCert(&sc);
return WOLFCERT_ERR_PARSE;
}

size_t inner = (size_t)ic.subjectRawLen + (size_t)sc.subjectRawLen;
size_t inner = (size_t)issuer_name_len + (size_t)sc.subjectRawLen;
size_t cap = inner + 8;
uint8_t* buf = (uint8_t*)WOLFCERT_XMALLOC(cap, heap);
if (buf == NULL) {
Expand All @@ -847,8 +858,8 @@ int wolfcert_scep_issuer_and_subject(const uint8_t* issuer_cert_der, size_t issu
}

size_t off = 1 + (size_t)ll;
memcpy(buf + off, ic.subjectRaw, (size_t)ic.subjectRawLen);
off += (size_t)ic.subjectRawLen;
memcpy(buf + off, issuer_name, (size_t)issuer_name_len);
off += (size_t)issuer_name_len;
memcpy(buf + off, sc.subjectRaw, (size_t)sc.subjectRawLen);
off += (size_t)sc.subjectRawLen;

Expand Down
206 changes: 206 additions & 0 deletions tests/unit/test_scep_msg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,96 @@ static int make_ca(uint8_t** cert_out, size_t* cert_out_len,
return ret;
}

/* Issue a certificate signed by the make_ca CA cert/key, so its issuer and
* subject names differ. `is_ca` sets the basic constraints CA flag. Ownership
* of the returned buffer passes to the caller (free with free()). */
static int make_signed_cert(const uint8_t* ca_der, size_t ca_der_len,
const uint8_t* ca_key_der, size_t ca_key_len,
const char* cn, int is_ca,
uint8_t** cert_out, size_t* cert_out_len)
{
RsaKey ca_key;
RsaKey sub_key;
WC_RNG rng;
Cert* cert = NULL;
uint8_t* der = NULL;
word32 idx = 0;
int ret = 0;
int body_n = 0;
int sign_n = 0;

if (wc_InitRng(&rng) != 0)
return -1;
if (wc_InitRsaKey(&ca_key, NULL) != 0) {
wc_FreeRng(&rng);
return -1;
}
if (wc_InitRsaKey(&sub_key, NULL) != 0) {
wc_FreeRsaKey(&ca_key);
wc_FreeRng(&rng);
return -1;
}

if (wc_RsaPrivateKeyDecode(ca_key_der, &idx, &ca_key,
(word32)ca_key_len) != 0)
ret = -1;

if (ret == 0 && wc_MakeRsaKey(&sub_key, 2048, WC_RSA_EXPONENT, &rng) != 0)
ret = -1;

if (ret == 0) {
der = (uint8_t*)malloc(4096);
if (der == NULL)
ret = -1;
}

if (ret == 0) {
cert = wc_CertNew(NULL);
if (cert == NULL)
ret = -1;
}

if (ret == 0) {
wc_InitCert_ex(cert, NULL, INVALID_DEVID);
strncpy(cert->subject.commonName, cn, CTC_NAME_SIZE - 1);
cert->subject.commonName[CTC_NAME_SIZE - 1] = '\0';
cert->isCA = is_ca;
cert->selfSigned = 0;
cert->sigType = CTC_SHA256wRSA;
cert->daysValid = 2;

if (wc_SetIssuerBuffer(cert, ca_der, (int)ca_der_len) != 0)
ret = -1;
}

if (ret == 0) {
body_n = wc_MakeCert(cert, der, 4096, &sub_key, NULL, &rng);
if (body_n <= 0)
ret = -1;
}

if (ret == 0) {
sign_n = wc_SignCert(cert->bodySz, cert->sigType, der, 4096, &ca_key,
NULL, &rng);
if (sign_n <= 0)
ret = -1;
}

if (ret == 0) {
*cert_out = der;
*cert_out_len = (size_t)sign_n;
der = NULL; /* ownership transferred */
}

if (cert != NULL)
wc_CertFree(cert);
free(der);
wc_FreeRsaKey(&sub_key);
wc_FreeRsaKey(&ca_key);
wc_FreeRng(&rng);
return ret;
}

#ifdef HAVE_ECC
/* Generate a throwaway self-signed ECC (P-256) CA cert, DER. Ownership of the
* returned buffer passes to the caller (free with free()). */
Expand Down Expand Up @@ -541,6 +631,120 @@ static int test_signer_subject_matches_csr(void)
return 0;
}

/* Build the IssuerAndSubject for `ra_der` and require its issuer Name to be
* the subject DN of `name_der`, followed by the CSR subject DN. */
static int check_issuer_and_subject(const uint8_t* ra_der, size_t ra_len,
const uint8_t* name_der, size_t name_len,
const uint8_t* csr_der, size_t csr_len)
{
WolfCertBuffer ias = { 0 };
DecodedCert nc;
DecodedCert sc;
size_t hdr = 0;
int rc = 0;

if (wolfcert_scep_issuer_and_subject(ra_der, ra_len, csr_der, csr_len,
&ias, NULL) != WOLFCERT_OK)
return 1;

wc_InitDecodedCert(&nc, name_der, (word32)name_len, NULL);
wc_InitDecodedCert(&sc, csr_der, (word32)csr_len, NULL);

if (wc_ParseCert(&nc, CERT_TYPE, NO_VERIFY, NULL) != 0 ||
wc_ParseCert(&sc, CERTREQ_TYPE, NO_VERIFY, NULL) != 0)
rc = 1;

if (rc == 0 && (nc.subjectRaw == NULL || nc.subjectRawLen <= 0 ||
sc.subjectRaw == NULL || sc.subjectRawLen <= 0))
rc = 1;

/* Outer SEQUENCE: tag octet plus short- or long-form length octets. */
if (rc == 0) {
if (ias.len < 2 || ias.data[0] != 0x30)
rc = 1;
else if ((ias.data[1] & 0x80) == 0)
hdr = 2;
else
hdr = 2 + (size_t)(ias.data[1] & 0x7F);
}

if (rc == 0 && ias.len != hdr + (size_t)nc.subjectRawLen +
(size_t)sc.subjectRawLen)
rc = 1;

if (rc == 0 && memcmp(ias.data + hdr, nc.subjectRaw,
(size_t)nc.subjectRawLen) != 0)
rc = 1;

if (rc == 0 && memcmp(ias.data + hdr + (size_t)nc.subjectRawLen,
sc.subjectRaw, (size_t)sc.subjectRawLen) != 0)
rc = 1;

wc_FreeDecodedCert(&nc);
wc_FreeDecodedCert(&sc);
wolfcert_buffer_free(&ias);
return rc;
}

/* RFC 8894 section 3.3.2: the IssuerAndSubject issuer Name identifies the CA
* that issues the requested cert - an RA contributes its issuer's name, a CA
* (including a sub-CA under an offline root) its own subject. */
static int test_issuer_and_subject_issuer_name(void)
{
RsaKey key;
WC_RNG rng;
uint8_t* ca_der = NULL;
size_t ca_len = 0;
uint8_t* ca_key_der = NULL;
size_t ca_key_len = 0;
uint8_t* ra_der = NULL;
size_t ra_len = 0;
uint8_t* sub_der = NULL;
size_t sub_len = 0;
uint8_t* csr_der = NULL;
size_t csr_len = 0;
int rc = 0;

REQUIRE(wc_InitRng(&rng) == 0);
REQUIRE(wc_InitRsaKey(&key, NULL) == 0);
REQUIRE(wc_MakeRsaKey(&key, 2048, WC_RSA_EXPONENT, &rng) == 0);

REQUIRE(make_ca(&ca_der, &ca_len, &ca_key_der, &ca_key_len) == 0);
REQUIRE(make_signed_cert(ca_der, ca_len, ca_key_der, ca_key_len,
"wolfCert Test RA Encryption", 0,
&ra_der, &ra_len) == 0);
REQUIRE(make_signed_cert(ca_der, ca_len, ca_key_der, ca_key_len,
"wolfCert Test Sub CA", 1,
&sub_der, &sub_len) == 0);
REQUIRE(make_csr(&key, &rng, "device-4711.example.org", "Widgets Inc",
&csr_der, &csr_len) == 0);

/* Split RA/CA: the RA is an end entity, so its issuer names the CA. */
rc = check_issuer_and_subject(ra_der, ra_len, ca_der, ca_len,
csr_der, csr_len);

/* Single self-signed CA as the envelope target. */
if (rc == 0)
rc = check_issuer_and_subject(ca_der, ca_len, ca_der, ca_len,
csr_der, csr_len);

/* Sub-CA with no RA: it issues, so it names itself. */
if (rc == 0)
rc = check_issuer_and_subject(sub_der, sub_len, sub_der, sub_len,
csr_der, csr_len);

free(csr_der);
free(sub_der);
free(ra_der);
free(ca_key_der);
free(ca_der);
wc_FreeRsaKey(&key);
wc_FreeRng(&rng);

REQUIRE(rc == 0);
return 0;
}

/* RFC 8894: a CertRep must be signed by the CA/RA certificate the client
* fetched via GetCACert. A response signed by any other certificate, as a
* MITM or rogue server would forge, must be rejected before the client
Expand Down Expand Up @@ -1276,6 +1480,8 @@ int main(void)
return 1;
if (test_signer_subject_matches_csr())
return 1;
if (test_issuer_and_subject_issuer_name())
return 1;
if (test_cert_rep_signer_trust())
return 1;
if (test_cert_rep_txid_and_type())
Expand Down
Loading