Issue #11401 has been updated by Usaku NAKAMURA.

Backport changed from 2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: DONE to 2.0.0: REQUIRED, 2.1: DONE, 2.2: DONE

ruby_2_1 r52799 merged revision(s) 52682.

----------------------------------------
Bug #11401: Net::HTTP SSL session resumption does not send SNI
https://bugs.ruby-lang.org/issues/11401#change-55162

* Author: Michiel Karnebeek
* Status: Closed
* Priority: Normal
* Assignee: openssl
* ruby -v: 
* Backport: 2.0.0: REQUIRED, 2.1: DONE, 2.2: DONE
----------------------------------------
See https://github.com/ruby/ruby/pull/964

## Problem

When an initial SSL request is done, Net::HTTP stores the OpenSSL::SSL::Session object in @ssl_session.

When (after the http-keep-alive timeout has expired, or the connection was closed for some other reason) a second http request is made by Net::Http, resulting in calling Net::Http#connect (see the relevant pieces of code below, while reading the following points).

* #connect first calls `OpenSSL::SSL::SSLSocket#session=` at http.rb:924, which, in C code, eventually calls the C-method ossl_ssl_setup
 * which executes the `if(!ssl) {` block at ossl_ssl.c:1205, but since @hostname has not been set yet, it will not execute SSL_set_tlsext_host_name. Also, because the OpenSSL::SSL::Session object does not contain a hostname, it is not known to OpenSSL at this point.
* it then calls `OpenSSL::SSL::SSLSocket#hostname=` at http.rb:927 which only sets @hostname on OpenSSL::SSL::SSLSocket
* and then it calls `OpenSSL::SSL::SSLSocket#connect` at http.rb:941
 * which is doing the second call to the C-method ossl_ssl_setup, but since the `if(!ssl) {` already ran, it won't run again, and won't set the hostname from @hostname to SSL_set_tlsext_host_name.

This causes the second request to contains a SSL Session Ticket, but not a SNI header. This is easily verified by doing 2 calls in a ruby script, with a sleep 2.1 in between (http-keep-alive timeout is 2 seconds by default) and checking the second Client Hello message in Wireshark.

Normally this does not cause any issues, because the server looks at the SSL Session Ticket and knows for which virtual host it issued the ticket. So when a second request comes in with that ticket, it assumes the request should be handled by that vhost.

However, this breaks when the client (Ruby) thinks the session ticket is still valid (#10533 did some fixing), sends it to the server, but the server denies it. The server then starts to renegotiate the SSL session, but since the SNI header is missing, it won't know for which vhost, and sends the SSL certificate for the default vhost, which may not be the vhost it wants to connect to. The client (Ruby) then checks the certificate against the hostname it was connecting to, and finds out it doesn't match.

So, this only occurs on SSL session resumption (the second http request after http-keep-alive expired, or the connection was closed), when connecting to non-default vhosts which has a different certificate set than the default vhost, and when the client thinks the SSL Session Ticket is valid, but the server disagrees.

Why would the server deny the SSL session ticket? The client already checked if it was valid, right? Well, all kind of reasons:

* Server may have invalidated the ticket earlier than the client
* Server rebooted
* Time drift
* ...
* but mostly because the SSL termination for a specific hostname may be handled by multiple servers, which are not sharing their SSL session tickets (or sharing them in a delayed matter).

## Solution
The solution is to move the call to `OpenSSL::SSL::SSLSocket#hostname=` before the call to `OpenSSL::SSL::SSLSocket#session=`, so the hostname gets set when ossl_ssl_set_session calls ossl_ssl_setup

## Relevant code pieces

http.rb

~~~
868	    def connect
...
878	      s = Timeout.timeout(@open_timeout, Net::OpenTimeout) {
...
885	      }
886	      s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1)
887	      D "opened"
888	      if use_ssl?
...
897	        @ssl_context = OpenSSL::SSL::SSLContext.new
898	        @ssl_context.set_params(ssl_parameters)
899	        D "starting SSL for #{conn_address}:#{conn_port}..."
900	        s = OpenSSL::SSL::SSLSocket.new(s, @ssl_context)              <-- it always re-creates the OpenSSL::SSL::SSLSocket object
901	        s.sync_close = true
902	        D "SSL established"
903	      end
904	      @socket = BufferedIO.new(s)
...
908	      if use_ssl?
909	        begin
910	          if proxy?
...
921	          end
922	          if @ssl_session and
923	             Process.clock_gettime(Process::CLOCK_REALTIME) < @ssl_session.time.to_f + @ssl_session.timeout
924	            s.session = @ssl_session if @ssl_session                 <-- (2nd request) this call already sets up the connection 
                                                                                 (calls the C-method ossl_ssl_setup)
                                                                                 also, this does not set the hostname, 
                                                                                 as OpenSSL::SSL::Session does not contain a hostname
925	          end
926	          # Server Name Indication (SNI) RFC 3546
927	          s.hostname = @address if s.respond_to? :hostname=          <-- Only sets the hostname to @hostname on OpenSSL::SSL::SSLSocket.
                                                                                 It relies on ossl_ssl_setup to actually set it to openssl 
                                                                                 (SSL_set_tlsext_host_name at ossl_ssl.c:1221
928	          if timeout = @open_timeout
929	            while true
930	              raise Net::OpenTimeout if timeout <= 0
931	              start = Process.clock_gettime Process::CLOCK_MONOTONIC
932	              # to_io is requied because SSLSocket doesn't have wait_readable yet
933	              case s.connect_nonblock(exception: false)
934	              when :wait_readable; s.to_io.wait_readable(timeout)
935	              when :wait_writable; s.to_io.wait_writable(timeout)
936	              else; break
937	              end
938	              timeout -= Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
939	            end
940	          else
941	            s.connect                                                <-- triggers another call to the C-method ossl_ssl_setup, 
                                                                                 but does not set up hostname, as it already ran once
                                                                                (see ossl_ssl.c:1205)
942	          end
943	          if @ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE
944	            s.post_connection_check(@address)
945	          end
946	          @ssl_session = s.session                                   <-- (1st request) does not store hostname, so the call at line 924 does not set it.
947	        rescue => exception
948	          D "Conn close because of connect error #{exception}"
949	          @socket.close if @socket and not @socket.closed?
950	          raise exception
951	        end
952	      end
953	      on_connect
954	    end
~~~

ossl_ssl.c

~~~
101    static const char *ossl_ssl_attrs[] = {
102    #ifdef HAVE_SSL_SET_TLSEXT_HOST_NAME
103        "hostname",
104    #endif
105        "sync_close",
106    };
...
1330    ossl_ssl_connect(VALUE self)
1331    {
1332        ossl_ssl_setup(self);
1333        return ossl_start_ssl(self, SSL_connect, "SSL_connect", 0, 0);
1334    }
...
1197    ossl_ssl_setup(VALUE self)    
1198    {    
1199        VALUE io, v_ctx, cb;    
1200        SSL_CTX *ctx;    
1201        SSL *ssl;    
1202        rb_io_t *fptr;    
1203        
1204        GetSSL(self, ssl);    
1205        if(!ssl){    
1206    #ifdef HAVE_SSL_SET_TLSEXT_HOST_NAME    
1207        VALUE hostname = rb_iv_get(self, "@hostname");
1208    #endif    
1209        
1210            v_ctx = ossl_ssl_get_ctx(self);    
1211            GetSSLCTX(v_ctx, ctx);    
1212        
1213            ssl = SSL_new(ctx);    
1214            if (!ssl) {    
1215                ossl_raise(eSSLError, "SSL_new");    
1216            }    
1217            DATA_PTR(self) = ssl;    
1218        
1219    #ifdef HAVE_SSL_SET_TLSEXT_HOST_NAME    
1220            if (!NIL_P(hostname)) {    
1221               if (SSL_set_tlsext_host_name(ssl, StringValuePtr(hostname)) != 1)    
1222                   ossl_raise(eSSLError, "SSL_set_tlsext_host_name");    
1223            }    
1224    #endif    
1225            io = ossl_ssl_get_io(self);    
1226            GetOpenFile(io, fptr);    
1227            rb_io_check_readable(fptr);    
1228            rb_io_check_writable(fptr);    
1229            SSL_set_fd(ssl, TO_SOCKET(FPTR_TO_FD(fptr)));    
1230            SSL_set_ex_data(ssl, ossl_ssl_ex_ptr_idx, (void*)self);
1231            cb = ossl_sslctx_get_verify_cb(v_ctx);
1232            SSL_set_ex_data(ssl, ossl_ssl_ex_vcb_idx, (void*)cb);
1233            cb = ossl_sslctx_get_client_cert_cb(v_ctx);
1234            SSL_set_ex_data(ssl, ossl_ssl_ex_client_cert_cb_idx, (void*)cb);
1235            cb = ossl_sslctx_get_tmp_dh_cb(v_ctx);
1236            SSL_set_ex_data(ssl, ossl_ssl_ex_tmp_dh_callback_idx, (void*)cb);
1237            SSL_set_info_callback(ssl, ssl_info_cb);
1238        }    
1239        
1240        return Qtrue;    
1241    }    
...
1825    ossl_ssl_set_session(VALUE self, VALUE arg1)
1826    {
1827        SSL *ssl;
1828        SSL_SESSION *sess;
1829    
1830    /* why is ossl_ssl_setup delayed? */
1831        ossl_ssl_setup(self);
1832    
1833        ossl_ssl_data_get_struct(self, ssl);
1834    
1835        SafeGetSSLSession(arg1, sess);
1836    
1837        if (SSL_set_session(ssl, sess) != 1)
1838            ossl_raise(eSSLError, "SSL_set_session");
1839    
1840        return arg1;
1841    }

~~~

Patch to http.rb:

~~~

@@ -914,12 +914,12 @@ module Net   #:nodoc:
             @socket.write(buf)
             HTTPResponse.read_new(@socket).value
           end
+          # Server Name Indication (SNI) RFC 3546
+          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
           end
-          # Server Name Indication (SNI) RFC 3546
-          s.hostname = @address if s.respond_to? :hostname=
           Timeout.timeout(@open_timeout, Net::OpenTimeout) { s.connect }
           if @ssl_context.verify_mode != OpenSSL::SSL::VERIFY_NONE
             s.post_connection_check(@address)
~~~

---Files--------------------------------
net.http.bug.patch (884 Bytes)


-- 
https://bugs.ruby-lang.org/