Added support for CRL check #62
Conversation
|
Tests do not work anymore with current logstash version because of #51. |
|
Sorry for delay! The plugin is lacking all TLS related tests, so I made an attempt to add some, including tests for reading malformed PEM files. I took inspiration from https://github.com/logstash-plugins/logstash-output-tcp for the setup of TLS server. I needed some test certificates, which I committed in the repo. I hope that is OK. I used my own tool certyaml to create those.
It seems to fail in a same way as it has been failing if user would pass invalid certificate or private key. For some reason, there is no error when loading malformed CA certificate, which may be a problem jruby-openssl. |
spec/outputs/syslog_tls_spec.rb
Outdated
| # TODO: there is no failure when reading malformed CA certificate | ||
| # | ||
| # context "invalid CA certificate" do | ||
| # let(:options ) { super().merge( | ||
| # "ssl_cert" => File.join(FIXTURES_PATH, "client.pem"), | ||
| # "ssl_key" => File.join(FIXTURES_PATH, "client-key.pem"), | ||
| # "ssl_cacert" => File.join(FIXTURES_PATH, "invalid.pem"), | ||
| # "ssl_crl" => File.join(FIXTURES_PATH, "ca-crl.pem") | ||
| # ) } | ||
|
|
||
| # it "register raises error" do | ||
| # expect { subject.register }.to raise_error(OpenSSL::PKey::RSAError, /malformed PEM data/) | ||
| # end | ||
| # end | ||
|
|
There was a problem hiding this comment.
| # TODO: there is no failure when reading malformed CA certificate | |
| # | |
| # context "invalid CA certificate" do | |
| # let(:options ) { super().merge( | |
| # "ssl_cert" => File.join(FIXTURES_PATH, "client.pem"), | |
| # "ssl_key" => File.join(FIXTURES_PATH, "client-key.pem"), | |
| # "ssl_cacert" => File.join(FIXTURES_PATH, "invalid.pem"), | |
| # "ssl_crl" => File.join(FIXTURES_PATH, "ca-crl.pem") | |
| # ) } | |
| # it "register raises error" do | |
| # expect { subject.register }.to raise_error(OpenSSL::PKey::RSAError, /malformed PEM data/) | |
| # end | |
| # end |
Remove commented code
spec/outputs/syslog_tls_spec.rb
Outdated
| context "ssl_verify == false, server with untrusted certificates" do | ||
| let(:options ) { super().merge("ssl_verify" => false) } | ||
| let(:server_cert_file) { File.join(FIXTURES_PATH, "untrusted-server.pem") } | ||
| let(:server_pkey_file) { File.join(FIXTURES_PATH, "untrusted-server-key.pem") } | ||
|
|
||
| it_behaves_like "syslog output" | ||
| end | ||
|
|
||
| context "ssl_verify == true, server with untrusted certificates" do | ||
| let(:options ) { super().merge("ssl_verify" => true) } | ||
| let(:server_cert_file) { File.join(FIXTURES_PATH, "untrusted-server.pem") } | ||
| let(:server_pkey_file) { File.join(FIXTURES_PATH, "untrusted-server-key.pem") } | ||
|
|
||
| it "syslog output refuses to connect" do | ||
| Thread.start { secure_server.accept rescue nil } | ||
| expect(subject.logger).to receive(:error).with(/SSL Error/i, hash_including(exception: OpenSSL::SSL::SSLError)).once.and_throw :TEST_DONE | ||
| expect { subject.receive event }.to throw_symbol(:TEST_DONE) | ||
| end | ||
| end |
There was a problem hiding this comment.
| context "ssl_verify == false, server with untrusted certificates" do | |
| let(:options ) { super().merge("ssl_verify" => false) } | |
| let(:server_cert_file) { File.join(FIXTURES_PATH, "untrusted-server.pem") } | |
| let(:server_pkey_file) { File.join(FIXTURES_PATH, "untrusted-server-key.pem") } | |
| it_behaves_like "syslog output" | |
| end | |
| context "ssl_verify == true, server with untrusted certificates" do | |
| let(:options ) { super().merge("ssl_verify" => true) } | |
| let(:server_cert_file) { File.join(FIXTURES_PATH, "untrusted-server.pem") } | |
| let(:server_pkey_file) { File.join(FIXTURES_PATH, "untrusted-server-key.pem") } | |
| it "syslog output refuses to connect" do | |
| Thread.start { secure_server.accept rescue nil } | |
| expect(subject.logger).to receive(:error).with(/SSL Error/i, hash_including(exception: OpenSSL::SSL::SSLError)).once.and_throw :TEST_DONE | |
| expect { subject.receive event }.to throw_symbol(:TEST_DONE) | |
| end | |
| end | |
| context "server with untrusted certificates" do | |
| let(:server_cert_file) { File.join(FIXTURES_PATH, "untrusted-server.pem") } | |
| let(:server_pkey_file) { File.join(FIXTURES_PATH, "untrusted-server-key.pem") } | |
| context "ss_verify disabled" do | |
| let(:options ) { super().merge("ssl_verify" => false) } | |
| it_behaves_like "syslog output" | |
| end | |
| context "ss_verify enabled" do | |
| let(:options ) { super().merge("ssl_verify" => true) } | |
| it "should refuse to connect" do | |
| Thread.start { secure_server.accept rescue nil } | |
| expect(subject.logger).to receive(:error).with(/SSL Error/i, hash_including(exception: OpenSSL::SSL::SSLError)).once.and_throw :TEST_DONE | |
| expect { subject.receive event }.to throw_symbol(:TEST_DONE) | |
| end | |
| end | |
| end |
A nested context could make it even more readable
There was a problem hiding this comment.
True, changed accordingly!
No problem, adding TLS tests is a good learning experience for a ruby newbie like me :) |
This change adds support for CRL (Certificate Revocation List) check, to validate that the certificate of the syslog server has not been revoked. The CRL is loaded from file, meaning it must be pre-loaded on the system where logstash is executing.
Part of elastic/logstash#15236