From 5608248c13130740ca94697b63a59245140e8092 Mon Sep 17 00:00:00 2001 From: Dalibor Nasevic Date: Sun, 31 Oct 2021 14:59:21 +0100 Subject: [PATCH] Support for cert_pem and key_pem with ssl_bind DSL (#2728) * Fix deprecation warning DEPRECATED: Use assert_nil if expecting nil from test/test_binder.rb:265. This will fail in Minitest 6. * Extend MiniSSL with support for cert_pem and key_pem * Extend Puma ssl_bind DSL with support for cert_pem and cert_key * Make some variables in binder test more readable --- ext/puma_http11/mini_ssl.c | 38 ++++++++-- lib/puma/binder.rb | 13 +++- lib/puma/dsl.rb | 29 +++++++ lib/puma/minissl.rb | 20 ++++- lib/puma/minissl/context_builder.rb | 14 ++-- test/test_binder.rb | 14 ++-- test/test_config.rb | 22 ++++++ test/test_integration_ssl.rb | 113 ++++++++++++++++++---------- test/test_minissl.rb | 14 ++++ test/test_puma_server_ssl.rb | 41 ++++++++++ 10 files changed, 257 insertions(+), 61 deletions(-) diff --git a/ext/puma_http11/mini_ssl.c b/ext/puma_http11/mini_ssl.c index 04bd1462..6974b634 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/binder.rb b/lib/puma/binder.rb index 6151889e..3d688296 100644 --- a/lib/puma/binder.rb +++ b/lib/puma/binder.rb @@ -30,6 +30,7 @@ module Puma def initialize(events, conf = Configuration.new) @events = events + @conf = conf @listeners = [] @inherited_fds = {} @activated_sockets = {} @@ -234,7 +235,17 @@ module Puma # 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 c3e93375..65b3bbed 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -447,6 +447,14 @@ module Puma # 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 @@ module Puma # 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 @@ module Puma 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.rb b/lib/puma/minissl.rb index 9f1bc818..f9161af7 100644 --- a/lib/puma/minissl.rb +++ b/lib/puma/minissl.rb @@ -208,6 +208,10 @@ module Puma 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 @@ module Puma 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 @@ module Puma @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/lib/puma/minissl/context_builder.rb b/lib/puma/minissl/context_builder.rb index a30a26dc..8cf16bbd 100644 --- a/lib/puma/minissl/context_builder.rb +++ b/lib/puma/minissl/context_builder.rb @@ -23,17 +23,19 @@ module Puma 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_binder.rb b/test/test_binder.rb index c4a027ab..4dddbcce 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -262,7 +262,7 @@ class TestBinder < TestBinderBase 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 @@ -308,11 +308,11 @@ class TestBinder < TestBinderBase 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 @@ class TestBinder < TestBinderBase 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 @@ class TestBinder < TestBinderBase @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 diff --git a/test/test_config.rb b/test/test_config.rb index 9ba56465..758910ca 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -77,6 +77,28 @@ class TestConfigFile < TestConfigFileBase 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 8f746e5a..9c3409a7 100644 --- a/test/test_integration_ssl.rb +++ b/test/test_integration_ssl.rb @@ -21,17 +21,47 @@ class TestIntegrationSSL < TestIntegration 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 = < 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