PKCS12Decoder.database_import() sometimes fails. All tests were done with python-nss-0.13.0 Two failure modes have been observed, either this error is returned ERROR [Errno -8099] (SEC_ERROR_PKCS12_UNABLE_TO_IMPORT_KEY) Unable to import. Error attempting to import private key.: 'PKCS12 decode import bags failed' or it will segfault. The failure is not consistent. On F17 i686 with nss versions 3.14.2-2 and 3.14.3-1 it runs successfully. However on F18 i686 with nss versions 3.14-7 and 3.14.3-1 it and it fails as well as on x86_64. Quick tests with valgrind on i686 did not report any memory usage problems but a more methodical approach is needed. When it does segfault it's deep inside NSS. Running pk12util on the same pkcs12 test file and NSS database was successful on on all arches and nss versions tested. The problem appears to be something in the binding but is not consistent, assuming uninitialized memory or incorrect data type declaration at the moment. Note, there was a memory corruption problem with pkcs12 decoding on 64-bit systems where the wrong data type was used resulting in a variable that was only half initialized (password length) but that was fixed, however we should verify the python-nss version being tested here also contains that fix and is not the potential culprit. The test pkcs12 file and a simple reproducer will be attached.
Created attachment 710852 [details] reproducer, use with matching p12 reproduer file
Created attachment 710853 [details] pkcs12 reproducer file
I found the problem, SEC_PKCS12DecoderStart copies the pointer to the pkcs12 password SECItem into it's decoder context, it does not however copy the contents, just the pointer. The SECItem was allocated on the stack, the decode operation occurred in a different stack frame, therefore the password data being referenced in the decoder context is pointing into a stack frame that no longer exists. Apparently the reason it sometimes worked and sometimes didn't had to do with whether the stack was data was overwritten or not. SEC_PKCS12DecoderStart should really copy the password SECItem using it's own arena. So instead of: p12dcx->pwitem = pwitem; it should be: SECItemCopyItem(p12dcx->arena, p12dcx->pwitem, pwitem); except once again the len field in the pwitem is abused in NSS :-( It's not the alloc len, it's been overwritten with the number of UCS2 characters in the pwitem! Therefore it can't do the sensible thing and decoder context data has to be manually managed outside the context, which is ugly and error prone. This bug confirms the abuse of the SECItem is error prone. I will attach a patch which manages the password SECItem in python-nss since it's not being managed by NSS.
Created attachment 742441 [details] manage password SECItem in Python object instead of on stack
python-nss-0.14.0-1.fc19 has been submitted as an update for Fedora 19. https://admin.fedoraproject.org/updates/python-nss-0.14.0-1.fc19
python-nss-0.14.0-1.fc19 has been pushed to the Fedora 19 stable repository. If problems still persist, please make note of it in this bug report.