nahi	2011-06-22 22:39:56 +0900 (Wed, 22 Jun 2011)

  New Revision: 32204

  http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=32204

  Log:
    * ext/openssl/ossl_ssl.c (ossl_sslctx_session_new_cb): Return 0 to
      OpenSSL from the callback for SSL_CTX_sess_set_get_cb().
      Returning 0 means to OpenSSL that the the session is still valid
      (since we created Ruby Session object) and was not freed by us with
      SSL_SESSION_free(). Call SSLContext#remove_session(sess) in
      session_get_cb block if you don't want OpenSSL to cache the session
      internally.
      This potential issue was pointed by Ippei Obayashi. See #4416.
    
    * test/openssl/test_ssl_session.rb (test_ctx_server_session_cb): Test
      it.

  Modified files:
    trunk/ChangeLog
    trunk/ext/openssl/ossl_ssl.c
    trunk/test/openssl/test_ssl_session.rb

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 32203)
+++ ChangeLog	(revision 32204)
@@ -1,3 +1,17 @@
+Wed Jun 22 22:34:05 2011  Hiroshi Nakamura  <nahi@r...>
+
+	* ext/openssl/ossl_ssl.c (ossl_sslctx_session_new_cb): Return 0 to
+	  OpenSSL from the callback for SSL_CTX_sess_set_get_cb().
+	  Returning 0 means to OpenSSL that the the session is still valid
+	  (since we created Ruby Session object) and was not freed by us with
+	  SSL_SESSION_free(). Call SSLContext#remove_session(sess) in
+	  session_get_cb block if you don't want OpenSSL to cache the session
+	  internally.
+	  This potential issue was pointed by Ippei Obayashi. See #4416.
+
+	* test/openssl/test_ssl_session.rb (test_ctx_server_session_cb): Test
+	  it.
+
 Wed Jun 22 22:21:17 2011  Martin Bosslet  <Martin.Bosslet@g...>
 
 	* ext/openssl/ossl_ssl_session.c: Respect T_BIGNUM time values. Patch by
Index: ext/openssl/ossl_ssl.c
===================================================================
--- ext/openssl/ossl_ssl.c	(revision 32203)
+++ ext/openssl/ossl_ssl.c	(revision 32204)
@@ -400,10 +400,16 @@
     ret_obj = rb_protect((VALUE(*)_((VALUE)))ossl_call_session_new_cb, ary, &state);
     if (state) {
         rb_ivar_set(ssl_obj, ID_callback_state, INT2NUM(state));
-        return 0; /* what should be returned here??? */
     }
 
-    return RTEST(ret_obj) ? 1 : 0;
+    /*
+     * return 0 which means to OpenSSL that the the session is still
+     * valid (since we created Ruby Session object) and was not freed by us
+     * with SSL_SESSION_free(). Call SSLContext#remove_session(sess) in
+     * session_get_cb block if you don't want OpenSSL to cache the session
+     * internally.
+     */
+    return 0;
 }
 
 static VALUE
Index: test/openssl/test_ssl_session.rb
===================================================================
--- test/openssl/test_ssl_session.rb	(revision 32203)
+++ test/openssl/test_ssl_session.rb	(revision 32204)
@@ -159,16 +159,18 @@
     called = {}
     ctx = OpenSSL::SSL::SSLContext.new("SSLv3")
     ctx.session_cache_mode = OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT
+
     ctx.session_new_cb = lambda { |ary|
       sock, sess = ary
       called[:new] = [sock, sess]
-      true
     }
+
     ctx.session_remove_cb = lambda { |ary|
       ctx, sess = ary
       called[:remove] = [ctx, sess]
       # any resulting value is OK (ignored)
     }
+
     start_server(PORT, OpenSSL::SSL::VERIFY_NONE, true) do |server, port|
       sock = TCPSocket.new("127.0.0.1", port)
       ssl = OpenSSL::SSL::SSLSocket.new(sock, ctx)
@@ -183,6 +185,63 @@
       ssl.close
     end
   end
+
+  def test_ctx_server_session_cb
+    called = {}
+
+    ctx_proc = Proc.new { |ctx, ssl|
+      ctx.session_cache_mode = OpenSSL::SSL::SSLContext::SESSION_CACHE_SERVER
+      last_server_session = nil
+
+      # get_cb is called whenever a client proposed to resume a session but
+      # the session could not be found in the internal session cache.
+      ctx.session_get_cb = lambda { |ary|
+        sess, data = ary
+        if last_server_session
+          called[:get2] = [sess, data]
+          last_server_session
+        else
+          called[:get1] = [sess, data]
+          last_server_session = sess
+          nil
+        end
+      }
+
+      ctx.session_new_cb = lambda { |ary|
+        sock, sess = ary
+        called[:new] = [sock, sess]
+        # SSL server doesn't cache sessions so get_cb is called next time.
+        ctx.session_remove(sess)
+      }
+
+      ctx.session_remove_cb = lambda { |ary|
+        ctx, sess = ary
+        called[:remove] = [ctx, sess]
+      }
+    }
+
+    server_proc = Proc.new { |c, ssl|
+      session = ssl.session
+      stats = c.session_cache_stats
+      readwrite_loop(c, ssl)
+    }
+    start_server(PORT, OpenSSL::SSL::VERIFY_NONE, true, :ctx_proc => ctx_proc, :server_proc => server_proc) do |server, port|
+      last_client_session = nil
+      3.times do
+        sock = TCPSocket.new("127.0.0.1", port)
+        ssl = OpenSSL::SSL::SSLSocket.new(sock, OpenSSL::SSL::SSLContext.new("SSLv3"))
+        ssl.sync_close = true
+        ssl.session = last_client_session if last_client_session
+        ssl.connect
+        last_client_session = ssl.session
+        ssl.close
+        assert(called.delete(:new))
+        assert(called.delete(:remove))
+      end
+    end
+    assert(called[:get1])
+    assert(called[:get2])
+  end
 end
 
 end

--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/