From 53a549000c3634f6b0a5ed262d5834c3145885d7 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 20 Feb 2008 09:56:26 +0000 Subject: - Based on initial work done by Gautam Kachroo to address a bug, we now keep better control at the exact state of the connection's SSL status so that we know exactly when it has completed the SSL negotiation or not so that there won't be accidental re-uses of connections that are wrongly believed to be in SSL-completed-negotiate state. --- CHANGES | 6 ++++++ RELEASE-NOTES | 3 ++- lib/gtls.c | 2 ++ lib/nss.c | 2 ++ lib/qssl.c | 3 +++ lib/sendf.c | 4 ++-- lib/sslgen.c | 4 +++- lib/ssluse.c | 7 ++++--- lib/url.c | 15 +++++++++++---- lib/urldata.h | 14 +++++++++++--- 10 files changed, 46 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index 4dd0b7dde..54485bea8 100644 --- a/CHANGES +++ b/CHANGES @@ -7,6 +7,12 @@ Changelog Daniel S (20 Feb 2008) +- Based on initial work done by Gautam Kachroo to address a bug, we now keep + better control at the exact state of the connection's SSL status so that we + know exactly when it has completed the SSL negotiation or not so that there + won't be accidental re-uses of connections that are wrongly believed to be + in SSL-completed-negotiate state. + - We no longer support setting the CURLOPT_URL option from inside a callback such as the CURLOPT_SSL_CTX_FUNCTION one treat that as if it was a Location: following. The patch that introduced this feature was done for 7.11.0, but diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 9279ea1fc..44b31da8c 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -27,6 +27,7 @@ This release includes the following bugfixes: problems with the cert o when using the multi interface and a handle is removed while still having a transfer going on, the connection is now closed by force + o bad re-use of SSL connections in non-complete state This release includes the following known bugs: @@ -45,6 +46,6 @@ advice from friends like these: Michal Marek, Dmitry Kurochkin, Niklas Angebrand, Günter Knauf, Yang Tse, Dan Fandrich, Mike Hommey, Pooyan McSporran, Jerome Muffat-Meridol, - Kaspar Brand + Kaspar Brand, Gautam Kachroo Thanks! (and sorry if I forgot to mention someone) diff --git a/lib/gtls.c b/lib/gtls.c index 2364c2778..3bf988194 100644 --- a/lib/gtls.c +++ b/lib/gtls.c @@ -501,6 +501,8 @@ Curl_gtls_connect(struct connectdata *conn, ptr = gnutls_mac_get_name(gnutls_mac_get(session)); infof(data, "\t MAC: %s\n", ptr); + connssl->state = ssl_connection_complete; + if(!ssl_sessionid) { /* this session was not previously in the cache, add it now */ diff --git a/lib/nss.c b/lib/nss.c index 6e3ee8604..c8d728d3e 100644 --- a/lib/nss.c +++ b/lib/nss.c @@ -1022,6 +1022,8 @@ CURLcode Curl_nss_connect(struct connectdata * conn, int sockindex) goto error; } + connssl->state = ssl_connection_complete; + display_conn_info(conn, connssl->handle); return CURLE_OK; diff --git a/lib/qssl.c b/lib/qssl.c index 0252b465e..ad04cc7b3 100644 --- a/lib/qssl.c +++ b/lib/qssl.c @@ -258,8 +258,11 @@ CURLcode Curl_qsossl_connect(struct connectdata * conn, int sockindex) SSL_Destroy(connssl->handle); connssl->handle = NULL; connssl->use = FALSE; + connssl->state = ssl_connection_none; } } + if (rc == CURLE_OK) + connssl->state = ssl_connection_complete; return rc; } diff --git a/lib/sendf.c b/lib/sendf.c index fe340236f..6aa4d8613 100644 --- a/lib/sendf.c +++ b/lib/sendf.c @@ -355,7 +355,7 @@ CURLcode Curl_write(struct connectdata *conn, CURLcode retcode; int num = (sockfd == conn->sock[SECONDARYSOCKET]); - if(conn->ssl[num].use) + if(conn->ssl[num].state == ssl_connection_complete) /* only TRUE if SSL enabled */ bytes_written = Curl_ssl_send(conn, num, mem, len); #ifdef USE_LIBSSH2 @@ -567,7 +567,7 @@ int Curl_read(struct connectdata *conn, /* connection data */ buffertofill = buf; } - if(conn->ssl[num].use) { + if(conn->ssl[num].state == ssl_connection_complete) { nread = Curl_ssl_recv(conn, num, buffertofill, bytesfromsocket); if(nread == -1) { diff --git a/lib/sslgen.c b/lib/sslgen.c index 5719c0bf2..bbcc8c56d 100644 --- a/lib/sslgen.c +++ b/lib/sslgen.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2007, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2008, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -207,6 +207,7 @@ Curl_ssl_connect(struct connectdata *conn, int sockindex) #ifdef USE_SSL /* mark this is being ssl enabled from here on. */ conn->ssl[sockindex].use = TRUE; + conn->ssl[sockindex].state = ssl_connection_negotiating; #ifdef USE_SSLEAY return Curl_ossl_connect(conn, sockindex); @@ -473,6 +474,7 @@ CURLcode Curl_ssl_shutdown(struct connectdata *conn, int sockindex) #endif /* USE_SSLEAY */ conn->ssl[sockindex].use = FALSE; /* get back to ordinary socket usage */ + conn->ssl[sockindex].state = ssl_connection_none; return CURLE_OK; } diff --git a/lib/ssluse.c b/lib/ssluse.c index 1e9b48a49..ac6b057cb 100644 --- a/lib/ssluse.c +++ b/lib/ssluse.c @@ -1737,7 +1737,8 @@ ossl_connect_common(struct connectdata *conn, connssl->connecting_state?sockfd:CURL_SOCKET_BAD; while(1) { - int what = Curl_socket_ready(readfd, writefd, nonblocking?0:(int)timeout_ms); + int what = Curl_socket_ready(readfd, writefd, + nonblocking?0:(int)timeout_ms); if(what > 0) /* readable or writable, go loop in the outer loop */ break; @@ -1775,11 +1776,11 @@ ossl_connect_common(struct connectdata *conn, } if(ssl_connect_done==connssl->connecting_state) { + connssl->state = ssl_connection_complete; *done = TRUE; } - else { + else *done = FALSE; - } /* Reset our connect state machine */ connssl->connecting_state = ssl_connect_1; diff --git a/lib/url.c b/lib/url.c index 71a533d25..3c3605c1a 100644 --- a/lib/url.c +++ b/lib/url.c @@ -2439,10 +2439,17 @@ ConnectionExists(struct SessionHandle *data, ssl options as well */ if(!Curl_ssl_config_matches(&needle->ssl_config, &check->ssl_config)) { - infof(data, - "Connection #%ld has different SSL parameters, " - "can't reuse\n", - check->connectindex ); + DEBUGF(infof(data, + "Connection #%ld has different SSL parameters, " + "can't reuse\n", + check->connectindex)); + continue; + } + else if(check->ssl[FIRSTSOCKET].state != ssl_connection_complete) { + DEBUGF(infof(data, + "Connection #%ld has not started ssl connect, " + "can't reuse\n", + check->connectindex)); continue; } } diff --git a/lib/urldata.h b/lib/urldata.h index 2eeb08c20..95ef36b82 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -168,11 +168,19 @@ typedef enum { ssl_connect_done } ssl_connect_state; +typedef enum { + ssl_connection_none, + ssl_connection_negotiating, + ssl_connection_complete +} ssl_connection_state; + /* struct for data related to each SSL connection */ struct ssl_connect_data { - bool use; /* use ssl encrypted communications TRUE/FALSE, not - necessarily using it atm but at least asked to or - meaning to use it */ + /* Use ssl encrypted communications TRUE/FALSE, not necessarily using it atm + but at least asked to or meaning to use it. See 'state' for the exact + current state of the connection. */ + bool use; + ssl_connection_state state; #ifdef USE_SSLEAY /* these ones requires specific SSL-types */ SSL_CTX* ctx; -- cgit v1.2.3