[1/2] dlls/crypt32: implement PFXImportCertStore()

[Philippe Casgrain]

On 2010-03-25, at 12:14 PM, Juan Lang wrote:

> Hi Philippe,
> 
>>> You accept the PKCS12 file even if the password is incorrect.  This is
>>> clearly wrong.
>> 
>> It is not accepted. If the verification fails, ERR is spewed out and the next step (parse, below) will fail as well.
> 
> Is this how Windows fails?  That is, with a parse error?  Please add a
> test to cover this case.

I don't have any password-protected certificates to test with, so I can't add such a test (it was not required for our implementation).

> 
>>> You don't support more than a single certificate in the PKCS12 file.
>>> This may be fine for the majority of uses, but at least a warning
>>> indicating more certificates are present would be helpful.
>> 
>> Hmmm. How do you suggest I do that? From <http://www.openssl.org/docs/crypto/PKCS12_parse.html> I get this:
>> 
>>    BUGS
>> 
>>    Only a single private key and corresponding certificate is returned by this function. More complex PKCS#12
>>    files with multiple private keys will only return the first match.
> 
> Look at the 5th parameter of PKCS12_parse.  It's true that OpenSSL
> will only return a single certificate with a private key, but not
> every certificate in a PKCS12 file need contain a private key.

So what you want is to import two new functions (sk_X509_new_null() and sk_X509_free()) and use them to create a STACK_OF(X509) whose sole purpose is to detect if there are more certificates, and then ERR or TRACE if there are, and then dispose?

I added a FIXME comment.

>>> Also, a
>>> PKCS12 file can contain more than just certificates, and the tests
>>> ought at least to check this.  For example, what about a PKCS12 file
>>> with a CRL in it?
>> 
>> I have not seen, nor needed to implement this, so I'm not sure how to test for it. Maybe add a comment to the test? Or a wine_todo test so we don't lose this information?
> 
> Test for it the way you should any Wine test:  on Windows.  Create a
> store with a CRL in it, export it to a PKCS12 file, and use that as
> your test case.

I added the information as comments inside the test case. Mailing lists are harder to search than source code.

>>> The Crypto API also supports setting such attributes, and if you
>>> aren't going to support these, at least the tests should cover them
>>> (and marked todo_wine) so we know they're still not done.
>> 
>> Same answer. I guess I can update the test set with more wine_todo().
> 
> Yes, I'd appreciate that.

I did not realize that you wanted the actual set of test cases, disabled with todo_wine in front. I have added this information as comments inside the test case.

>> If you create a store with no name, you run the risk of it not being created (if there is another store with no name).
> 
> Not for a memory store, it's just a linked list.

Ah. In that case, it does not really matter what the string is, right? I can remove it if you don't want it.

Philippe