Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no_tlsv1_3 to Puma::DSL#ssl_bindings and Puma::MiniSSL::Context #2426

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -2,6 +2,7 @@

* Features
* Your feature goes here <Most recent on the top, like GitHub> (#Github Number)
* Add no_tlsv1_3 to Puma::DSL#ssl_bindings and Puma::MiniSSL::Context (#2426)

* Bugfixes
* Cleanup daemonization in rc.d script (#2409)
Expand Down
9 changes: 4 additions & 5 deletions ext/puma_http11/extconf.rb
Expand Up @@ -17,12 +17,11 @@
have_header "openssl/bio.h"

# below is yes for 1.0.2 & later
have_func "DTLS_method" , "openssl/ssl.h"
have_func "DTLS_method" , "openssl/ssl.h"

# below are yes for 1.1.0 & later, may need to check func rather than macro
# with versions after 1.1.1
have_func "TLS_server_method" , "openssl/ssl.h"
have_macro "SSL_CTX_set_min_proto_version", "openssl/ssl.h"
# below are yes for 1.1.0 & later
have_func "TLS_server_method" , "openssl/ssl.h"
have_func "SSL_CTX_set_min_proto_version(NULL, 0)", "openssl/ssl.h"
end
end

Expand Down
52 changes: 29 additions & 23 deletions ext/puma_http11/mini_ssl.c
Expand Up @@ -171,6 +171,9 @@ VALUE engine_init_server(VALUE self, VALUE mini_ssl_ctx) {
ID sym_no_tlsv1_1 = rb_intern("no_tlsv1_1");
VALUE no_tlsv1_1 = rb_funcall(mini_ssl_ctx, sym_no_tlsv1_1, 0);

ID sym_no_tlsv1_3 = rb_intern("no_tlsv1_3");
VALUE no_tlsv1_3 = rb_funcall(mini_ssl_ctx, sym_no_tlsv1_3, 0);

#ifdef HAVE_TLS_SERVER_METHOD
ctx = SSL_CTX_new(TLS_server_method());
#else
Expand Down Expand Up @@ -198,11 +201,14 @@ VALUE engine_init_server(VALUE self, VALUE mini_ssl_ctx) {
else {
min = TLS1_VERSION;
}

SSL_CTX_set_min_proto_version(ctx, min);

SSL_CTX_set_options(ctx, ssl_options);
if (RTEST(no_tlsv1_3)) {
SSL_CTX_set_max_proto_version(ctx, TLS1_2_VERSION);
}

SSL_CTX_set_options(ctx, ssl_options);
#else
/* As of 1.0.2f, SSL_OP_SINGLE_DH_USE key use is always on */
ssl_options |= SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_SINGLE_DH_USE;
Expand Down Expand Up @@ -504,27 +510,27 @@ void Init_mini_ssl(VALUE puma) {
#else
rb_define_const(mod, "OPENSSL_LIBRARY_VERSION", rb_str_new2(SSLeay_version(SSLEAY_VERSION)));
#endif
#if defined(OPENSSL_NO_SSL3) || defined(OPENSSL_NO_SSL3_METHOD)
/* True if SSL3 is not available */
rb_define_const(mod, "OPENSSL_NO_SSL3", Qtrue);
#else
rb_define_const(mod, "OPENSSL_NO_SSL3", Qfalse);
#endif

#if defined(OPENSSL_NO_TLS1) || defined(OPENSSL_NO_TLS1_METHOD)
/* True if TLS1 is not available */
rb_define_const(mod, "OPENSSL_NO_TLS1", Qtrue);
#else
rb_define_const(mod, "OPENSSL_NO_TLS1", Qfalse);
#endif

#if defined(OPENSSL_NO_TLS1_1) || defined(OPENSSL_NO_TLS1_1_METHOD)
/* True if TLS1_1 is not available */
rb_define_const(mod, "OPENSSL_NO_TLS1_1", Qtrue);
#else
rb_define_const(mod, "OPENSSL_NO_TLS1_1", Qfalse);
#endif

#if defined(OPENSSL_NO_SSL3) || defined(OPENSSL_NO_SSL3_METHOD)
/* True if SSL3 is not available */
rb_define_const(mod, "OPENSSL_NO_SSL3", Qtrue);
#else
rb_define_const(mod, "OPENSSL_NO_SSL3", Qfalse);
#endif

#if defined(OPENSSL_NO_TLS1) || defined(OPENSSL_NO_TLS1_METHOD)
/* True if TLS1 is not available */
rb_define_const(mod, "OPENSSL_NO_TLS1", Qtrue);
#else
rb_define_const(mod, "OPENSSL_NO_TLS1", Qfalse);
#endif

#if defined(OPENSSL_NO_TLS1_1) || defined(OPENSSL_NO_TLS1_1_METHOD)
/* True if TLS1_1 is not available */
rb_define_const(mod, "OPENSSL_NO_TLS1_1", Qtrue);
#else
rb_define_const(mod, "OPENSSL_NO_TLS1_1", Qfalse);
#endif

rb_define_singleton_method(mod, "check", noop, 0);

Expand Down
24 changes: 20 additions & 4 deletions lib/puma/dsl.rb
Expand Up @@ -369,6 +369,12 @@ def threads(min, max)
# Instead of `bind 'ssl://127.0.0.1:9292?key=key_path&cert=cert_path'` you
# can also use the this method.
#
# Protocols can be disabled by the use of `no_tlsv1`, `no_tlsv1_1`, and
# `no_tlsv1_3`. `no_tlsv1`, and `no_tlsv1_1` set the minimum protocol to
# TLSv1.1 and TLSv1.2 respectively. `no_tlsv1_3` should only be used when a
# front-end does not support TLSv1.3.
# The default value for all three is `false`, set them to `true` to enable.
#
# @example
# ssl_bind '127.0.0.1', '9292', {
# cert: path_to_cert,
Expand All @@ -387,16 +393,26 @@ def threads(min, max)
# }
def ssl_bind(host, port, opts)
verify = opts.fetch(:verify_mode, 'none').to_s
no_tlsv1 = opts.fetch(:no_tlsv1, 'false')
no_tlsv1_1 = opts.fetch(:no_tlsv1_1, 'false')

tls_str = nil
tls_str = if opts[:no_tlsv1_1]
'&no_tlsv1_1=true'
elsif opts[:no_tlsv1]
'&no_tlsv1=true'
end

if opts[:no_tlsv1_3]
tls_str = "#{tls_str || ''}&no_tlsv1_3=true"
end

ca_additions = "&ca=#{opts[:ca]}" if ['peer', 'force_peer'].include?(verify)

if defined?(JRUBY_VERSION)
keystore_additions = "keystore=#{opts[:keystore]}&keystore-pass=#{opts[:keystore_pass]}"
bind "ssl://#{host}:#{port}?cert=#{opts[:cert]}&key=#{opts[:key]}&#{keystore_additions}&verify_mode=#{verify}&no_tlsv1=#{no_tlsv1}&no_tlsv1_1=#{no_tlsv1_1}#{ca_additions}"
bind "ssl://#{host}:#{port}?cert=#{opts[:cert]}&key=#{opts[:key]}&#{keystore_additions}&verify_mode=#{verify}#{tls_str}#{ca_additions}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just realized how silly it is that we pass around arguments to bind internally as a string rather than as arguments. File that one away for the future...

else
ssl_cipher_filter = "&ssl_cipher_filter=#{opts[:ssl_cipher_filter]}" if opts[:ssl_cipher_filter]
bind "ssl://#{host}:#{port}?cert=#{opts[:cert]}&key=#{opts[:key]}#{ssl_cipher_filter}&verify_mode=#{verify}&no_tlsv1=#{no_tlsv1}&no_tlsv1_1=#{no_tlsv1_1}#{ca_additions}"
bind "ssl://#{host}:#{port}?cert=#{opts[:cert]}&key=#{opts[:key]}#{ssl_cipher_filter}&verify_mode=#{verify}#{tls_str}#{ca_additions}"
end
end

Expand Down
13 changes: 10 additions & 3 deletions lib/puma/minissl.rb
Expand Up @@ -217,11 +217,12 @@ class SSLError < StandardError

class Context
attr_accessor :verify_mode
attr_reader :no_tlsv1, :no_tlsv1_1
attr_reader :no_tlsv1, :no_tlsv1_1, :no_tlsv1_3

def initialize
@no_tlsv1 = false
@no_tlsv1_1 = false
@no_tlsv1_3 = false
end

if IS_JRUBY
Expand Down Expand Up @@ -267,20 +268,26 @@ def check
end
end

# disables TLSv1
# Disables TLSv1
# @!attribute [w] no_tlsv1=
def no_tlsv1=(tlsv1)
raise ArgumentError, "Invalid value of no_tlsv1=" unless ['true', 'false', true, false].include?(tlsv1)
@no_tlsv1 = tlsv1
end

# disables TLSv1 and TLSv1.1. Overrides `#no_tlsv1=`
# Disables TLSv1 and TLSv1.1. Overrides `#no_tlsv1=`
# @!attribute [w] no_tlsv1_1=
def no_tlsv1_1=(tlsv1_1)
raise ArgumentError, "Invalid value of no_tlsv1_1=" unless ['true', 'false', true, false].include?(tlsv1_1)
@no_tlsv1_1 = tlsv1_1
end

# Disables TLSv1.3.
# @!attribute [w] no_tlsv1_3=
def no_tlsv1_3=(tlsv1_3)
raise ArgumentError, "Invalid value of no_tlsv1_3=" unless ['true', 'false', true, false].include?(tlsv1_3)
@no_tlsv1_3 = tlsv1_3 if HAS_TLS1_3
end
end

VERIFY_NONE = 0
Expand Down
3 changes: 2 additions & 1 deletion lib/puma/minissl/context_builder.rb
Expand Up @@ -45,8 +45,9 @@ def context
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 = true if params['no_tlsv1'] == 'true'
ctx.no_tlsv1_1 = true if params['no_tlsv1_1'] == 'true'
ctx.no_tlsv1_3 = true if params['no_tlsv1_3'] == 'true'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At what point to we decide there's a more elegant interface here than just disabling individual TLS minor versions? I seem to remember something was discussed a long ways back...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we drop support for OpenSSL 1.0.2? 1.1.1 has min/max functions, with 1.0.2 you have to supply a list of allowed protocols.

JFYI, TLSv1.2 came out in 2008, TLSv1.3 in 2018. What we have now will probably work for a few years...


if params['verify_mode']
ctx.verify_mode = case params['verify_mode']
Expand Down
78 changes: 76 additions & 2 deletions test/test_config.rb
Expand Up @@ -16,7 +16,6 @@ def test_default_max_threads
assert_equal max_threads, Puma::Configuration.new.default_max_threads
end


def test_app_from_rackup
conf = Puma::Configuration.new do |c|
c.rackup "test/rackup/hello-bind.ru"
Expand Down Expand Up @@ -74,7 +73,82 @@ def test_ssl_bind

conf.load

ssl_binding = "ssl://0.0.0.0:9292?cert=/path/to/cert&key=/path/to/key&verify_mode=the_verify_mode&no_tlsv1=false&no_tlsv1_1=false"
ssl_binding = "ssl://0.0.0.0:9292?cert=/path/to/cert&key=/path/to/key&verify_mode=the_verify_mode"
assert_equal [ssl_binding], conf.options[:binds]
end

def test_ssl_bind
skip_on :jruby
skip 'No ssl support' unless ::Puma::HAS_SSL

conf = Puma::Configuration.new do |c|
c.ssl_bind "0.0.0.0", "9292", {
cert: "/path/to/cert",
key: "/path/to/key",
verify_mode: "the_verify_mode",
}
end

conf.load

ssl_binding = "ssl://0.0.0.0:9292?cert=/path/to/cert&key=/path/to/key&verify_mode=the_verify_mode"
assert_equal [ssl_binding], conf.options[:binds]
end

def test_ssl_bind_no_tlsv1
skip_on :jruby
skip 'No ssl support' unless ::Puma::HAS_SSL

conf = Puma::Configuration.new do |c|
c.ssl_bind "0.0.0.0", "9292", {
cert: "/path/to/cert",
key: "/path/to/key",
verify_mode: "the_verify_mode",
no_tlsv1: true,
}
end

conf.load

ssl_binding = "ssl://0.0.0.0:9292?cert=/path/to/cert&key=/path/to/key&verify_mode=the_verify_mode&no_tlsv1=true"
assert_equal [ssl_binding], conf.options[:binds]
end

def test_ssl_bind_no_tlsv1_1
skip_on :jruby
skip 'No ssl support' unless ::Puma::HAS_SSL

conf = Puma::Configuration.new do |c|
c.ssl_bind "0.0.0.0", "9292", {
cert: "/path/to/cert",
key: "/path/to/key",
verify_mode: "the_verify_mode",
no_tlsv1_1: true,
}
end

conf.load

ssl_binding = "ssl://0.0.0.0:9292?cert=/path/to/cert&key=/path/to/key&verify_mode=the_verify_mode&no_tlsv1_1=true"
assert_equal [ssl_binding], conf.options[:binds]
end

def test_ssl_bind_no_tlsv1_3
skip_on :jruby
skip 'No ssl support' unless ::Puma::HAS_SSL

conf = Puma::Configuration.new do |c|
c.ssl_bind "0.0.0.0", "9292", {
cert: "/path/to/cert",
key: "/path/to/key",
verify_mode: "the_verify_mode",
no_tlsv1_3: true,
}
end

conf.load

ssl_binding = "ssl://0.0.0.0:9292?cert=/path/to/cert&key=/path/to/key&verify_mode=the_verify_mode&no_tlsv1_3=true"
assert_equal [ssl_binding], conf.options[:binds]
end

Expand Down
17 changes: 17 additions & 0 deletions test/test_puma_server_ssl.rb
Expand Up @@ -204,6 +204,23 @@ def test_tls_v1_1_rejection
end
end

def test_tls_v1_3_rejection
skip("No TLSv1.3 support") unless ::Puma::MiniSSL::HAS_TLS1_3
ssl_version = ''
start_server { |ctx| ctx.no_tlsv1_3 = true }

ctx = OpenSSL::SSL::SSLContext.new
ctx.verify_mode = OpenSSL::SSL::VERIFY_NONE
port = @server.connected_ports[0]
socket = OpenSSL::SSL::SSLSocket.new TCPSocket.new(@host, port), ctx
socket.connect
socket.syswrite 'help Me'
ssl_version = socket.ssl_version
socket.close

assert_equal 'TLSv1.2', ssl_version
end

def test_http_rejection
body_http = nil
body_https = nil
Expand Down