Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi-threaded "libssh2_init()" #61

Closed
doublex opened this issue Nov 8, 2015 · 12 comments
Closed

multi-threaded "libssh2_init()" #61

doublex opened this issue Nov 8, 2015 · 12 comments
Labels

Comments

@doublex
Copy link
Contributor

doublex commented Nov 8, 2015

Please add a flag LIBSSH2_INIT_MULTITHREADED to libssh2_init(int flags).

Openssl and gnutls need to be initialized in a special way in order to be thread safe.
It's quite easy, but as far as I can see libssh2 does not support a thread safe init.

Openssl:
http://stackoverflow.com/questions/3919420/tutorial-on-using-openssl-with-pthreads
Gnutls:
http://gnu.ist.utl.pt/software/gnutls/manual/html_node/Multi_002dthreaded-applications.html

@bagder
Copy link
Member

bagder commented Nov 9, 2015

It is not that easy to "just add" such a flag unless we also make assumptions of what thread system/model your application is using. That's why it is documented to not be thread-safe so you have to do the mutexing yourself.

@doublex
Copy link
Contributor Author

doublex commented Nov 9, 2015

Thanks a lot for your answer. My problem is, that I can't initialize openssl/gnutls thread safe outside libssh2, because as far as I can see, libssh2 does not export whether it uses openssl or gnutls.
Example (simplified):

#ifdef LIBSSH_USES_OPENSSL
    CRYPTO_set_id_callback(thread_id);
    CRYPTO_set_locking_callback(lock_callback);
#else 
    gcry_control (GCRYCTL_SET_THREAD_CBS, &gcry_threads_pthread);
    gnutls_global_init();
#endif
libssh2_init(0);

@bagder
Copy link
Member

bagder commented Nov 9, 2015

libssh2 should init the crypto lib when you call libssh2_init(). Doesn't it?

You just need to make sure that libssh2_init() isn't called in a thread-unsafe way,

@doublex
Copy link
Contributor Author

doublex commented Nov 9, 2015

Sorry - my description was a mess.
The problem is: openssl and gnutls need a special kind of initialization to be thread safe.
E.g.:

gcry_control (GCRYCTL_SET_THREAD_CBS, &gcry_threads_pthread);

My idea was to add a flag to libssh2_init() which tells libssh2 to initialize the crypto backend in a thread-safe manner.

@doublex
Copy link
Contributor Author

doublex commented Nov 12, 2015

Maybe:
a) A --enable-pthread configure option?
b) libssh2 could export the backend (openssl or gnutls) as a #define so that an application could initialize the crypto library?

Our application is threaded - and we are getting core dumps only if multiple threads are calling libssh2:

info threads
  7 Thread 0x7f3f0665c700 (LWP 26756)  0x0000003abf8e13b3 in select () at syscall-template.S:82
  6 Thread 0x7f3f0705d700 (LWP 26752)  0x0000003abf8e13b3 in select () at syscall-template.S:82
  5 Thread 0x7f3f07a5e700 (LWP 26750)  0x0000003abf8e13b3 in select () at syscall-template.S:82
  4 Thread 0x7f3f0f43c700 (LWP 26603)  0x0000003abf8e8f33 in epoll_wait () at syscall-template.S:82
  3 Thread 0x7f3f356ca820 (LWP 26602)  pthread_cond_wait@@GLIBC_2.3.2 () at pthread_cond_wait.S:183
  2 Thread 0x7f3f0e743700 (LWP 26649)  0x0000003abf8e13b3 in select () at syscall-template.S:82
* 1 Thread 0x7f3ef3fff700 (LWP 26803)  sha1_block_data_order_avx () at sha1-x86_64.s:3392
#0  sha1_block_data_order_avx () at sha1-x86_64.s:3392
#1  0xca62c1d6ca62c1d6 in ?? ()
#2  0xca62c1d6ca62c1d6 in ?? ()
#3  0xca62c1d6ca62c1d6 in ?? ()
#4  0xca62c1d6ca62c1d6 in ?? ()
#5  0xca62c1d6ca62c1d6 in ?? ()
#6  0xca62c1d6ca62c1d6 in ?? ()
#7  0xca62c1d6ca62c1d6 in ?? ()
#8  0xca62c1d6ca62c1d6 in ?? ()
#9  0x0000003ac9fdf7f8 in state () from libcrypto.so.1.0.1e
#10 0x00007f3eec07eff0 in ?? ()
#11 0x0000000000000027 in ?? () at stl_tree.h:874
#12 0x0000003ac9c72d67 in SHA1_Update (c=0xa670dc42, data_=<value optimized out>, len=<value optimized out>) at md32_common.h:325
#13 0x0000003ac9ce62e4 in ssleay_rand_bytes (
    buf=0x7f3eec01b82b "\325(2[...]", <incomplete sequence \347>, num=0, pseudo=0) at md_rand.c:477
#14 0x0000003acc01a61c in _libssh2_transport_send (session=0x7f3eec0175a0, data=0x7f3eec024d0c "^", data_len=<value optimized out>, data2=0x7f3eec07faa4 "", data2_len=<value optimized out>)
    at transport.c:818
#15 0x0000003acc006469 in _libssh2_channel_write (channel=0x7f3eec024aa0, stream_id=0, buf=0x7f3eec07faa4 "", buflen=<value optimized out>) at channel.c:2058
#16 0x0000003acc015e5c in sftp_read (hnd=0x7f3eec024ec0, buffer=0x7f3ef3fcdf90 "\270\"\b\354>\177", buffer_maxlen=504) at sftp.c:1405
#17 libssh2_sftp_read (hnd=0x7f3eec024ec0, buffer=0x7f3ef3fcdf90 "\270\"\b\354>\177", buffer_maxlen=504) at sftp.c:1568

@doublex
Copy link
Contributor Author

doublex commented Nov 22, 2015

The great Daniel Stenberg :-) wrote some examples on this topic:
http://curl.haxx.se/libcurl/c/opensslthreadlock.html
http://curl.haxx.se/libcurl/c/threaded-ssl.html

My problem:
I can't neither tell libssh to initialize the crypto backend thread-safe - nor can I do it myself properly because the application doesn't know if libssh uses openssl or gnutls (I could only create workaround).

IHMO the best option would be:
a) a configure-option --enable-pthread
b) libssh2_init( LIBSSH2_INIT_MULTITHREADED );

@ghost
Copy link

ghost commented Apr 18, 2016

Hi,
I'm having exactly the same problem at moment even though I have initialized OpenSSL for a multithreaded environment (I used static locking callbacks) as in your examples.
I verified that callbacks are triggered by the underlying openssl stratum.

Anyway my application crashes when opening an SFTP connection.

In detail, libssh2_session_handshake crashes while invoking HMAC_Init_ex, EVP_DigestInit_ex, CRYPTO_free:
#0 0x00007fe3600eb0a7 in raise () from /lib64/libc.so.6
#1 0x00007fe3600ec458 in abort () from /lib64/libc.so.6
#2 0x00007fe360128764 in __libc_message () from /lib64/libc.so.6
#3 0x00007fe36012dfce in malloc_printerr () from /lib64/libc.so.6
#4 0x00007fe361e6b3fd in CRYPTO_free () from /lib64/libcrypto.so.1.0.0
#5 0x00007fe361ef4009 in EVP_DigestInit_ex () from /lib64/libcrypto.so.1.0.0
#6 0x00007fe361e79360 in HMAC_Init_ex () from /lib64/libcrypto.so.1.0.0
#7 0x00007fe36291ee7a in ?? () from /usr/lib64/libssh2.so.1
#8 0x00007fe36292d3e9 in ?? () from /usr/lib64/libssh2.so.1
#9 0x00007fe362924bd3 in libssh2_session_handshake () from /usr/lib64/libssh2.so.1

I guess there's problem in the setup of my environment.
I'm using SUSE SLES12. The same code works on SUSE SLES11.

Any idea?

@ghost
Copy link

ghost commented Apr 20, 2016

Problem fixed!
Thanks to the libssh2 community who helped me :-)

I fixed the problem by installing a newer version of libssh2.

I came across this: http://trac.libssh2.org/ticket/279
In fact on my system I was using libssh2 1.4.3.

The problem was initially fixed in the 1.5.0, then completely fixed the 1.6.0 version (see the change log https://www.libssh2.org/changes.html)

@doublex I guess that it's not possible to add such a LIBSSH2_INIT_MULTITHREADED flag for the libssh2_init function because libssh2 users are free to use different crypto libraries that in turn require different initializations. But this is just my understanding as libssh2 user.

@doublex
Copy link
Contributor Author

doublex commented Oct 1, 2016

Would it be possible to add a "configure" option like "--enable-gthreads"?

@bagder
Copy link
Member

bagder commented Oct 1, 2016

Possible perhaps, but what would it do and how would it work?

@doublex
Copy link
Contributor Author

doublex commented Oct 1, 2016

Maybe it would be best to modify libssh2_init() and libssh2_exit() in global.c.
A flag LIBSSH2_INIT_MULTITHREADED would preserve backward compatibility.


static void init_multithreaded();
static void exit_multithreaded();

static int _libssh2_initialized = 0;
static int _libssh2_init_flags = 0;

LIBSSH2_API int
libssh2_init(int flags)
{
    if (_libssh2_initialized == 0 && !(flags & LIBSSH2_INIT_NO_CRYPTO)) {
        if (flags & LIBSSH2_INIT_MULTITHREADED)
            init_multithreaded();
        libssh2_crypto_init();
        _libssh2_init_aes_ctr();
    }

    _libssh2_initialized++;
    _libssh2_init_flags |= flags;

    return 0;
}

LIBSSH2_API void
libssh2_exit(void)
{
    if (_libssh2_initialized == 0)
        return;

    _libssh2_initialized--;

    if (!(_libssh2_init_flags & LIBSSH2_INIT_NO_CRYPTO)) {
        libssh2_crypto_exit();
        if (flags & LIBSSH2_INIT_MULTITHREADED)
            exit_multithreaded();
    }

    return;
}


#ifdef LIBSSH2_MULTITHREADED

#ifdef LIBSSH2_OPENSSL
#include <pthread.h>
static unsigned long crypto_thread_id()
{
    return (unsigned long)thread_id();
}
static pthread_mutex_t *lockarray = NULL;
static void crypto_lock_callback( int mode, int type, const char *file, int line )
{
    ASSERT( lockarray != NULL );
    if( mode & CRYPTO_LOCK )
        pthread_mutex_lock( &(lockarray[type]) );
    else
        pthread_mutex_unlock( &(lockarray[type]) );
}
static void init_multithreaded()
{
    ASSERT( lockarray == NULL );
    lockarray = (pthread_mutex_t *) malloc( CRYPTO_num_locks() * sizeof(pthread_mutex_t) );
    for( int i=0; i < CRYPTO_num_locks(); ++i )
        pthread_mutex_init( &(lockarray[i]), NULL );
    CRYPTO_set_id_callback( crypto_thread_id );
    CRYPTO_set_locking_callback( crypto_lock_callback );
}
static void exit_multithreaded()
{
    // kill locks
    if( lockarray == NULL )
        return ;
    CRYPTO_set_locking_callback( NULL );
    for( int i=0; i < CRYPTO_num_locks(); ++i )
        pthread_mutex_destroy( &(lockarray[i]) );
    free( lockarray );
    lockarray = NULL;

    ENGINE_cleanup();
    CRYPTO_cleanup_all_ex_data();
    ERR_free_strings();
    ERR_remove_state(0);
    EVP_cleanup();
}    

#elif defined(LIBSSH2_LIBGCRYPT)

GCRY_THREAD_OPTION_PTHREAD_IMPL;
static void init_multithreaded()
{
    gcry_control( GCRYCTL_SET_THREAD_CBS );
}
static void exit_multithreaded()
{
}

#elif defined(LIBSSH2_WINCNG)

static void init_multithreaded()
{
}
static void exit_multithreaded()
{
}

#endif

#else

static void init_multithreaded()
{
}
static void exit_multithreaded()
{
}

#endif

@stale
Copy link

stale bot commented Mar 21, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 21, 2019
@stale stale bot closed this as completed Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants