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

[Andrew Fan]

Hi Aleksey,

Thanks for the review, you'll help us a lot at improving the quality.

Aleksey Sanin wrote:

> Hi, Channdler, Andrew!
>
> I started reviewing the changes for xmlsec-mscrypto library and I
> have few questions about your patch:
>
> 0) After applying the patch, I have quite a lot of failures in
> xmlsec regression test suite. I wonder if you run the tests and know
> the reasons for these failures?
>
If I correct, the patch doesn't implement app* routines, which are not 
used at openoffice, so I don't implement it in time. Because some APIs 
have changed or added to the kernel, so it is necessary to adjust the 
app* programs. However, I think Channdler and Michael may be in the 
implementing process.

> 1) xmlsec/include/xmlsec/mscrypto/akmngr.c, xmlsec/src/mscrypto/akmngr.c
> Why do you need "AppliedKeyManager"? How is it different from the
> "DefaultKeyManager" and do you think it would be easier to just
> merge the two?
>
The AppliedKeyManager enable user specify their preferred key store and 
certificate store. It would be a good idea just simply support both of 
the two manager.

> 2) xmlsec/src/mscrypto/certkeys.c
> I understand that you are using refcounting for HCRYPTKEY and
> HCRYPTPROV instead of system "duplicate" functionality to support
> NT 4.0. However, it seems a little bit dangerouse to me to re-use
> the same key handler from multiple threads. Do you know if MS
> documentation says anything about this? Did you do any tests in
> multithreading environment?
>
Good suggest, we will do tests on multithreads. Or add some syn 
mechanism if necessary.

> 3) xmlsec/src/mscrypto/x509.c,
> xmlSecMSCryptoKeyDataX509VerifyAndExtractKey function
> You commented out the code to get public key from a verified certificate
> and replaced it with code that gets either public or private key.
> I am not sure I understand why would you need a private key for
> a "verify cert" operation. It seems impossible to me.
>
I don't think this function only used to verify, sometime it is also 
used to sign. In our cases, all of the signature/encryption process are 
controlled by signature/encryption template, the function is called 
during signning.

Thanks,
Andrew

>
> Thanks,
> Aleksey
>
>
>
> Chandler Peng wrote:
>
>> Aleksey Sanin wrote:
>>
>>> Thanks, Chandler and Andrew!
>>>
>>> I'l review these files during next week. But will you mind to re-send
>>> them to xmlsec at aleksey.com mailing list, please? There are more folks
>>> who can help me with review.
>>>
>> OK , no problem :-)
>>
>>> Thanks!
>>> Aleksey
>>>
>>> Chandler Peng wrote:
>>>
>>>> Andrew ,
>>>> I have created a new diff file with "diff -uN" on xmlsec_1.2.6. 
>>>> This diff file only include the difference on the source and the 
>>>> related makefile . Our new source file need add to xmlsec is in the 
>>>> xmlsec.zip .
>>>>
>>>> Chandler Peng..
>>>>