Merge pull request #767 from rhenium/ky/ssl-respect-default-ssl-options

ssl: do not enable OpenSSL::SSL::OP_ALL by default
ruby · Dec 7, 2024 · 63db970 · 63db970
2 parents 3b3c950 + 77c3db2
commit 63db970
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 6 deletions.
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
@@ -746,7 +746,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)
@@ -761,7 +764,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 (<tt>|</tt>). 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)

diff --git 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
@@ -145,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
@@ -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,31 @@ 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
+        ctx.set_params
+        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