[xmlsec] xmlsec-mscrypto code review

[Wouter]

> --------------------------------------------------------------
> --------------------------------------------------------------
> 0) It's my first close expirience with MS Crypto API  and I could not 
> resist to
> express my opinion about it. While MS Crypto API is OK for 
> writing applications (i.e. same as OpenSSL, NSS, BSAFE, etc.) 
> but it's *absolutely* not OK for writing secure applications. 
> I was just suprised to see how 
> *difficult* is
> to ensure that everything is OK (also see item 1) bellow). I 
> knew it's a 
> badly
> designed API but I never thought that it is soo bad :(

It is a terrible thing. I still don't understand why I'm crazy enough to
actually use this stuff. I think I'm under the spell of Bill... :) Or
was it money ;)
 
> 
> 2) Memory leaks. I fixed a lot of them (especially in error cases). 
> Wouter, I wonder
> if you can run some memory checking tool (purify?) on the 
> xmlsec-mscrypto code.
> I feel that there are too many places to spot everything on 
> code review.

I'm not sure I've access to such a tool. I do have numega, but an old
version, and that will defenitely not run on .NET environment. 
 
> 
> Correct error reason is very important. For example,  some 
> applications 
> ignore all the
> XMLSEC_ERRORS_R_XMLSEC_FAILED errors to get to the bottom of 
> the errors 
> stack and report
> the actual problem.

Speaking about errors. Why did you remove all the calls to GetLastError
to fill the error msgs created by MS Crypto API error with some
(especially during debugging) native error code?
 
> 8) xmlSecMSCryptoNodeGetBigNumValue(), 
> xmlSecMSCryptoNodeSetBigNumValue() and reading/writing
> RSA/DSA keys
> I rewrote these functions to avoid un-necessary xmlSecBuffer 
> allocations 
> and simplify the code. I feel that

Hmm, ok, I thought that it was desirable to use the buffer instead of
direct (xml)mallocs in the xmlsec lib code. But I didn't use the
xmlSecBuffer consistently, especially in the early code, simply because
of my ignorance (I can be very ignorant, you know :)

> 
> 9) xmlSecMSCryptoKeyDataDuplicate() : there was a comment 
> that it's not 
> clear what exactly should be done.
> I made a change and would appreciate if someone (Wouter?) 
> would look at it.

After looking at it twice by now, I think this is the correct approach
(though you never can be sure with MS Crypto API).

Wouter