From 9fbe471156f2cddf49e1e007934f7192b9e19237 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 3 Jun 2019 07:52:50 +0700 Subject: [PATCH 1/3] fix check on ret value and add test case --- tests/api.c | 24 ++++- wolfcrypt/src/pkcs7.c | 183 +++++++++++++++++++++----------------- wolfssl/wolfcrypt/pkcs7.h | 1 + 3 files changed, 124 insertions(+), 84 deletions(-) diff --git a/tests/api.c b/tests/api.c index 72c3b5f3e..0d679fa9f 100644 --- a/tests/api.c +++ b/tests/api.c @@ -17246,7 +17246,7 @@ static void test_PKCS7_signed_enveloped(void) AssertIntGT((envSz = wc_PKCS7_EncodeEnvelopedData(pkcs7, env, envSz)), 0); wc_PKCS7_Free(pkcs7); - /* create signed enveloped data */ + /* create bad signed enveloped data */ sigSz = FOURK_BUF * 2; AssertNotNull(pkcs7 = wc_PKCS7_New(NULL, 0)); AssertIntEQ(wc_InitRng(&rng), 0); @@ -17267,11 +17267,31 @@ static void test_PKCS7_signed_enveloped(void) AssertIntGT((sigSz = wc_PKCS7_EncodeSignedData(pkcs7, sig, sigSz)), 0); pkcs7->certList = (Pkcs7Cert*)pt; /* restore pointer for PKCS7 free call */ wc_PKCS7_Free(pkcs7); + + /* check verify fails */ + AssertNotNull(pkcs7 = wc_PKCS7_New(NULL, 0)); + AssertIntEQ(wc_PKCS7_InitWithCert(pkcs7, cert, certSz), 0); + AssertIntNE(wc_PKCS7_VerifySignedData(pkcs7, sig, sigSz), 0); + wc_PKCS7_Free(pkcs7); + + /* create valid degenerate bundle */ + sigSz = FOURK_BUF * 2; + AssertNotNull(pkcs7 = wc_PKCS7_New(NULL, 0)); + pkcs7->content = env; + pkcs7->contentSz = envSz; + pkcs7->contentOID = DATA; + pkcs7->privateKey = key; + pkcs7->privateKeySz = keySz; + pkcs7->encryptOID = RSAk; + pkcs7->hashOID = SHA256h; + pkcs7->rng = &rng; + AssertIntEQ(wc_PKCS7_SetSignerIdentifierType(pkcs7, DEGENERATE_SID), 0); + AssertIntGT((sigSz = wc_PKCS7_EncodeSignedData(pkcs7, sig, sigSz)), 0); wc_FreeRng(&rng); /* check verify */ AssertNotNull(pkcs7 = wc_PKCS7_New(NULL, 0)); - AssertIntEQ(wc_PKCS7_InitWithCert(pkcs7, cert, certSz), 0); + AssertIntEQ(wc_PKCS7_Init(pkcs7, HEAP_HINT, devId), 0); AssertIntEQ(wc_PKCS7_VerifySignedData(pkcs7, sig, sigSz), 0); AssertNotNull(pkcs7->content); diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index ef259b187..1a24197dc 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1896,19 +1896,21 @@ static int PKCS7_EncodeSigned(PKCS7* pkcs7, ESD* esd, } signedDataOidSz = ret; - esd->hashType = wc_OidGetHash(pkcs7->hashOID); - if (wc_HashGetDigestSize(esd->hashType) != (int)hashSz) { - WOLFSSL_MSG("hashSz did not match hashOID"); -#ifdef WOLFSSL_SMALL_STACK - XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return BUFFER_E; - } + if (pkcs7->sidType != DEGENERATE_SID) { + esd->hashType = wc_OidGetHash(pkcs7->hashOID); + if (wc_HashGetDigestSize(esd->hashType) != (int)hashSz) { + WOLFSSL_MSG("hashSz did not match hashOID"); + #ifdef WOLFSSL_SMALL_STACK + XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + #endif + return BUFFER_E; + } - /* include hash */ - esd->contentDigest[0] = ASN_OCTET_STRING; - esd->contentDigest[1] = (byte)hashSz; - XMEMCPY(&esd->contentDigest[2], hashBuf, hashSz); + /* include hash */ + esd->contentDigest[0] = ASN_OCTET_STRING; + esd->contentDigest[1] = (byte)hashSz; + XMEMCPY(&esd->contentDigest[2], hashBuf, hashSz); + } if (pkcs7->detached == 1) { /* do not include content if generating detached signature */ @@ -1949,6 +1951,8 @@ static int PKCS7_EncodeSigned(PKCS7* pkcs7, ESD* esd, /* version MUST be 3 */ esd->signerVersionSz = SetMyVersion(3, esd->signerVersion, 0); + } else if (pkcs7->sidType == DEGENERATE_SID) { + /* no signer info added */ } else { #ifdef WOLFSSL_SMALL_STACK XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); @@ -1956,78 +1960,80 @@ static int PKCS7_EncodeSigned(PKCS7* pkcs7, ESD* esd, return SKID_E; } - signerInfoSz += esd->signerVersionSz; - esd->signerDigAlgoIdSz = SetAlgoID(pkcs7->hashOID, esd->signerDigAlgoId, - oidHashType, 0); - signerInfoSz += esd->signerDigAlgoIdSz; + if (pkcs7->sidType != DEGENERATE_SID) { + signerInfoSz += esd->signerVersionSz; + esd->signerDigAlgoIdSz = SetAlgoID(pkcs7->hashOID, esd->signerDigAlgoId, + oidHashType, 0); + signerInfoSz += esd->signerDigAlgoIdSz; - /* set signatureAlgorithm */ - ret = wc_PKCS7_SignedDataGetEncAlgoId(pkcs7, &digEncAlgoId, - &digEncAlgoType); - if (ret < 0) { -#ifdef WOLFSSL_SMALL_STACK - XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); -#endif - return ret; - } - esd->digEncAlgoIdSz = SetAlgoID(digEncAlgoId, esd->digEncAlgoId, - digEncAlgoType, 0); - signerInfoSz += esd->digEncAlgoIdSz; - - /* build up signed attributes, include contentType, signingTime, and - messageDigest by default */ - ret = wc_PKCS7_BuildSignedAttributes(pkcs7, esd, pkcs7->contentType, - pkcs7->contentTypeSz, - contentTypeOid, sizeof(contentTypeOid), - messageDigestOid, sizeof(messageDigestOid), - signingTimeOid, sizeof(signingTimeOid), - signingTime, sizeof(signingTime)); - if (ret < 0) { + /* set signatureAlgorithm */ + ret = wc_PKCS7_SignedDataGetEncAlgoId(pkcs7, &digEncAlgoId, + &digEncAlgoType); + if (ret < 0) { #ifdef WOLFSSL_SMALL_STACK - XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); #endif - return ret; - } + return ret; + } + esd->digEncAlgoIdSz = SetAlgoID(digEncAlgoId, esd->digEncAlgoId, + digEncAlgoType, 0); + signerInfoSz += esd->digEncAlgoIdSz; - if (esd->signedAttribsSz > 0) { - flatSignedAttribs = (byte*)XMALLOC(esd->signedAttribsSz, pkcs7->heap, - DYNAMIC_TYPE_PKCS7); - flatSignedAttribsSz = esd->signedAttribsSz; - if (flatSignedAttribs == NULL) { + /* build up signed attributes, include contentType, signingTime, and + messageDigest by default */ + ret = wc_PKCS7_BuildSignedAttributes(pkcs7, esd, pkcs7->contentType, + pkcs7->contentTypeSz, + contentTypeOid, sizeof(contentTypeOid), + messageDigestOid, sizeof(messageDigestOid), + signingTimeOid, sizeof(signingTimeOid), + signingTime, sizeof(signingTime)); + if (ret < 0) { #ifdef WOLFSSL_SMALL_STACK XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); #endif - return MEMORY_E; + return ret; } - FlattenAttributes(flatSignedAttribs, - esd->signedAttribs, esd->signedAttribsCount); - esd->signedAttribSetSz = SetImplicit(ASN_SET, 0, esd->signedAttribsSz, - esd->signedAttribSet); - } else { - esd->signedAttribSetSz = 0; + if (esd->signedAttribsSz > 0) { + flatSignedAttribs = (byte*)XMALLOC(esd->signedAttribsSz, pkcs7->heap, + DYNAMIC_TYPE_PKCS7); + flatSignedAttribsSz = esd->signedAttribsSz; + if (flatSignedAttribs == NULL) { + #ifdef WOLFSSL_SMALL_STACK + XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + #endif + return MEMORY_E; + } + + FlattenAttributes(flatSignedAttribs, + esd->signedAttribs, esd->signedAttribsCount); + esd->signedAttribSetSz = SetImplicit(ASN_SET, 0, esd->signedAttribsSz, + esd->signedAttribSet); + } else { + esd->signedAttribSetSz = 0; + } + + /* Calculate the final hash and encrypt it. */ + ret = wc_PKCS7_SignedDataBuildSignature(pkcs7, flatSignedAttribs, + flatSignedAttribsSz, esd); + if (ret < 0) { + if (pkcs7->signedAttribsSz != 0) + XFREE(flatSignedAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + #ifdef WOLFSSL_SMALL_STACK + XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + #endif + return ret; + } + + signerInfoSz += flatSignedAttribsSz + esd->signedAttribSetSz; + + esd->signerDigestSz = SetOctetString(esd->encContentDigestSz, + esd->signerDigest); + signerInfoSz += esd->signerDigestSz + esd->encContentDigestSz; + + esd->signerInfoSeqSz = SetSequence(signerInfoSz, esd->signerInfoSeq); + signerInfoSz += esd->signerInfoSeqSz; } - - /* Calculate the final hash and encrypt it. */ - ret = wc_PKCS7_SignedDataBuildSignature(pkcs7, flatSignedAttribs, - flatSignedAttribsSz, esd); - if (ret < 0) { - if (pkcs7->signedAttribsSz != 0) - XFREE(flatSignedAttribs, pkcs7->heap, DYNAMIC_TYPE_PKCS7); - #ifdef WOLFSSL_SMALL_STACK - XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); - #endif - return ret; - } - - signerInfoSz += flatSignedAttribsSz + esd->signedAttribSetSz; - - esd->signerDigestSz = SetOctetString(esd->encContentDigestSz, - esd->signerDigest); - signerInfoSz += esd->signerDigestSz + esd->encContentDigestSz; - - esd->signerInfoSeqSz = SetSequence(signerInfoSz, esd->signerInfoSeq); - signerInfoSz += esd->signerInfoSeqSz; esd->signerInfoSetSz = SetSet(signerInfoSz, esd->signerInfoSet); signerInfoSz += esd->signerInfoSetSz; @@ -2043,11 +2049,12 @@ static int PKCS7_EncodeSigned(PKCS7* pkcs7, ESD* esd, if (certSetSz > 0) esd->certsSetSz = SetImplicit(ASN_SET, 0, certSetSz, esd->certsSet); - esd->singleDigAlgoIdSz = SetAlgoID(pkcs7->hashOID, esd->singleDigAlgoId, + if (pkcs7->sidType != DEGENERATE_SID) { + esd->singleDigAlgoIdSz = SetAlgoID(pkcs7->hashOID, esd->singleDigAlgoId, oidHashType, 0); + } esd->digAlgoIdSetSz = SetSet(esd->singleDigAlgoIdSz, esd->digAlgoIdSet); - esd->versionSz = SetMyVersion(1, esd->version, 0); totalSz = esd->versionSz + esd->singleDigAlgoIdSz + esd->digAlgoIdSetSz + @@ -2168,6 +2175,8 @@ static int PKCS7_EncodeSigned(PKCS7* pkcs7, ESD* esd, idx += esd->issuerSKIDSz; XMEMCPY(output2 + idx, pkcs7->issuerSubjKeyId, KEYID_SIZE); idx += KEYID_SIZE; + } else if (pkcs7->sidType == DEGENERATE_SID) { + /* no signer infos in degenerate case */ } else { #ifdef WOLFSSL_SMALL_STACK XFREE(esd, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); @@ -3919,12 +3928,13 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, idx++; if (GetLength(pkiMsg2, &idx, &length, pkiMsg2Sz) < 0) ret = ASN_PARSE_E; + } - if (ret != 0) { - break; - } + if (ret != 0) { + break; + } #ifndef NO_PKCS7_STREAM - if (content != NULL && pkcs7->stream->flagOne) { + if (content != NULL && pkcs7->stream->flagOne && length > 0) { stateIdx = idx; /* case where all data was read from in2 */ } @@ -3937,6 +3947,11 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, } else { pkcs7->stream->expected = MAX_SEQ_SZ; + if (pkcs7->stream->expected > (pkcs7->stream->maxLen - + pkcs7->stream->totalRd) + pkcs7->stream->length) { + pkcs7->stream->expected = (pkcs7->stream->maxLen - + pkcs7->stream->totalRd) + pkcs7->stream->length; + } } #endif wc_PKCS7_ChangeState(pkcs7, WC_PKCS7_VERIFY_STAGE4); @@ -4055,7 +4070,6 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, } } idx += length; - } if (!detached) { /* set content and size after init of PKCS7 structure */ @@ -4296,6 +4310,10 @@ static int PKCS7_VerifySignedData(PKCS7* pkcs7, const byte* hashBuf, if (ret < 0) { WOLFSSL_MSG("Failed to set public key OID from signature"); } + else { + /* if previous return was positive then was success */ + ret = 0; + } } if (idx >= pkiMsg2Sz) @@ -6051,7 +6069,8 @@ int wc_PKCS7_SetSignerIdentifierType(PKCS7* pkcs7, int type) return BAD_FUNC_ARG; if (type != CMS_ISSUER_AND_SERIAL_NUMBER && - type != CMS_SKID) { + type != CMS_SKID && + type != DEGENERATE_SID) { return BAD_FUNC_ARG; } diff --git a/wolfssl/wolfcrypt/pkcs7.h b/wolfssl/wolfcrypt/pkcs7.h index 4f26dd939..48a79739b 100644 --- a/wolfssl/wolfcrypt/pkcs7.h +++ b/wolfssl/wolfcrypt/pkcs7.h @@ -159,6 +159,7 @@ enum Cms_Options { CMS_SKID = 1, CMS_ISSUER_AND_SERIAL_NUMBER = 2, }; +#define DEGENERATE_SID 3 /* CMS/PKCS#7 RecipientInfo types, RFC 5652, Section 6.2 */ enum Pkcs7_RecipientInfo_Types { From cb4f9afd6dc31350012927a8cc2163b08c5bb4d8 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Tue, 25 Jun 2019 15:24:39 -0600 Subject: [PATCH 2/3] free memory in test case --- tests/api.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/api.c b/tests/api.c index 0d679fa9f..a109a52c3 100644 --- a/tests/api.c +++ b/tests/api.c @@ -17288,6 +17288,7 @@ static void test_PKCS7_signed_enveloped(void) AssertIntEQ(wc_PKCS7_SetSignerIdentifierType(pkcs7, DEGENERATE_SID), 0); AssertIntGT((sigSz = wc_PKCS7_EncodeSignedData(pkcs7, sig, sigSz)), 0); wc_FreeRng(&rng); + wc_PKCS7_Free(pkcs7); /* check verify */ AssertNotNull(pkcs7 = wc_PKCS7_New(NULL, 0)); From e7fd45537de764e16428733b4b72e905c67f0598 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Wed, 26 Jun 2019 11:58:53 -0600 Subject: [PATCH 3/3] update comments for DEGENERATE_SID use --- wolfcrypt/src/pkcs7.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 1a24197dc..8a1463454 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -6059,8 +6059,11 @@ static int wc_PKCS7_GenerateBlock(PKCS7* pkcs7, WC_RNG* rng, byte* out, * IssuerAndSerialNumber unless set with this function or explicitly * overriden via options when adding RecipientInfo type. * + * Using the type DEGENERATE_SID skips over signer information. In degenerate + * cases there are no signers. + * * pkcs7 - pointer to initialized PKCS7 structure - * type - either CMS_ISSUER_AND_SERIAL_NUMBER or CMS_SKID + * type - either CMS_ISSUER_AND_SERIAL_NUMBER, CMS_SKID or DEGENERATE_SID * * return 0 on success, negative upon error */ int wc_PKCS7_SetSignerIdentifierType(PKCS7* pkcs7, int type)