From 9d9495f8b6c7ca0c5c81bef761907eabdde35e26 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Fri, 12 Jul 2019 16:23:48 -0700 Subject: [PATCH 1/3] allow CSR challenge pass to be encoded as PrintableString --- wolfcrypt/src/asn.c | 20 ++++++++++++++++---- wolfssl/wolfcrypt/asn_public.h | 1 + 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 5dfcc3a62..e457b0cbf 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -10065,6 +10065,12 @@ typedef struct DerCert { #ifdef WOLFSSL_CERT_REQ /* Write a set header to output */ +static word32 SetPrintableString(word32 len, byte* output) +{ + output[0] = ASN_PRINTABLE_STRING; + return SetLength(len, output + 1) + 1; +} + static word32 SetUTF8String(word32 len, byte* output) { output[0] = ASN_UTF8STRING; @@ -11961,7 +11967,8 @@ int wc_MakeNtruCert(Cert* cert, byte* derBuffer, word32 derSz, #ifdef WOLFSSL_CERT_REQ -static int SetReqAttrib(byte* output, char* pw, int extSz) +static int SetReqAttrib(byte* output, char* pw, int pwPrintableString, + int extSz) { static const byte cpOid[] = { ASN_OBJECT_ID, 0x09, 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, @@ -11990,7 +11997,11 @@ static int SetReqAttrib(byte* output, char* pw, int extSz) if (pw && pw[0]) { pwSz = (int)XSTRLEN(pw); - cpStrSz = SetUTF8String(pwSz, cpStr); + if (pwPrintableString) { + cpStrSz = SetPrintableString(pwSz, cpStr); + } else { + cpStrSz = SetUTF8String(pwSz, cpStr); + } cpSetSz = SetSet(cpStrSz + pwSz, cpSet); cpSeqSz = SetSequence(sizeof(cpOid) + cpSetSz + cpStrSz + pwSz, cpSeq); cpSz = cpSeqSz + sizeof(cpOid) + cpSetSz + cpStrSz + pwSz; @@ -12198,8 +12209,9 @@ static int EncodeCertReq(Cert* cert, DerCert* der, RsaKey* rsaKey, #endif /* WOLFSSL_CERT_EXT */ } - der->attribSz = SetReqAttrib(der->attrib, - cert->challengePw, der->extensionsSz); + der->attribSz = SetReqAttrib(der->attrib, cert->challengePw, + cert->challengePwPrintableString, + der->extensionsSz); if (der->attribSz <= 0) return REQ_ATTRIBUTE_E; diff --git a/wolfssl/wolfcrypt/asn_public.h b/wolfssl/wolfcrypt/asn_public.h index 5261a6c6e..9f5371e6d 100644 --- a/wolfssl/wolfcrypt/asn_public.h +++ b/wolfssl/wolfcrypt/asn_public.h @@ -296,6 +296,7 @@ typedef struct Cert { #endif #ifdef WOLFSSL_CERT_REQ char challengePw[CTC_NAME_SIZE]; + int challengePwPrintableString; /* encode as PrintableString */ #endif void* decodedCert; /* internal DecodedCert allocated from heap */ byte* der; /* Pointer to buffer of current DecodedCert cache */ From 7367da3fb6093e7e3303531c478caf5e72a30312 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Fri, 12 Jul 2019 16:36:15 -0700 Subject: [PATCH 2/3] sort PKCS7 signed attributes when generating bundle --- wolfcrypt/src/pkcs7.c | 169 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 160 insertions(+), 9 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 9b907711a..3a9c8bf8d 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1298,12 +1298,99 @@ static int EncodeAttributes(EncodedAttrib* ea, int eaSz, } -static int FlattenAttributes(byte* output, EncodedAttrib* ea, int eaSz) -{ - int i, idx; +typedef struct FlatAttrib { + byte* data; + word32 dataSz; +} FlatAttrib; + + +/* Free FlatAttrib array and memory allocated to internal struct members */ +static void FreeAttribArray(PKCS7* pkcs7, FlatAttrib** arr, int rows) +{ + int i; + + if (arr) { + for (i = 0; i < rows; i++) { + if (arr[i]) { + if (arr[i]->data) { + XMEMSET(arr[i]->data, 0, arr[i]->dataSz); + XFREE(arr[i]->data, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + } + XMEMSET(arr[i], 0, sizeof(FlatAttrib)); + XFREE(arr[i], pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + } + } + XMEMSET(arr, 0, rows); + XFREE(arr, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + } + (void)pkcs7; +} + + +/* Sort FlatAttrib array in ascending order */ +static int SortAttribArray(FlatAttrib** arr, int rows) +{ + int i, j; + word32 minSz, minIdx; + FlatAttrib* a = NULL; + FlatAttrib* b = NULL; + FlatAttrib* tmp = NULL; + + if (arr == NULL) { + return BAD_FUNC_ARG; + } + + for (i = 0; i < rows; i++) { + a = arr[i]; + minSz = a->dataSz; + minIdx = i; + for (j = i+1; j < rows; j++) { + b = arr[j]; + if (b->dataSz < minSz) { + minSz = b->dataSz; + minIdx = j; + } + } + if (minSz < a->dataSz) { + /* swap array positions */ + tmp = arr[i]; + arr[i] = arr[minIdx]; + arr[minIdx] = tmp; + } + } + + return 0; +} + + +/* Build up array of FlatAttrib structs from EncodedAttrib ones. FlatAttrib + * holds flattened DER encoding of each attribute */ +static int FlattenEncodedAttribs(PKCS7* pkcs7, FlatAttrib** derArr, int rows, + EncodedAttrib* ea, int eaSz) +{ + int i, idx, sz; + byte* output = NULL; + FlatAttrib* fa = NULL; + + if (pkcs7 == NULL || derArr == NULL || ea == NULL) { + WOLFSSL_MSG("Invalid arguments to FlattenEncodedAttribs"); + return BAD_FUNC_ARG; + } + + if (rows != eaSz) { + WOLFSSL_MSG("DER array not large enough to hold attribute count"); + return BAD_FUNC_ARG; + } - idx = 0; for (i = 0; i < eaSz; i++) { + sz = ea[i].valueSeqSz + ea[i].oidSz + ea[i].valueSetSz + ea[i].valueSz; + + output = (byte*)XMALLOC(sz, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); + if (output == NULL) { + return MEMORY_E; + } + + idx = 0; XMEMCPY(output + idx, ea[i].valueSeq, ea[i].valueSeqSz); idx += ea[i].valueSeqSz; XMEMCPY(output + idx, ea[i].oid, ea[i].oidSz); @@ -1311,8 +1398,70 @@ static int FlattenAttributes(byte* output, EncodedAttrib* ea, int eaSz) XMEMCPY(output + idx, ea[i].valueSet, ea[i].valueSetSz); idx += ea[i].valueSetSz; XMEMCPY(output + idx, ea[i].value, ea[i].valueSz); - idx += ea[i].valueSz; + + fa = derArr[i]; + fa->data = output; + fa->dataSz = sz; } + + return 0; +} + + +/* Sort and Flatten EncodedAttrib attributes into output buffer */ +static int FlattenAttributes(PKCS7* pkcs7, byte* output, EncodedAttrib* ea, + int eaSz) +{ + int i, idx, ret; + FlatAttrib** derArr = NULL; + FlatAttrib* fa = NULL; + + if (pkcs7 == NULL || output == NULL || ea == NULL) { + return BAD_FUNC_ARG; + } + + /* create array of FlatAttrib struct pointers to hold DER attribs */ + derArr = (FlatAttrib**) XMALLOC(eaSz * sizeof(FlatAttrib*), pkcs7->heap, + DYNAMIC_TYPE_TMP_BUFFER); + if (derArr == NULL) { + return MEMORY_E; + } + XMEMSET(derArr, 0, eaSz); + + for (i = 0; i < eaSz; i++) { + derArr[i] = (FlatAttrib*) XMALLOC(sizeof(FlatAttrib), pkcs7->heap, + DYNAMIC_TYPE_TMP_BUFFER); + if (derArr[i] == NULL) { + FreeAttribArray(pkcs7, derArr, eaSz); + return MEMORY_E; + } + XMEMSET(derArr[i], 0, sizeof(FlatAttrib)); + } + + /* flatten EncodedAttrib into DER byte arrays */ + ret = FlattenEncodedAttribs(pkcs7, derArr, eaSz, ea, eaSz); + if (ret != 0) { + FreeAttribArray(pkcs7, derArr, eaSz); + return ret; + } + + /* SET OF DER signed attributes must be sorted in ascending order */ + ret = SortAttribArray(derArr, eaSz); + if (ret != 0) { + FreeAttribArray(pkcs7, derArr, eaSz); + return ret; + } + + /* copy sorted DER attribute arrays into output buffer */ + idx = 0; + for (i = 0; i < eaSz; i++) { + fa = derArr[i]; + XMEMCPY(output + idx, fa->data, fa->dataSz); + idx += fa->dataSz; + } + + FreeAttribArray(pkcs7, derArr, eaSz); + return 0; } @@ -1999,7 +2148,7 @@ static int PKCS7_EncodeSigned(PKCS7* pkcs7, ESD* esd, return MEMORY_E; } - FlattenAttributes(flatSignedAttribs, + FlattenAttributes(pkcs7, flatSignedAttribs, esd->signedAttribs, esd->signedAttribsCount); esd->signedAttribSetSz = SetImplicit(ASN_SET, 0, esd->signedAttribsSz, esd->signedAttribSet); @@ -9722,7 +9871,8 @@ int wc_PKCS7_EncodeAuthEnvelopedData(PKCS7* pkcs7, byte* output, return MEMORY_E; } - FlattenAttributes(flatAuthAttribs, authAttribs, authAttribsCount); + FlattenAttributes(pkcs7, flatAuthAttribs, authAttribs, + authAttribsCount); authAttribsSetSz = SetImplicit(ASN_SET, 1, authAttribsSz, authAttribSet); @@ -9766,7 +9916,8 @@ int wc_PKCS7_EncodeAuthEnvelopedData(PKCS7* pkcs7, byte* output, return MEMORY_E; } - FlattenAttributes(flatUnauthAttribs, unauthAttribs, unauthAttribsCount); + FlattenAttributes(pkcs7, flatUnauthAttribs, unauthAttribs, + unauthAttribsCount); unauthAttribsSetSz = SetImplicit(ASN_SET, 2, unauthAttribsSz, unauthAttribSet); } @@ -10713,7 +10864,7 @@ int wc_PKCS7_EncodeEncryptedData(PKCS7* pkcs7, byte* output, word32 outputSz) return MEMORY_E; } - FlattenAttributes(flatAttribs, attribs, attribsCount); + FlattenAttributes(pkcs7, flatAttribs, attribs, attribsCount); attribsSetSz = SetImplicit(ASN_SET, 1, attribsSz, attribSet); } else { From d51b95833446adbc4f431ed386e7d47b4736390c Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Tue, 16 Jul 2019 08:13:47 -0700 Subject: [PATCH 3/3] use ForceZero instead of MEMSET in PKCS7 signed attribute handling --- wolfcrypt/src/pkcs7.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 3a9c8bf8d..b0493455d 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1313,14 +1313,14 @@ static void FreeAttribArray(PKCS7* pkcs7, FlatAttrib** arr, int rows) for (i = 0; i < rows; i++) { if (arr[i]) { if (arr[i]->data) { - XMEMSET(arr[i]->data, 0, arr[i]->dataSz); + ForceZero(arr[i]->data, arr[i]->dataSz); XFREE(arr[i]->data, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); } - XMEMSET(arr[i], 0, sizeof(FlatAttrib)); + ForceZero(arr[i], sizeof(FlatAttrib)); XFREE(arr[i], pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); } } - XMEMSET(arr, 0, rows); + ForceZero(arr, rows); XFREE(arr, pkcs7->heap, DYNAMIC_TYPE_TMP_BUFFER); } (void)pkcs7; @@ -1426,7 +1426,7 @@ static int FlattenAttributes(PKCS7* pkcs7, byte* output, EncodedAttrib* ea, if (derArr == NULL) { return MEMORY_E; } - XMEMSET(derArr, 0, eaSz); + ForceZero(derArr, eaSz); for (i = 0; i < eaSz; i++) { derArr[i] = (FlatAttrib*) XMALLOC(sizeof(FlatAttrib), pkcs7->heap, @@ -1435,7 +1435,7 @@ static int FlattenAttributes(PKCS7* pkcs7, byte* output, EncodedAttrib* ea, FreeAttribArray(pkcs7, derArr, eaSz); return MEMORY_E; } - XMEMSET(derArr[i], 0, sizeof(FlatAttrib)); + ForceZero(derArr[i], sizeof(FlatAttrib)); } /* flatten EncodedAttrib into DER byte arrays */