On Tue, Nov 4, 2008 at 7:46 PM, Robert Relyea <[EMAIL PROTECTED]> wrote:
> Ludovic Rousseau wrote:
>>
>> On Fri, Oct 31, 2008 at 12:25 AM, Robert Relyea <[EMAIL PROTECTED]>
>> wrote:
>>
>>>
>>> Stanislav Brabec wrote:
>>>
>>>>
>>>> Could it be implemented using CERT_DecodeDERCertificate()?
>>>>
>>>> extern CERTCertificate *
>>>> CERT_DecodeDERCertificate (SECItem *derSignedCert, PRBool copyDER, char
>>>> *nickname);
>>>>
>>>>
>>>
>>> Don't use CERT_DecodeDERCertificate. It's a private symbol that requires
>>> much care (it doesn't return a fully usable CERTCertificate *). To
>>> inidcate
>>> this you will find that it's exported as __CERT_DecodeDERCertificate()
>>> and
>>> requires work to get to.
>>>
>>> Use CERT_NewTempCertificate() instead. Older versions of NSS exported
>>> this
>>> symbol as __CERT_NewTemp.... as well, but we've officially blessed it as
>>> safe. In nss 3.12 it's fully exported (the old symbol will continue to be
>>> available as well).
>>>
>>> It's Signature is
>>>
>>> extern CERTCertificate *
>>> CERT_NewTempCertificate (CERTCertDBHandle *handle, SECItem *derCert,
>>> char *nickname, PRBool isperm, PRBool copyDER);
>>>
>>> handle is an historical dreg passing it CERT_GetDefaultCertDB() is fine.
>>> Nickname can be NULL, isPerm should be set to FALSE (or it will try to
>>> load
>>> the cert into the default database).
>>>
>>
>> I propose the included patch.
>>
>> Notes:
>> - I used 0 for copyDER. But I don't know what it is used for.
>> - I have not tested the code. It compiles and all symbols are resolved.
>>
>> Using libnss 1.8.0.15~pre080614d-0etch1 (from Debian Etch) I can
>> compile using CERT_NewTempCertificate() but the symbol is not found:
>> undefined symbol: CERT_NewTempCertificate
>> (src/mappers/.libs/ldap_mapper.so)
>> Using __CERT_NewTempCertificate() works but it is not a nice name
>>
>> Using libnss 3.12.0-5 (from Debian Lenny, released soon) I can use
>> CERT_NewTempCertificate(). I do not plan to support Etch.
>>
>> Does the patch looks good for you Bob?
>>
>
> I see some potential issues.
>
> 1) does ldap_x509[v] ever get freed? In the NSS case it will need to be
> freed by CERT_DestroyCertificate().
ldap_x509[v] elements were not freed. Only the ldap_x509[] was.
I added code to call CERT_DestroyCertificate() on the elements.
> 2) if bv_vals[rv]->bv_val has the same lifetime as the ldap_x509[rv] then
> it's ok to set copyDER to 0, otherwise it should be set to 1. If CopyDER is
> set to zero, then the CERTCertificate's derData field will point to the
> actual data in bv_val. If bv_val goes away before ldap_x509[rv] then bad
> things could happen.
I used 1 to be on the safe side.
> 3) is bv_val a SECITEM or just a data pointer. I notice that the code is
> missing a cast, which seems to indicate that it is a SECITEM, but it doesn't
> seem consistent with the usage above. SECITEMs have both data and length:
>
> #ifdef HAVE_NSS
> {
> SECITEM derdata;
> derdata.data = bv_val;
> derdata.len = bvals[rv]->bv_len;
>
> ldap_x509[rv] = CERT_NewTempCertificate(CERT_GetDefaultCertDB(), &derData,
> NULL, 0, 1);
> }
> #else
>
> if bv_val is already a SECITEM then the existing code is fine.
bv_val is a void *.
I used your code (with SECItem instead of SECITEM).
Change committed in revision 359
http://www.opensc-project.org/pam_pkcs11/changeset/359
Thanks
--
Dr. Ludovic Rousseau
_______________________________________________
opensc-devel mailing list
opensc-devel@lists.opensc-project.org
http://www.opensc-project.org/mailman/listinfo/opensc-devel
