--- Michael Jung <[EMAIL PROTECTED]> wrote:
> > 2. A CSP isn't allocating a "real" handle, it's sort of a
> pseudohandle, so
> > the multiple-of-four business isn't necessary.  When I was testing
> SSPs, I
> > found some of them return 0 as a valid handle value.  So I think using
> the
> > index directly would be fine.
> I would rather keep this, since it isn't really a performance problem.
> Perhaps 
> sometime somebody else will use the code with "real" handles.

No, this won't be the case.  By real handle I mean one which is sharable
between processes, is refcounted by the wineserver, etc.  That isn't the
case with the Crypto API handles.  So no, it doesn't cause a performance
problem, but it is confusing.

> This handle 
> managing code is actually another example where I would love to have a 
> library, which would be linked statically by multiple dlls.

I agree that one should be available.  Robert Shearman posted a patch to
add handle tables to ntdll:
http://www.winehq.org/hypermail/wine-patches/2004/10/0339.html
It isn't committed yet, but it might be a usable replacement.  It depends
how much you need the features it doesn't provide.

> When I wrote the code I thought a double call to EnterCriticalSection on
> the  same mutex might deadlock. I later found out that I doesn't, but
> thought that  it might be faster to avoid multiple calls to
> EnterCriticalSection, since those call wineserver.

It doesn't appear as though critical sections involve the wine server. 
Take a look at ntdll/critsection.c and RtlEnterCriticalSection.

> > 4. Some of the OpenSSL checks don't seem valid.  In init_hash_impl,
> for
> > example, you fail if libcrypto isn't available, even if it isn't
> necessary
> > for the hash.  Please fail only if you really need an OpenSSL func. 
> E.g.,
> > if (!pMD2_Init) { SetLastError()... } seems better to me.
> Are you saying that we should consider the case where we actually have a
> libcrypto.so at runtime, but which only exports a subset of the
> functions  rsaenh was compiled with?

No, in this case I'm pointing out that most of the hash functions don't
require OpenSSL at all (MD4, MD5, and SHA1), so why should they be
prevented from working if it isn't available?  Right now the check is done
at the entry to the function, so all will fail if libcrypto.so isn't
there.  In my above example I move the check to the function that depends
on libcrypto.so.

> Now, this would mean we are considering the case, where we only have a
> subset of the libssl-dev headers at compile time, right?

Right, or that the OpenSSL you compiled against had some of the algorithms
disabled.  The check as you have it will prevent MD2 from working if DES
wasn't available at compile time, and vice-versa.  There's no guarantee
how OpenSSL was configured.

A similar comment, that is also open to debate, is the implementation of
gen_rand_impl.  I know, rsabase already does it this way.. but it might be
better to try to use /dev/random instead of RAND_bytes, since that will
work on many platforms whether or not OpenSSL is available.

--Juan


                
__________________________________
Do you Yahoo!?
Yahoo! Mail Address AutoComplete - You start. We finish.
http://promotions.yahoo.com/new_mail 

Reply via email to