From 7eb10f7b75735254a17c5996ab329a56ea083e4a Mon Sep 17 00:00:00 2001
From: Kazuki Yamaguchi <k@rhe.jp>
Date: Thu, 10 Aug 2023 02:45:15 +0900
Subject: [PATCH] ssl: raise SSLError if loading ca_file or ca_path fails

When compiled with OpenSSL <= 1.1.1, OpenSSL::SSL::SSLContext#setup
does not raise an exception on an error return from
SSL_CTX_load_verify_locations(), but instead only prints a verbose-mode
warning. This is not helpful since it very likely indicates an actual
error, such as the specified file not being readable.

Also, OpenSSL's error queue is not correctly cleared:

	$ ruby -w -ropenssl -e'OpenSSL.debug=true; ctx=OpenSSL::SSL::SSLContext.new; ctx.ca_file="bad-path"; ctx.setup; pp OpenSSL.errors'
	-e:1: warning: can't set verify locations
	["error:02001002:system library:fopen:No such file or directory",
	 "error:2006D080:BIO routines:BIO_new_file:no such file",
	 "error:0B084002:x509 certificate routines:X509_load_cert_crl_file: system lib"]

The behavior is currently different when compiled with OpenSSL >= 3.0:
SSLError is raised if SSL_CTX_load_verify_file() or
SSL_CTX_load_verify_dir() fails.

This inconsistency was unintentionally introduced by commit 5375a55ffc35
("ssl: use SSL_CTX_load_verify_{file,dir}() if available", 2020-02-22).
However, raising SSLError seems more appropriate in this situation.
Let's adjust the OpenSSL <= 1.1.1 code so that it behaves the same way
as the OpenSSL >= 3.0 code currently does.

Fixes: https://github.com/ruby/openssl/issues/649
---
 ext/openssl/ossl_ssl.c   |  6 +++---
 test/openssl/test_ssl.rb | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index ce6a5d4e4..5384893e6 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -885,9 +885,9 @@ ossl_sslctx_setup(VALUE self)
     if (ca_path && !SSL_CTX_load_verify_dir(ctx, ca_path))
         ossl_raise(eSSLError, "SSL_CTX_load_verify_dir");
 #else
-    if(ca_file || ca_path){
-	if (!SSL_CTX_load_verify_locations(ctx, ca_file, ca_path))
-	    rb_warning("can't set verify locations");
+    if (ca_file || ca_path) {
+        if (!SSL_CTX_load_verify_locations(ctx, ca_file, ca_path))
+            ossl_raise(eSSLError, "SSL_CTX_load_verify_locations");
     }
 #endif
 
diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb
index db76f1dc4..5e5b0386a 100644
--- a/test/openssl/test_ssl.rb
+++ b/test/openssl/test_ssl.rb
@@ -481,6 +481,40 @@ def test_exception_in_verify_callback_is_ignored
     }
   end
 
+  def test_ca_file
+    start_server(ignore_listener_error: true) { |port|
+      # X509_STORE is shared; setting ca_file to SSLContext affects store
+      store = OpenSSL::X509::Store.new
+      assert_equal false, store.verify(@svr_cert)
+
+      ctx = Tempfile.create("ca_cert.pem") { |f|
+        f.puts(@ca_cert.to_pem)
+        f.close
+
+        ctx = OpenSSL::SSL::SSLContext.new
+        ctx.verify_mode = OpenSSL::SSL::VERIFY_PEER
+        ctx.cert_store = store
+        ctx.ca_file = f.path
+        ctx.setup
+        ctx
+      }
+      assert_nothing_raised {
+        server_connect(port, ctx) { |ssl| ssl.puts("abc"); ssl.gets }
+      }
+      assert_equal true, store.verify(@svr_cert)
+    }
+  end
+
+  def test_ca_file_not_found
+    path = Tempfile.create("ca_cert.pem") { |f| f.path }
+    ctx = OpenSSL::SSL::SSLContext.new
+    ctx.ca_file = path
+    # OpenSSL >= 1.1.0: /no certificate or crl found/
+    assert_raise(OpenSSL::SSL::SSLError) {
+      ctx.setup
+    }
+  end
+
   def test_finished_messages
     server_finished = nil
     server_peer_finished = nil