From ad49ea5e9c5614b2bd3e1a69a2c45d44e7d59fc7 Mon Sep 17 00:00:00 2001 From: MSP-Greg Date: Sun, 29 Sep 2019 13:21:59 -0500 Subject: [PATCH] Update per PR #1987 --- lib/puma/binder.rb | 173 ++++++++++++++++++++---------------- test/test_binder.rb | 212 +++++++++++++++++++++++++++++++------------- 2 files changed, 249 insertions(+), 136 deletions(-) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index a5e48c8c28..9df8c5ba12 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -86,6 +86,7 @@ def import_from_env end def parse(binds, logger) + @logger = logger binds.each do |str| uri = URI.parse str case uri.scheme @@ -103,17 +104,6 @@ def parse(binds, logger) bak = params.fetch('backlog', 1024).to_i io = add_tcp_listener uri.host, uri.port, opt, bak - - @ios.each do |i| - next unless TCPServer === i - addr = if i.local_address.ipv6? - "[#{i.local_address.ip_unpack[0]}]:#{i.local_address.ip_unpack[1]}" - else - i.local_address.ip_unpack.join(':') - end - - logger.log "* Listening on tcp://#{addr}" - end end @listeners << [str, io] if io @@ -158,60 +148,7 @@ def parse(binds, logger) MiniSSL.check - ctx = MiniSSL::Context.new - - if defined?(JRUBY_VERSION) - unless params['keystore'] - @events.error "Please specify the Java keystore via 'keystore='" - end - - ctx.keystore = params['keystore'] - - unless params['keystore-pass'] - @events.error "Please specify the Java keystore password via 'keystore-pass='" - end - - ctx.keystore_pass = params['keystore-pass'] - ctx.ssl_cipher_list = params['ssl_cipher_list'] if params['ssl_cipher_list'] - else - unless params['key'] - @events.error "Please specify the SSL key via 'key='" - end - - ctx.key = params['key'] - - unless params['cert'] - @events.error "Please specify the SSL cert via 'cert='" - end - - ctx.cert = params['cert'] - - if ['peer', 'force_peer'].include?(params['verify_mode']) - unless params['ca'] - @events.error "Please specify the SSL ca via 'ca='" - end - end - - ctx.ca = params['ca'] if params['ca'] - ctx.ssl_cipher_filter = params['ssl_cipher_filter'] if params['ssl_cipher_filter'] - end - - ctx.no_tlsv1 = true if params['no_tlsv1'] == 'true' - ctx.no_tlsv1_1 = true if params['no_tlsv1_1'] == 'true' - - if params['verify_mode'] - ctx.verify_mode = case params['verify_mode'] - when "peer" - MiniSSL::VERIFY_PEER - when "force_peer" - MiniSSL::VERIFY_PEER | MiniSSL::VERIFY_FAIL_IF_NO_PEER_CERT - when "none" - MiniSSL::VERIFY_NONE - else - @events.error "Please specify a valid verify_mode=" - MiniSSL::VERIFY_NONE - end - end + ctx = ssl_ctx_init params if fd = @inherited_fds.delete(str) logger.log "* Inherited #{str}" @@ -221,7 +158,6 @@ def parse(binds, logger) logger.log "* Activated #{str}" else io = add_ssl_listener uri.host, uri.port, ctx - logger.log "* Listening on #{str}" end @listeners << [str, io] if io @@ -258,6 +194,7 @@ def parse(binds, logger) # We have to unlink a unix socket path that's not being used File.unlink key[1] if key[0] == :unix end + @logger = nil end def loopback_addresses @@ -274,6 +211,7 @@ def loopback_addresses # allow to accumulate before returning connection refused. # def add_tcp_listener(host, port, optimize_for_latency=true, backlog=1024) + host_str = nil if host == "localhost" loopback_addresses.each do |addr| add_tcp_listener addr, port, optimize_for_latency, backlog @@ -281,17 +219,31 @@ def add_tcp_listener(host, port, optimize_for_latency=true, backlog=1024) return end - host = host[1..-2] if host and host[0..0] == '[' + # ':' means IPV6, loopback addresses may not have brackets + host_str = (host and host.include? ':' and !host.start_with? '[') ? + "[#{host}]" : host + s = TCPServer.new(host, port) + port = s.local_address.ip_port if !port || port.zero? + if optimize_for_latency s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) end s.setsockopt(Socket::SOL_SOCKET,Socket::SO_REUSEADDR, true) s.listen backlog - @connected_port = s.addr[1] + @connected_port = port - @ios << s - s + str = "tcp://#{host_str}:#{port}" + # called from parse + if defined?(@logger) + @logger.log "* Listening on #{str}" + @listeners << [str, s] if s + @ios << s + nil + else + @ios << s + s + end end attr_reader :connected_port @@ -312,6 +264,7 @@ def add_ssl_listener(host, port, ctx, require 'puma/minissl' MiniSSL.check + host_str = nil if host == "localhost" loopback_addresses.each do |addr| @@ -320,22 +273,34 @@ def add_ssl_listener(host, port, ctx, return end - host = host[1..-2] if host[0..0] == '[' + # ':' means IPV6, loopback addresses may not have brackets + host_str = (host and host.include? ':' and !host.start_with? '[') ? + "[#{host}]" : host + s = TCPServer.new(host, port) + port = s.local_address.ip_port if !port || port.zero? + if optimize_for_latency s.setsockopt(Socket::IPPROTO_TCP, Socket::TCP_NODELAY, 1) end s.setsockopt(Socket::SOL_SOCKET,Socket::SO_REUSEADDR, true) s.listen backlog - ssl = MiniSSL::Server.new s, ctx env = @proto_env.dup env[HTTPS_KEY] = HTTPS @envs[ssl] = env - - @ios << ssl - s + str = "ssl://#{host_str}:#{port}" + # called from parse + if defined?(@logger) + @logger.log "* Listening on #{str}" + @listeners << [str, ssl] if ssl + @ios << ssl + nil + else + @ios << ssl + ssl + end end def inherit_ssl_listener(fd, ctx) @@ -428,6 +393,64 @@ def close_unix_paths @unix_paths.each { |up| File.unlink(up) if File.exist? up } end + def ssl_ctx_init(params) + ctx = MiniSSL::Context.new + + if defined?(JRUBY_VERSION) + unless params['keystore'] + @events.error "Please specify the Java keystore via 'keystore='" + end + + ctx.keystore = params['keystore'] + + unless params['keystore-pass'] + @events.error "Please specify the Java keystore password via 'keystore-pass='" + end + + ctx.keystore_pass = params['keystore-pass'] + ctx.ssl_cipher_list = params['ssl_cipher_list'] if params['ssl_cipher_list'] + else + unless params['key'] + @events.error "Please specify the SSL key via 'key='" + end + + ctx.key = params['key'] + + unless params['cert'] + @events.error "Please specify the SSL cert via 'cert='" + end + + ctx.cert = params['cert'] + + if ['peer', 'force_peer'].include?(params['verify_mode']) + unless params['ca'] + @events.error "Please specify the SSL ca via 'ca='" + end + end + + ctx.ca = params['ca'] if params['ca'] + ctx.ssl_cipher_filter = params['ssl_cipher_filter'] if params['ssl_cipher_filter'] + end + + ctx.no_tlsv1 = true if params['no_tlsv1'] == 'true' + ctx.no_tlsv1_1 = true if params['no_tlsv1_1'] == 'true' + + if params['verify_mode'] + ctx.verify_mode = case params['verify_mode'] + when "peer" + MiniSSL::VERIFY_PEER + when "force_peer" + MiniSSL::VERIFY_PEER | MiniSSL::VERIFY_FAIL_IF_NO_PEER_CERT + when "none" + MiniSSL::VERIFY_NONE + else + @events.error "Please specify a valid verify_mode=" + MiniSSL::VERIFY_NONE + end + end + ctx + end + def redirects_for_restart redirects = {:close_others => true} @listeners.each_with_index do |(l, io), i| diff --git a/test/test_binder.rb b/test/test_binder.rb index e7f3e40d05..fdddce91ff 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -5,157 +5,247 @@ require "puma/binder" require "puma/puma_http11" +# TODO: add coverage for activated & inherited binds, maybe verify sockets +# work with reads/writes + class TestBinderBase < Minitest::Test + + HAS_IP6 = !Socket.ip_address_list.select { |ai| ai.ipv6_loopback? }.empty? + + LOOPBACK_ADDRS = Puma::Binder.new(Puma::Events.strings).loopback_addresses + .map do |addr| + (addr.include? ':' and !addr.start_with? '[') ? "[#{addr}]" : addr + end.map { |addr| Regexp.escape addr } + def setup @events = Puma::Events.strings @binder = Puma::Binder.new(@events) end + def teardown + return if skipped? + # Binder#close does this without the 'checks', but teardown should cleanup + # no matter what happens,ie broken Binder + @binder.instance_variable_get(:@ios).each do |sock| + if defined?(Puma::MiniSSL::Server) && Puma::MiniSSL::Server === sock + sock.close + elsif sock.is_a? BasicSocket and !sock.closed? + sock.close + end + end + end + private - def key - @key ||= File.expand_path "../../examples/puma/puma_keypair.pem", __FILE__ + # calls Binder#parse with bind argument, returns 'log' and @listeners array + def parse(binds) + @binder.parse binds, @events + [@events.stdout.string, @binder.instance_variable_get(:@listeners)] end - def cert - @cert ||= File.expand_path "../../examples/puma/cert_puma.pem", __FILE__ + # concats @listeners string elements (URIs) for use with Regex match tests + def listener_string(l) + l.map(&:first).join '' end - def ssl_context_for_binder(binder) + # below assumes ssl bind of interest is the first, need to add querying + # if ssl bind isn't the first or more than one ssl bind is used + def ssl_context_for_binder(binder = @binder) binder.instance_variable_get(:@ios)[0].instance_variable_get(:@ctx) end + + def ssl_query + @ssl_query ||= if Puma.jruby? + @keystore = File.expand_path "../../examples/puma/keystore.jks", __FILE__ + @ssl_cipher_list = "TLS_DHE_RSA_WITH_DES_CBC_SHA,TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA" + "keystore=#{@keystore}&keystore-pass=pswd&ssl_cipher_list=#{@ssl_cipher_list}" + else + cert = File.expand_path "../../examples/puma/cert_puma.pem", __FILE__ + key = File.expand_path "../../examples/puma/puma_keypair.pem", __FILE__ + "key=#{key}&cert=#{cert}" + end + end end +# Tests pass URI's (fixed, localhost, port 0, different protocols) to +# Binder#parse, then check output and/or @listener array string +# elements for maches +# class TestBinder < TestBinderBase def test_localhost_addresses_dont_alter_listeners_for_tcp_addresses - @binder.parse(["tcp://localhost:10001"], @events) + out, l = parse ["tcp://localhost:10001"] + + l_str = listener_string l - assert_equal [], @binder.instance_variable_get(:@listeners) + LOOPBACK_ADDRS.each do |addr| + assert_match %r!tcp://#{addr}:(\d+)!, out + assert_match %r!tcp://#{addr}:(\d+)!, l_str + end end def test_correct_zero_port - @binder.parse(["tcp://localhost:0"], @events) + out, _ = parse ["tcp://localhost:0"] - m = %r!tcp://127.0.0.1:(\d+)!.match(@events.stdout.string) - port = m[1].to_i + port = out[%r!tcp://127.0.0.1:(\d+)!, 1].to_i refute_equal 0, port end def test_logs_all_localhost_bindings - @binder.parse(["tcp://localhost:0"], @events) + out, _ = parse ["tcp://localhost:0"] - assert_match %r!tcp://127.0.0.1:(\d+)!, @events.stdout.string - if @binder.loopback_addresses.include?("::1") - assert_match %r!tcp://\[::1\]:(\d+)!, @events.stdout.string - end + LOOPBACK_ADDRS.each { |addr| assert_match %r!tcp://#{addr}:(\d+)!, out } end - def test_allows_both_unix_and_tcp - skip UNIX_SKT_MSG unless UNIX_SKT_EXIST + def test_correct_zero_port_ssl + out, _ = parse ["ssl://localhost:0?#{ssl_query}"] - uri_unix = "unix://test/#{name}_server.sock" - uri_tcp = "tcp://127.0.0.1:#{UniquePort.call}" + port = out[%r!ssl://127.0.0.1:(\d+)!, 1].to_i - @binder.parse([uri_unix, uri_tcp], @events) + refute_equal 0, port - stdout = @events.stdout.string - assert stdout.include?(uri_unix), "\n#{stdout}\n" - assert stdout.include?(uri_tcp) , "\n#{stdout}\n" + LOOPBACK_ADDRS.each { |addr| assert_match %r!ssl://#{addr}:(\d+)!, out } + end - ensure - if UNIX_SKT_EXIST - @binder.close - @binder.close_listeners - end + def test_ios_and_listeners_correct_length + out, l = parse ["ssl://localhost:0?#{ssl_query}", "tcp://localhost:0"] + + len = 2 * LOOPBACK_ADDRS.length + + assert len, out.lines + assert len, l.length + end + + def test_double_ssl_then_tcp_both_localhost + out, l = parse ["ssl://localhost:0?#{ssl_query}", "tcp://localhost:0"] + + l_str = listener_string l + + LOOPBACK_ADDRS.each do |addr| + assert_match %r!ssl://#{addr}:(\d+)!, out + assert_match %r!tcp://#{addr}:(\d+)!, out + assert_match %r!ssl://#{addr}:(\d+)!, l_str + assert_match %r!tcp://#{addr}:(\d+)!, l_str + end end - def test_allows_both_tcp_and_unix - skip UNIX_SKT_MSG unless UNIX_SKT_EXIST + def test_double_unix_then_tcp + mult_binds [:unix, :tcp] + end + + def test_double_tcp_then_unix + mult_binds [:unix, :tcp] + end + + def test_ipv6 + # TODO: Ubuntu & macOS have the below error + # SocketError: getaddrinfo: nodename nor servname provided, or not known + skip unless HAS_IP6 && windows? + out, _ = parse ["tcp://[::1]:0"] + assert_match %r!tcp://\[::1\]:(\d+)!, out + end + + private + + def mult_binds(ary) + skip UNIX_SKT_MSG if ary.include?(:unix) && !UNIX_SKT_EXIST - uri_unix = "unix://test/#{name}_server.sock" - uri_tcp = "tcp://127.0.0.1:#{UniquePort.call}" + binds = [] - @binder.parse([uri_tcp, uri_unix], @events) + uri_ssl = "ssl://127.0.0.1:#{UniquePort.call}" + uri_tcp = "tcp://127.0.0.1:#{UniquePort.call}" - stdout = @events.stdout.string - assert stdout.include?(uri_unix), "\n#{stdout}\n" - assert stdout.include?(uri_tcp) , "\n#{stdout}\n" + if ary.include? :unix + path_unix = "test/#{name}_server.sock" + uri_unix ="unix://#{path_unix}" + end + + ary.each do |type| + binds << + case type + when :ssl then uri_ssl + when :tcp then uri_tcp + when :unix then uri_unix + end + end + + out, _ = parse binds + + assert(out.include? uri_ssl) if ary.include? :ssl + assert(out.include? uri_tcp) if ary.include? :tcp + assert(out.include? uri_unix) if ary.include? :unix ensure if UNIX_SKT_EXIST @binder.close - @binder.close_listeners + File.unlink(path_unix) if File.exist? path_unix end end - end class TestBinderJRuby < TestBinderBase def setup - super skip_unless :jruby + super end def test_binder_parses_jruby_ssl_options - keystore = File.expand_path "../../examples/puma/keystore.jks", __FILE__ - ssl_cipher_list = "TLS_DHE_RSA_WITH_DES_CBC_SHA,TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA" - - @binder.parse(["ssl://0.0.0.0:8080?keystore=#{keystore}&keystore-pass=&ssl_cipher_list=#{ssl_cipher_list}"], @events) + @binder.parse(["ssl://0.0.0.0:8080?#{ssl_query}"], @events) - assert_equal keystore, ssl_context_for_binder(@binder).keystore - assert_equal ssl_cipher_list, ssl_context_for_binder(@binder).ssl_cipher_list + assert_equal @keystore , ssl_context_for_binder.keystore + assert_equal @ssl_cipher_list, ssl_context_for_binder.ssl_cipher_list end end class TestBinderMRI < TestBinderBase def setup - super skip_on :jruby + super end def test_localhost_addresses_dont_alter_listeners_for_ssl_addresses - @binder.parse(["ssl://localhost:10002?key=#{key}&cert=#{cert}"], @events) + out, l = parse ["ssl://localhost:10002?#{ssl_query}"] - assert_equal [], @binder.instance_variable_get(:@listeners) + strings = l.map(&:first) + strings.each { |s| assert out.include?(s) } end def test_binder_parses_ssl_cipher_filter ssl_cipher_filter = "AES@STRENGTH" - @binder.parse(["ssl://0.0.0.0?key=#{key}&cert=#{cert}&ssl_cipher_filter=#{ssl_cipher_filter}"], @events) + @binder.parse(["ssl://0.0.0.0?#{ssl_query}&ssl_cipher_filter=#{ssl_cipher_filter}"], @events) - assert_equal ssl_cipher_filter, ssl_context_for_binder(@binder).ssl_cipher_filter + assert_equal ssl_cipher_filter, ssl_context_for_binder.ssl_cipher_filter end def test_binder_parses_tlsv1_disabled - @binder.parse(["ssl://0.0.0.0?key=#{key}&cert=#{cert}&no_tlsv1=true"], @events) + @binder.parse(["ssl://0.0.0.0?#{ssl_query}&no_tlsv1=true"], @events) - assert ssl_context_for_binder(@binder).no_tlsv1 + assert ssl_context_for_binder.no_tlsv1 end def test_binder_parses_tlsv1_enabled - @binder.parse(["ssl://0.0.0.0?key=#{key}&cert=#{cert}&no_tlsv1=false"], @events) + @binder.parse(["ssl://0.0.0.0?#{ssl_query}&no_tlsv1=false"], @events) - refute ssl_context_for_binder(@binder).no_tlsv1 + refute ssl_context_for_binder.no_tlsv1 end def test_binder_parses_tlsv1_tlsv1_1_unspecified_defaults_to_enabled - @binder.parse(["ssl://0.0.0.0?key=#{key}&cert=#{cert}"], @events) + @binder.parse(["ssl://0.0.0.0?#{ssl_query}"], @events) - refute ssl_context_for_binder(@binder).no_tlsv1 - refute ssl_context_for_binder(@binder).no_tlsv1_1 + refute ssl_context_for_binder.no_tlsv1 + refute ssl_context_for_binder.no_tlsv1_1 end def test_binder_parses_tlsv1_1_disabled - @binder.parse(["ssl://0.0.0.0?key=#{key}&cert=#{cert}&no_tlsv1_1=true"], @events) + @binder.parse(["ssl://0.0.0.0?#{ssl_query}&no_tlsv1_1=true"], @events) - assert ssl_context_for_binder(@binder).no_tlsv1_1 + assert ssl_context_for_binder.no_tlsv1_1 end def test_binder_parses_tlsv1_1_enabled - @binder.parse(["ssl://0.0.0.0?key=#{key}&cert=#{cert}&no_tlsv1_1=false"], @events) + @binder.parse(["ssl://0.0.0.0?#{ssl_query}&no_tlsv1_1=false"], @events) - refute ssl_context_for_binder(@binder).no_tlsv1_1 + refute ssl_context_for_binder.no_tlsv1_1 end end