libssh multi-threading bug in crypto initialization

["Mark Riordan" <mriordan@xxxxxxxxxxxx>]

Hello.  I recently fixed a bug in libssh's initialization of the OpenSSL
Cryptographic module that caused crashes in my multithreaded application.
The fix was for libssh 0.4.8, but I see that the bug appears in 0.5.2, and
the fix is much the same.

Failing to get red.libssh.org to work, I'm attaching a zip file containing a
couple of new source code files.  
You'll want to add these files to CMakeLists.txt. 
Also, dh.c needs to be changed to add a call to the new function provided by
critsec.c.  I haven't patched 0.5.2, but the 0.4.8 patch looks like:

@@ -55,6 +55,7 @@
 #include "libssh/session.h"
 #include "libssh/keys.h"
 #include "libssh/dh.h"
+#include "libssh/critsec.h"

 /* todo: remove it */
 #include "libssh/string.h"
@@ -134,6 +135,7 @@
       return -1;
     }
 #elif defined HAVE_LIBCRYPTO
+       libssh_init_critsec_crypto();
     p = bignum_new();
     if (p == NULL) {
       bignum_free(g);

The problem is that libcrypto maintains a pool of pseudorandom numbers
intended to be used by all threads.  Appropriate locking is done in
libcrypto,
BUT this locking doesn't work at all unless you provide some non-trivial
locking initialization in the code that uses libcrypto.  
If you fail to provide the extra initialization, the resulting lack of
locking in libcrypto may result in two indices being out-of-sync, resulting
in -N rather than N being passed as a byte count in a call to memcpy.
N is typically rather small--for instance, 12.  Doing a memcpy of -12 bytes
(which looks like over 4 billion when viewed as an unsigned integer)
is very bad for the health of libssh.

Regards,

Mark R

Attachment: libssh-critsec.zip
Description: Zip compressed data