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

OpenSSL::X509::Certificate load entire certificate chain #288

Closed
ioquatix opened this issue Nov 4, 2019 · 4 comments · Fixed by #441
Closed

OpenSSL::X509::Certificate load entire certificate chain #288

ioquatix opened this issue Nov 4, 2019 · 4 comments · Fixed by #441
Assignees
Milestone

Comments

@ioquatix
Copy link
Member

ioquatix commented Nov 4, 2019

We do have support for SSL_CTX_use_certificate_chain_file 46e4bdb

But this makes assumptions that the certificates can be read from a file on disk, i.e. user code cannot handle this abstractly but instead must use a path.

I would like to decouple this, i.e. certificates might come from a file on disk or they might come from some other store (e.g. Redis).

I've been looking at how SSL_CTX_use_certificate_chain_file is implemented, and it's relatively straight forward. I'd like to add something like OpenSSL::X509::Certificate.load_file(path) which returns an array of certificates.

This is a quick hack I did in the past:

require 'openssl/x509'

module OpenSSL::X509
	CERTIFICATE_PATTERN = /-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----/m
	
	def self.load_certificates(path)
		File.read(path).scan(CERTIFICATE_PATTERN).collect do |text|
			Certificate.new(text)
		end
	end
end

But I think we can do better than this using the BIO_ methods from OpenSSL. Maybe we should have:

OpenSSL::X509.load_file(path)

# and/or
OpenSSL::X509::Certificate.load_file(path)

PEM files can contain more things, but for me that's enough for my use case. However, I'm not adverse to considering how to load and/or support other things.

@ioquatix ioquatix added this to the v2.2.0 milestone Nov 4, 2019
@ioquatix
Copy link
Member Author

ioquatix commented Nov 4, 2019

See also #228

@ioquatix
Copy link
Member Author

#305

@ioquatix
Copy link
Member Author

rhenium added a commit to rhenium/ruby-openssl that referenced this issue May 13, 2020
Let's revert the changes for now, as it cannot be included in the 2.2.0
release.

My comment on ruby#257:

> A blocker is OpenSSL::SSL::SSLContext#add_certificate_chain_file. It
> has a pending change and I don't want to include it in an incomplete
> state.
>
> The initial implementation in commit 46e4bdb was not really
> useful. The issue is described in ruby#305. ruby#309 extended it
> to take the corresponding private key together. However, the new
> implementation was incompatible on Windows and was reverted by ruby#320 to
> the initial one.
>
> (The prerequisite to implement it in) an alternative way is ruby#288, and
> it's still cooking.

This effectively reverts the following commits:

 - dacd089 ("ssl: suppress test failure with SSLContext#add_certificate_chain_file", 2020-03-09)
 - 46e4bdb ("Add support for SSL_CTX_use_certificate_chain_file. Fixes ruby#254.", 2019-06-13)
@rhenium rhenium modified the milestones: v2.2.0, v3.0.0 May 13, 2020
@ioquatix
Copy link
Member Author

Implemented in #441

@ioquatix ioquatix linked a pull request May 13, 2021 that will close this issue
@ioquatix ioquatix self-assigned this May 13, 2021
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