curl_easy_perform locks when using mbedtls #737

damv · 2016-03-29T12:38:43Z

I got an issue with libcurl after migrating from cyassl to mbedtls.

I configure my easy_handle to do a HTTPS POST than I call curl_easy_perform which locks and never returns. The same code works like a charm when using cyassl.
More precisely, with cyassl I can see that my write function (CURLOPT_WRITEFUNCTION) is called 2 times, one time for 212 bytes, and then for 9 bytes.
With mbedtls I receive the first call only. Sometimes the second call arrives after a long delay (several minutes).
Maybe it is the same bug than #334 ?
I use libcurl 7.47.1

bagder · 2016-03-29T17:19:36Z

Could be the same. Can you tell us how to reproduce this issue, ideally against a public URL?

damv · 2016-03-30T11:57:53Z

Here is an example which triggers the problem.
For some reason, in this example easy_perform always returns, with a smaller delay (approximately 15 seconds).
I chose the size of the web page in order to have at least two calls to my write function. If the page is small enough, the write function is called only once, and there is no issue.

For information, the platform is a cortex M4 chip and the ciphersuite used is TLS-RSA-WITH-AES-128-GCM-SHA256. I will perform test with other ciphersuites.

Let me know if you are able to reproduce.

#define SIZE_SMLBUF       212
#define SIZE_LRGBUF       1800

int test_dvi ()
{
    char resbuf[SIZE_LRGBUF];
    char param[SIZE_SMLBUF];
    char *p
    printf("max_write_size %d\n", CURL_MAX_WRITE_SIZE);

    size_t writefunction(void *ptr, size_t size, size_t nmemb, void *stream) {
        if (p == NULL)
            return (size * nmemb);
        if ((p + size * nmemb) > resbuf + SIZE_LRGBUF + 100) {
            printf("[buff ovflow (%d>%d)\n", (int)((p - resbuf) + size * nmemb), (int)SIZE_LRGBUF + 100);
            resbuf[0] = 0;
            return (-1);
        }
        memcpy(p, ptr, size * nmemb);
        p += size * nmemb;
        p[0] = 0;
        printf("End of write, size = %d\n", size*nmemb);
        return (size * nmemb);
    }

    p = resbuf;
    if (resbuf != NULL) {
        p[0] = 0;
    }

    CURL *curl = curl_easy_init();
    if(curl) {
        curl_easy_setopt(curl, CURLOPT_VERBOSE, 1);
        curl_easy_setopt(curl, CURLOPT_URL, "https://178.62.74.69/");
        curl_easy_setopt(curl, CURLOPT_POST, 1);
        curl_easy_setopt(curl, CURLOPT_READFUNCTION, NULL);
        curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, &writefunction);
        curl_easy_setopt(curl, CURLOPT_POSTFIELDS, param);
        curl_easy_setopt(curl, CURLOPT_POSTFIELDSIZE, strlen(param));
        curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0);
        curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0);
        printf("easy perform...\n");
        curl_easy_perform(curl);
        printf("Result %s\n", resbuf);
        printf("clean up...\n");
        curl_easy_cleanup(curl);
    }

    return 0;
}

bagder · 2016-04-02T15:29:26Z

I tried to reproduce it from my linux box, but failed:

$ ./src/curl -V
curl 7.49.0-DEV (x86_64-pc-linux-gnu) libcurl/7.49.0-DEV mbedTLS/2.2.1 zlib/1.2.8 c-ares/1.11.0 libidn/1.32 libpsl/0.11.0 (+libicu/55.1) libssh2/1.5.0 nghttp2/1.9.1 librtmp/2.3
$ time ./src/curl --data-binary @libtool https://178.62.74.69/ -k > /dev/null
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  332k  100   711  100  332k   1531   715k --:--:-- --:--:-- --:--:--  714k

real    0m0.474s
user    0m0.048s
sys     0m0.000s
$ echo $?
0

bagder · 2016-04-02T21:53:34Z

Can you use a debugger or strace or something to figure out which function that gets stuck, or where the loop is?

damv · 2016-04-06T16:45:44Z

I have finally found the bug.
To know if there is still data to be retrieved from mbedtls, the "function" called is :

#define curlssl_data_pending(x,y) (x=x, y=y, 0)

... which always returns 0.

I got the good behaviour with :

#define curlssl_data_pending(x,y) mbedtls_data_pending(x,y)
int mbedtls_data_pending(struct connectdata *conn, int sockindex)
{
    return mbedtls_ssl_pending(&conn->ssl[sockindex].ssl);
}

This problem will only occur with small values of CURL_MAX_WRITE_SIZE (=500 in my embedded application). Indeed, if CURL_MAX_WRITE_SIZE is sufficiently high, the expected return value will always be 0 and the trivial implementation works.

bagder · 2016-04-06T21:45:25Z

@damv, Can you please produce a full patch or pull-request with the change you made to get it working?

jay · 2016-04-06T23:02:34Z

mbedtls_ssl_pending? where is that? I can't find it in the API.

damv · 2016-04-07T08:53:18Z

My mistake, I had to define it myself, in mbedTLS :

int mbedtls_ssl_pending( mbedtls_ssl_context *ssl)
{
    return ssl->in_msglen != 0;
}

Without modifying mbedTLS we could define mbedtls_data_pending as :

int mbedtls_data_pending(struct connectdata *conn, int sockindex)
{
    mbedtls_ssl_context *ssl = &conn->ssl[sockindex].ssl;
    return ssl->in_msglen != 0;
}

bagder · 2016-04-07T13:00:27Z

I don't think our own functions should use mbedtls_ as prefix. I know we have that already established but we should stop that so that it becomes easier to see the actual mbedtls functions.

If the function is to be provided to other source files within libcurl it needs to be prefixed with Curl_.

bagder · 2016-04-07T13:17:01Z

Fixed my first point in 5446549

bagder · 2016-04-07T13:39:47Z

So if I understand you correctly, the patch would be something like this?

diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c
index 77ff326..4f887a1 100644
--- a/lib/vtls/mbedtls.c
+++ b/lib/vtls/mbedtls.c
@@ -862,8 +862,13 @@ int Curl_mbedtls_init(void)
 void Curl_mbedtls_cleanup(void)
 {
   (void)Curl_polarsslthreadlock_thread_cleanup();
 }

-
+int Curl_mbedtls_data_pending(const struct connectdata *conn, int sockindex)
+{
+  mbedtls_ssl_context *ssl =
+    (mbedtls_ssl_context *)&conn->ssl[sockindex].ssl;
+  return ssl->in_msglen != 0;
+}

 #endif /* USE_MBEDTLS */
diff --git a/lib/vtls/mbedtls.h b/lib/vtls/mbedtls.h
index eb6192e..bd1c15b 100644
--- a/lib/vtls/mbedtls.h
+++ b/lib/vtls/mbedtls.h
@@ -29,10 +29,11 @@
 #include <mbedtls/sha256.h>

 /* Called on first use mbedTLS, setup threading if supported */
 int  Curl_mbedtls_init(void);
 void Curl_mbedtls_cleanup(void);
+int Curl_mbedtls_data_pending(const struct connectdata *conn, int sockindex);

 CURLcode Curl_mbedtls_connect(struct connectdata *conn, int sockindex);

 CURLcode Curl_mbedtls_connect_nonblocking(struct connectdata *conn,
                                            int sockindex,
@@ -61,11 +62,11 @@ int Curl_mbedtls_shutdown(struct connectdata *conn, int sockindex);
 #define curlssl_set_engine(x,y) (x=x, y=y, CURLE_NOT_BUILT_IN)
 #define curlssl_set_engine_default(x) (x=x, CURLE_NOT_BUILT_IN)
 #define curlssl_engines_list(x) (x=x, (struct curl_slist *)NULL)
 #define curlssl_version Curl_mbedtls_version
 #define curlssl_check_cxn(x) (x=x, -1)
-#define curlssl_data_pending(x,y) (x=x, y=y, 0)
+#define curlssl_data_pending(x,y) Curl_mbedtls_data_pending(x, y)
 #define CURL_SSL_BACKEND CURLSSLBACKEND_MBEDTLS
 #define curlssl_sha256sum(a,b,c,d) mbedtls_sha256(a,b,c,0)

 /* This might cause libcurl to use a weeker random!
    TODO: implement proper use of Polarssl's CTR-DRBG or HMAC-DRBG and use that

damv · 2016-04-07T13:54:32Z

yes, exactly.

damv · 2016-04-07T14:00:58Z

It is not related but I also had to do this patch to use mbedtls debug

From 29f05bacec716371049d636a14e31be999f50305 Mon Sep 17 00:00:00 2001
From: Damien Vielpeau <damien.vielpeau@withings.com>
Date: Thu, 7 Apr 2016 15:58:11 +0200
Subject: [PATCH] Fix mbedTLS debug

---
 lib/vtls/mbedtls.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c
index 77ff326..9e61b07 100644
--- a/lib/vtls/mbedtls.c
+++ b/lib/vtls/mbedtls.c
@@ -101,7 +101,7 @@ static int entropy_func_mutex(void *data, unsigned char *output, size_t len)
 #undef MBEDTLS_DEBUG

 #ifdef MBEDTLS_DEBUG
-static void mbed_debug(void *context, int level, const char *line)
+static void mbed_debug(void *context, int level, const char *f_name, int line_nb, const char *line)
 {
   struct SessionHandle *data = NULL;

@@ -423,7 +423,7 @@ mbed_connect_step1(struct connectdata *conn,
 #endif

 #ifdef MBEDTLS_DEBUG
-  mbedtls_ssl_conf_dbg(&connssl->ssl, mbedtls_debug, data);
+  mbedtls_ssl_conf_dbg(&connssl->config, mbedtls_debug, data);
 #endif

   connssl->connecting_state = ssl_connect_2;
-- 
2.5.0

bagder · 2016-04-07T14:13:59Z

Thanks, I've pushed these commits now!

bagder added TLS HTTP labels Mar 29, 2016

bagder self-assigned this Mar 29, 2016

bagder closed this as completed in c111178 Apr 7, 2016

lock bot locked as resolved and limited conversation to collaborators May 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

curl_easy_perform locks when using mbedtls #737

curl_easy_perform locks when using mbedtls #737

damv commented Mar 29, 2016

bagder commented Mar 29, 2016

damv commented Mar 30, 2016

bagder commented Apr 2, 2016

bagder commented Apr 2, 2016

damv commented Apr 6, 2016

bagder commented Apr 6, 2016

jay commented Apr 6, 2016

damv commented Apr 7, 2016

bagder commented Apr 7, 2016

bagder commented Apr 7, 2016

bagder commented Apr 7, 2016

damv commented Apr 7, 2016

damv commented Apr 7, 2016

bagder commented Apr 7, 2016

curl_easy_perform locks when using mbedtls #737

curl_easy_perform locks when using mbedtls #737

Comments

damv commented Mar 29, 2016

bagder commented Mar 29, 2016

damv commented Mar 30, 2016

bagder commented Apr 2, 2016

bagder commented Apr 2, 2016

damv commented Apr 6, 2016

bagder commented Apr 6, 2016

jay commented Apr 6, 2016

damv commented Apr 7, 2016

bagder commented Apr 7, 2016

bagder commented Apr 7, 2016

bagder commented Apr 7, 2016

damv commented Apr 7, 2016

damv commented Apr 7, 2016

bagder commented Apr 7, 2016