From 1dfc377ae3b174b043d3f0ed36de57b0296b34d0 Mon Sep 17 00:00:00 2001

From: rhe <rhe@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>

Date: Wed, 8 Aug 2018 14:13:55 +0000

Subject: [PATCH] net/http, net/ftp: fix session resumption with TLS 1.3

When TLS 1.3 is in use, the session ticket may not have been sent yet

even though a handshake has finished. Also, the ticket could change if

multiple session ticket messages are sent by the server. Use

SSLContext#session_new_cb instead of calling SSLSocket#session

immediately after a handshake. This way also works with earlier protocol

versions.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@64234 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

---

 lib/net/ftp.rb              |  5 ++++-

 lib/net/http.rb             |  7 +++++--

 test/net/http/test_https.rb | 35 ++++++++++-------------------------

 3 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/lib/net/ftp.rb b/lib/net/ftp.rb

index c3ee47ef4d36..9902f9dc657a 100644

--- a/lib/net/ftp.rb

+++ b/lib/net/ftp.rb

@@ -230,6 +230,10 @@ def initialize(host = nil, user_or_options = {}, passwd = nil, acct = nil)

         if defined?(VerifyCallbackProc)

           @ssl_context.verify_callback = VerifyCallbackProc

         end

+        @ssl_context.session_cache_mode =

+          OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT |

+          OpenSSL::SSL::SSLContext::SESSION_CACHE_NO_INTERNAL_STORE

+        @ssl_context.session_new_cb = proc {|sock, sess| @ssl_session = sess }

         @ssl_session = nil

         if options[:private_data_connection].nil?

           @private_data_connection = true

@@ -349,7 +353,6 @@ def start_tls_session(sock)

       if @ssl_context.verify_mode != VERIFY_NONE

         ssl_sock.post_connection_check(@host)

       end

-      @ssl_session = ssl_sock.session

       return ssl_sock

     end

     private :start_tls_session

diff --git a/lib/net/http.rb b/lib/net/http.rb

index 281b15cedff0..683a884f5dbe 100644

--- a/lib/net/http.rb

+++ b/lib/net/http.rb

@@ -969,6 +969,10 @@ def connect

         end

         @ssl_context = OpenSSL::SSL::SSLContext.new

         @ssl_context.set_params(ssl_parameters)

+        @ssl_context.session_cache_mode =

+          OpenSSL::SSL::SSLContext::SESSION_CACHE_CLIENT |

+          OpenSSL::SSL::SSLContext::SESSION_CACHE_NO_INTERNAL_STORE

+        @ssl_context.session_new_cb = proc {|sock, sess| @ssl_session = sess }

         D "starting SSL for #{conn_address}:#{conn_port}..."

         s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)

         s.sync_close = true

@@ -976,13 +980,12 @@ def connect

         s.hostname = @address if s.respond_to? :hostname=

         if @ssl_session and

            Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout

-          s.session = @ssl_session if @ssl_session

+          s.session = @ssl_session

         end

         ssl_socket_connect(s, @open_timeout)

         if @ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE

           s.post_connection_check(@address)

         end

-        @ssl_session = s.session

         D "SSL established"

       end

       @socket = BufferedIO.new(s, read_timeout: @read_timeout,

diff --git a/test/net/http/test_https.rb b/test/net/http/test_https.rb

index 8004d5c5f29f..a5182a1fe9db 100644

--- a/test/net/http/test_https.rb

+++ b/test/net/http/test_https.rb

@@ -71,20 +71,11 @@ def test_session_reuse

     http.get("/")

     http.finish

-    http.start

-    http.get("/")

-    http.finish # three times due to possible bug in OpenSSL 0.9.8

-

-    sid = http.instance_variable_get(:@ssl_session).id

-

     http.start

     http.get("/")

     socket = http.instance_variable_get(:@socket).io

-

-    assert socket.session_reused?

-

-    assert_equal sid, http.instance_variable_get(:@ssl_session).id

+    assert_equal true, socket.session_reused?

     http.finish

   rescue SystemCallError

@@ -101,16 +92,12 @@ def test_session_reuse_but_expire

     http.get("/")

     http.finish

-    sid = http.instance_variable_get(:@ssl_session).id

-

     http.start

     http.get("/")

     socket = http.instance_variable_get(:@socket).io

     assert_equal false, socket.session_reused?

-    assert_not_equal sid, http.instance_variable_get(:@ssl_session).id

-

     http.finish

   rescue SystemCallError

     skip $!

@@ -160,15 +147,16 @@ def test_certificate_verify_failure

   end

   def test_identity_verify_failure

+    # the certificate's subject has CN=localhost

     http = Net::HTTP.new("127.0.0.1", config("port"))

     http.use_ssl = true

-    http.verify_callback = Proc.new do |preverify_ok, store_ctx|

-      true

-    end

+    http.cert_store = TEST_STORE

+    @log_tester = lambda {|_| }

     ex = assert_raise(OpenSSL::SSL::SSLError){

       http.request_get("/") {|res| }

     }

-    assert_match(/hostname \"127.0.0.1\" does not match/, ex.message)

+    re_msg = /certificate verify failed|hostname \"127.0.0.1\" does not match/

+    assert_match(re_msg, ex.message)

   end

   def test_timeout_during_SSL_handshake

@@ -193,16 +181,13 @@ def test_timeout_during_SSL_handshake

   end

   def test_min_version

-    http = Net::HTTP.new("127.0.0.1", config("port"))

+    http = Net::HTTP.new("localhost", config("port"))

     http.use_ssl = true

     http.min_version = :TLS1

-    http.verify_callback = Proc.new do |preverify_ok, store_ctx|

-      true

-    end

-    ex = assert_raise(OpenSSL::SSL::SSLError){

-      http.request_get("/") {|res| }

+    http.cert_store = TEST_STORE

+    http.request_get("/") {|res|

+      assert_equal($test_net_http_data, res.body)

     }

-    assert_match(/hostname \"127.0.0.1\" does not match/, ex.message)

   end

   def test_max_version