From 571e61ab36bc1277edc9c973d41aa1acefd76179 Mon Sep 17 00:00:00 2001 From: Michael Nitschinger Date: Mon, 31 Aug 2020 10:34:44 +0200 Subject: [PATCH] Allow to customize how SslMasterKeyHandler is enabled (#10513) Motivation: Right now after a SslMasterKeyHandler is added to the pipeline, it also needs to be enabled via a system property explicitly. In some environments where the handler is conditionally added to the pipeline this is redundant and a bit confusing. Modifications: This changeset keeps the default behavior, but allows child implementations to tweak the way on how it detects that it should actually handle the event when it is being raised. Also, removed a private static that is not used in the wireshark handler. Result: Child implementations can be more flexible in deciding when and how the handler should perform its work (without changing any of the defaults). --- .../handler/ssl/SslMasterKeyHandler.java | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslMasterKeyHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslMasterKeyHandler.java index c86ec92563..17ee6bac39 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslMasterKeyHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslMasterKeyHandler.java @@ -62,7 +62,7 @@ public abstract class SslMasterKeyHandler implements ChannelHandler { private static final Throwable UNAVAILABILITY_CAUSE; static { - Throwable cause = null; + Throwable cause; Class clazz = null; Field field = null; try { @@ -120,34 +120,41 @@ public abstract class SslMasterKeyHandler implements ChannelHandler { @Override public final void userEventTriggered(ChannelHandlerContext ctx, Object evt) { //only try to log the session info if the ssl handshake has successfully completed. - if (evt == SslHandshakeCompletionEvent.SUCCESS) { - boolean shouldHandle = SystemPropertyUtil.getBoolean(SYSTEM_PROP_KEY, false); + if (evt == SslHandshakeCompletionEvent.SUCCESS && masterKeyHandlerEnabled()) { + final SslHandler handler = ctx.pipeline().get(SslHandler.class); + final SSLEngine engine = handler.engine(); + final SSLSession sslSession = engine.getSession(); - if (shouldHandle) { - final SslHandler handler = ctx.pipeline().get(SslHandler.class); - final SSLEngine engine = handler.engine(); - final SSLSession sslSession = engine.getSession(); - - //the OpenJDK does not expose a way to get the master secret, so try to use reflection to get it. - if (isSunSslEngineAvailable() && sslSession.getClass().equals(SSL_SESSIONIMPL_CLASS)) { - final SecretKey secretKey; - try { - secretKey = (SecretKey) SSL_SESSIONIMPL_MASTER_SECRET_FIELD.get(sslSession); - } catch (IllegalAccessException e) { - throw new IllegalArgumentException("Failed to access the field 'masterSecret' " + - "via reflection.", e); - } - accept(secretKey, sslSession); - } else if (OpenSsl.isAvailable() && engine instanceof ReferenceCountedOpenSslEngine) { - SecretKeySpec secretKey = ((ReferenceCountedOpenSslEngine) engine).masterKey(); - accept(secretKey, sslSession); + //the OpenJDK does not expose a way to get the master secret, so try to use reflection to get it. + if (isSunSslEngineAvailable() && sslSession.getClass().equals(SSL_SESSIONIMPL_CLASS)) { + final SecretKey secretKey; + try { + secretKey = (SecretKey) SSL_SESSIONIMPL_MASTER_SECRET_FIELD.get(sslSession); + } catch (IllegalAccessException e) { + throw new IllegalArgumentException("Failed to access the field 'masterSecret' " + + "via reflection.", e); } + accept(secretKey, sslSession); + } else if (OpenSsl.isAvailable() && engine instanceof ReferenceCountedOpenSslEngine) { + SecretKeySpec secretKey = ((ReferenceCountedOpenSslEngine) engine).masterKey(); + accept(secretKey, sslSession); } } ctx.fireUserEventTriggered(evt); } + /** + * Checks if the handler is set up to actually handle/accept the event. + * By default the {@link #SYSTEM_PROP_KEY} property is checked, but any implementations of this class are + * free to override if they have different mechanisms of checking. + * + * @return true if it should handle, false otherwise. + */ + protected boolean masterKeyHandlerEnabled() { + return SystemPropertyUtil.getBoolean(SYSTEM_PROP_KEY, false); + } + /** * Create a {@link WiresharkSslMasterKeyHandler} instance. * This TLS master key handler logs the master key and session-id in a format @@ -171,8 +178,6 @@ public abstract class SslMasterKeyHandler implements ChannelHandler { private static final InternalLogger wireshark_logger = InternalLoggerFactory.getInstance("io.netty.wireshark"); - private static final char[] hexCode = "0123456789ABCDEF".toCharArray(); - @Override protected void accept(SecretKey masterKey, SSLSession session) { if (masterKey.getEncoded().length != 48) {