Skip to content

Commit

Permalink
ssl: raise SSLError if loading ca_file or ca_path fails
Browse files Browse the repository at this point in the history
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. Commit 5375a55 ("ssl: use
SSL_CTX_load_verify_{file,dir}() if available", 2020-02-22) introduced
this inconsistency unintentionally, but raising SSLError seems more
appropriate in this situation. Let's adjust the OpenSSL <= 1.1.1 code
so that it behave the same way.
  • Loading branch information
rhenium committed Aug 10, 2023
1 parent 97fb410 commit 72ae17e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 3 deletions.
6 changes: 3 additions & 3 deletions ext/openssl/ossl_ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
34 changes: 34 additions & 0 deletions test/openssl/test_ssl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 72ae17e

Please sign in to comment.