Support indefinite length BER encodings in PKCS #7 #1380
Conversation
SparkiDev
force-pushed
the
ber_indef
branch
2 times, most recently
from
53e6812 to
5158da6
Compare
wolfcrypt/src/asn.c
Outdated
| * 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) |
There was a problem hiding this comment.
Since this is a public function we should have a length input for Der size as well so we don't overwrite the Der buffer. It will also catch programming errors on our side.
wolfcrypt/src/pkcs7.c
Outdated
| pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); | ||
| if (pkcs7->der == NULL) | ||
| return MEMORY_E; | ||
| wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der); |
There was a problem hiding this comment.
We should check the return code here, there's no guarantee in the future that if the first call succeeds on the length check that the second call will succeed as well. Also a good static analyzer should warn on this.
wolfcrypt/src/pkcs7.c
Outdated
| pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); | ||
| if (pkcs7->der == NULL) | ||
| return MEMORY_E; | ||
| wc_BerToDer(pkiMsg, pkiMsgSz, pkcs7->der); |
There was a problem hiding this comment.
return code check needed.
|
Looks good, please resolve conflict. |
wolfcrypt/src/pkcs7.c
Outdated
| ret = wc_BerToDer(pkiMsg, pkiMsgSz, NULL, &len); | ||
| if (ret != LENGTH_ONLY_E) | ||
| return ret; | ||
| pkcs7->der = XMALLOC(len, pkcs7->heap, DYNAMIC_TYPE_PKCS7); |
There was a problem hiding this comment.
We find results from XMALLOC need to have a cast in front due to some compilers. (byte*)XMALLOC().
wolfcrypt/test/test.c
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
Recommend doing an wc_PKCS7_Init above and remove this. Our test examples should do the init, so the PKCS7 struct is memset to 0.
|
@dgarske sent an example case by email just in case we still need one. |
Also support explicit octet string in enveloped data.