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

Can no longer pass file contents as ca_file property with OpenSSL 3 #649

Closed
thoiberg opened this issue Jul 5, 2023 · 3 comments · Fixed by #659
Closed

Can no longer pass file contents as ca_file property with OpenSSL 3 #649

thoiberg opened this issue Jul 5, 2023 · 3 comments · Fixed by #659

Comments

@thoiberg
Copy link

thoiberg commented Jul 5, 2023

Hi, I have some code that was retrieving the string contents of a certificate from a database (so "-----BEGIN CERTIFICATE...") and setting it to the ca_file property with OpenSSL. This works for OpenSSL 1.1.1, but on OpenSSL 3.0.9 raises SSL_CTX_load_verify_file: system lib (OpenSSL::SSL::SSLError). From what I've read I'm guessing this might have always been unsupported behaviour with OpenSSL 1.1.1, but I was wondering if this was an intentional breaking change, or if there is an alternative way to pass the contents of a certificate to OpenSSL.

I have a set of minimal reproduction steps based of the wget sample below:

OpenSSL 1.1.1

docker run -it ruby:3.1-bullseye
require 'net/https'

pem_contents = File.read(OpenSSL::X509::DEFAULT_CERT_FILE)
uri = URI.parse("https://github.com/ruby/openssl")
h = Net::HTTP.new(uri.host, uri.port)
h.use_ssl = true
h.ca_file = pem_contents
path = uri.path.empty? ? "/" : uri.path
h.get2(path)
# returns <Net::HTTPOK 200 OK readbody=true>

OpenSSL 3.0.9

docker run -it ruby:3.1-bookworm
require 'net/https'

pem_contents = File.read(OpenSSL::X509::DEFAULT_CERT_FILE)
uri = URI.parse("https://github.com/ruby/openssl")
h = Net::HTTP.new(uri.host, uri.port)
h.use_ssl = true
h.ca_file = pem_contents
path = uri.path.empty? ? "/" : uri.path
h.get2(path)
# raises SSL_CTX_load_verify_file: system lib (OpenSSL::SSL::SSLError)

full stacktrace

/usr/local/lib/ruby/3.1.0/net/http.rb:1081:in `initialize': SSL_CTX_load_verify_file: system lib (OpenSSL::SSL::SSLError)
	from /usr/local/lib/ruby/3.1.0/net/http.rb:1081:in `new'
	from /usr/local/lib/ruby/3.1.0/net/http.rb:1081:in `connect'
	from /usr/local/lib/ruby/3.1.0/net/http.rb:995:in `do_start'
	from /usr/local/lib/ruby/3.1.0/net/http.rb:984:in `start'
	from /usr/local/lib/ruby/3.1.0/net/http.rb:1564:in `request'
	from /usr/local/lib/ruby/3.1.0/net/http.rb:1474:in `request_get'
	from (irb):10:in `<main>'
	from /usr/local/lib/ruby/gems/3.1.0/gems/irb-1.4.1/exe/irb:11:in `<top (required)>'
	from /usr/local/bin/irb:25:in `load'
	from /usr/local/bin/irb:25:in `<main>'
@rhenium
Copy link
Member

rhenium commented Jul 5, 2023

Yes, Net::HTTP#ca_file (OpenSSL::SSL::SSLContext#ca_file) wants the path to the file.

I don't think setting the file contents to the attribute ever worked. With OpenSSL 1.1.1, it is simply ignored with a verbose-mode warning:

#ifdef HAVE_SSL_CTX_LOAD_VERIFY_FILE
if (ca_file && !SSL_CTX_load_verify_file(ctx, ca_file))
ossl_raise(eSSLError, "SSL_CTX_load_verify_file");
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");
}
#endif

$ ruby -w -ropenssl -e'ctx=OpenSSL::SSL::SSLContext.new; ctx.ca_file=File.read("/etc/ssl/certs/Go_Daddy_Class_2_CA.pem"); ctx.setup'
-e:1: warning: can't set verify locations

The behavior difference was not intentional, but I think the OpenSSL <= 1.1.1 code should've also raised an exception since this is clearly an error case.

@thoiberg
Copy link
Author

thoiberg commented Jul 6, 2023

ahhhhhh, ok that makes a lot more sense 😄 . Thanks for investigating 🙇

@thoiberg thoiberg closed this as completed Jul 6, 2023
@rhenium
Copy link
Member

rhenium commented Jul 12, 2023

Reopening because the OpenSSL <= 1.1.1 code should be fixed.

@rhenium rhenium reopened this Jul 12, 2023
rhenium added a commit to rhenium/ruby-openssl that referenced this issue Aug 10, 2023
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 5375a55
("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: ruby#649
matzbot pushed a commit to ruby/ruby that referenced this issue Aug 16, 2023
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 ruby/openssl@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: ruby/openssl#649

ruby/openssl@7eb10f7b75
anakinj pushed a commit to anakinj/openssl that referenced this issue Feb 17, 2024
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 5375a55
("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: ruby#649
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants