[xmlsec] Big patch to xmlsec in recent OpenOffice.org sources

[Aleksey Sanin]

I agree with you about the "original" aka xmlsec-1.2.7 code.
However, take a look at the new code I wrote yesterday!
I believe it does exactly the same thing as the code in your
patch:

http://cvs.gnome.org/viewcvs/xmlsec/src/mscrypto/certkeys.c?rev=1.7&view=markup

Aleksey

Andrew Fan wrote:
> Aleksey Sanin wrote:
> 
>>>> 6) src/mscrypto/certkeys.c, 
>>>> xmlSecMSCryptoX509StoreConstructCertsChain()
>>>> function:
>>>
>>>
>> ...
>>
>> As far as I can understand your patch, it *does not* search untrusted
>> certs store if the certificate is self signed ("if(!selfSigned)...").
>> And this is exactly what happens in my code:
>>  1) Search trusted store for the cert subject and return TRUE if found
>>  2) Check if cert is self signed and return FALSE if it is the case
>>  3) Search trusted store for the cert issuer, check signature,
>>     revocation, etc. and return TRUE if everything is OK
>>  4) Search issuer cert in the list of other input certs, check
>>     signature, revocation, etc. and return recurse if everything is OK
>>  5) Search issuer cert in the list of untrusted certs, check signature,
>>     revocation, etc. and return recurse if everything is OK
> 
> 
> I have a little different views. At the original file:
> (1). line 290-291:
> ----------------
> /* try the untrusted certs in the chain */
> issuerCert = CertFindCertificateInStore(certs,
> .....
> --here, it the first step to find the cert from the stored 
> certs-------------
> 
> (2). line 297-299:
> ----------------
>    if(issuerCert == cert) {
>    /* self signed cert, forget it */
>    CertFreeCertificateContext(issuerCert);
> --here, you will go forward to next step: find the cert at untrsuted store;
> --the compare "issuerCert == cert" is not correct, it only compared the 
> handler instead of the certificate content, it is common that the same 
> certificate is bound with different handler because of difference of 
> who/when create handler, where the cert is reposited, etc. ---------------
> 
> (3). line 316-317:
> ----------------
>    /* try the untrusted certs in the store */
>    issuerCert = CertFindCertificateInStore(ctx->untrusted,
> --The second step to find a cert from untrusted store. -------------
> 
> (4). line 323-324:
> ----------------
>    if(issuerCert == cert) {
>    /* self signed cert, forget it */
> --The same as (2)--------------
> 
> (5). line 341-342:
> ----------------
>    /* try to find issuer cert in the trusted cert in the store */
>    issuerCert = CertFindCertificateInStore(ctx->trusted,
> --Finally, try to find the self-signed cert in trusted store. Notes, 
> goes here, the cert must be a self-signed cert, otherwise, it must be 
> switched off.--------------
> 
> Considering two cases.
> 1. I have self-signed cert in my key store, code goes to find the 
> self-signed cert, the process like:
>   a. try to find it at cert chains, i.e, the stored certs which maybe 
> read from xml or set by user;
>   b. if found, because it is a self-signed cert, ignore and goes 
> forward; if not goes forward;
>   c. try to find it at untrusted store;
>   d. if found, because it is a self-signed cert, ignore and goes 
> forward; if not goes forward;
>   e. try to find it at trusted store. and we get it at last.
> 
>  --We have four step useless, step a to d. And at step c, for large 
> scale PKI system, it maybe connect to a remote directory server, it is 
> quite time consumption. In fact, we can directly try to find the cert 
> firstly from the trusted store.
> 
> 2. I have personal certificate with private key in my key store, but I 
> have no root certificate in my key store, I want to sign or decrypt some 
> data. Because I have private key, for sure, I trust it.
>    The codes will be failed to find the cert.
> 
> So I think, the find cert from trusted store should be moved up, and 
> return immediately after the cert is found at the trusted store no 
> matter whether it is a self-signed cert.
> 
>>    It seems to me that this covers all the cases and it is not much
>> different from your code. In your code, step 3) was done after 4) and 5)
>> and you did it for self signed certs too. But if cert is self signed,
>> then subject == issuer and you'll find it (or not find it) in the
>> trusted store on step 1) anyway and there is no need to repeat
>> the search on step 3) again.
>>
> Yes, in my codes, the lastest block are redundant, which should be 
> deleted and add return false in the above two block where found the 
> self-signed cert.
> 
> Thanks,
> Andrew
> _______________________________________________
> xmlsec mailing list
> xmlsec at aleksey.com
> http://www.aleksey.com/mailman/listinfo/xmlsec