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

Juan Lang juan.lang at gmail.com
Mon Mar 22 13:40:46 CDT 2010


Hi Phillippe,

this attempt looks pretty incomplete.  First off:

+    ret = pPKCS12_verify_mac(pkcs12, password, len);
+    if (ret == 0)
+        ERR_(crypt)("failed to verify pkcs12 {%p} with password
\"%s\" using func {%p}\n", pkcs12, password, pPKCS12_verify_mac);
+    else
+        TRACE_(crypt)("verified pkcs12 {%p} with password \"%s\"
using func {%p}\n", pkcs12, password, pPKCS12_verify_mac);

You accept the PKCS12 file even if the password is incorrect.  This is
clearly wrong.

+    /* extract private key and certificate */
+    ret = pPKCS12_parse(pkcs12, password, &pkey, &cert, NULL);

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.  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?

OpenSSL also supports at least a couple of attributes associated with
the certificates.  From the man page for PKCS12_parse:
       The friendlyName and localKeyID attributes (if present) on each
       certificate will be stored in the alias and keyid attributes of the
       X509 structure.
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.

More questions:
+        const WCHAR storeName[] = {'S', 't', 'o', 'r', 'e', ' ', 'N',
'a', 'm', 'e', 0};
+        store = CertOpenStore(CERT_STORE_PROV_MEMORY, 0,
(HCRYPTPROV)NULL, CERT_STORE_CREATE_NEW_FLAG, storeName);

What's the purpose of the store name?  Does the native implementation
do this?  If not, it can be omitted.  If so, the tests should check
it.

Thanks,
--Juan



More information about the wine-devel mailing list