From 9120fcde6a3ea173c0891de7de59af6c6ca931f9 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 12 Jun 2024 02:26:18 +0900 Subject: [PATCH 1/3] ssl: improve documentation of SSLContext#options= --- ext/openssl/ossl_ssl.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c index 457630ddc..28f1d6201 100644 --- a/ext/openssl/ossl_ssl.c +++ b/ext/openssl/ossl_ssl.c @@ -757,7 +757,10 @@ ssl_info_cb(const SSL *ssl, int where, int val) } /* - * Gets various OpenSSL options. + * call-seq: + * ctx.options -> integer + * + * Gets various \OpenSSL options. */ static VALUE ossl_sslctx_get_options(VALUE self) @@ -772,7 +775,17 @@ ossl_sslctx_get_options(VALUE self) } /* - * Sets various OpenSSL options. + * call-seq: + * ctx.options = integer + * + * Sets various \OpenSSL options. The options are a bit field and can be + * combined with the bitwise OR operator (|). Available options are + * defined as constants in OpenSSL::SSL that begin with +OP_+. + * + * For backwards compatibility, passing +nil+ has the same effect as passing + * OpenSSL::SSL::OP_ALL. + * + * See also man page SSL_CTX_set_options(3). */ static VALUE ossl_sslctx_set_options(VALUE self, VALUE options) From 00bec0d905d5ed25d1d00d0f5d12f7f097077643 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 12 Jun 2024 02:29:46 +0900 Subject: [PATCH 2/3] ssl: do not enable OpenSSL::SSL::OP_ALL by default Respect the SSL options set by default by SSL_CTX() and by the system-wide OpenSSL configuration file. OpenSSL::SSL::SSLContext#initialize currently adds OpenSSL::SSL::OP_ALL on top of the default SSL options. Let's stop doing it. OpenSSL::SSL::OP_ALL is a set of options that changes OpenSSL's behavior to workaround various TLS implementation bugs. Using it is considered usually safe, but is not completely harmless. --- lib/openssl/ssl.rb | 1 - test/openssl/test_ssl.rb | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index 2186f5f43..f28239bab 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -125,7 +125,6 @@ class SSLContext # that this form is deprecated. New applications should use #min_version= # and #max_version= as necessary. def initialize(version = nil) - self.options |= OpenSSL::SSL::OP_ALL self.ssl_version = version if version self.verify_mode = OpenSSL::SSL::VERIFY_NONE self.verify_hostname = false diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index f011e881e..088bd602c 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -15,11 +15,16 @@ def test_bad_socket end end + def test_ctx_setup + ctx = OpenSSL::SSL::SSLContext.new + assert_equal true, ctx.setup + assert_predicate ctx, :frozen? + assert_equal nil, ctx.setup + end + def test_ctx_options ctx = OpenSSL::SSL::SSLContext.new - assert (OpenSSL::SSL::OP_ALL & ctx.options) == OpenSSL::SSL::OP_ALL, - "OP_ALL is set by default" ctx.options = 4 assert_equal 4, ctx.options & 4 if ctx.options != 4 @@ -33,6 +38,29 @@ def test_ctx_options assert_equal nil, ctx.setup end + def test_ctx_options_config + omit "LibreSSL does not support OPENSSL_CONF" if libressl? + omit "OpenSSL < 1.1.1 does not support system_default" if openssl? && !openssl?(1, 1, 1) + + Tempfile.create("openssl.cnf") { |f| + f.puts(<<~EOF) + openssl_conf = default_conf + [default_conf] + ssl_conf = ssl_sect + [ssl_sect] + system_default = ssl_default_sect + [ssl_default_sect] + Options = -SessionTicket + EOF + f.close + + assert_separately([{ "OPENSSL_CONF" => f.path }, "-ropenssl"], <<~"end;") + ctx = OpenSSL::SSL::SSLContext.new + assert_equal OpenSSL::SSL::OP_NO_TICKET, ctx.options & OpenSSL::SSL::OP_NO_TICKET + end; + } + end + def test_ssl_with_server_cert ctx_proc = -> ctx { ctx.cert = @svr_cert From 77c3db2d6587d6430e16c9d01611bda9ccf65e70 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 12 Jun 2024 03:01:54 +0900 Subject: [PATCH 3/3] ssl: do not clear existing SSL options in SSLContext#set_params Apply SSL options set in DEFAULT_PARAMS without clearing existing options. It currently clears options in order to avoid setting one of the options included in OpenSSL::SSL::OP_ALL unless explicitly specified, namely OpenSSL::SSL::OP_DONT_INSERT_EMPTY_FRAGMENTS. Now that OpenSSL::SSL::OP_ALL has been removed from SSLContext#initialize, it is no longer necessary. --- lib/openssl/ssl.rb | 2 +- test/openssl/test_ssl.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/openssl/ssl.rb b/lib/openssl/ssl.rb index f28239bab..d6cb260f9 100644 --- a/lib/openssl/ssl.rb +++ b/lib/openssl/ssl.rb @@ -144,7 +144,7 @@ def initialize(version = nil) # used. def set_params(params={}) params = DEFAULT_PARAMS.merge(params) - self.options = params.delete(:options) # set before min_version/max_version + self.options |= params.delete(:options) # set before min_version/max_version params.each{|name, value| self.__send__("#{name}=", value) } if self.verify_mode != OpenSSL::SSL::VERIFY_NONE unless self.ca_file or self.ca_path or self.cert_store diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb index 088bd602c..459efcc18 100644 --- a/test/openssl/test_ssl.rb +++ b/test/openssl/test_ssl.rb @@ -57,6 +57,8 @@ def test_ctx_options_config assert_separately([{ "OPENSSL_CONF" => f.path }, "-ropenssl"], <<~"end;") ctx = OpenSSL::SSL::SSLContext.new assert_equal OpenSSL::SSL::OP_NO_TICKET, ctx.options & OpenSSL::SSL::OP_NO_TICKET + ctx.set_params + assert_equal OpenSSL::SSL::OP_NO_TICKET, ctx.options & OpenSSL::SSL::OP_NO_TICKET end; } end