From 3bcb75efbb9073a3674fff6d930fe63049188549 Mon Sep 17 00:00:00 2001 From: Dalibor Nasevic Date: Mon, 4 Oct 2021 13:28:59 +0200 Subject: [PATCH 1/4] Fix deprecation warning DEPRECATED: Use assert_nil if expecting nil from test/test_binder.rb:265. This will fail in Minitest 6. --- test/test_binder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_binder.rb b/test/test_binder.rb index c4a027ab62..df0b8efc42 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -262,7 +262,7 @@ def test_env_contains_protoenv env_hash = @binder.envs[@binder.ios.first] @binder.proto_env.each do |k,v| - assert_equal env_hash[k], v + assert env_hash[k] == v end end From 156908422b5c12584f32dc9839e86c02282d610d Mon Sep 17 00:00:00 2001 From: Dalibor Nasevic Date: Fri, 8 Oct 2021 10:49:19 +0200 Subject: [PATCH 2/4] Extend MiniSSL with support for cert_pem and key_pem --- ext/puma_http11/mini_ssl.c | 38 ++++++++++++++++++++++++++++----- lib/puma/minissl.rb | 20 ++++++++++++++++-- test/test_minissl.rb | 14 ++++++++++++ test/test_puma_server_ssl.rb | 41 ++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 7 deletions(-) diff --git a/ext/puma_http11/mini_ssl.c b/ext/puma_http11/mini_ssl.c index 04bd1462df..6974b6349c 100644 --- a/ext/puma_http11/mini_ssl.c +++ b/ext/puma_http11/mini_ssl.c @@ -208,8 +208,11 @@ sslctx_initialize(VALUE self, VALUE mini_ssl_ctx) { #endif int ssl_options; VALUE key, cert, ca, verify_mode, ssl_cipher_filter, no_tlsv1, no_tlsv1_1, - verification_flags, session_id_bytes; + verification_flags, session_id_bytes, cert_pem, key_pem; DH *dh; + BIO *bio; + X509 *x509; + EVP_PKEY *pkey; #if OPENSSL_VERSION_NUMBER < 0x10002000L EC_KEY *ecdh; @@ -218,13 +221,15 @@ sslctx_initialize(VALUE self, VALUE mini_ssl_ctx) { TypedData_Get_Struct(self, SSL_CTX, &sslctx_type, ctx); key = rb_funcall(mini_ssl_ctx, rb_intern_const("key"), 0); - StringValue(key); cert = rb_funcall(mini_ssl_ctx, rb_intern_const("cert"), 0); - StringValue(cert); ca = rb_funcall(mini_ssl_ctx, rb_intern_const("ca"), 0); + cert_pem = rb_funcall(mini_ssl_ctx, rb_intern_const("cert_pem"), 0); + + key_pem = rb_funcall(mini_ssl_ctx, rb_intern_const("key_pem"), 0); + verify_mode = rb_funcall(mini_ssl_ctx, rb_intern_const("verify_mode"), 0); ssl_cipher_filter = rb_funcall(mini_ssl_ctx, rb_intern_const("ssl_cipher_filter"), 0); @@ -233,8 +238,31 @@ sslctx_initialize(VALUE self, VALUE mini_ssl_ctx) { no_tlsv1_1 = rb_funcall(mini_ssl_ctx, rb_intern_const("no_tlsv1_1"), 0); - SSL_CTX_use_certificate_chain_file(ctx, RSTRING_PTR(cert)); - SSL_CTX_use_PrivateKey_file(ctx, RSTRING_PTR(key), SSL_FILETYPE_PEM); + if (!NIL_P(cert)) { + StringValue(cert); + SSL_CTX_use_certificate_chain_file(ctx, RSTRING_PTR(cert)); + } + + if (!NIL_P(key)) { + StringValue(key); + SSL_CTX_use_PrivateKey_file(ctx, RSTRING_PTR(key), SSL_FILETYPE_PEM); + } + + if (!NIL_P(cert_pem)) { + bio = BIO_new(BIO_s_mem()); + BIO_puts(bio, RSTRING_PTR(cert_pem)); + x509 = PEM_read_bio_X509(bio, NULL, NULL, NULL); + + SSL_CTX_use_certificate(ctx, x509); + } + + if (!NIL_P(key_pem)) { + bio = BIO_new(BIO_s_mem()); + BIO_puts(bio, RSTRING_PTR(key_pem)); + pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); + + SSL_CTX_use_PrivateKey(ctx, pkey); + } verification_flags = rb_funcall(mini_ssl_ctx, rb_intern_const("verification_flags"), 0); diff --git a/lib/puma/minissl.rb b/lib/puma/minissl.rb index 9f1bc81854..f9161af763 100644 --- a/lib/puma/minissl.rb +++ b/lib/puma/minissl.rb @@ -208,6 +208,10 @@ class Context def initialize @no_tlsv1 = false @no_tlsv1_1 = false + @key = nil + @cert = nil + @key_pem = nil + @cert_pem = nil end if IS_JRUBY @@ -230,6 +234,8 @@ def check attr_reader :key attr_reader :cert attr_reader :ca + attr_reader :cert_pem + attr_reader :key_pem attr_accessor :ssl_cipher_filter attr_accessor :verification_flags @@ -248,9 +254,19 @@ def ca=(ca) @ca = ca end + def cert_pem=(cert_pem) + raise ArgumentError, "'cert_pem' is not a String" unless cert_pem.is_a? String + @cert_pem = cert_pem + end + + def key_pem=(key_pem) + raise ArgumentError, "'key_pem' is not a String" unless key_pem.is_a? String + @key_pem = key_pem + end + def check - raise "Key not configured" unless @key - raise "Cert not configured" unless @cert + raise "Key not configured" if @key.nil? && @key_pem.nil? + raise "Cert not configured" if @cert.nil? && @cert_pem.nil? end end diff --git a/test/test_minissl.rb b/test/test_minissl.rb index f9caf6d4e4..f3b0dbee19 100644 --- a/test/test_minissl.rb +++ b/test/test_minissl.rb @@ -25,5 +25,19 @@ def test_raises_with_invalid_cert_file exception = assert_raises(ArgumentError) { ctx.cert = "/no/such/cert" } assert_equal("No such cert file '/no/such/cert'", exception.message) end + + def test_raises_with_invalid_key_pem + ctx = Puma::MiniSSL::Context.new + + exception = assert_raises(ArgumentError) { ctx.key_pem = nil } + assert_equal("'key_pem' is not a String", exception.message) + end + + def test_raises_with_invalid_cert_pem + ctx = Puma::MiniSSL::Context.new + + exception = assert_raises(ArgumentError) { ctx.cert_pem = nil } + assert_equal("'cert_pem' is not a String", exception.message) + end end end if ::Puma::HAS_SSL diff --git a/test/test_puma_server_ssl.rb b/test/test_puma_server_ssl.rb index a3e843f5b1..c387cf7c44 100644 --- a/test/test_puma_server_ssl.rb +++ b/test/test_puma_server_ssl.rb @@ -333,3 +333,44 @@ def test_verify_client_cert end end end if ::Puma::HAS_SSL + +class TestPumaServerSSLWithCertPemAndKeyPem < Minitest::Test + CERT_PATH = File.expand_path "../examples/puma/client-certs", __dir__ + + def test_server_ssl_with_cert_pem_and_key_pem + host = "localhost" + port = 0 + ctx = Puma::MiniSSL::Context.new.tap { |ctx| + ctx.cert_pem = File.read("#{CERT_PATH}/server.crt") + ctx.key_pem = File.read("#{CERT_PATH}/server.key") + } + + app = lambda { |env| [200, {}, [env['rack.url_scheme']]] } + events = SSLEventsHelper.new STDOUT, STDERR + server = Puma::Server.new app, events + server.add_ssl_listener host, port, ctx + host_addrs = server.binder.ios.map { |io| io.to_io.addr[2] } + server.run + + http = Net::HTTP.new host, server.connected_ports[0] + http.use_ssl = true + http.ca_file = "#{CERT_PATH}/ca.crt" + + client_error = nil + begin + http.start do + req = Net::HTTP::Get.new "/", {} + http.request(req) + end + rescue OpenSSL::SSL::SSLError, EOFError, Errno::ECONNRESET => e + # Errno::ECONNRESET TruffleRuby + client_error = e + # closes socket if open, may not close on error + http.send :do_finish + end + + assert_nil client_error + ensure + server.stop(true) if server + end +end if ::Puma::HAS_SSL && !Puma::IS_JRUBY From 5f478b9f16f08bf1ee2b271ff071e23578a5ce6e Mon Sep 17 00:00:00 2001 From: Dalibor Nasevic Date: Fri, 8 Oct 2021 10:55:52 +0200 Subject: [PATCH 3/4] Extend Puma ssl_bind DSL with support for cert_pem and cert_key --- lib/puma/binder.rb | 13 +++- lib/puma/dsl.rb | 29 ++++++++ lib/puma/minissl/context_builder.rb | 14 ++-- test/test_config.rb | 22 ++++++ test/test_integration_ssl.rb | 105 ++++++++++++++++++---------- 5 files changed, 140 insertions(+), 43 deletions(-) diff --git a/lib/puma/binder.rb b/lib/puma/binder.rb index 6151889e79..3d688296bd 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -30,6 +30,7 @@ class Binder def initialize(events, conf = Configuration.new) @events = events + @conf = conf @listeners = [] @inherited_fds = {} @activated_sockets = {} @@ -234,7 +235,17 @@ def parse(binds, logger, log_msg = 'Listening') # Load localhost authority if not loaded. ctx = localhost_authority && localhost_authority_context if params.empty? - ctx ||= MiniSSL::ContextBuilder.new(params, @events).context + ctx ||= + begin + # Extract cert_pem and key_pem from options[:store] if present + ['cert', 'key'].each do |v| + if params[v] && params[v].start_with?('store:') + index = Integer(params.delete(v).split('store:').last) + params["#{v}_pem"] = @conf.options[:store][index] + end + end + MiniSSL::ContextBuilder.new(params, @events).context + end if fd = @inherited_fds.delete(str) logger.log "* Inherited #{str}" diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb index c3e9337513..65b3bbed9b 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -447,6 +447,14 @@ def threads(min, max) # verify_mode: verify_mode, # default 'none' # verification_flags: flags, # optional, not supported by JRuby # } + # + # Alternatively, you can provide the cert_pem and key_pem: + # @example + # ssl_bind '127.0.0.1', '9292', { + # cert_pem: File.read(path_to_cert), + # key_pem: File.read(path_to_key), + # } + # # @example For JRuby, two keys are required: keystore & keystore_pass. # ssl_bind '127.0.0.1', '9292', { # keystore: path_to_keystore, @@ -455,6 +463,7 @@ def threads(min, max) # verify_mode: verify_mode # default 'none' # } def ssl_bind(host, port, opts) + add_pem_values_to_options_store(opts) bind self.class.ssl_bind_str(host, port, opts) end @@ -927,5 +936,25 @@ def io_selector_backend(backend) def mutate_stdout_and_stderr_to_sync_on_write(enabled=true) @options[:mutate_stdout_and_stderr_to_sync_on_write] = enabled end + + private + + # To avoid adding cert_pem and key_pem as URI params, we store them on the + # options[:store] from where Puma binder knows how to find and extract them. + def add_pem_values_to_options_store(opts) + return if defined?(JRUBY_VERSION) + + @options[:store] ||= [] + + # Store cert_pem and key_pem to options[:store] if present + [:cert, :key].each do |v| + opt_key = :"#{v}_pem" + if opts[opt_key] + index = @options[:store].length + @options[:store] << opts[opt_key] + opts[v] = "store:#{index}" + end + end + end end end diff --git a/lib/puma/minissl/context_builder.rb b/lib/puma/minissl/context_builder.rb index a30a26dc3d..8cf16bbd30 100644 --- a/lib/puma/minissl/context_builder.rb +++ b/lib/puma/minissl/context_builder.rb @@ -23,17 +23,19 @@ def context 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='" + if params['key'].nil? && params['key_pem'].nil? + events.error "Please specify the SSL key via 'key=' or 'key_pem='" end - ctx.key = params['key'] + ctx.key = params['key'] if params['key'] + ctx.key_pem = params['key_pem'] if params['key_pem'] - unless params['cert'] - events.error "Please specify the SSL cert via 'cert='" + if params['cert'].nil? && params['cert_pem'].nil? + events.error "Please specify the SSL cert via 'cert=' or 'cert_pem='" end - ctx.cert = params['cert'] + ctx.cert = params['cert'] if params['cert'] + ctx.cert_pem = params['cert_pem'] if params['cert_pem'] if ['peer', 'force_peer'].include?(params['verify_mode']) unless params['ca'] diff --git a/test/test_config.rb b/test/test_config.rb index 9ba564653b..758910ca18 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -77,6 +77,28 @@ def test_ssl_bind assert_equal [ssl_binding], conf.options[:binds] end + def test_ssl_bind_with_cert_and_key_pem + skip_if :jruby + skip_unless :ssl + + cert_path = File.expand_path "../examples/puma/client-certs", __dir__ + cert_pem = File.read("#{cert_path}/server.crt") + key_pem = File.read("#{cert_path}/server.key") + + conf = Puma::Configuration.new do |c| + c.ssl_bind "0.0.0.0", "9292", { + cert_pem: cert_pem, + key_pem: key_pem, + verify_mode: "the_verify_mode", + } + end + + conf.load + + ssl_binding = "ssl://0.0.0.0:9292?cert=store:0&key=store:1&verify_mode=the_verify_mode" + assert_equal [ssl_binding], conf.options[:binds] + end + def test_ssl_bind_jruby skip_unless :jruby skip_unless :ssl diff --git a/test/test_integration_ssl.rb b/test/test_integration_ssl.rb index 8f746e5ab7..9c3409a7b5 100644 --- a/test/test_integration_ssl.rb +++ b/test/test_integration_ssl.rb @@ -21,17 +21,47 @@ def teardown super end - def generate_config(opts = nil) - @bind_port = UniquePort.call - @control_tcp_port = UniquePort.call + def bind_port + @bind_port ||= UniquePort.call + end + + def control_tcp_port + @control_tcp_port ||= UniquePort.call + end + + def with_server(config) + config_file = Tempfile.new %w(config .rb) + config_file.write config + config_file.close + config_file.path + + # start server + cmd = "#{BASE} bin/puma -C #{config_file.path}" + @server = IO.popen cmd, 'r' + wait_for_server_to_boot + @pid = @server.pid + http = Net::HTTP.new HOST, bind_port + http.use_ssl = true + http.verify_mode = OpenSSL::SSL::VERIFY_NONE + + yield http + + # stop server + sock = TCPSocket.new HOST, control_tcp_port + @ios_to_close << sock + sock.syswrite "GET /stop?token=#{TOKEN} HTTP/1.1\r\n\r\n" + sock.read + assert_match 'Goodbye!', @server.read + end + + def test_ssl_run config = < Date: Fri, 8 Oct 2021 10:57:08 +0200 Subject: [PATCH 4/4] Make some variables in binder test more readable --- test/test_binder.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/test_binder.rb b/test/test_binder.rb index df0b8efc42..4dddbcce5c 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -308,11 +308,11 @@ def test_redirects_for_restart_env def test_close_listeners_closes_ios @binder.parse ["tcp://127.0.0.1:#{UniquePort.call}"], @events - refute @binder.listeners.any? { |u, l| l.closed? } + refute @binder.listeners.any? { |_l, io| io.closed? } @binder.close_listeners - assert @binder.listeners.all? { |u, l| l.closed? } + assert @binder.listeners.all? { |_l, io| io.closed? } end def test_close_listeners_closes_ios_unless_closed? @@ -322,11 +322,11 @@ def test_close_listeners_closes_ios_unless_closed? bomb.close def bomb.close; raise "Boom!"; end # the bomb has been planted - assert @binder.listeners.any? { |u, l| l.closed? } + assert @binder.listeners.any? { |_l, io| io.closed? } @binder.close_listeners - assert @binder.listeners.all? { |u, l| l.closed? } + assert @binder.listeners.all? { |_l, io| io.closed? } end def test_listeners_file_unlink_if_unix_listener @@ -344,8 +344,8 @@ def test_import_from_env_listen_inherit @binder.parse ["tcp://127.0.0.1:0"], @events removals = @binder.create_inherited_fds(@binder.redirects_for_restart_env) - @binder.listeners.each do |url, io| - assert_equal io.to_i, @binder.inherited_fds[url] + @binder.listeners.each do |l, io| + assert_equal io.to_i, @binder.inherited_fds[l] end assert_includes removals, "PUMA_INHERIT_0" end