Skip to content

Commit 4c335a3

Browse files
kareskares
authored
Feat: improved error handling/logging/unwraping (#133)
The unwrapping at the Java level is for exceptions wrapped by Netty. Full exception details will be logged at debug level from the Java side - since we seem to prefer (manual) exception logging at the plugin level. We also make sure to log cause, if any, on the Ruby side which now catches all (expected) Java exceptions. resolves #134
1 parent d7534e9 commit 4c335a3

File tree

7 files changed

+140
-40
lines changed

7 files changed

+140
-40
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
## 3.3.7
2+
- Feat: improved error handling/logging/unwraping [#133](https://github.com/logstash-plugins/logstash-input-http/pull/133)
3+
14
## 3.3.6
25
- Fixes a regression introduced in 3.1.0's migration to the Netty back-end that broke some users'
36
browser-based workflows. When an instance of this plugin that is configured to require Basic

‎VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.3.6
1+
3.3.7

‎lib/logstash/inputs/http.rb‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -217,30 +217,24 @@ def create_http_server(message_handler)
217217
def build_ssl_params
218218
return nil unless @ssl
219219

220-
ssl_builder = nil
221-
222220
if @keystore && @keystore_password
223221
ssl_builder = org.logstash.plugins.inputs.http.util.JksSslBuilder.new(@keystore, @keystore_password.value)
224222
else
225223
begin
226-
ssl_builder = org.logstash.plugins.inputs.http.util.SslSimpleBuilder.new(@ssl_certificate, @ssl_key, @ssl_key_passphrase.nil? ? nil : @ssl_key_passphrase.value)
227-
.setCipherSuites(normalized_ciphers)
224+
ssl_builder = org.logstash.plugins.inputs.http.util.SslSimpleBuilder
225+
.new(@ssl_certificate, @ssl_key, @ssl_key_passphrase.nil? ? nil : @ssl_key_passphrase.value)
226+
.setCipherSuites(normalized_ciphers)
228227
rescue java.lang.IllegalArgumentException => e
229-
raise LogStash::ConfigurationError.new(e)
228+
@logger.error("SSL configuration invalid", error_details(e))
229+
raise LogStash::ConfigurationError, e
230230
end
231231

232232
if client_authentication?
233233
ssl_builder.setCertificateAuthorities(@ssl_certificate_authorities)
234234
end
235235
end
236236

237-
ssl_context = ssl_builder.build()
238-
ssl_handler_provider = org.logstash.plugins.inputs.http.util.SslHandlerProvider.new(ssl_context)
239-
ssl_handler_provider.setVerifyMode(@ssl_verify_mode_final.upcase)
240-
ssl_handler_provider.setProtocols(convert_protocols)
241-
ssl_handler_provider.setHandshakeTimeoutMilliseconds(@ssl_handshake_timeout)
242-
243-
ssl_handler_provider
237+
new_ssl_handshake_provider(ssl_builder)
244238
end
245239

246240
def ssl_key_configured?
@@ -259,6 +253,8 @@ def require_certificate_authorities?
259253
@ssl_verify_mode_final == "force_peer" || @ssl_verify_mode_final == "peer"
260254
end
261255

256+
private
257+
262258
def normalized_ciphers
263259
@cipher_suites.map(&:upcase)
264260
end
@@ -267,4 +263,31 @@ def convert_protocols
267263
TLS.get_supported(@tls_min_version..@tls_max_version).map(&:name)
268264
end
269265

266+
def new_ssl_handshake_provider(ssl_builder)
267+
begin
268+
ssl_handler_provider = org.logstash.plugins.inputs.http.util.SslHandlerProvider.new(ssl_builder.build())
269+
ssl_handler_provider.setVerifyMode(@ssl_verify_mode_final.upcase)
270+
ssl_handler_provider.setProtocols(convert_protocols)
271+
ssl_handler_provider.setHandshakeTimeoutMilliseconds(@ssl_handshake_timeout)
272+
ssl_handler_provider
273+
rescue java.lang.IllegalArgumentException => e
274+
@logger.error("SSL configuration invalid", error_details(e))
275+
raise LogStash::ConfigurationError, e
276+
rescue java.lang.Exception => e
277+
@logger.error("SSL configuration failed", error_details(e, true))
278+
raise e
279+
end
280+
end
281+
282+
def error_details(e, trace = false)
283+
error_details = { :exception => e.class, :message => e.message }
284+
error_details[:backtrace] = e.backtrace if trace || @logger.debug?
285+
cause = e.cause
286+
if cause && e != cause
287+
error_details[:cause] = { :exception => cause.class, :message => cause.message }
288+
error_details[:cause][:backtrace] = cause.backtrace if trace || @logger.debug?
289+
end
290+
error_details
291+
end
292+
270293
end # class LogStash::Inputs::Http

‎spec/inputs/http_spec.rb‎

Lines changed: 71 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -386,21 +386,21 @@
386386
let(:ssl_certificate) { ssc.certificate }
387387
let(:ssl_key) { ssc.private_key }
388388

389+
let(:config) do
390+
{ "port" => port, "ssl" => true, "ssl_certificate" => ssl_certificate.path, "ssl_key" => ssl_key.path }
391+
end
392+
389393
after(:each) { ssc.delete }
390394

391-
subject { LogStash::Inputs::Http.new("port" => port, "ssl" => true,
392-
"ssl_certificate" => ssl_certificate.path,
393-
"ssl_key" => ssl_key.path) }
395+
subject { LogStash::Inputs::Http.new(config) }
396+
394397
it "should not raise exception" do
395398
expect { subject.register }.to_not raise_exception
396399
end
397400

398401
context "with ssl_verify_mode = none" do
399-
subject { LogStash::Inputs::Http.new("port" => port, "ssl" => true,
400-
"ssl_certificate" => ssl_certificate.path,
401-
"ssl_key" => ssl_key.path,
402-
"ssl_verify_mode" => "none"
403-
) }
402+
subject { LogStash::Inputs::Http.new(config.merge("ssl_verify_mode" => "none")) }
403+
404404
it "should not raise exception" do
405405
expect { subject.register }.to_not raise_exception
406406
end
@@ -419,11 +419,8 @@
419419
end
420420
end
421421
context "with verify_mode = none" do
422-
subject { LogStash::Inputs::Http.new("port" => port, "ssl" => true,
423-
"ssl_certificate" => ssl_certificate.path,
424-
"ssl_key" => ssl_key.path,
425-
"verify_mode" => "none"
426-
) }
422+
subject { LogStash::Inputs::Http.new(config.merge("verify_mode" => "none")) }
423+
427424
it "should not raise exception" do
428425
expect { subject.register }.to_not raise_exception
429426
end
@@ -441,6 +438,67 @@
441438
end
442439
end
443440
end
441+
442+
context "with invalid cipher_suites" do
443+
let(:config) { super.merge("cipher_suites" => "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38") }
444+
445+
it "should raise a configuration error" do
446+
expect( subject.logger ).to receive(:error) do |msg, opts|
447+
expect( msg ).to match /.*?configuration invalid/
448+
expect( opts[:message] ).to match /TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA38.*? not available/
449+
end
450+
expect { subject.register }.to raise_error(LogStash::ConfigurationError)
451+
end
452+
end
453+
454+
context "with invalid ssl certificate" do
455+
before do
456+
cert = File.readlines path = config["ssl_certificate"]
457+
i = cert.index { |line| line.index('END CERTIFICATE') }
458+
cert[i - 1] = ''
459+
File.write path, cert.join("\n")
460+
end
461+
462+
it "should raise a configuration error" do
463+
expect( subject.logger ).to receive(:error) do |msg, opts|
464+
expect( msg ).to match /SSL configuration invalid/
465+
expect( opts[:message] ).to match /File does not contain valid certificate/i
466+
end
467+
expect { subject.register }.to raise_error(LogStash::ConfigurationError)
468+
end
469+
end
470+
471+
context "with invalid ssl key config" do
472+
let(:config) { super.merge("ssl_key_passphrase" => "1234567890") }
473+
474+
it "should raise a configuration error" do
475+
expect( subject.logger ).to receive(:error) do |msg, opts|
476+
expect( msg ).to match /SSL configuration invalid/
477+
expect( opts[:message] ).to match /File does not contain valid private key/i
478+
end
479+
expect { subject.register }.to raise_error(LogStash::ConfigurationError)
480+
end
481+
end
482+
483+
context "with invalid ssl certificate_authorities" do
484+
let(:config) do
485+
super.merge("ssl_verify_mode" => "peer",
486+
"ssl_certificate_authorities" => [ ssc.certificate.path, ssc.private_key.path ])
487+
end
488+
489+
it "should raise a cert error" do
490+
expect( subject.logger ).to receive(:error) do |msg, opts|
491+
expect( msg ).to match(/SSL configuration failed/), lambda { "unexpected: logger.error #{msg.inspect}, #{opts.inspect}" }
492+
expect( opts[:message] ).to match /signed fields invalid/
493+
end
494+
begin
495+
subject.register
496+
rescue Java::JavaSecurityCert::CertificateParsingException
497+
:pass
498+
end
499+
end
500+
end
501+
444502
end
445503
end
446504
end

‎src/main/java/org/logstash/plugins/inputs/http/util/JksSslBuilder.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,13 @@
11
package org.logstash.plugins.inputs.http.util;
22

3+
import io.netty.handler.ssl.SslContext;
34
import io.netty.handler.ssl.SslContextBuilder;
45

5-
66
import javax.net.ssl.KeyManagerFactory;
77
import javax.net.ssl.TrustManagerFactory;
8-
9-
import io.netty.handler.ssl.SslContext;
108
import java.io.FileInputStream;
11-
import java.io.IOException;
12-
import java.security.KeyManagementException;
139
import java.security.KeyStore;
14-
import java.security.KeyStoreException;
15-
import java.security.NoSuchAlgorithmException;
1610
import java.security.Security;
17-
import java.security.UnrecoverableKeyException;
18-
import java.security.cert.CertificateException;
1911

2012
public class JksSslBuilder implements SslBuilder {
2113
private static final String ALGORITHM_SUN_X509 = "SunX509";
@@ -28,7 +20,7 @@ public JksSslBuilder(String keyStorePath, String keyStorePassword) {
2820
this.keyStorePassword = keyStorePassword.toCharArray();
2921
}
3022

31-
public SslContext build() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException, UnrecoverableKeyException, KeyManagementException {
23+
public SslContext build() throws Exception {
3224
String algorithm = Security.getProperty(ALGORITHM);
3325
if (algorithm == null) {
3426
algorithm = ALGORITHM_SUN_X509;
@@ -47,6 +39,6 @@ public SslContext build() throws IOException, KeyStoreException, CertificateExce
4739
SslContextBuilder builder = SslContextBuilder.forServer(kmf);
4840
builder.trustManager(tmf);
4941

50-
return builder.build();
42+
return SslSimpleBuilder.doBuild(builder);
5143
}
5244
}

‎src/main/java/org/logstash/plugins/inputs/http/util/SslBuilder.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111

1212
public interface SslBuilder {
1313

14-
SslContext build() throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException, UnrecoverableKeyException, KeyManagementException;
14+
/**
15+
* @return context
16+
* @throws Exception (IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException, UnrecoverableKeyException, KeyManagementException)
17+
*/
18+
SslContext build() throws Exception;
1519

1620
}

‎src/main/java/org/logstash/plugins/inputs/http/util/SslSimpleBuilder.java‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Arrays;
1919
import java.util.List;
2020
import javax.crypto.Cipher;
21+
import javax.net.ssl.SSLException;
2122
import javax.net.ssl.SSLServerSocketFactory;
2223

2324
public class SslSimpleBuilder implements SslBuilder {
@@ -85,7 +86,7 @@ public SslSimpleBuilder setCertificateAuthorities(String[] cert) {
8586
return this;
8687
}
8788

88-
public SslContext build() throws IOException, NoSuchAlgorithmException, CertificateException {
89+
public SslContext build() throws Exception {
8990
SslContextBuilder builder = SslContextBuilder.forServer(sslCertificateFile, sslKeyFile, passPhrase);
9091

9192
if(logger.isDebugEnabled()) {
@@ -102,7 +103,26 @@ public SslContext build() throws IOException, NoSuchAlgorithmException, Certific
102103
builder.trustManager(loadCertificateCollection(certificateAuthorities));
103104
}
104105

105-
return builder.build();
106+
return doBuild(builder);
107+
}
108+
109+
// NOTE: copy-pasta from input-beats
110+
static SslContext doBuild(final SslContextBuilder builder) throws Exception {
111+
try {
112+
return builder.build();
113+
} catch (SSLException e) {
114+
logger.debug("Failed to initialize SSL", e);
115+
// unwrap generic wrapped exception from Netty's JdkSsl{Client|Server}Context
116+
if ("failed to initialize the server-side SSL context".equals(e.getMessage()) ||
117+
"failed to initialize the client-side SSL context".equals(e.getMessage())) {
118+
// Netty catches Exception and simply wraps: throw new SSLException("...", e);
119+
if (e.getCause() instanceof Exception) throw (Exception) e.getCause();
120+
}
121+
throw e;
122+
} catch (Exception e) {
123+
logger.debug("Failed to initialize SSL", e);
124+
throw e;
125+
}
106126
}
107127

108128
private X509Certificate[] loadCertificateCollection(String[] certificates) throws IOException, CertificateException {

0 commit comments

Comments
 (0)