Skip to content

Commit 8153ea6

Browse files
JacobBarthelmehJacobBarthelmeh
authored
Merge pull request #9559 from cconlon/pkcs7SignedNonOctet
Fix PKCS#7 SignedData parsing for non-OCTET_STRING content types
2 parents bbc3a72 + afe82b9 commit 8153ea6

File tree

3 files changed

+155
-4
lines changed

3 files changed

+155
-4
lines changed

‎tests/api/test_pkcs7.c‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4383,4 +4383,135 @@ int test_wc_PKCS7_DecodeCompressedData(void)
43834383
return EXPECT_RESULT();
43844384
}
43854385

4386+
/*
4387+
* Test for PKCS#7 SignedData with non-OCTET_STRING content
4388+
* (PKCS#7 style vs CMS)
4389+
*
4390+
* Tests parsing PKCS#7 SignedData where the encapsulated content
4391+
* is a SEQUENCE (as allowed by original PKCS#7 spec "ANY DEFINED BY
4392+
* contentType") rather than an OCTET STRING (as mandated by CMS). This showed
4393+
* up in use case of Authenticode signatures.
4394+
*/
4395+
int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void)
4396+
{
4397+
EXPECT_DECLS;
4398+
#if defined(HAVE_PKCS7)
4399+
PKCS7* pkcs7 = NULL;
4400+
#ifndef NO_PKCS7_STREAM
4401+
word32 idx;
4402+
int ret;
4403+
#endif
4404+
4405+
/*
4406+
* Hand-crafted PKCS#7 SignedData (degenerate, no signers) with:
4407+
* - Content type OID (1.3.6.1.4.1.311.2.1.4 = SPC_INDIRECT_DATA)
4408+
* - Content is a SEQUENCE, NOT an OCTET STRING
4409+
* - eContent is encoded as "ANY" type per original PKCS#7 spec.
4410+
*
4411+
* This test ensures wolfSSL's PKCS7 streaming code can correctly
4412+
* parse SignedData types when the encapsulated content is not an OCTET
4413+
* STRING (as CMS requires) but rather a SEQUENCE or other type
4414+
* (as PKCS#7's "ANY" type allows). Microsoft Authenticode signatures
4415+
* use this format with SPC_INDIRECT_DATA content.
4416+
*
4417+
* Structure:
4418+
* ContentInfo SEQUENCE
4419+
* contentType OID signedData
4420+
* [0] SignedData SEQUENCE
4421+
* version INTEGER 1
4422+
* digestAlgorithms SET { sha256 }
4423+
* encapContentInfo SEQUENCE
4424+
* eContentType OID 1.3.6.1.4.1.311.2.1.4
4425+
* [0] eContent
4426+
* SEQUENCE { OID, OCTET STRING } - SEQUENCE not OCTET STRING
4427+
* signerInfos SET {} (empty = degenerate)
4428+
*/
4429+
static const byte pkcs7Content[] = {
4430+
/* ContentInfo SEQUENCE */
4431+
0x30, 0x56,
4432+
/* contentType OID: 1.2.840.113549.1.7.2 (signedData) */
4433+
0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x07, 0x02,
4434+
/* [0] EXPLICIT - content */
4435+
0xA0, 0x49,
4436+
/* SignedData SEQUENCE */
4437+
0x30, 0x47,
4438+
/* version INTEGER 1 */
4439+
0x02, 0x01, 0x01,
4440+
/* digestAlgorithms SET */
4441+
0x31, 0x0F,
4442+
/* AlgorithmIdentifier SEQUENCE */
4443+
0x30, 0x0D,
4444+
/* OID sha256: 2.16.840.1.101.3.4.2.1 */
4445+
0x06, 0x09, 0x60, 0x86, 0x48, 0x01, 0x65, 0x03,
4446+
0x04, 0x02, 0x01,
4447+
/* NULL */
4448+
0x05, 0x00,
4449+
/* encapContentInfo SEQUENCE */
4450+
0x30, 0x2F,
4451+
/* eContentType OID: 1.3.6.1.4.1.311.2.1.4 (SPC_INDIRECT_DATA) */
4452+
0x06, 0x0A, 0x2B, 0x06, 0x01, 0x04, 0x01, 0x82,
4453+
0x37, 0x02, 0x01, 0x04,
4454+
/* [0] EXPLICIT - eContent */
4455+
0xA0, 0x21,
4456+
/* Content SEQUENCE (0x30), not OCTET STRING (0x04)
4457+
* Following PKCS#7 "ANY" type, not CMS OCTET STRING */
4458+
0x30, 0x1F,
4459+
/* Content: SEQUENCE { OID, OCTET STRING with 24 bytes } */
4460+
0x06, 0x03, 0x55, 0x04, 0x03, /* OID 2.5.4.3 (5 bytes) */
4461+
0x04, 0x18, /* OCTET STRING length 24 */
4462+
0x54, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20, /* "This is " */
4463+
0x74, 0x65, 0x73, 0x74, 0x20, 0x63, 0x6F, 0x6E, /* "test con" */
4464+
0x74, 0x65, 0x6E, 0x74, 0x20, 0x64, 0x61, 0x74, /* "tent dat" */
4465+
/* signerInfos SET - empty for degenerate */
4466+
0x31, 0x00
4467+
};
4468+
4469+
/* Test non-streaming verification */
4470+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
4471+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
4472+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
4473+
ExpectIntEQ(wc_PKCS7_VerifySignedData(pkcs7, (byte*)pkcs7Content,
4474+
(word32)sizeof(pkcs7Content)), 0);
4475+
4476+
/* Verify content was parsed correctly */
4477+
if (pkcs7 != NULL) {
4478+
/* contentIsPkcs7Type should be set */
4479+
ExpectIntEQ(pkcs7->contentIsPkcs7Type, 1);
4480+
/* Content should have been parsed (33 bytes) */
4481+
ExpectIntEQ(pkcs7->contentSz, 33);
4482+
ExpectNotNull(pkcs7->content);
4483+
}
4484+
wc_PKCS7_Free(pkcs7);
4485+
pkcs7 = NULL;
4486+
4487+
#ifndef NO_PKCS7_STREAM
4488+
/* Test streaming verification - feed data byte by byte */
4489+
ExpectNotNull(pkcs7 = wc_PKCS7_New(HEAP_HINT, testDevId));
4490+
ExpectIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, INVALID_DEVID), 0);
4491+
ExpectIntEQ(wc_PKCS7_InitWithCert(pkcs7, NULL, 0), 0);
4492+
4493+
/* Feed data byte by byte to exercise streaming path */
4494+
ret = WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E);
4495+
for (idx = 0; idx < (word32)sizeof(pkcs7Content) && ret != 0; idx++) {
4496+
ret = wc_PKCS7_VerifySignedData(pkcs7,
4497+
(byte*)pkcs7Content + idx, 1);
4498+
if (ret < 0 && ret != WC_NO_ERR_TRACE(WC_PKCS7_WANT_READ_E)) {
4499+
/* Unexpected error */
4500+
break;
4501+
}
4502+
}
4503+
4504+
/* Expecting ret = 0, not ASN_PARSE_E or other negative error */
4505+
ExpectIntEQ(ret, 0);
4506+
4507+
if (pkcs7 != NULL) {
4508+
ExpectIntEQ(pkcs7->contentIsPkcs7Type, 1);
4509+
ExpectIntEQ(pkcs7->contentSz, 33);
4510+
ExpectNotNull(pkcs7->content);
4511+
}
4512+
wc_PKCS7_Free(pkcs7);
4513+
#endif /* !NO_PKCS7_STREAM */
4514+
#endif /* HAVE_PKCS7 */
4515+
return EXPECT_RESULT();
4516+
}
43864517

‎tests/api/test_pkcs7.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ int test_wc_PKCS7_SetOriEncryptCtx(void);
4848
int test_wc_PKCS7_SetOriDecryptCtx(void);
4949
int test_wc_PKCS7_DecodeCompressedData(void);
5050
int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void);
51+
int test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq(void);
5152

5253

5354
#define TEST_PKCS7_DECLS \
@@ -63,7 +64,8 @@ int test_wc_PKCS7_DecodeEnvelopedData_multiple_recipients(void);
6364
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_ECC), \
6465
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_Degenerate), \
6566
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_BER), \
66-
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_NoDefaultSignedAttribs)
67+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_NoDefaultSignedAttribs), \
68+
TEST_DECL_GROUP("pkcs7_sd", test_wc_PKCS7_VerifySignedData_PKCS7ContentSeq)
6769

6870
#define TEST_PKCS7_ENCRYPTED_DATA_DECLS \
6971
TEST_DECL_GROUP("pkcs7_ed", test_wc_PKCS7_DecodeEnvelopedData_stream), \

‎wolfcrypt/src/pkcs7.c‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5455,7 +5455,6 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
54555455
if (ret == 0 && GetMyVersion(pkiMsg, &idx, &version, pkiMsgSz) < 0)
54565456
ret = ASN_PARSE_E;
54575457

5458-
54595458
/* version 1 follows RFC 2315 */
54605459
/* version 3 follows RFC 4108 */
54615460
if (ret == 0 && (version != 1 && version != 3)) {
@@ -5673,6 +5672,15 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
56735672
* this as start of content. */
56745673
localIdx = start;
56755674
pkcs7->contentIsPkcs7Type = 1;
5675+
5676+
#ifndef NO_PKCS7_STREAM
5677+
/* Set streaming variables for PKCS#7 type content.
5678+
* length contains the size from [0] EXPLICIT wrapper */
5679+
pkcs7->stream->multi = 0;
5680+
pkcs7->stream->currContIdx = localIdx;
5681+
pkcs7->stream->currContSz = (word32)length;
5682+
pkcs7->stream->currContRmnSz = (word32)length;
5683+
#endif
56765684
}
56775685
else {
56785686
/* CMS eContent OCTET_STRING */
@@ -5762,7 +5770,6 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
57625770
idx = localIdx;
57635771
}
57645772
else {
5765-
57665773
/* If either pkcs7->content and pkcs7->contentSz are set
57675774
* (detached signature where user has set content explicitly
57685775
* into pkcs7->content/contentSz) OR pkcs7->hashBuf and
@@ -5862,7 +5869,7 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
58625869

58635870
/* copy content to pkcs7->contentDynamic */
58645871
if (keepContent && pkcs7->stream->content &&
5865-
pkcs7->stream->contentSz >0) {
5872+
pkcs7->stream->contentSz > 0) {
58665873
pkcs7->contentDynamic = (byte*)XMALLOC(pkcs7->stream->contentSz,
58675874
pkcs7->heap, DYNAMIC_TYPE_PKCS7);
58685875
if (pkcs7->contentDynamic == NULL) {
@@ -6412,6 +6419,17 @@ static int PKCS7_VerifySignedData(wc_PKCS7* pkcs7, const byte* hashBuf,
64126419
NO_USER_CHECK) < 0)
64136420
ret = ASN_PARSE_E;
64146421

6422+
/* Update degenerate flag based on if signerInfos SET is empty.
6423+
* The earlier degenerate check at digestAlgorithms is an early
6424+
* optimization, but depending on degenerate case may not be
6425+
* detected until here. */
6426+
if (ret == 0) {
6427+
degenerate = (length == 0) ? 1 : 0;
6428+
#ifndef NO_PKCS7_STREAM
6429+
pkcs7->stream->degenerate = (degenerate != 0);
6430+
#endif
6431+
}
6432+
64156433
if (ret != 0)
64166434
break;
64176435
#ifndef NO_PKCS7_STREAM

0 commit comments

Comments
 (0)