Fix test_create_with_mac_iter accidently setting keytype not maciter #762
Conversation
Looks like their mirror is having a bad day, I'll come back and re-run the tests later. |
|
#763 - this'll fix the build. |
There was a problem hiding this comment.
Good catch!
I think this is where the MSIE keytype is explained.
https://learn.microsoft.com/en-us/windows/win32/api/wincrypt/nf-wincrypt-pfximportcertstore
An X.509 KeyUsage is supposed to be a 9-bit bit field. But looking at the OpenSSL code, it doesn't seem to intend to support anything other than the two values.
- https://github.com/openssl/openssl/blob/0285160ffa3b8c2b5491222243042593808298c4/crypto/pkcs12/p12_attr.c#L29-L34 is taking only the lower 8 bits. This function would also create an illegal
ASN1_BIT_STRINGif the lower 8 bits are all zero, which leads to the out-of-bounds read as caught by ASAN. - https://github.com/openssl/openssl/blob/0285160ffa3b8c2b5491222243042593808298c4/apps/pkcs12.c#L208-L213 is overwriting the
keytypevalue instead of or'ing together.
The change looks good to me. Thank you!
This test was accidentally passing the value 2048 into the keytype parameter of PKCS12_create, not the mac_iter parameter (because it had one too many `nil`s in the call). This value is invalid, and will make OpenSSL perform an out-of-bounds read which is caught when compiling with ASAN. This commit fixes the tests, and also adds some validation to PKCS12.create to make sure any keytype passed is actually valid. Since there only two valid keytype constants, and the whole feature is an export-grade crypto era thing only ever supported by old MSIE, it seems far more likely that code in the whild is using keytype similarly by mistake rather than as intended. So this validation might catch that.
KJTsanaktsidis
force-pushed
the
ktsanaktsidis/fix_asan_error
branch
from
d6d4c44 to
4702868
Compare
Ah, thanks for explaining this - I was looking at |
This test was accidentally passing the value 2048 into the keytype parameter of PKCS12_create, not the mac_iter parameter (because it had one too many
nils in the call). This value is invalid, and will make OpenSSL perform an out-of-bounds read which is caught when compiling with ASAN.This commit fixes the tests, and also adds some validation to PKCS12.create to make sure any keytype passed is actually valid. Since there only two valid keytype constants, and the whole feature is an export-grade crypto era thing only ever supported by old MSIE, it seems far more likely that code in the wild is using keytype similarly by mistake rather than as intended. So this validation might catch that.