Add chained certificate support #20
Conversation
|
@cspenceiv can you sign the CLA and rebase please? https://github.com/elasticsearch/logstash/blob/master/CONTRIBUTING.md#contribution-steps |
This update does not make any changes to input. It merely parses and properly loads chained certificates from the file pointed to by ssl_certificate.
|
Rebase complete. Already signed the CLA a while back for a similar issue on logstash-plugins/logstash-input-tcp. |
| @ssl.cert = OpenSSL::X509::Certificate.new(File.read(@options[:ssl_certificate])) | ||
| @ssl.key = OpenSSL::PKey::RSA.new(File.read(@options[:ssl_key]), | ||
| ssl = OpenSSL::SSL::SSLContext.new | ||
| raw_cert = File.read(@options[:ssl_certificate]) |
There was a problem hiding this comment.
I suspect there's a library method for reading certificates from a file that we can reuse instead of crafting our own here. I don't know what that is right now, though.
There was a problem hiding this comment.
This new code might be best in a separate file as a function that returns an array of OpenSSL::X509::Certificates.
|
Hello and thank you for this work :) While I work on manual tests for this, I did notice that there are no unit or integration tests. Without those, there's no simple way for us to know if this code continues to work over time. Can you add tests to verify that both chained and unchained certs still work? |
| certEnd = raw_cert.length - 1 | ||
| end | ||
| certLink = OpenSSL::X509::Certificate.new(raw_cert[certPosition[i]..certEnd]) | ||
| ssl.extra_chain_cert.push(certLink) |
There was a problem hiding this comment.
Like most things in Ruby OpenSSL, the extra_chain_cert setting has practically no documentation available nor are there any decent examples I can find of its usage online (all the examples I see are copy/pasted from Ruby's own source tree, blah).
I'll have to read MRI's code to figure out what this really does under the hood.
There was a problem hiding this comment.
(Notes from reading MRI's code) extra_chain_cert is an accessor (in MRI, anyway) that is used in both the SSLContext initialize and connect(?) methods. It invokes this OpenSSL method: SSL_CTX_add_extra_chain_cert for each item in the Enumerable extra_chain_cert accessor.
|
Unfortunately, the project I was working on that needed the ability to utilize chained SSL certificates is tabled for now due to an emergency project redirection at my work. Additionally, since it appears that there will be a lot of needed development in the area of fully compliant SSL (at least from a testing standpoint), it's unlikely that I will be continuing the evaluation of logstash since the project does not meet my employer's security requirements in the short term. Additionally, the library you currently use for certificates, Flores::PKI, only generates self-signed certificates which is useless for the kind of testing you're asking me to write. This would require the writing of a more complete test suite than Flores currently provides. Again, I won't have the time to invest into such a project. I guess at this point, my contribution is more or less a take it or leave it kind of commit depending upon how much of a priority fully compliant SSL is to your project. If you prefer, I can redirect the pull request to a development branch upon request. Anyway, I was glad to provide some initial work for you guys, and I wish you the best of luck! [Edited out inaccuracy.] |
This update does not make any changes to input. It merely parses and properly loads chained certificates from the file pointed to by ssl_certificate.
This should resolve issue #19 and is a similar fix as proposed for logstash-input-tcp.
This pull request will be in direct conflict with the solution made by pull request #2. This solution does not require changing input to the Server Class so it will work without requiring modification by other plugins. Additionally, it will operate the way other systems process chained certificates (without separating the chain from the cert).