From d09f26a69f34d624897c800d1fbd0be4ba138bab Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Mon, 19 Feb 2018 13:40:18 +1000 Subject: [PATCH 1/5] Support indefinite length BER encodings in PKCS #7 --- configure.ac | 15 ++++ wolfcrypt/src/asn.c | 122 ++++++++++++++++++++++++++++++++ wolfcrypt/src/error.c | 3 + wolfcrypt/src/pkcs7.c | 72 ++++++++++++++++++- wolfcrypt/test/test.c | 3 + wolfssl/wolfcrypt/asn.h | 2 + wolfssl/wolfcrypt/error-crypt.h | 4 +- wolfssl/wolfcrypt/pkcs7.h | 3 + 8 files changed, 222 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 59ae80a72..bf5d630ae 100644 --- a/configure.ac +++ b/configure.ac @@ -217,6 +217,7 @@ then enable_aeskeywrap=yes enable_x963kdf=yes enable_scrypt=yes + enable_indef=yes AM_CFLAGS="-DHAVE_AES_DECRYPT $AM_CFLAGS" fi @@ -2662,6 +2663,19 @@ fi AM_CONDITIONAL([BUILD_SRP], [test "x$ENABLED_SRP" = "xyes"]) +# Indefinite length encoded BER message support +AC_ARG_ENABLE([indef], + [AS_HELP_STRING([--enable-indef],[Enable parsing of indefinite length encoded msgs (default: disabled)])], + [ ENABLED_BER_INDEF=$enableval ], + [ ENABLED_BER_INDEF=no ] + ) + +if test "x$ENABLED_BER_INDEF" = "xyes" +then + AM_CFLAGS="$AM_CFLAGS -DASN_BER_TO_DER" +fi + + # Small Stack AC_ARG_ENABLE([smallstack], @@ -4247,6 +4261,7 @@ echo " * SIGNAL: $ENABLED_SIGNAL" echo " * ERROR_STRINGS: $ENABLED_ERROR_STRINGS" echo " * DTLS: $ENABLED_DTLS" echo " * SCTP: $ENABLED_SCTP" +echo " * Indefinite Length: $ENABLED_BER_INDEF" echo " * Multicast: $ENABLED_MCAST" echo " * Old TLS Versions: $ENABLED_OLD_TLS" echo " * SSL version 3.0: $ENABLED_SSLV3" diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 63d842390..d90931ecd 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -786,6 +786,128 @@ static word32 SetBitString(word32 len, byte unusedBits, byte* output) } #endif /* !NO_RSA || HAVE_ECC || HAVE_ED25519 */ +#ifdef ASN_BER_TO_DER +/* Convert a BER encoding with indefinite length items to DER. + * + * ber BER encoded data. + * length Length of BER encoded data. + * der Buffer to hold DER encoded version of data. + * NULL indicates only the length is required. + * returns the length of the DER data on success, ASN_PARSE_E if the BER data is + * invalid and BAD_FUNC_ARG if ber is NULL. + */ +int wc_BerToDer(byte* ber, word32 length, byte* der) +{ + int ret; + word32 i, j, k; + int len, l; + int indef; + int depth = 1; + word32 cnt; + byte lenBytes[4]; + + if (ber == NULL) + return BAD_FUNC_ARG; + + for (i = 0, j = 0; i < length; ) { + if (i + 1 >= length) + return ASN_PARSE_E; + + if (ber[i] == 0 && ber[i+1] == 0) { + if (--depth == 0) + break; + + i += 2; + continue; + } + + indef = ber[i+1] == 0x80; + if (indef && ber[i] < 0x40 && ber[i] != 0x30 && ber[i] != 0x31) { + if (der != NULL) + der[j] = ber[i] & 0x1f; + i++; j++; + i++; + + if (i + 1 >= length) + return ASN_PARSE_E; + + len = 0; + k = i; + while (ber[k] != 0x00) { + k++; + ret = GetLength(ber, &k, &l, length); + if (ret < 0) + return ASN_PARSE_E; + k += l; + len += l; + + if (k + 1 >= length) + return ASN_PARSE_E; + } + + if (der == NULL) { + j += SetLength(len, lenBytes); + j += len; + } + else { + j += SetLength(len, der + j); + k = i; + while (ber[k] != 0x00) { + k++; + ret = GetLength(ber, &k, &l, length); + if (ret < 0) + return ASN_PARSE_E; + XMEMCPY(der + j, ber + k, l); + k += l; j += l; + } + } + i = k + 2; + + continue; + } + + if (der != NULL) + der[j] = ber[i]; + i++; j++; + + cnt = i; + ret = GetLength(ber, &cnt, &len, length); + if (ret < 0) + return ASN_PARSE_E; + cnt -= i; + if (der != NULL) { + for (k = 0; k < cnt; k++) + der[j + k] = ber[i + k]; + } + i += cnt; j += cnt; + if (cnt == 0) { + i++; + len = wc_BerToDer(ber + i, length - i, NULL); + if (len < 0) + return len; + if (der != NULL) + j += SetLength(len, der + j); + else + j += SetLength(len, lenBytes); + } + + if (!indef) { + if (i + len > length) + return ASN_PARSE_E; + + if (der != NULL) + XMEMCPY(der + j, ber + i, len); + i += len; + j += len; + } + else + depth++; + } + + return j; +} +#endif + #if defined(WOLFSSL_CERT_GEN) || defined(WOLFSSL_KEY_GEN) #if (!defined(NO_RSA) && !defined(HAVE_USER_RSA)) || \ diff --git a/wolfcrypt/src/error.c b/wolfcrypt/src/error.c index 7b52791c9..414b77a3f 100644 --- a/wolfcrypt/src/error.c +++ b/wolfcrypt/src/error.c @@ -449,6 +449,9 @@ const char* wc_GetErrorString(int error) case PRIME_GEN_E: return "Unable to find a prime for RSA key"; + case BER_INDEF_E: + return "Unable to decode an indefinite length encoded message"; + default: return "unknown error number"; diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 10287d1ad..9432a09f9 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -338,6 +338,11 @@ void wc_PKCS7_Free(PKCS7* pkcs7) return; wc_PKCS7_FreeDecodedAttrib(pkcs7->decodedAttrib, pkcs7->heap); + +#ifdef ASN_BER_TO_DER + if (pkcs7->der != NULL) + XFREE(pkcs7->der, pkcs7->heap, DYNAMIC_TYPE_PKCS7); +#endif } @@ -1849,6 +1854,28 @@ int wc_PKCS7_VerifySignedData(PKCS7* pkcs7, byte* pkiMsg, word32 pkiMsgSz) if (GetSequence(pkiMsg, &idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; + if (length == 0 && pkiMsg[idx-1] == 0x80) { +#ifdef ASN_BER_TO_DER + int len; + + len = wc_BerToDer(pkiMsg, pkiMsgSz, NULL); + if (len < 0) + return len; + pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + if (pkcs7->der == NULL) + return MEMORY_E; + wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der); + + pkiMsg = pkcs7->der; + pkiMsgSz = len; + idx = 0; + if (GetSequence(pkiMsg, &idx, &length, pkiMsgSz) < 0) + return ASN_PARSE_E; +#else + return BER_INDEF_E; +#endif + } + /* Get the contentInfo contentType */ if (wc_GetContentType(pkiMsg, &idx, &contentType, pkiMsgSz) < 0) return ASN_PARSE_E; @@ -4299,6 +4326,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* pkiMsg, int encryptedContentSz; byte padLen; byte* encryptedContent = NULL; + int explicitOctet; if (pkcs7 == NULL || pkcs7->singleCert == NULL || pkcs7->singleCertSz == 0 || pkcs7->privateKey == NULL || @@ -4313,6 +4341,28 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* pkiMsg, if (GetSequence(pkiMsg, &idx, &length, pkiMsgSz) < 0) return ASN_PARSE_E; + if (length == 0 && pkiMsg[idx-1] == 0x80) { +#ifdef ASN_BER_TO_DER + int len; + + len = wc_BerToDer(pkiMsg, pkiMsgSz, NULL); + if (len < 0) + return len; + pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + if (pkcs7->der == NULL) + return MEMORY_E; + wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der); + + pkiMsg = pkcs7->der; + pkiMsgSz = len; + idx = 0; + if (GetSequence(pkiMsg, &idx, &length, pkiMsgSz) < 0) + return ASN_PARSE_E; +#else + return BER_INDEF_E; +#endif + } + if (wc_GetContentType(pkiMsg, &idx, &contentType, pkiMsgSz) < 0) return ASN_PARSE_E; @@ -4435,13 +4485,17 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* pkiMsg, XMEMCPY(tmpIv, &pkiMsg[idx], length); idx += length; + explicitOctet = pkiMsg[idx] == (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | 0); + /* read encryptedContent, cont[0] */ - if (pkiMsg[idx++] != (ASN_CONTEXT_SPECIFIC | 0)) { + if (pkiMsg[idx] != (ASN_CONTEXT_SPECIFIC | 0) && + pkiMsg[idx] != (ASN_CONTEXT_SPECIFIC | ASN_CONSTRUCTED | 0)) { #ifdef WOLFSSL_SMALL_STACK XFREE(decryptedKey, NULL, DYNAMIC_TYPE_PKCS7); #endif return ASN_PARSE_E; } + idx++; if (GetLength(pkiMsg, &idx, &encryptedContentSz, pkiMsgSz) <= 0) { #ifdef WOLFSSL_SMALL_STACK @@ -4450,6 +4504,22 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* pkiMsg, return ASN_PARSE_E; } + if (explicitOctet) { + if (pkiMsg[idx++] != ASN_OCTET_STRING) { +#ifdef WOLFSSL_SMALL_STACK + XFREE(decryptedKey, NULL, DYNAMIC_TYPE_PKCS7); +#endif + return ASN_PARSE_E; + } + + if (GetLength(pkiMsg, &idx, &encryptedContentSz, pkiMsgSz) <= 0) { +#ifdef WOLFSSL_SMALL_STACK + XFREE(decryptedKey, NULL, DYNAMIC_TYPE_PKCS7); +#endif + return ASN_PARSE_E; + } + } + encryptedContent = (byte*)XMALLOC(encryptedContentSz, pkcs7->heap, DYNAMIC_TYPE_PKCS7); if (encryptedContent == NULL) { diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index f9d58ed21..d26f00957 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -16702,6 +16702,9 @@ int pkcs7encrypted_test(void) pkcs7.unprotectedAttribs = testVectors[i].attribs; pkcs7.unprotectedAttribsSz = testVectors[i].attribsSz; pkcs7.heap = HEAP_HINT; +#ifdef ASN_BER_TO_DER + pkcs7.der = NULL; +#endif /* encode encryptedData */ encryptedSz = wc_PKCS7_EncodeEncryptedData(&pkcs7, encrypted, diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 5777e1dd9..8e06317da 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -765,6 +765,8 @@ struct TrustedPeerCert { #define WOLFSSL_ASN_API WOLFSSL_LOCAL #endif +WOLFSSL_ASN_API int wc_BerToDer(byte* ber, word32 length, byte* der); + WOLFSSL_ASN_API void FreeAltNames(DNS_entry*, void*); #ifndef IGNORE_NAME_CONSTRAINTS WOLFSSL_ASN_API void FreeNameSubtrees(Base_entry*, void*); diff --git a/wolfssl/wolfcrypt/error-crypt.h b/wolfssl/wolfcrypt/error-crypt.h index c047dfec9..6088bd9cb 100644 --- a/wolfssl/wolfcrypt/error-crypt.h +++ b/wolfssl/wolfcrypt/error-crypt.h @@ -198,7 +198,9 @@ enum { PSS_SALTLEN_E = -250, /* PSS length of salt is to long for hash */ PRIME_GEN_E = -251, /* Failure finding a prime. */ - WC_LAST_E = -251, /* Update this to indicate last error */ + BER_INDEF_E = -252, /* Cannot decode indefinite length BER. */ + + WC_LAST_E = -252, /* Update this to indicate last error */ MIN_CODE_E = -300 /* errors -101 - -299 */ /* add new companion error id strings for any new error codes diff --git a/wolfssl/wolfcrypt/pkcs7.h b/wolfssl/wolfcrypt/pkcs7.h index efa3d7d18..ca6afec7c 100644 --- a/wolfssl/wolfcrypt/pkcs7.h +++ b/wolfssl/wolfcrypt/pkcs7.h @@ -100,6 +100,9 @@ typedef struct PKCS7 { byte* issuer; /* issuer name of singleCert */ byte* privateKey; /* private key, DER, not owner */ void* heap; /* heap hint for dynamic memory */ +#ifdef ASN_BER_TO_DER + byte* der; /* DER encoded version of message */ +#endif byte* cert[MAX_PKCS7_CERTS]; /* Encrypted-data Content Type */ From 6dad94c0faa221d1c2eb5164120d76e023725ea0 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 20 Feb 2018 09:49:56 +1000 Subject: [PATCH 2/5] Change wc_BerToDer signature to have length as param Clean up code to make readable --- wolfcrypt/src/asn.c | 174 +++++++++++++++++++++++++++++----------- wolfcrypt/src/pkcs7.c | 24 +++--- wolfssl/wolfcrypt/asn.h | 6 +- 3 files changed, 144 insertions(+), 60 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index d90931ecd..c5f82a610 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -789,30 +789,43 @@ static word32 SetBitString(word32 len, byte unusedBits, byte* output) #ifdef ASN_BER_TO_DER /* Convert a BER encoding with indefinite length items to DER. * - * ber BER encoded data. - * length Length of BER encoded data. - * der Buffer to hold DER encoded version of data. - * NULL indicates only the length is required. - * returns the length of the DER data on success, ASN_PARSE_E if the BER data is - * invalid and BAD_FUNC_ARG if ber is NULL. + * ber BER encoded data. + * berSz Length of BER encoded data. + * der Buffer to hold DER encoded version of data. + * NULL indicates only the length is required. + * derSz The size of the buffer to hold the DER encoded data. + * Will be set if der is NULL, otherwise the value is checked as der is + * filled. + * returns ASN_PARSE_E if the BER data is invalid and BAD_FUNC_ARG if ber or + * derSz are NULL. */ -int wc_BerToDer(byte* ber, word32 length, byte* der) +int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) { int ret; word32 i, j, k; int len, l; int indef; int depth = 1; - word32 cnt; + byte type; + word32 cnt, sz; + word32 outSz; byte lenBytes[4]; - if (ber == NULL) + if (ber == NULL || derSz == NULL) return BAD_FUNC_ARG; - for (i = 0, j = 0; i < length; ) { - if (i + 1 >= length) + outSz = *derSz; + + for (i = 0, j = 0; i < berSz; ) { + /* Check that there is data for an ASN item to parse. */ + if (i + 1 > berSz) return ASN_PARSE_E; + /* End Of Content (EOC) mark end of indefinite length items. + * EOCs are not encoded in DER. + * Keep track of no. indefinite length items that have not been + * terminated in depth. + */ if (ber[i] == 0 && ber[i+1] == 0) { if (--depth == 0) break; @@ -821,90 +834,155 @@ int wc_BerToDer(byte* ber, word32 length, byte* der) continue; } - indef = ber[i+1] == 0x80; - if (indef && ber[i] < 0x40 && ber[i] != 0x30 && ber[i] != 0x31) { - if (der != NULL) - der[j] = ber[i] & 0x1f; + /* Indefinite length is encoded as: 0x80 */ + type = ber[i]; + indef = ber[i+1] == ASN_INDEF_LENGTH; + if (indef && (type & 0xC0) == 0 && + ber[i] != (ASN_SEQUENCE | ASN_CONSTRUCTED) && + ber[i] != (ASN_SET | ASN_CONSTRUCTED)) { + /* Indefinite length OCTET STRING or other simple type. + * Put all the data into one entry. + */ + + /* Type no longer constructed. */ + type &= ~ASN_CONSTRUCTED; + if (der != NULL) { + /* Ensure space for type. */ + if (j + 1 >= outSz) + return BUFFER_E; + der[j] = type; + } i++; j++; + /* Skip indefinite length. */ i++; - if (i + 1 >= length) + /* There must be further ASN1 items to combine. */ + if (i + 1 >= berSz) return ASN_PARSE_E; + /* Calculate length of combined data. */ len = 0; k = i; while (ber[k] != 0x00) { + /* Each ASN item must be the same type as the constructed. */ + if (ber[k] != type) + return ASN_PARSE_E; k++; - ret = GetLength(ber, &k, &l, length); + + ret = GetLength(ber, &k, &l, berSz); if (ret < 0) return ASN_PARSE_E; k += l; len += l; - if (k + 1 >= length) + /* Must at least have terminating EOC. */ + if (k + 1 >= berSz) return ASN_PARSE_E; } + /* Ensure a valid EOC ASN item. */ + if (ber[k+1] != 0x00) + return ASN_PARSE_E; if (der == NULL) { + /* Add length of ASN item length encoding and data. */ j += SetLength(len, lenBytes); j += len; } else { + /* Check space for encoded length. */ + if (SetLength(len, lenBytes) > outSz - j) + return BUFFER_E; + /* Encode new length. */ j += SetLength(len, der + j); + + /* Encode data in single item. */ k = i; while (ber[k] != 0x00) { + /* Skip ASN type. */ k++; - ret = GetLength(ber, &k, &l, length); + + /* Find length of data in ASN item. */ + ret = GetLength(ber, &k, &l, berSz); if (ret < 0) return ASN_PARSE_E; + + /* Ensure space for data and copy in. */ + if (j + l > outSz) + return BUFFER_E; XMEMCPY(der + j, ber + k, l); k += l; j += l; } } + /* Continue conversion after EOC. */ i = k + 2; continue; } - if (der != NULL) + if (der != NULL) { + /* Ensure space for type and at least one byte of length. */ + if (j + 1 >= outSz) + return BUFFER_E; + /* Put in type. */ der[j] = ber[i]; + } i++; j++; - cnt = i; - ret = GetLength(ber, &cnt, &len, length); - if (ret < 0) - return ASN_PARSE_E; - cnt -= i; - if (der != NULL) { - for (k = 0; k < cnt; k++) - der[j + k] = ber[i + k]; - } - i += cnt; j += cnt; - if (cnt == 0) { + if (indef) { + /* Skip indefinite length. */ i++; - len = wc_BerToDer(ber + i, length - i, NULL); - if (len < 0) - return len; - if (der != NULL) - j += SetLength(len, der + j); - else - j += SetLength(len, lenBytes); - } + /* Calculate the size of the data inside constructed. */ + ret = wc_BerToDer(ber + i, berSz - i, NULL, &sz); + if (ret != LENGTH_ONLY_E) + return ret; - if (!indef) { - if (i + len > length) + if (der != NULL) { + /* Ensure space for encoded length. */ + if (SetLength(sz, lenBytes) > outSz - j) + return BUFFER_E; + /* Encode real length. */ + j += SetLength(sz, der + j); + } + else { + /* Add size of encoded length. */ + j += SetLength(sz, lenBytes); + } + + /* Another EOC to find. */ + depth++; + } + else { + /* Get the size of the encode length and length value. */ + cnt = i; + ret = GetLength(ber, &cnt, &len, berSz); + if (ret < 0) + return ASN_PARSE_E; + cnt -= i; + + /* Check there is enough data to copy out. */ + if (i + cnt + len > berSz) return ASN_PARSE_E; - if (der != NULL) - XMEMCPY(der + j, ber + i, len); - i += len; - j += len; + if (der != NULL) { + /* Ensure space in DER buffer. */ + if (j + cnt + len > outSz) + return BUFFER_E; + /* Copy length and data into DER buffer. */ + XMEMCPY(der + j, ber + i, cnt + len); + } + /* Continue conversion after this ASN item. */ + i += cnt + len; + j += cnt + len; } - else - depth++; } - return j; + /* Return length if no buffer to write to. */ + if (der == NULL) { + *derSz = j; + return LENGTH_ONLY_E; + } + + return 0; } #endif diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 9432a09f9..be00af375 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1856,15 +1856,17 @@ int wc_PKCS7_VerifySignedData(PKCS7* pkcs7, byte* pkiMsg, word32 pkiMsgSz) if (length == 0 && pkiMsg[idx-1] == 0x80) { #ifdef ASN_BER_TO_DER - int len; + word32 len; - len = wc_BerToDer(pkiMsg, pkiMsgSz, NULL); - if (len < 0) - return len; + ret = wc_BerToDer(pkiMsg, pkiMsgSz, NULL, &len); + if (ret != LENGTH_ONLY_E) + return ret; pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); if (pkcs7->der == NULL) return MEMORY_E; - wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der); + ret = wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der, &len); + if (ret < 0) + return ret; pkiMsg = pkcs7->der; pkiMsgSz = len; @@ -4343,15 +4345,17 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* pkiMsg, if (length == 0 && pkiMsg[idx-1] == 0x80) { #ifdef ASN_BER_TO_DER - int len; + word32 len; - len = wc_BerToDer(pkiMsg, pkiMsgSz, NULL); - if (len < 0) - return len; + ret = wc_BerToDer(pkiMsg, pkiMsgSz, NULL, &len); + if (ret != LENGTH_ONLY_E) + return ret; pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); if (pkcs7->der == NULL) return MEMORY_E; - wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der); + ret = wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der, &len); + if (ret < 0) + return ret; pkiMsg = pkcs7->der; pkiMsgSz = len; diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 8e06317da..3be1029c3 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -86,7 +86,8 @@ enum ASN_Tags { ASN_GENERALIZED_TIME = 0x18, CRL_EXTENSIONS = 0xa0, ASN_EXTENSIONS = 0xa3, - ASN_LONG_LENGTH = 0x80 + ASN_LONG_LENGTH = 0x80, + ASN_INDEF_LENGTH = 0x80 }; #define ASN_UTC_TIME_SIZE 14 @@ -765,7 +766,8 @@ struct TrustedPeerCert { #define WOLFSSL_ASN_API WOLFSSL_LOCAL #endif -WOLFSSL_ASN_API int wc_BerToDer(byte* ber, word32 length, byte* der); +WOLFSSL_ASN_API int wc_BerToDer(byte* ber, word32 berSz, byte* der, + word32* derSz); WOLFSSL_ASN_API void FreeAltNames(DNS_entry*, void*); #ifndef IGNORE_NAME_CONSTRAINTS From 3dfc2d87f359802c49c5632b9efa3b475f654d5f Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Tue, 20 Feb 2018 11:22:11 +1000 Subject: [PATCH 3/5] Fix leak when wc_PKCS7_InitWithCert is called in verify --- wolfcrypt/src/pkcs7.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index be00af375..595411bee 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1844,6 +1844,9 @@ int wc_PKCS7_VerifySignedData(PKCS7* pkcs7, byte* pkiMsg, word32 pkiMsgSz) byte* signedAttrib = NULL; int contentSz = 0, sigSz = 0, certSz = 0, signedAttribSz = 0; byte degenerate; +#ifdef ASN_BER_TO_DER + byte* der; +#endif if (pkcs7 == NULL || pkiMsg == NULL || pkiMsgSz == 0) return BAD_FUNC_ARG; @@ -1988,8 +1991,14 @@ int wc_PKCS7_VerifySignedData(PKCS7* pkcs7, byte* pkiMsg, word32 pkiMsgSz) certSz += (certIdx - idx); } +#ifdef ASN_BER_TO_DER + der = pkcs7->der; +#endif /* This will reset PKCS7 structure and then set the certificate */ wc_PKCS7_InitWithCert(pkcs7, cert, certSz); +#ifdef ASN_BER_TO_DER + pkcs7->der = der; +#endif /* iterate through any additional certificates */ if (MAX_PKCS7_CERTS > 0) { From 76b0464a3be0f7111fb8a132a62498de1d3507fb Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 22 Feb 2018 08:31:19 +1000 Subject: [PATCH 4/5] Fixes from review --- wolfcrypt/src/pkcs7.c | 4 ++-- wolfcrypt/test/test.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/wolfcrypt/src/pkcs7.c b/wolfcrypt/src/pkcs7.c index 595411bee..b6fe2bcd6 100644 --- a/wolfcrypt/src/pkcs7.c +++ b/wolfcrypt/src/pkcs7.c @@ -1864,7 +1864,7 @@ int wc_PKCS7_VerifySignedData(PKCS7* pkcs7, byte* pkiMsg, word32 pkiMsgSz) ret = wc_BerToDer(pkiMsg, pkiMsgSz, NULL, &len); if (ret != LENGTH_ONLY_E) return ret; - pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + pkcs7->der = (byte*)XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); if (pkcs7->der == NULL) return MEMORY_E; ret = wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der, &len); @@ -4359,7 +4359,7 @@ WOLFSSL_API int wc_PKCS7_DecodeEnvelopedData(PKCS7* pkcs7, byte* pkiMsg, ret = wc_BerToDer(pkiMsg, pkiMsgSz, NULL, &len); if (ret != LENGTH_ONLY_E) return ret; - pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); + pkcs7->der = (byte*)XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); if (pkcs7->der == NULL) return MEMORY_E; ret = wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der, &len); diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index d26f00957..6d9b5bbbe 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -16693,6 +16693,10 @@ int pkcs7encrypted_test(void) testSz = sizeof(testVectors) / sizeof(pkcs7EncryptedVector); for (i = 0; i < testSz; i++) { + ret = wc_PKCS7_Init(&pkcs7, HEAP_HINT, devId); + if (ret != 0) + return -7599; + pkcs7.content = (byte*)testVectors[i].content; pkcs7.contentSz = testVectors[i].contentSz; pkcs7.contentOID = testVectors[i].contentOID; @@ -16701,10 +16705,6 @@ int pkcs7encrypted_test(void) pkcs7.encryptionKeySz = testVectors[i].encryptionKeySz; pkcs7.unprotectedAttribs = testVectors[i].attribs; pkcs7.unprotectedAttribsSz = testVectors[i].attribsSz; - pkcs7.heap = HEAP_HINT; -#ifdef ASN_BER_TO_DER - pkcs7.der = NULL; -#endif /* encode encryptedData */ encryptedSz = wc_PKCS7_EncodeEncryptedData(&pkcs7, encrypted, From 274110a10c99a70076f1fb87149d5becf6a57dd4 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 22 Feb 2018 14:48:14 +1000 Subject: [PATCH 5/5] Added tests and fixes from testing --- wolfcrypt/src/asn.c | 15 +++-- wolfcrypt/test/test.c | 118 ++++++++++++++++++++++++++++++++++++++++ wolfssl/wolfcrypt/asn.h | 2 +- 3 files changed, 129 insertions(+), 6 deletions(-) diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index c5f82a610..e3c07e625 100755 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -799,13 +799,13 @@ static word32 SetBitString(word32 len, byte unusedBits, byte* output) * returns ASN_PARSE_E if the BER data is invalid and BAD_FUNC_ARG if ber or * derSz are NULL. */ -int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) +int wc_BerToDer(const byte* ber, word32 berSz, byte* der, word32* derSz) { int ret; word32 i, j, k; int len, l; int indef; - int depth = 1; + int depth = 0; byte type; word32 cnt, sz; word32 outSz; @@ -818,7 +818,7 @@ int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) for (i = 0, j = 0; i < berSz; ) { /* Check that there is data for an ASN item to parse. */ - if (i + 1 > berSz) + if (i + 2 > berSz) return ASN_PARSE_E; /* End Of Content (EOC) mark end of indefinite length items. @@ -827,6 +827,8 @@ int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) * terminated in depth. */ if (ber[i] == 0 && ber[i+1] == 0) { + if (depth == 0) + break; if (--depth == 0) break; @@ -857,7 +859,7 @@ int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) i++; /* There must be further ASN1 items to combine. */ - if (i + 1 >= berSz) + if (i + 2 > berSz) return ASN_PARSE_E; /* Calculate length of combined data. */ @@ -876,7 +878,7 @@ int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) len += l; /* Must at least have terminating EOC. */ - if (k + 1 >= berSz) + if (k + 2 > berSz) return ASN_PARSE_E; } /* Ensure a valid EOC ASN item. */ @@ -976,6 +978,9 @@ int wc_BerToDer(byte* ber, word32 berSz, byte* der, word32* derSz) } } + if (depth >= 1) + return ASN_PARSE_E; + /* Return length if no buffer to write to. */ if (der == NULL) { *derSz = j; diff --git a/wolfcrypt/test/test.c b/wolfcrypt/test/test.c index 6d9b5bbbe..45678923d 100644 --- a/wolfcrypt/test/test.c +++ b/wolfcrypt/test/test.c @@ -316,6 +316,9 @@ int memory_test(void); #ifdef HAVE_VALGRIND int mp_test(void); #endif +#ifdef ASN_BER_TO_DER +int berder_test(void); +#endif int logging_test(void); int mutex_test(void); #if defined(USE_WOLFSSL_MEMORY) && !defined(FREERTOS) @@ -883,6 +886,13 @@ int wolfcrypt_test(void* args) printf( "mp test passed!\n"); #endif +#ifdef ASN_BER_TO_DER + if ( (ret = berder_test()) != 0) + return err_sys("ber-der test failed!\n", ret); + else + printf( "ber-der test passed!\n"); +#endif + if ( (ret = logging_test()) != 0) return err_sys("logging test failed!\n", ret); else @@ -17352,6 +17362,114 @@ done: } #endif +#ifdef ASN_BER_TO_DER +typedef struct berDerTestData { + const byte *in; + word32 inSz; + const byte *out; + word32 outSz; +} berDerTestData; + +int berder_test(void) +{ + int ret; + int i; + word32 len, l; + byte out[32]; + static const byte good1_in[] = { 0x30, 0x80, 0x00, 0x00 }; + static const byte good1_out[] = { 0x30, 0x00 }; + static const byte good2_in[] = { 0x30, 0x80, 0x02, 0x01, 0x01, 0x00, 0x00 }; + static const byte good2_out[] = { 0x30, 0x03, 0x02, 0x01, 0x01 }; + static const byte good3_in[] = { + 0x24, 0x80, 0x04, 0x01, 0x01, 0x00, 0x00 + }; + static const byte good3_out[] = { 0x04, 0x1, 0x01 }; + static const byte good4_in[] = { + 0x30, 0x80, + 0x02, 0x01, 0x01, + 0x30, 0x80, + 0x24, 0x80, + 0x04, 0x01, 0x01, + 0x04, 0x02, 0x02, 0x03, + 0x00, 0x00, + 0x06, 0x01, 0x01, + 0x00, 0x00, + 0x31, 0x80, + 0x06, 0x01, 0x01, + 0x00, 0x00, + 0x00, 0x00, + }; + static const byte good4_out[] = { + 0x30, 0x0d, + 0x02, 0x01, 0x01, + 0x30, 0x08, + 0x04, 0x03, 0x01, 0x02, 0x03, + 0x06, 0x01, 0x01, + 0x31, 0x03, + 0x06, 0x01, 0x01 + }; + + berDerTestData testData[] = { + { good1_in, sizeof(good1_in), good1_out, sizeof(good1_out) }, + { good2_in, sizeof(good2_in), good2_out, sizeof(good2_out) }, + { good3_in, sizeof(good3_in), good3_out, sizeof(good3_out) }, + { good4_in, sizeof(good4_in), good4_out, sizeof(good4_out) }, + }; + + for (i = 0; i < (int)(sizeof(testData) / sizeof(*testData)); i++) { + ret = wc_BerToDer(testData[i].in, testData[i].inSz, NULL, &len); + if (ret != LENGTH_ONLY_E) + return -7830 - i; + if (len != testData[i].outSz) + return -7840 - i; + len = testData[i].outSz; + ret = wc_BerToDer(testData[i].in, testData[i].inSz, out, &len); + if (ret != 0) + return -7850 - i; + if (XMEMCMP(out, testData[i].out, len) != 0) + return -7860 - i; + + for (l = 1; l < testData[i].inSz; l++) { + ret = wc_BerToDer(testData[i].in, l, NULL, &len); + if (ret != ASN_PARSE_E) + return -7870; + len = testData[i].outSz; + ret = wc_BerToDer(testData[i].in, l, out, &len); + if (ret != ASN_PARSE_E) + return -7871; + } + } + + ret = wc_BerToDer(NULL, 4, NULL, NULL); + if (ret != BAD_FUNC_ARG) + return -7880; + ret = wc_BerToDer(out, 4, NULL, NULL); + if (ret != BAD_FUNC_ARG) + return -7881; + ret = wc_BerToDer(NULL, 4, NULL, &len); + if (ret != BAD_FUNC_ARG) + return -7882; + ret = wc_BerToDer(NULL, 4, out, NULL); + if (ret != BAD_FUNC_ARG) + return -7883; + ret = wc_BerToDer(out, 4, out, NULL); + if (ret != BAD_FUNC_ARG) + return -7884; + ret = wc_BerToDer(NULL, 4, out, &len); + if (ret != BAD_FUNC_ARG) + return -7885; + + for (l = 1; l < sizeof(good4_out); l++) { + len = l; + ret = wc_BerToDer(good4_in, sizeof(good4_in), out, &len); + if (ret != BUFFER_E) + return -7890; + } + + return 0; +} +#endif + #ifdef DEBUG_WOLFSSL static THREAD_LS_T int log_cnt = 0; static void my_Logging_cb(const int logLevel, const char *const logMessage) diff --git a/wolfssl/wolfcrypt/asn.h b/wolfssl/wolfcrypt/asn.h index 3be1029c3..2d43c6a89 100644 --- a/wolfssl/wolfcrypt/asn.h +++ b/wolfssl/wolfcrypt/asn.h @@ -766,7 +766,7 @@ struct TrustedPeerCert { #define WOLFSSL_ASN_API WOLFSSL_LOCAL #endif -WOLFSSL_ASN_API int wc_BerToDer(byte* ber, word32 berSz, byte* der, +WOLFSSL_ASN_API int wc_BerToDer(const byte* ber, word32 berSz, byte* der, word32* derSz); WOLFSSL_ASN_API void FreeAltNames(DNS_entry*, void*);