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