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

[Andrew Fan]

Why double get the same cert from the trusted store before and after the 
self-singed cert checking? Otherwise, looks good, and much more better 
than mine coding style. :-P

-Andrew

Aleksey Sanin wrote:

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