Re: torture_rand segfaults with mbedtls

[Juraj Vijtiuk <juraj.vijtiuk@xxxxxxxxxx>]

Hello Andreas,

On Wed, Jan 10, 2018 at 10:22:05PM +0100, Andreas Schneider wrote:
> torture_rand segfaults with mbedtls. Could you please investigate?

the segfault appears when mbedTLS is built without threading support,
which seems to be the case on Fedora.

ssh_init is called in torture_rand.c:22, but the return value isn't
checked, so the test continues and then segfaults even though initialisation
failed because mbedTLS wasn't built with thread support.

Below is a patch that avoids the segfault by adding a return value check
to torture_rand. I've added the check to torture_server_x11, since the
return value of ssh_init isn't checked there either.

The patch also updates the README.mbedtls file to document the need to build
mbedTLS with threading support if it's used in a multi threaded
application.

Regards,
Juraj

From 444a8ff240e9f1fac4c7e0dc8e80b0e0e7fa7780 Mon Sep 17 00:00:00 2001
From: jvijtiuk <juraj.vijtiuk@xxxxxxxxxx>
Date: Mon, 12 Feb 2018 18:01:48 +0100
Subject: [PATCH] Fix segfault that appears in tests that use threading when
 mbedTLS is built without threading support.

torture_rand and torture_server_x11 call ssh_init without checking
the return value. If mbedTLS is built without threading support
ssh_init fails but the tests continue and then segfault since threading
wasn't correctly initialised.

Add a section that documents requirements for mbedTLS usage in a
multi threaded environment to README.mbedtls.

Signed-off-by: Juraj Vijtiuk <juraj.vijtiuk@xxxxxxxxxx>
---
 README.mbedtls                       | 16 ++++++++++++++++
 tests/unittests/torture_rand.c       |  6 +++++-
 tests/unittests/torture_server_x11.c |  6 +++++-
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/README.mbedtls b/README.mbedtls
index 5411f301..dd1f505d 100644
--- a/README.mbedtls
+++ b/README.mbedtls
@@ -1,3 +1,6 @@
+libssh mbedTLS ECDSA support
+=============================
+
 When built with mbedTLS, libssh currently does not support ECDSA key comparison.
 Since the comparison function is used during the verification of publickey
 authentication requests a libssh server will not be able to deal with ECDSA
@@ -5,3 +8,16 @@ keys.
 
 In general, if the ssh_key_cmp function is used with mbedTLS, ECDSA key
 comparison won't work.
+
+
+mbedTLS and libssh in multithreaded applications
+==================================================
+
+To use libssh with mbedTLS in a multithreaded application, mbedTLS has to be
+built with threading support enabled.
+
+If threading support is not available and multi threading is used, ssh_init
+will fail.
+
+More information about building mbedTLS with threading support can be found
+in the mbedTLS documentation.
diff --git a/tests/unittests/torture_rand.c b/tests/unittests/torture_rand.c
index 46815c48..6e666dfb 100644
--- a/tests/unittests/torture_rand.c
+++ b/tests/unittests/torture_rand.c
@@ -17,9 +17,13 @@
 
 static int setup(void **state) {
     (void) state;
+    int rc;
 
     ssh_threads_set_callbacks(ssh_threads_get_pthread());
-    ssh_init();
+    rc = ssh_init();
+    if (rc != SSH_OK) {
+        return -1;
+    }
 
     return 0;
 }
diff --git a/tests/unittests/torture_server_x11.c b/tests/unittests/torture_server_x11.c
index b12be556..4395dbf1 100644
--- a/tests/unittests/torture_server_x11.c
+++ b/tests/unittests/torture_server_x11.c
@@ -21,9 +21,13 @@ struct hostkey_state {
 static int setup(void **state) {
     struct hostkey_state *h;
     mode_t mask;
+    int rc;
 
     ssh_threads_set_callbacks(ssh_threads_get_pthread());
-    ssh_init();
+    rc = ssh_init();
+    if (rc != SSH_OK) {
+        return -1;
+    }
 
     h = malloc(sizeof(struct hostkey_state));
     assert_non_null(h);
-- 
2.11.0