diff --git a/src/main/java/fr/xephi/authme/Log4JFilter.java b/src/main/java/fr/xephi/authme/Log4JFilter.java index dccaee6f..365b2cec 100644 --- a/src/main/java/fr/xephi/authme/Log4JFilter.java +++ b/src/main/java/fr/xephi/authme/Log4JFilter.java @@ -6,80 +6,50 @@ import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.Logger; import org.apache.logging.log4j.message.Message; +import fr.xephi.authme.util.StringUtils; + /** - * + * Implements a filter for Log4j to skip sensitive AuthMe commands. * @author Xephi59 */ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { + + /** List of commands (lower-case) to skip. */ + private static final String[] COMMANDS_TO_SKIP = { "/login ", "/l ", "/reg ", "/changepassword ", + "/unregister ", "/authme register ", "/authme changepassword ", "/authme reg ", "/authme cp ", + "/register " }; + /** Constructor. */ public Log4JFilter() { } @Override public Result filter(LogEvent record) { - try { - if (record == null || record.getMessage() == null) - return Result.NEUTRAL; - String logM = record.getMessage().getFormattedMessage().toLowerCase(); - if (!logM.contains("issued server command:")) - return Result.NEUTRAL; - if (!logM.contains("/login ") && !logM.contains("/l ") && !logM.contains("/reg ") && !logM.contains("/changepassword ") && !logM.contains("/unregister ") && !logM.contains("/authme register ") && !logM.contains("/authme changepassword ") && !logM.contains("/authme reg ") && !logM.contains("/authme cp ") && !logM.contains("/register ")) - return Result.NEUTRAL; - return Result.DENY; - } catch (NullPointerException npe) { - return Result.NEUTRAL; - } + if (record == null) { + return Result.NEUTRAL; + } + return validateMessage(record.getMessage()); } @Override public Result filter(Logger arg0, Level arg1, Marker arg2, String message, Object... arg4) { - try { - if (message == null) - return Result.NEUTRAL; - String logM = message.toLowerCase(); - if (!logM.contains("issued server command:")) - return Result.NEUTRAL; - if (!logM.contains("/login ") && !logM.contains("/l ") && !logM.contains("/reg ") && !logM.contains("/changepassword ") && !logM.contains("/unregister ") && !logM.contains("/authme register ") && !logM.contains("/authme changepassword ") && !logM.contains("/authme reg ") && !logM.contains("/authme cp ") && !logM.contains("/register ")) - return Result.NEUTRAL; - return Result.DENY; - } catch (NullPointerException npe) { - return Result.NEUTRAL; - } + return validateMessage(message); } @Override public Result filter(Logger arg0, Level arg1, Marker arg2, Object message, Throwable arg4) { - try { - if (message == null) - return Result.NEUTRAL; - String logM = message.toString().toLowerCase(); - if (!logM.contains("issued server command:")) - return Result.NEUTRAL; - if (!logM.contains("/login ") && !logM.contains("/l ") && !logM.contains("/reg ") && !logM.contains("/changepassword ") && !logM.contains("/unregister ") && !logM.contains("/authme register ") && !logM.contains("/authme changepassword ") && !logM.contains("/authme reg ") && !logM.contains("/authme cp ") && !logM.contains("/register ")) - return Result.NEUTRAL; - return Result.DENY; - } catch (NullPointerException npe) { - return Result.NEUTRAL; - } + if (message == null) { + return Result.NEUTRAL; + } + return validateMessage(message.toString()); } @Override public Result filter(Logger arg0, Level arg1, Marker arg2, Message message, Throwable arg4) { - try { - if (message == null) - return Result.NEUTRAL; - String logM = message.getFormattedMessage().toLowerCase(); - if (!logM.contains("issued server command:")) - return Result.NEUTRAL; - if (!logM.contains("/login ") && !logM.contains("/l ") && !logM.contains("/reg ") && !logM.contains("/changepassword ") && !logM.contains("/unregister ") && !logM.contains("/authme register ") && !logM.contains("/authme changepassword ") && !logM.contains("/authme reg ") && !logM.contains("/authme cp ") && !logM.contains("/register ")) - return Result.NEUTRAL; - return Result.DENY; - } catch (NullPointerException npe) { - return Result.NEUTRAL; - } + return validateMessage(message); } @Override @@ -92,4 +62,39 @@ public class Log4JFilter implements org.apache.logging.log4j.core.Filter { return Result.NEUTRAL; } + /** + * Validates a Message instance and returns the {@link Result} value + * depending depending on whether the message contains sensitive AuthMe + * data. + * + * @param message the Message object to verify + * @return the Result value + */ + private static Result validateMessage(Message message) { + if (message == null) { + return Result.NEUTRAL; + } + return validateMessage(message.getFormattedMessage()); + } + + /** + * Validates a message and returns the {@link Result} value depending + * depending on whether the message contains sensitive AuthMe data. + * + * @param message the message to verify + * @return the Result value + */ + private static Result validateMessage(String message) { + if (message == null) { + return Result.NEUTRAL; + } + + String lowerMessage = message.toLowerCase(); + if (lowerMessage.contains("issued server command:") + && StringUtils.containsAny(lowerMessage, COMMANDS_TO_SKIP)) { + return Result.DENY; + } + return Result.NEUTRAL; + } + } diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index eab7ea15..da6f91ba 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -25,4 +25,16 @@ public class StringUtils { // Determine the difference value, return the result return Math.abs(service.score(first, second) - 1.0); } + + public static boolean containsAny(String str, String... pieces) { + if (str == null) { + return false; + } + for (String piece : pieces) { + if (str.contains(piece)) { + return true; + } + } + return false; + } } diff --git a/test/main/java/fr/xephi/authme/Log4JFilterTest.java b/test/main/java/fr/xephi/authme/Log4JFilterTest.java index e7d11410..5614eb51 100644 --- a/test/main/java/fr/xephi/authme/Log4JFilterTest.java +++ b/test/main/java/fr/xephi/authme/Log4JFilterTest.java @@ -52,6 +52,20 @@ public class Log4JFilterTest { assertThat(result, equalTo(Result.NEUTRAL)); } + @Test + public void shouldNotFilterNonCommandLogEvent() { + // given + Message message = mockMessage(OTHER_COMMAND); + LogEvent event = Mockito.mock(LogEvent.class); + when(event.getMessage()).thenReturn(message); + + // when + Result result = log4JFilter.filter(event); + + // then + assertThat(result, equalTo(Result.NEUTRAL)); + } + @Test public void shouldNotFilterLogEventWithNullMessage() { // given @@ -96,6 +110,15 @@ public class Log4JFilterTest { assertThat(result, equalTo(Result.NEUTRAL)); } + @Test + public void shouldNotFilterNonCommandStringMessage() { + // given / when + Result result = log4JFilter.filter(null, null, null, OTHER_COMMAND, new Object[0]); + + // then + assertThat(result, equalTo(Result.NEUTRAL)); + } + @Test public void shouldReturnNeutralForNullMessage() { // given / when @@ -120,7 +143,7 @@ public class Log4JFilterTest { @Test public void shouldNotFilterNullObjectParam() { // given / when - Result result = log4JFilter.filter(null, null, null, null, new Exception()); + Result result = log4JFilter.filter(null, null, null, (Object) null, new Exception()); // then assertThat(result, equalTo(Result.NEUTRAL)); @@ -135,6 +158,15 @@ public class Log4JFilterTest { assertThat(result, equalTo(Result.NEUTRAL)); } + @Test + public void shouldNotFilterNonSensitiveCommand() { + // given / when + Result result = log4JFilter.filter(null, null, null, NORMAL_COMMAND, new Exception()); + + // then + assertThat(result, equalTo(Result.NEUTRAL)); + } + // -------- // Test filter(Logger, Level, Marker, Message, Throwable) // -------- @@ -162,10 +194,22 @@ public class Log4JFilterTest { assertThat(result, equalTo(Result.NEUTRAL)); } + @Test + public void shouldNotFilterNonCommandMessage() { + // given + Message message = mockMessage(OTHER_COMMAND); + + // when + Result result = log4JFilter.filter(null, null, null, message, new Exception()); + + // then + assertThat(result, equalTo(Result.NEUTRAL)); + } + @Test public void shouldNotFilterNullMessage() { // given / when - Result result = log4JFilter.filter(null, null, null, null, new Exception()); + Result result = log4JFilter.filter(null, null, null, (Message) null, new Exception()); // then assertThat(result, equalTo(Result.NEUTRAL));