From 9f498de9a28e4a4a3f73108756fb7e50af9ec014 Mon Sep 17 00:00:00 2001 From: Jay Satiro Date: Thu, 28 Apr 2016 02:57:12 -0400 Subject: [PATCH] mbedtls: Fix session resume This also fixes PolarSSL session resume. Prior to this change the TLS session information wasn't properly saved and restored for PolarSSL and mbedTLS. Bug: https://curl.haxx.se/mail/lib-2016-01/0070.html Reported-by: Thomas Glanzmann Bug: https://curl.haxx.se/mail/lib-2016-04/0095.html Reported-by: Moti Avrahami --- docs/libcurl/opts/CURLINFO_TLS_SSL_PTR.3 | 4 +- lib/getinfo.c | 4 +- lib/urldata.h | 2 - lib/vtls/mbedtls.c | 65 +++++++++++------------- lib/vtls/polarssl.c | 63 +++++++++++------------ 5 files changed, 63 insertions(+), 75 deletions(-) diff --git a/docs/libcurl/opts/CURLINFO_TLS_SSL_PTR.3 b/docs/libcurl/opts/CURLINFO_TLS_SSL_PTR.3 index b82faecc9..5e58f6f81 100644 --- a/docs/libcurl/opts/CURLINFO_TLS_SSL_PTR.3 +++ b/docs/libcurl/opts/CURLINFO_TLS_SSL_PTR.3 @@ -80,9 +80,9 @@ as well: .IP axTLS SSL * .IP mbedTLS -mbedtls_ssl_session * +mbedtls_ssl_context * .IP PolarSSL -ssl_session * +ssl_context * .IP "Secure Channel (WinSSL)" CtxtHandle * .IP "Secure Transport (DarwinSSL)" diff --git a/lib/getinfo.c b/lib/getinfo.c index 39189cb60..d4b01bf29 100644 --- a/lib/getinfo.c +++ b/lib/getinfo.c @@ -307,7 +307,7 @@ static CURLcode getinfo_slist(struct SessionHandle *data, CURLINFO info, #elif defined(USE_GSKIT) tsi->internals = (void *)conn->ssl[i].handle; #elif defined(USE_MBEDTLS) - tsi->internals = (void *)&conn->ssl[i].ssn; + tsi->internals = (void *)&conn->ssl[i].ssl; #elif defined(USE_NSS) tsi->internals = (void *)conn->ssl[i].handle; #elif defined(USE_OPENSSL) @@ -316,7 +316,7 @@ static CURLcode getinfo_slist(struct SessionHandle *data, CURLINFO info, (void *)conn->ssl[i].ctx : (void *)conn->ssl[i].handle); #elif defined(USE_POLARSSL) - tsi->internals = (void *)&conn->ssl[i].ssn; + tsi->internals = (void *)&conn->ssl[i].ssl; #elif defined(USE_SCHANNEL) tsi->internals = (void *)&conn->ssl[i].ctxt->ctxt_handle; #elif defined(USE_SSL) diff --git a/lib/urldata.h b/lib/urldata.h index f190fbdd6..c0b2e2f7f 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -290,7 +290,6 @@ struct ssl_connect_data { mbedtls_ctr_drbg_context ctr_drbg; mbedtls_entropy_context entropy; mbedtls_ssl_context ssl; - mbedtls_ssl_session ssn; int server_fd; mbedtls_x509_crt cacert; mbedtls_x509_crt clicert; @@ -302,7 +301,6 @@ struct ssl_connect_data { ctr_drbg_context ctr_drbg; entropy_context entropy; ssl_context ssl; - ssl_session ssn; int server_fd; x509_crt cacert; x509_crt clicert; diff --git a/lib/vtls/mbedtls.c b/lib/vtls/mbedtls.c index f61d7a96d..6b26a9747 100644 --- a/lib/vtls/mbedtls.c +++ b/lib/vtls/mbedtls.c @@ -170,7 +170,6 @@ mbed_connect_step1(struct connectdata *conn, struct in_addr addr; #endif void *old_session = NULL; - size_t old_session_size = 0; char errorbuf[128]; errorbuf[0]=0; @@ -187,8 +186,7 @@ mbed_connect_step1(struct connectdata *conn, mbedtls_ctr_drbg_init(&connssl->ctr_drbg); ret = mbedtls_ctr_drbg_seed(&connssl->ctr_drbg, entropy_func_mutex, - &entropy, connssl->ssn.id, - connssl->ssn.id_len); + &entropy, NULL, 0); if(ret) { #ifdef MBEDTLS_ERROR_C mbedtls_strerror(ret, errorbuf, sizeof(errorbuf)); @@ -201,8 +199,7 @@ mbed_connect_step1(struct connectdata *conn, mbedtls_ctr_drbg_init(&connssl->ctr_drbg); ret = mbedtls_ctr_drbg_seed(&connssl->ctr_drbg, mbedtls_entropy_func, - &connssl->entropy, connssl->ssn.id, - connssl->ssn.id_len); + &connssl->entropy, NULL, 0); if(ret) { #ifdef MBEDTLS_ERROR_C mbedtls_strerror(ret, errorbuf, sizeof(errorbuf)); @@ -377,14 +374,15 @@ mbed_connect_step1(struct connectdata *conn, mbedtls_ssl_conf_ciphersuites(&connssl->config, mbedtls_ssl_list_ciphersuites()); - if(!Curl_ssl_getsessionid(conn, &old_session, &old_session_size)) { - memcpy(&connssl->ssn, old_session, old_session_size); + if(!Curl_ssl_getsessionid(conn, &old_session, NULL)) { + ret = mbedtls_ssl_set_session(&connssl->ssl, old_session); + if(ret) { + failf(data, "mbedtls_ssl_set_session returned -0x%x", -ret); + return CURLE_SSL_CONNECT_ERROR; + } infof(data, "mbedTLS re-using session\n"); } - mbedtls_ssl_set_session(&connssl->ssl, - &connssl->ssn); - mbedtls_ssl_conf_ca_chain(&connssl->config, &connssl->cacert, &connssl->crl); @@ -601,38 +599,32 @@ mbed_connect_step3(struct connectdata *conn, struct ssl_connect_data *connssl = &conn->ssl[sockindex]; struct SessionHandle *data = conn->data; void *old_ssl_sessionid = NULL; - mbedtls_ssl_session *our_ssl_sessionid = &conn->ssl[sockindex].ssn; - int incache; + mbedtls_ssl_session *our_ssl_sessionid; + int ret; DEBUGASSERT(ssl_connect_3 == connssl->connecting_state); - /* Save the current session data for possible re-use */ - incache = !(Curl_ssl_getsessionid(conn, &old_ssl_sessionid, NULL)); - if(incache) { - if(old_ssl_sessionid != our_ssl_sessionid) { - infof(data, "old SSL session ID is stale, removing\n"); - Curl_ssl_delsessionid(conn, old_ssl_sessionid); - incache = FALSE; - } + our_ssl_sessionid = malloc(sizeof(mbedtls_ssl_session)); + if(!our_ssl_sessionid) + return CURLE_OUT_OF_MEMORY; + + mbedtls_ssl_session_init(our_ssl_sessionid); + + ret = mbedtls_ssl_get_session(&connssl->ssl, our_ssl_sessionid); + if(ret) { + failf(data, "mbedtls_ssl_get_session returned -0x%x", -ret); + return CURLE_SSL_CONNECT_ERROR; } - if(!incache) { - void *new_session = malloc(sizeof(mbedtls_ssl_session)); - if(new_session) { - memcpy(new_session, our_ssl_sessionid, - sizeof(mbedtls_ssl_session)); + /* If there's already a matching session in the cache, delete it */ + if(!Curl_ssl_getsessionid(conn, &old_ssl_sessionid, NULL)) + Curl_ssl_delsessionid(conn, old_ssl_sessionid); - retcode = Curl_ssl_addsessionid(conn, new_session, - sizeof(mbedtls_ssl_session)); - } - else { - retcode = CURLE_OUT_OF_MEMORY; - } - - if(retcode) { - failf(data, "failed to store ssl session"); - return retcode; - } + retcode = Curl_ssl_addsessionid(conn, our_ssl_sessionid, 0); + if(retcode) { + free(our_ssl_sessionid); + failf(data, "failed to store ssl session"); + return retcode; } connssl->connecting_state = ssl_connect_done; @@ -704,6 +696,7 @@ static ssize_t mbed_recv(struct connectdata *conn, int num, void Curl_mbedtls_session_free(void *ptr) { + mbedtls_ssl_session_free(ptr); free(ptr); } diff --git a/lib/vtls/polarssl.c b/lib/vtls/polarssl.c index 0c6ef239e..6c7a7864b 100644 --- a/lib/vtls/polarssl.c +++ b/lib/vtls/polarssl.c @@ -151,7 +151,6 @@ polarssl_connect_step1(struct connectdata *conn, struct in_addr addr; #endif void *old_session = NULL; - size_t old_session_size = 0; char errorbuf[128]; errorbuf[0]=0; @@ -167,7 +166,7 @@ polarssl_connect_step1(struct connectdata *conn, entropy_init_mutex(&entropy); if((ret = ctr_drbg_init(&connssl->ctr_drbg, entropy_func_mutex, &entropy, - connssl->ssn.id, connssl->ssn.length)) != 0) { + NULL, 0)) != 0) { #ifdef POLARSSL_ERROR_C error_strerror(ret, errorbuf, sizeof(errorbuf)); #endif /* POLARSSL_ERROR_C */ @@ -178,7 +177,7 @@ polarssl_connect_step1(struct connectdata *conn, entropy_init(&connssl->entropy); if((ret = ctr_drbg_init(&connssl->ctr_drbg, entropy_func, &connssl->entropy, - connssl->ssn.id, connssl->ssn.length)) != 0) { + NULL, 0)) != 0) { #ifdef POLARSSL_ERROR_C error_strerror(ret, errorbuf, sizeof(errorbuf)); #endif /* POLARSSL_ERROR_C */ @@ -338,14 +337,15 @@ polarssl_connect_step1(struct connectdata *conn, net_send, &conn->sock[sockindex]); ssl_set_ciphersuites(&connssl->ssl, ssl_list_ciphersuites()); - if(!Curl_ssl_getsessionid(conn, &old_session, &old_session_size)) { - memcpy(&connssl->ssn, old_session, old_session_size); + if(!Curl_ssl_getsessionid(conn, &old_session, NULL)) { + ret = ssl_set_session(&connssl->ssl, old_session); + if(ret) { + failf(data, "ssl_set_session returned -0x%x", -ret); + return CURLE_SSL_CONNECT_ERROR; + } infof(data, "PolarSSL re-using session\n"); } - ssl_set_session(&connssl->ssl, - &connssl->ssn); - ssl_set_ca_chain(&connssl->ssl, &connssl->cacert, &connssl->crl, @@ -551,40 +551,36 @@ static CURLcode polarssl_connect_step3(struct connectdata *conn, int sockindex) { - CURLcode result = CURLE_OK; + CURLcode retcode = CURLE_OK; struct ssl_connect_data *connssl = &conn->ssl[sockindex]; struct SessionHandle *data = conn->data; void *old_ssl_sessionid = NULL; - ssl_session *our_ssl_sessionid = &conn->ssl[sockindex].ssn; - bool incache; + ssl_session *our_ssl_sessionid; + int ret; DEBUGASSERT(ssl_connect_3 == connssl->connecting_state); - /* Save the current session data for possible re-use */ - incache = !(Curl_ssl_getsessionid(conn, &old_ssl_sessionid, NULL)); - if(incache) { - if(old_ssl_sessionid != our_ssl_sessionid) { - infof(data, "old SSL session ID is stale, removing\n"); - Curl_ssl_delsessionid(conn, old_ssl_sessionid); - incache = FALSE; - } + our_ssl_sessionid = malloc(sizeof(ssl_session)); + if(!our_ssl_sessionid) + return CURLE_OUT_OF_MEMORY; + + ssl_session_init(our_ssl_sessionid); + + ret = ssl_get_session(&connssl->ssl, our_ssl_sessionid); + if(ret) { + failf(data, "ssl_get_session returned -0x%x", -ret); + return CURLE_SSL_CONNECT_ERROR; } - if(!incache) { - void *new_session = malloc(sizeof(ssl_session)); + /* If there's already a matching session in the cache, delete it */ + if(!Curl_ssl_getsessionid(conn, &old_ssl_sessionid, NULL)) + Curl_ssl_delsessionid(conn, old_ssl_sessionid); - if(new_session) { - memcpy(new_session, our_ssl_sessionid, sizeof(ssl_session)); - - result = Curl_ssl_addsessionid(conn, new_session, sizeof(ssl_session)); - } - else - result = CURLE_OUT_OF_MEMORY; - - if(result) { - failf(data, "failed to store ssl session"); - return result; - } + retcode = Curl_ssl_addsessionid(conn, our_ssl_sessionid, 0); + if(retcode) { + free(our_ssl_sessionid); + failf(data, "failed to store ssl session"); + return retcode; } connssl->connecting_state = ssl_connect_done; @@ -649,6 +645,7 @@ static ssize_t polarssl_recv(struct connectdata *conn, void Curl_polarssl_session_free(void *ptr) { + ssl_session_free(ptr); free(ptr); }