diff options
| -rw-r--r-- | ChangeLog | 14 | ||||
| -rw-r--r-- | ext/openssl/ossl_ssl.c | 10 | ||||
| -rw-r--r-- | test/openssl/test_ssl_session.rb | 61 |
3 files changed, 82 insertions, 3 deletions
@@ -1,3 +1,17 @@ +Wed Jun 22 22:34:05 2011 Hiroshi Nakamura <nahi@ruby-lang.org> + + * 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@googlemail.com> * ext/openssl/ossl_ssl_session.c: Respect T_BIGNUM time values. Patch by diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index fd7b9f5e44..a9f31020eb 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -400,10 +400,16 @@ ossl_sslctx_session_new_cb(SSL *ssl, SSL_SESSION *sess) 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 diff --git a/test/openssl/test_ssl_session.rb b/test/openssl/test_ssl_session.rb index b4563bb126..05e9f61b7c 100644 --- a/test/openssl/test_ssl_session.rb +++ b/test/openssl/test_ssl_session.rb @@ -159,16 +159,18 @@ class OpenSSL::TestSSLSession < OpenSSL::SSLTestCase 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 @@ class OpenSSL::TestSSLSession < OpenSSL::SSLTestCase 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 |