- Michael Smith posted bug report #2786255

  (http://curl.haxx.se/bug/view.cgi?id=2786255) with a patch, identifying how
  libcurl did not deal with SSL session ids properly if the server rejected a
  re-use of one. Starting now, it will forget the rejected one and remember
  the new. This change was for OpenSSL only, it is likely that other SSL lib
  code needs similar fixes.

5 files changed, 54 insertions, 19 deletions

diff --git a/CHANGES b/CHANGES
index bef9b1262..e1bea9fc8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,14 @@
 
                                   Changelog
 
+Daniel Stenberg (4 May 2009)
+- Michael Smith posted bug report #2786255
+  (http://curl.haxx.se/bug/view.cgi?id=2786255) with a patch, identifying how
+  libcurl did not deal with SSL session ids properly if the server rejected a
+  re-use of one. Starting now, it will forget the rejected one and remember
+  the new. This change was for OpenSSL only, it is likely that other SSL lib
+  code needs similar fixes.
+
 Yang Tse (4 May 2009)
 - Applied David McCreedy's "transfer.c fixes for CURL_DO_LINEEND_CONV and
   non-ASCII platform HTTP requests" patch addressing two HTTP PUT problems:
diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 11e3a6648..af2197c8e 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -39,6 +39,7 @@ This release includes the following bugfixes:
  o TFTP problems after a failed transfer to the same host
  o improved out of the box TPF compatibility
  o HTTP PUT protocol line endings portions mangled from CRLF to CRCRLF
+ o Rejected SSL session ids are killed properly (for OpenSSL builds)
 
 This release includes the following known bugs:
 
@@ -51,6 +52,6 @@ advice from friends like these:
  Andre Guibert de Bruet, Andreas Farber, Frank Hempel, Pierre Brico,
  Kamil Dudka, Jim Freeman, Daniel Johnson, Toshio Kuratomi, Martin Storsjo,
  Pramod Sharma, Gisle Vanem, Leanic Lefever, Rainer Koenig, Sven Wegener,
- Tim Chen, Constantine Sapuntzakis, David McCreedy
+ Tim Chen, Constantine Sapuntzakis, David McCreedy, Michael Smith
 
         Thanks! (and sorry if I forgot to mention someone)
diff --git a/lib/sslgen.c b/lib/sslgen.c
index f512a8807..6352224f1 100644
--- a/lib/sslgen.c
+++ b/lib/sslgen.c
@@ -272,6 +272,22 @@ static int kill_session(struct curl_ssl_session *session)
 }
 
 /*
+ * Delete the given session ID from the cache.
+ */
+void Curl_ssl_delsessionid(struct connectdata *conn, void *ssl_sessionid)
+{
+  int i;
+  for(i=0; i< conn->data->set.ssl.numsessions; i++) {
+    struct curl_ssl_session *check = &conn->data->state.session[i];
+
+    if (check->sessionid == ssl_sessionid) {
+      kill_session(check);
+      break;
+    }
+  }
+}
+
+/*
  * Store session id in the session cache. The ID passed on to this function
  * must already have been extracted and allocated the proper way for the SSL
  * layer. Curl_XXXX_session_free() will be called to free/kill the session ID
diff --git a/lib/sslgen.h b/lib/sslgen.h
index bbe7de7e9..d55bd1bb3 100644
--- a/lib/sslgen.h
+++ b/lib/sslgen.h
@@ -71,6 +71,8 @@ int Curl_ssl_getsessionid(struct connectdata *conn,
 CURLcode Curl_ssl_addsessionid(struct connectdata *conn,
                                void *ssl_sessionid,
                                size_t idsize);
+/* delete a session from the cache */
+void Curl_ssl_delsessionid(struct connectdata *conn, void *ssl_sessionid);
 
 #define SSL_SHUTDOWN_TIMEOUT 10000 /* ms */
 
diff --git a/lib/ssluse.c b/lib/ssluse.c
index 2e6928d77..a86c2808b 100644
--- a/lib/ssluse.c
+++ b/lib/ssluse.c
@@ -2177,35 +2177,43 @@ ossl_connect_step3(struct connectdata *conn,
                    int sockindex)
 {
   CURLcode retcode = CURLE_OK;
-  void *ssl_sessionid=NULL;
+  void *old_ssl_sessionid=NULL;
   struct SessionHandle *data = conn->data;
   struct ssl_connect_data *connssl = &conn->ssl[sockindex];
+  int incache;
+  SSL_SESSION *our_ssl_sessionid;
 
   DEBUGASSERT(ssl_connect_3 == connssl->connecting_state);
 
-  if(Curl_ssl_getsessionid(conn, &ssl_sessionid, NULL)) {
-    /* Since this is not a cached session ID, then we want to stach this one
-       in the cache! */
-    SSL_SESSION *our_ssl_sessionid;
 #ifdef HAVE_SSL_GET1_SESSION
-    our_ssl_sessionid = SSL_get1_session(connssl->handle);
+  our_ssl_sessionid = SSL_get1_session(connssl->handle);
 
-    /* SSL_get1_session() will increment the reference
-       count and the session will stay in memory until explicitly freed with
-       SSL_SESSION_free(3), regardless of its state.
-       This function was introduced in openssl 0.9.5a. */
+  /* SSL_get1_session() will increment the reference
+     count and the session will stay in memory until explicitly freed with
+     SSL_SESSION_free(3), regardless of its state.
+     This function was introduced in openssl 0.9.5a. */
 #else
-    our_ssl_sessionid = SSL_get_session(connssl->handle);
+  our_ssl_sessionid = SSL_get_session(connssl->handle);
 
-    /* if SSL_get1_session() is unavailable, use SSL_get_session().
-       This is an inferior option because the session can be flushed
-       at any time by openssl. It is included only so curl compiles
-       under versions of openssl < 0.9.5a.
+  /* if SSL_get1_session() is unavailable, use SSL_get_session().
+     This is an inferior option because the session can be flushed
+     at any time by openssl. It is included only so curl compiles
+     under versions of openssl < 0.9.5a.
 
-       WARNING: How curl behaves if it's session is flushed is
-       untested.
-    */
+     WARNING: How curl behaves if it's session is flushed is
+     untested.
+  */
 #endif
+
+  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;
+    }
+  }
+  if (!incache) {
     retcode = Curl_ssl_addsessionid(conn, our_ssl_sessionid,
                                     0 /* unknown size */);
     if(retcode) {