Code review comment for lp:~rick-fdd/pyopenssl/pkcs12_mod_and_export2

Revision history for this message
Jean-Paul Calderone (exarkun) wrote :

I wrote a nicer review, but firefox crashed and ate it. This one's a bit terser, sorry.

  1. crypto_PKCS12_set_ca_certificates should convert cacerts to a tuple first, then iterate over that tuple and check the types of the elements. A broken or weird __iter__ could sneak a non-x509 past the checks as the code is now.
  2. The return type of crypto_PKCS12_get_friendlyname shouldn't really be crypto_PKeyObj, should it?
  3. crypto_PKCS12_set_friendlyname should use PyString_CheckExact, to reject str subclasses.
  4. It's hard to tell for sure, but I *think* PKCS12_create doesn't take over the cacerts stack passed to it. So I think crypto_PKCS12_export is currently leaking a STACK_OF(X509).
  5. In crypto_PKCS12_New, cacerts is leaked in all the error cases.

« Back to merge proposal