Skip to content

Commit 6449ca0

Browse files
committed
PKCS#7: support degenerate certs-only encode and harden signed-attribute handling
Server-side PKCS#7 encode improvements that let downstream EST/SCEP enrollment code (wolfCert) drive the existing encoder through the public API rather than hand-rolling DER. Everything is gated under the existing HAVE_PKCS7 — no new build options and no new public functions; the convenience wrappers live caller-side. Allow degenerate (certs-only) SignedData encode Relax the hashOID != 0 requirement in PKCS7_EncodeSigned() when sidType == DEGENERATE_SID, so a caller can produce a certs-only bundle (no signer, attributes, or eContent — the form used by EST /cacerts and SCEP GetCACert) by selecting DEGENERATE_SID via wc_PKCS7_SetSignerIdentifierType() and calling wc_PKCS7_EncodeSignedData(). The output round-trips through wc_PKCS7_VerifySignedData(). Size the signed-attribute array to the actual count The SignerInfo attribute working array is now sized to the real attribute count instead of a fixed [7] array. An inline buffer (sized MAX_SIGNED_ATTRIBS_SZ, the historical footprint) covers the common allocation-free case; a heap buffer is used only when the count exceeds it. The default-attribute count comes from a single helper (wc_PKCS7_GetDefaultSignedAttribCount) so the sizing matches the emission logic exactly, and the canned-attribute write is bound-checked against the array capacity. This also fixes a latent overflow where the backing array was hardcoded [7] while the bound check used MAX_SIGNED_ATTRIBS_SZ. The macro is retained for source compatibility but no longer caps the count. Document the decoded-attribute value shape Documented the stable shape of PKCS7DecodedAttrib.value (the contents of the SET OF AttributeValue, outer SET tag stripped) so callers can rely on it. No behavior change. Fix multi-certificate decode in non-streaming builds Bound the additional-certificate loop in wc_PKCS7_VerifySignedData against the absolute end of the certificate set (idx + length) rather than the relative length. In NO_PKCS7_STREAM builds the old bound dropped trailing certificates (all but the first when a large eContent preceded the set), failing verification when the signer cert was among those dropped. Streaming builds were unaffected. Tests Added coverage in pkcs7signed_test: degenerate certs-only encode via the public API, nine-attribute encode (beyond the inline capacity), decoded attribute value shape for PrintableString and OCTET STRING, and a multi-certificate decode regression with large content that triggers the bound bug under NO_PKCS7_STREAM. Config-sensitive cases are guarded.
1 parent dd6da70 commit 6449ca0

3 files changed

Lines changed: 602 additions & 11 deletions

File tree

wolfcrypt/src/pkcs7.c

Lines changed: 148 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,7 +1618,14 @@ typedef struct ESD {
16181618
byte digEncAlgoId[MAX_ALGO_SZ];
16191619
#endif
16201620
byte signedAttribSet[MAX_SET_SZ];
1621-
EncodedAttrib signedAttribs[7];
1621+
/* Working attribute array. signedAttribs points at
1622+
* the inline buffer for the common case (no heap use,
1623+
* important for no-malloc/static-memory builds) and is
1624+
* redirected to a heap allocation only when the
1625+
* attribute count exceeds MAX_SIGNED_ATTRIBS_SZ.
1626+
* signedAttribsCap holds the usable entry count. */
1627+
EncodedAttrib signedAttribsInline[MAX_SIGNED_ATTRIBS_SZ];
1628+
EncodedAttrib* signedAttribs;
16221629
byte signerDigest[MAX_OCTET_STR_SZ];
16231630
word32 innerOctetsSz, innerContSeqSz, contentInfoSeqSz;
16241631
word32 outerSeqSz, outerContentSz, innerSeqSz, versionSz, digAlgoIdSetSz,
@@ -1627,7 +1634,7 @@ typedef struct ESD {
16271634
issuerSnSeqSz, issuerNameSz, issuerSnSz, issuerSKIDSz,
16281635
issuerSKIDSeqSz, signerDigAlgoIdSz, digEncAlgoIdSz, signerDigestSz;
16291636
word32 encContentDigestSz, signedAttribsSz, signedAttribsCount,
1630-
signedAttribSetSz;
1637+
signedAttribSetSz, signedAttribsCap;
16311638
} ESD;
16321639

16331640

@@ -2238,11 +2245,46 @@ static int wc_PKCS7_GetSignSize(wc_PKCS7* pkcs7)
22382245
}
22392246

22402247

2248+
/* Number of default ("canned") signed attributes that
2249+
* wc_PKCS7_BuildSignedAttributes() will emit for the current
2250+
* pkcs7->defaultSignedAttribs selection. This is the single source of truth for
2251+
* that count: it must stay in lock step with the emission logic in
2252+
* wc_PKCS7_BuildSignedAttributes() below, and is used by PKCS7_EncodeSigned() to
2253+
* size the working attribute array to the exact count. */
2254+
static word32 wc_PKCS7_GetDefaultSignedAttribCount(wc_PKCS7* pkcs7)
2255+
{
2256+
word32 cnt = 0;
2257+
word16 flags;
2258+
2259+
if (pkcs7 == NULL)
2260+
return 0;
2261+
2262+
flags = pkcs7->defaultSignedAttribs;
2263+
if (flags == WOLFSSL_NO_ATTRIBUTES)
2264+
return 0;
2265+
2266+
if ((flags & WOLFSSL_CONTENT_TYPE_ATTRIBUTE) || flags == 0)
2267+
cnt++;
2268+
#ifndef NO_ASN_TIME
2269+
if ((flags & WOLFSSL_SIGNING_TIME_ATTRIBUTE) || flags == 0)
2270+
cnt++;
2271+
#endif
2272+
if ((flags & WOLFSSL_MESSAGE_DIGEST_ATTRIBUTE) || flags == 0)
2273+
cnt++;
2274+
2275+
return cnt;
2276+
}
2277+
2278+
22412279
/* builds up SignedData signed attributes, including default ones.
22422280
*
22432281
* pkcs7 - pointer to initialized PKCS7 structure
22442282
* esd - pointer to initialized ESD structure, used for output
22452283
*
2284+
* The number of default attributes emitted here must match
2285+
* wc_PKCS7_GetDefaultSignedAttribCount(), which sizes the working array; the
2286+
* runtime bound-check below turns any drift into BUFFER_E rather than overflow.
2287+
*
22462288
* return 0 on success, negative on error */
22472289
static int wc_PKCS7_BuildSignedAttributes(wc_PKCS7* pkcs7, ESD* esd,
22482290
const byte* contentType, word32 contentTypeSz,
@@ -2315,6 +2357,13 @@ static int wc_PKCS7_BuildSignedAttributes(wc_PKCS7* pkcs7, ESD* esd,
23152357
idx++;
23162358
}
23172359

2360+
/* the working array is sized for the canned count by PKCS7_EncodeSigned()
2361+
* via wc_PKCS7_GetDefaultSignedAttribCount(); bound-check here so a
2362+
* future drift between the two can never overflow it */
2363+
if (esd->signedAttribs == NULL ||
2364+
atrIdx + idx > esd->signedAttribsCap)
2365+
return BUFFER_E;
2366+
23182367
esd->signedAttribsCount += idx;
23192368
encAttribsSz = EncodeAttributes(&esd->signedAttribs[atrIdx],
23202369
(int)idx, cannedAttribs, (int)idx);
@@ -2329,9 +2378,13 @@ static int wc_PKCS7_BuildSignedAttributes(wc_PKCS7* pkcs7, ESD* esd,
23292378

23302379
/* add custom signed attributes if set */
23312380
if (pkcs7->signedAttribsSz > 0 && pkcs7->signedAttribs != NULL) {
2332-
word32 availableSpace = MAX_SIGNED_ATTRIBS_SZ - atrIdx;
2381+
/* esd->signedAttribs was allocated to hold all attributes, but guard
2382+
* against writing past it in case the working array was undersized */
2383+
word32 availableSpace = (esd->signedAttribsCap > atrIdx) ?
2384+
(esd->signedAttribsCap - atrIdx) : 0;
23332385

2334-
if (pkcs7->signedAttribsSz > availableSpace)
2386+
if (esd->signedAttribs == NULL ||
2387+
pkcs7->signedAttribsSz > availableSpace)
23352388
return BUFFER_E;
23362389

23372390
esd->signedAttribsCount += pkcs7->signedAttribsSz;
@@ -3054,7 +3107,10 @@ static int PKCS7_EncodeSigned(wc_PKCS7* pkcs7,
30543107

30553108
byte signingTime[MAX_TIME_STRING_SZ];
30563109

3057-
if (pkcs7 == NULL || pkcs7->hashOID == 0 ||
3110+
/* hashOID is unused on the degenerate (certs-only) path, so only require
3111+
* it when an actual signer is present */
3112+
if (pkcs7 == NULL ||
3113+
(pkcs7->hashOID == 0 && pkcs7->sidType != DEGENERATE_SID) ||
30583114
outputSz == NULL) {
30593115
WOLFSSL_MSG("PKCS7 struct / outputSz null, or hashOID is 0");
30603116
return BAD_FUNC_ARG;
@@ -3065,9 +3121,12 @@ static int PKCS7_EncodeSigned(wc_PKCS7* pkcs7,
30653121
}
30663122

30673123
/* signature size varies with ECDSA; RSA-PSS signs digest directly like
3068-
* ECDSA. For both, content hash must be known to build ASN.1 before signing */
3124+
* ECDSA. For both, content hash must be known to build ASN.1 before signing.
3125+
* The degenerate (certs-only) path has no signer, so this does not apply
3126+
* even though InitWithCert may have parsed an ECDSA/RSA-PSS cert and left
3127+
* publicKeyOID set. */
30693128
#if defined(HAVE_ECC) || defined(WC_RSA_PSS)
3070-
if (hashBuf == NULL &&
3129+
if (pkcs7->sidType != DEGENERATE_SID && hashBuf == NULL &&
30713130
(pkcs7->publicKeyOID == ECDSAk
30723131
#ifdef WC_RSA_PSS
30733132
|| pkcs7->publicKeyOID == RSAPSSk
@@ -3260,6 +3319,53 @@ static int PKCS7_EncodeSigned(wc_PKCS7* pkcs7,
32603319
}
32613320
signerInfoSz += esd->digEncAlgoIdSz;
32623321

3322+
/* Point the working attribute array at the inline buffer, sized for the
3323+
* actual attribute count: the user-supplied attributes plus the exact
3324+
* number of CMS auto-defaults that will be emitted for this
3325+
* pkcs7->defaultSignedAttribs selection. Only fall back to a heap
3326+
* allocation when more attributes are needed than fit inline, so the
3327+
* common case stays allocation-free. */
3328+
{
3329+
word32 defaultAttribCap =
3330+
wc_PKCS7_GetDefaultSignedAttribCount(pkcs7);
3331+
word32 neededCap = defaultAttribCap + pkcs7->signedAttribsSz;
3332+
3333+
/* detect addition overflow */
3334+
if (neededCap < pkcs7->signedAttribsSz) {
3335+
idx = BUFFER_E;
3336+
goto out;
3337+
}
3338+
3339+
if (neededCap <= MAX_SIGNED_ATTRIBS_SZ) {
3340+
esd->signedAttribs = esd->signedAttribsInline;
3341+
esd->signedAttribsCap = MAX_SIGNED_ATTRIBS_SZ;
3342+
}
3343+
else {
3344+
#ifdef WOLFSSL_NO_MALLOC
3345+
/* cannot grow beyond the inline array without a heap */
3346+
idx = BUFFER_E;
3347+
goto out;
3348+
#else
3349+
/* detect multiplication overflow */
3350+
if (neededCap > ((word32)WC_MAX_SINT_OF(int) /
3351+
(word32)sizeof(EncodedAttrib))) {
3352+
idx = BUFFER_E;
3353+
goto out;
3354+
}
3355+
esd->signedAttribs = (EncodedAttrib*)XMALLOC(
3356+
neededCap * (word32)sizeof(EncodedAttrib),
3357+
pkcs7->heap, DYNAMIC_TYPE_PKCS7);
3358+
if (esd->signedAttribs == NULL) {
3359+
idx = MEMORY_E;
3360+
goto out;
3361+
}
3362+
esd->signedAttribsCap = neededCap;
3363+
#endif
3364+
}
3365+
XMEMSET(esd->signedAttribs, 0,
3366+
esd->signedAttribsCap * (word32)sizeof(EncodedAttrib));
3367+
}
3368+
32633369
/* build up signed attributes, include contentType, signingTime, and
32643370
messageDigest by default */
32653371
ret = wc_PKCS7_BuildSignedAttributes(pkcs7, esd, pkcs7->contentType,
@@ -3688,6 +3794,20 @@ static int PKCS7_EncodeSigned(wc_PKCS7* pkcs7,
36883794

36893795
XFREE(flatSignedAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
36903796

3797+
/* free the working attribute array only if it was heap-allocated (i.e. it
3798+
* is not the inline buffer) before freeing esd. In small-stack builds esd
3799+
* is heap-allocated and may be NULL here. */
3800+
#ifdef WOLFSSL_SMALL_STACK
3801+
if (esd != NULL)
3802+
#endif
3803+
{
3804+
if (esd->signedAttribs != NULL &&
3805+
esd->signedAttribs != esd->signedAttribsInline) {
3806+
XFREE(esd->signedAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7);
3807+
}
3808+
esd->signedAttribs = NULL;
3809+
}
3810+
36913811
WC_FREE_VAR_EX(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
36923812
WC_FREE_VAR_EX(signedDataOid, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER);
36933813

@@ -3858,12 +3978,16 @@ int wc_PKCS7_EncodeSignedData(wc_PKCS7* pkcs7, byte* output, word32 outputSz)
38583978
return BAD_FUNC_ARG;
38593979
}
38603980

3861-
/* pre-calculate content hash for ECDSA and RSA-PSS (both sign digest directly) */
3862-
if (pkcs7->publicKeyOID == ECDSAk
3981+
/* pre-calculate content hash for ECDSA and RSA-PSS (both sign digest
3982+
* directly). Skipped on the degenerate (certs-only) path: there is no
3983+
* signer to hash for, and hashOID is 0 so deriving a hash type would fail
3984+
* even though InitWithCert may have left publicKeyOID set to the cert's. */
3985+
if (pkcs7->sidType != DEGENERATE_SID &&
3986+
(pkcs7->publicKeyOID == ECDSAk
38633987
#ifdef WC_RSA_PSS
38643988
|| pkcs7->publicKeyOID == RSAPSSk
38653989
#endif
3866-
) {
3990+
)) {
38673991
int hashSz;
38683992
enum wc_HashType hashType;
38693993
byte hashBuf[WC_MAX_DIGEST_SIZE];
@@ -7021,14 +7145,27 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
70217145
if (ret == 0 && MAX_PKCS7_CERTS > 0) {
70227146
int sz = 0;
70237147
int i;
7148+
/* Absolute end of the certificate set within pkiMsg2.
7149+
* idx is the start of the set, so the set spans
7150+
* [idx, idx + length). In non-streaming mode idx is the
7151+
* absolute offset into the message; in streaming mode it
7152+
* is typically 0 (the set was copied to a standalone
7153+
* buffer). Bounding the loop with the relative length
7154+
* alone stops short by idx bytes in non-streaming mode
7155+
* and can drop the last certificate. Clamp to pkiMsg2Sz
7156+
* to guard against overflow/over-long length (reads stay
7157+
* bounded by the certIdx + 1 < pkiMsg2Sz check below). */
7158+
word32 certSetEnd = idx + (word32)length;
7159+
if (certSetEnd < idx || certSetEnd > pkiMsg2Sz)
7160+
certSetEnd = pkiMsg2Sz;
70247161

70257162
pkcs7->cert[0] = cert;
70267163
pkcs7->certSz[0] = (word32)certSz;
70277164
certIdx = idx + (word32)certSz;
70287165

70297166
for (i = 1; i < MAX_PKCS7_CERTS &&
70307167
certIdx + 1 < pkiMsg2Sz &&
7031-
certIdx + 1 < (word32)length; i++) {
7168+
certIdx + 1 < certSetEnd; i++) {
70327169
localIdx = certIdx;
70337170

70347171
if (ret == 0 && GetASNTag(pkiMsg2, &certIdx, &tag,

0 commit comments

Comments
 (0)