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

[Aleksey Sanin]

Sorry, wrong link:

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

Aleksey

Aleksey Sanin wrote:
> 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
> 
> _______________________________________________
> xmlsec mailing list
> xmlsec at aleksey.com
> http://www.aleksey.com/mailman/listinfo/xmlsec