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

[Andrew Fan]

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