From e9f09ba1fe6b168bed7fff59d0bdbfd65351cf9d Mon Sep 17 00:00:00 2001 From: Karol Bucek Date: Mon, 4 Jul 2022 16:16:31 +0200 Subject: [PATCH] [jruby] support setting TLS protocols + rename ssl_cipher_list (#2899) * [jruby] support setting TLS protocols + rename ssl_cipher_list follow Java naming as we already do with keystore/truststore ... Context now does the string split and accepts an Array * [test] cipher_suites and protocols behavior * [jruby] support new TLS settings in DSL --- ext/puma_http11/org/jruby/puma/MiniSSL.java | 35 ++++++++++++-------- lib/puma/dsl.rb | 18 +++++------ lib/puma/minissl.rb | 17 +++++++++- lib/puma/minissl/context_builder.rb | 3 +- test/test_binder.rb | 21 +++++++++--- test/test_config.rb | 24 ++++++++++++++ test/test_puma_server_ssl.rb | 36 +++++++++++++++++++++ 7 files changed, 125 insertions(+), 29 deletions(-) diff --git a/ext/puma_http11/org/jruby/puma/MiniSSL.java b/ext/puma_http11/org/jruby/puma/MiniSSL.java index 1c2366d7..dd42f7af 100644 --- a/ext/puma_http11/org/jruby/puma/MiniSSL.java +++ b/ext/puma_http11/org/jruby/puma/MiniSSL.java @@ -1,6 +1,7 @@ package org.jruby.puma; import org.jruby.Ruby; +import org.jruby.RubyArray; import org.jruby.RubyClass; import org.jruby.RubyModule; import org.jruby.RubyObject; @@ -224,18 +225,25 @@ public class MiniSSL extends RubyObject { // MiniSSL::Engine handshake = false; engine = sslCtx.createSSLEngine(); - String[] protocols; - if (miniSSLContext.callMethod(context, "no_tlsv1").isTrue()) { - protocols = new String[] { "TLSv1.1", "TLSv1.2", "TLSv1.3" }; + String[] enabledProtocols; + IRubyObject protocols = miniSSLContext.callMethod(context, "protocols"); + if (protocols.isNil()) { + if (miniSSLContext.callMethod(context, "no_tlsv1").isTrue()) { + enabledProtocols = new String[] { "TLSv1.1", "TLSv1.2", "TLSv1.3" }; + } else { + enabledProtocols = new String[] { "TLSv1", "TLSv1.1", "TLSv1.2", "TLSv1.3" }; + } + + if (miniSSLContext.callMethod(context, "no_tlsv1_1").isTrue()) { + enabledProtocols = new String[] { "TLSv1.2", "TLSv1.3" }; + } + } else if (protocols instanceof RubyArray) { + enabledProtocols = (String[]) ((RubyArray) protocols).toArray(new String[0]); } else { - protocols = new String[] { "TLSv1", "TLSv1.1", "TLSv1.2", "TLSv1.3" }; + throw context.runtime.newTypeError(protocols, context.runtime.getArray()); } + engine.setEnabledProtocols(enabledProtocols); - if (miniSSLContext.callMethod(context, "no_tlsv1_1").isTrue()) { - protocols = new String[] { "TLSv1.2", "TLSv1.3" }; - } - - engine.setEnabledProtocols(protocols); engine.setUseClientMode(false); long verify_mode = miniSSLContext.callMethod(context, "verify_mode").convertToInteger("to_i").getLongValue(); @@ -246,10 +254,11 @@ public class MiniSSL extends RubyObject { // MiniSSL::Engine engine.setNeedClientAuth(true); } - IRubyObject sslCipherListObject = miniSSLContext.callMethod(context, "ssl_cipher_list"); - if (!sslCipherListObject.isNil()) { - String[] sslCipherList = sslCipherListObject.convertToString().asJavaString().split(","); - engine.setEnabledCipherSuites(sslCipherList); + IRubyObject cipher_suites = miniSSLContext.callMethod(context, "cipher_suites"); + if (cipher_suites instanceof RubyArray) { + engine.setEnabledCipherSuites((String[]) ((RubyArray) cipher_suites).toArray(new String[0])); + } else if (!cipher_suites.isNil()) { + throw context.runtime.newTypeError(cipher_suites, context.runtime.getArray()); } SSLSession session = engine.getSession(); diff --git a/lib/puma/dsl.rb b/lib/puma/dsl.rb index 94e97bfd..7026cbc7 100644 --- a/lib/puma/dsl.rb +++ b/lib/puma/dsl.rb @@ -52,8 +52,9 @@ module Puma backlog_str = opts[:backlog] ? "&backlog=#{Integer(opts[:backlog])}" : '' if defined?(JRUBY_VERSION) - ssl_cipher_list = opts[:ssl_cipher_list] ? - "&ssl_cipher_list=#{opts[:ssl_cipher_list]}" : nil + cipher_suites = opts[:ssl_cipher_list] ? "&ssl_cipher_list=#{opts[:ssl_cipher_list]}" : nil # old name + cipher_suites = "#{cipher_suites}&cipher_suites=#{opts[:cipher_suites]}" if opts[:cipher_suites] + protocols = opts[:protocols] ? "&protocols=#{opts[:protocols]}" : nil keystore_additions = "keystore=#{opts[:keystore]}&keystore-pass=#{opts[:keystore_pass]}" keystore_additions = "#{keystore_additions}&keystore-type=#{opts[:keystore_type]}" if opts[:keystore_type] @@ -63,20 +64,17 @@ module Puma truststore_additions = "#{truststore_additions}&truststore-type=#{opts[:truststore_type]}" if opts[:truststore_type] end - "ssl://#{host}:#{port}?#{keystore_additions}#{truststore_additions}#{ssl_cipher_list}" \ + "ssl://#{host}:#{port}?#{keystore_additions}#{truststore_additions}#{cipher_suites}#{protocols}" \ "&verify_mode=#{verify}#{tls_str}#{ca_additions}#{backlog_str}" else - ssl_cipher_filter = opts[:ssl_cipher_filter] ? - "&ssl_cipher_filter=#{opts[:ssl_cipher_filter]}" : nil - - v_flags = (ary = opts[:verification_flags]) ? - "&verification_flags=#{Array(ary).join ','}" : nil + ssl_cipher_filter = opts[:ssl_cipher_filter] ? "&ssl_cipher_filter=#{opts[:ssl_cipher_filter]}" : nil + v_flags = (ary = opts[:verification_flags]) ? "&verification_flags=#{Array(ary).join ','}" : nil cert_flags = (cert = opts[:cert]) ? "cert=#{Puma::Util.escape(cert)}" : nil key_flags = (key = opts[:key]) ? "&key=#{Puma::Util.escape(key)}" : nil - "ssl://#{host}:#{port}?#{cert_flags}#{key_flags}" \ - "#{ssl_cipher_filter}&verify_mode=#{verify}#{tls_str}#{ca_additions}#{v_flags}#{backlog_str}" + "ssl://#{host}:#{port}?#{cert_flags}#{key_flags}#{ssl_cipher_filter}" \ + "&verify_mode=#{verify}#{tls_str}#{ca_additions}#{v_flags}#{backlog_str}" end end diff --git a/lib/puma/minissl.rb b/lib/puma/minissl.rb index 49a158f9..616e73ab 100644 --- a/lib/puma/minissl.rb +++ b/lib/puma/minissl.rb @@ -223,7 +223,8 @@ module Puma attr_reader :truststore attr_reader :truststore_type attr_accessor :truststore_pass - attr_accessor :ssl_cipher_list + attr_reader :cipher_suites + attr_reader :protocols def keystore=(keystore) check_file keystore, 'Keystore' @@ -249,6 +250,20 @@ module Puma @truststore_type = type end + def cipher_suites=(list) + list = list.split(',').map(&:strip) if list.is_a?(String) + @cipher_suites = list + end + + # aliases for backwards compatibility + alias_method :ssl_cipher_list, :cipher_suites + alias_method :ssl_cipher_list=, :cipher_suites= + + def protocols=(list) + list = list.split(',').map(&:strip) if list.is_a?(String) + @protocols = list + end + def check raise "Keystore not configured" unless @keystore # @truststore defaults to @keystore due backwards compatibility diff --git a/lib/puma/minissl/context_builder.rb b/lib/puma/minissl/context_builder.rb index 85c5060b..b03f266e 100644 --- a/lib/puma/minissl/context_builder.rb +++ b/lib/puma/minissl/context_builder.rb @@ -29,7 +29,8 @@ module Puma ctx.truststore_type = params['truststore-type'] end - ctx.ssl_cipher_list = params['ssl_cipher_list'] if params['ssl_cipher_list'] + ctx.cipher_suites = params['cipher_suites'] || params['ssl_cipher_list'] + ctx.protocols = params['protocols'] if params['protocols'] else if params['key'].nil? && params['key_pem'].nil? log_writer.error "Please specify the SSL key via 'key=' or 'key_pem='" diff --git a/test/test_binder.rb b/test/test_binder.rb index 908001c7..454056c0 100644 --- a/test/test_binder.rb +++ b/test/test_binder.rb @@ -478,13 +478,26 @@ class TestBinderJRuby < TestBinderBase def test_binder_parses_jruby_ssl_options skip_unless :ssl - keystore = File.expand_path "../../examples/puma/keystore.jks", __FILE__ - ssl_cipher_list = "TLS_DHE_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" + cipher_suites = ['TLS_DHE_RSA_WITH_AES_128_CBC_SHA', 'TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256'] @binder.parse ["ssl://0.0.0.0:8080?#{ssl_query}"], @log_writer - assert_equal keystore, ssl_context_for_binder.keystore - assert_equal ssl_cipher_list, ssl_context_for_binder.ssl_cipher_list + assert_equal @keystore, ssl_context_for_binder.keystore + assert_equal cipher_suites, ssl_context_for_binder.cipher_suites + assert_equal cipher_suites, ssl_context_for_binder.ssl_cipher_list + end + + def test_binder_parses_jruby_ssl_protocols_and_cipher_suites_options + skip_unless :ssl + + keystore = File.expand_path "../../examples/puma/keystore.jks", __FILE__ + cipher = "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" + ssl_query = "keystore=#{keystore}&keystore-pass=jruby_puma&cipher_suites=#{cipher}&protocols=TLSv1.3,TLSv1.2" + + @binder.parse ["ssl://0.0.0.0:8080?#{ssl_query}"], @log_writer + + assert_equal [ 'TLSv1.3', 'TLSv1.2' ], ssl_context_for_binder.protocols + assert_equal [ cipher ], ssl_context_for_binder.cipher_suites end end if ::Puma::IS_JRUBY diff --git a/test/test_config.rb b/test/test_config.rb index 24af48aa..65297941 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -149,6 +149,30 @@ class TestConfigFile < TestConfigFileBase skip_unless :jruby skip_unless :ssl + ciphers = "TLS_DHE_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" + + conf = Puma::Configuration.new do |c| + c.ssl_bind "0.0.0.0", "9292", { + keystore: "/path/to/keystore", + keystore_pass: "password", + cipher_suites: ciphers, + protocols: 'TLSv1.2', + verify_mode: "the_verify_mode" + } + end + + conf.load + + ssl_binding = "ssl://0.0.0.0:9292?keystore=/path/to/keystore" \ + "&keystore-pass=password&cipher_suites=#{ciphers}&protocols=TLSv1.2" \ + "&verify_mode=the_verify_mode" + assert_equal [ssl_binding], conf.options[:binds] + end + + def test_ssl_bind_jruby_with_ssl_cipher_list + skip_unless :jruby + skip_unless :ssl + cipher_list = "TLS_DHE_RSA_WITH_AES_128_CBC_SHA,TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256" conf = Puma::Configuration.new do |c| diff --git a/test/test_puma_server_ssl.rb b/test/test_puma_server_ssl.rb index 1767d923..6808e589 100644 --- a/test/test_puma_server_ssl.rb +++ b/test/test_puma_server_ssl.rb @@ -453,6 +453,42 @@ class TestPumaServerSSLClient < Minitest::Test end end if Puma.jruby? + def test_allows_to_specify_cipher_suites_and_protocols + ctx = CTX.dup + ctx.cipher_suites = [ 'TLS_RSA_WITH_AES_128_GCM_SHA256' ] + ctx.protocols = 'TLSv1.2' + + assert_ssl_client_error_match(false, context: ctx) do |http| + key = "#{CERT_PATH}/client.key" + crt = "#{CERT_PATH}/client.crt" + http.key = OpenSSL::PKey::RSA.new File.read(key) + http.cert = OpenSSL::X509::Certificate.new File.read(crt) + http.ca_file = "#{CERT_PATH}/ca.crt" + http.verify_mode = OpenSSL::SSL::VERIFY_PEER + + http.ssl_version = :TLSv1_2 + http.ciphers = [ 'TLS_RSA_WITH_AES_128_GCM_SHA256' ] + end + end if Puma.jruby? + + def test_fails_when_no_cipher_suites_in_common + ctx = CTX.dup + ctx.cipher_suites = [ 'TLS_RSA_WITH_AES_128_GCM_SHA256' ] + ctx.protocols = 'TLSv1.2' + + assert_ssl_client_error_match(/no cipher suites in common/, context: ctx) do |http| + key = "#{CERT_PATH}/client.key" + crt = "#{CERT_PATH}/client.crt" + http.key = OpenSSL::PKey::RSA.new File.read(key) + http.cert = OpenSSL::X509::Certificate.new File.read(crt) + http.ca_file = "#{CERT_PATH}/ca.crt" + http.verify_mode = OpenSSL::SSL::VERIFY_PEER + + http.ssl_version = :TLSv1_2 + http.ciphers = [ 'TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384' ] + end + end if Puma.jruby? + end if ::Puma::HAS_SSL class TestPumaServerSSLWithCertPemAndKeyPem < Minitest::Test